fix regex, remove unsafe pointer usage, and add graceful HTTP shutdown
- Fix versionMatcher regex: gorilla/mux does not support (?:) syntax, and -dirty suffix was required instead of optional - Replace unsafe.Pointer LUN casts with binary.LittleEndian.Uint64 in sbc.go, scsi.go, and target.go - Implement graceful HTTP server shutdown with 5s timeout using srv.Shutdown() instead of raw listener close - Replace golang.org/x/net/context with standard library context - Respect existing req.Cancel value in canceler to avoid overwriting - Add early context cancellation check in Do() to fail fast Based on review of PR #120 by @orzhang, with fixes applied. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -14,7 +14,10 @@ import (
|
|||||||
)
|
)
|
||||||
|
|
||||||
func canceler(client transport.Sender, req *http.Request) func() {
|
func canceler(client transport.Sender, req *http.Request) func() {
|
||||||
// TODO(djd): Respect any existing value of req.Cancel.
|
// Respect any existing value of req.Cancel.
|
||||||
|
if req.Cancel != nil {
|
||||||
|
return nop
|
||||||
|
}
|
||||||
ch := make(chan struct{})
|
ch := make(chan struct{})
|
||||||
req.Cancel = ch
|
req.Cancel = ch
|
||||||
|
|
||||||
|
|||||||
@@ -35,6 +35,13 @@ func Do(ctx context.Context, client transport.Sender, req *http.Request) (*http.
|
|||||||
client = http.DefaultClient
|
client = http.DefaultClient
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// If the context is already done, return early.
|
||||||
|
select {
|
||||||
|
case <-ctx.Done():
|
||||||
|
return nil, ctx.Err()
|
||||||
|
default:
|
||||||
|
}
|
||||||
|
|
||||||
// Request cancelation changed in Go 1.5, see canceler.go and canceler_go14.go.
|
// Request cancelation changed in Go 1.5, see canceler.go and canceler_go14.go.
|
||||||
cancel := canceler(client, req)
|
cancel := canceler(client, req)
|
||||||
|
|
||||||
|
|||||||
@@ -19,11 +19,13 @@ package apiserver
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"crypto/tls"
|
"crypto/tls"
|
||||||
|
"context"
|
||||||
"fmt"
|
"fmt"
|
||||||
"net"
|
"net"
|
||||||
"net/http"
|
"net/http"
|
||||||
"strconv"
|
"strconv"
|
||||||
"strings"
|
"strings"
|
||||||
|
"time"
|
||||||
|
|
||||||
systemdActivation "github.com/coreos/go-systemd/activation"
|
systemdActivation "github.com/coreos/go-systemd/activation"
|
||||||
"github.com/docker/go-connections/sockets"
|
"github.com/docker/go-connections/sockets"
|
||||||
@@ -34,12 +36,11 @@ import (
|
|||||||
"github.com/gostor/gotgt/pkg/apiserver/router/lu"
|
"github.com/gostor/gotgt/pkg/apiserver/router/lu"
|
||||||
"github.com/gostor/gotgt/pkg/apiserver/router/target"
|
"github.com/gostor/gotgt/pkg/apiserver/router/target"
|
||||||
log "github.com/sirupsen/logrus"
|
log "github.com/sirupsen/logrus"
|
||||||
"golang.org/x/net/context"
|
|
||||||
)
|
)
|
||||||
|
|
||||||
// versionMatcher defines a variable matcher to be parsed by the router
|
// versionMatcher defines a variable matcher to be parsed by the router
|
||||||
// when a request is about to be served.
|
// when a request is about to be served.
|
||||||
const versionMatcher = "/v{version:[0-9.]+(?:-dirty)}"
|
const versionMatcher = "/v{version:[0-9.]+(-dirty)?}"
|
||||||
|
|
||||||
// Config provides the configuration for the API server
|
// Config provides the configuration for the API server
|
||||||
type Config struct {
|
type Config struct {
|
||||||
@@ -137,7 +138,9 @@ func (s *HTTPServer) Serve() error {
|
|||||||
|
|
||||||
// Close closes the HTTPServer from listening for the inbound requests.
|
// Close closes the HTTPServer from listening for the inbound requests.
|
||||||
func (s *HTTPServer) Close() error {
|
func (s *HTTPServer) Close() error {
|
||||||
return s.l.Close()
|
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||||
|
defer cancel()
|
||||||
|
return s.srv.Shutdown(ctx)
|
||||||
}
|
}
|
||||||
|
|
||||||
func (s *Server) initTCPSocket(addr string) (l net.Listener, err error) {
|
func (s *Server) initTCPSocket(addr string) (l net.Listener, err error) {
|
||||||
|
|||||||
@@ -21,7 +21,6 @@ import (
|
|||||||
"bytes"
|
"bytes"
|
||||||
"encoding/binary"
|
"encoding/binary"
|
||||||
"fmt"
|
"fmt"
|
||||||
"unsafe"
|
|
||||||
|
|
||||||
"github.com/gostor/gotgt/pkg/api"
|
"github.com/gostor/gotgt/pkg/api"
|
||||||
"github.com/gostor/gotgt/pkg/util"
|
"github.com/gostor/gotgt/pkg/util"
|
||||||
@@ -498,7 +497,7 @@ func SBCReserve(host int, cmd *api.SCSICommand) api.SAMStat {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func SBCRelease(host int, cmd *api.SCSICommand) api.SAMStat {
|
func SBCRelease(host int, cmd *api.SCSICommand) api.SAMStat {
|
||||||
lun := *(*uint64)(unsafe.Pointer(&cmd.Lun))
|
lun := binary.LittleEndian.Uint64(cmd.Lun[:])
|
||||||
if err := deviceRelease(cmd.Target.TID, cmd.ITNexusID, lun, false); err != nil {
|
if err := deviceRelease(cmd.Target.TID, cmd.ITNexusID, lun, false); err != nil {
|
||||||
return api.SAMStatReservationConflict
|
return api.SAMStatReservationConflict
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -21,7 +21,6 @@ import (
|
|||||||
"encoding/binary"
|
"encoding/binary"
|
||||||
"fmt"
|
"fmt"
|
||||||
"sync"
|
"sync"
|
||||||
"unsafe"
|
|
||||||
|
|
||||||
"github.com/gostor/gotgt/pkg/api"
|
"github.com/gostor/gotgt/pkg/api"
|
||||||
log "github.com/sirupsen/logrus"
|
log "github.com/sirupsen/logrus"
|
||||||
@@ -96,7 +95,7 @@ func (s *SCSITargetService) AddCommandQueue(tid int, scmd *api.SCSICommand) erro
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
scmd.ITNexus = itn
|
scmd.ITNexus = itn
|
||||||
lun := *(*uint64)(unsafe.Pointer(&scmd.Lun))
|
lun := binary.LittleEndian.Uint64(scmd.Lun[:])
|
||||||
scmd.Device = target.Devices[lun]
|
scmd.Device = target.Devices[lun]
|
||||||
|
|
||||||
log.Debugf("scsi opcode: 0x%x, LUN: %d", int(scmd.SCB[0]), binary.LittleEndian.Uint64(scmd.Lun[:]))
|
log.Debugf("scsi opcode: 0x%x, LUN: %d", int(scmd.SCB[0]), binary.LittleEndian.Uint64(scmd.Lun[:]))
|
||||||
|
|||||||
@@ -17,8 +17,8 @@ limitations under the License.
|
|||||||
package scsi
|
package scsi
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"encoding/binary"
|
||||||
"fmt"
|
"fmt"
|
||||||
"unsafe"
|
|
||||||
|
|
||||||
"github.com/google/uuid"
|
"github.com/google/uuid"
|
||||||
"github.com/gostor/gotgt/pkg/api"
|
"github.com/gostor/gotgt/pkg/api"
|
||||||
@@ -97,7 +97,7 @@ func RemoveITNexus(target *api.SCSITarget, itnexus *api.ITNexus) {
|
|||||||
|
|
||||||
func deviceReserve(cmd *api.SCSICommand) error {
|
func deviceReserve(cmd *api.SCSICommand) error {
|
||||||
var lu *api.SCSILu
|
var lu *api.SCSILu
|
||||||
lun := *(*uint64)(unsafe.Pointer(&cmd.Lun))
|
lun := binary.LittleEndian.Uint64(cmd.Lun[:])
|
||||||
|
|
||||||
for tgtLUN, lunDev := range cmd.Target.Devices {
|
for tgtLUN, lunDev := range cmd.Target.Devices {
|
||||||
if tgtLUN == lun {
|
if tgtLUN == lun {
|
||||||
|
|||||||
Reference in New Issue
Block a user