usbip: drop dead code from host-seam refactor
The ExportHost.Reconcile changed bool is computed by both Linux and Darwin implementations but discarded at the only call site (reconcileAndBroadcast); the ledger already detects change in BroadcastIfChanged. linuxImportHost.trackPort had a dead add=true branch — all callers passed false. darwinExport.Snapshot was setting StatusReason to the same string as State (tautological). LeaseCheck for stale captures returned the State name as the reason. Also trim narrative comments in export_ledger.go that referenced refactor history or the deleted lease.go file, inline the trivial checkAvailableUnderLock helper at its only caller, and replace cloneIsoPackets with slices.Clone.
This commit is contained in:
@@ -16,8 +16,7 @@ import (
|
||||
// exportLedger owns the server's authoritative mutable state: which
|
||||
// devices it is exporting, which busids are currently in use, who is
|
||||
// subscribed to control-channel updates, and which import leases are
|
||||
// outstanding. It absorbs what previously lived as four fields plus a
|
||||
// LeaseManager on ServerService.
|
||||
// outstanding.
|
||||
//
|
||||
// Synchronization:
|
||||
//
|
||||
@@ -29,11 +28,6 @@ import (
|
||||
//
|
||||
// fast: broadcast bookkeeping. seq, nextSubID, subs, state.
|
||||
// slow: inventory and leases. exports, busy, leases, nextLeaseID.
|
||||
//
|
||||
// The previous controlAccess -> LeaseManager.access -> access
|
||||
// ordering documented in lease.go disappears because the only
|
||||
// remaining cross-state operation (IssueLease) reads seq under fast,
|
||||
// releases, then uses slow.
|
||||
type exportLedger struct {
|
||||
logger log.ContextLogger
|
||||
now func() time.Time
|
||||
@@ -80,8 +74,7 @@ func newExportLedger(logger log.ContextLogger, ttl time.Duration, now func() tim
|
||||
}
|
||||
}
|
||||
|
||||
// IsBusy reports whether the busid currently has an active import. The
|
||||
// signature matches ExportHost.Reconcile's isBusy callback.
|
||||
// IsBusy reports whether the busid currently has an active import.
|
||||
func (l *exportLedger) IsBusy(busid string) bool {
|
||||
l.slow.Lock()
|
||||
defer l.slow.Unlock()
|
||||
@@ -187,9 +180,15 @@ func (l *exportLedger) BroadcastIfChanged(ctx context.Context) bool {
|
||||
// mark is only inserted after the lease check passes and the second
|
||||
// availability re-check confirms no other goroutine raced in.
|
||||
func (l *exportLedger) TryReserveForImport(ctx context.Context, busid string) (Export, bool, string) {
|
||||
export, reason, ok := l.checkAvailableUnderLock(busid)
|
||||
if !ok {
|
||||
return nil, false, reason
|
||||
l.slow.Lock()
|
||||
export, found := l.exports[busid]
|
||||
busy := l.busy[busid]
|
||||
l.slow.Unlock()
|
||||
if !found {
|
||||
return nil, false, "unknown busid"
|
||||
}
|
||||
if busy {
|
||||
return nil, false, deviceStateBusy
|
||||
}
|
||||
leaseOK, leaseReason := export.LeaseCheck(ctx)
|
||||
if !leaseOK {
|
||||
@@ -208,22 +207,7 @@ func (l *exportLedger) TryReserveForImport(ctx context.Context, busid string) (E
|
||||
return export, true, ""
|
||||
}
|
||||
|
||||
func (l *exportLedger) checkAvailableUnderLock(busid string) (Export, string, bool) {
|
||||
l.slow.Lock()
|
||||
defer l.slow.Unlock()
|
||||
export, found := l.exports[busid]
|
||||
if !found {
|
||||
return nil, "unknown busid", false
|
||||
}
|
||||
if l.busy[busid] {
|
||||
return nil, deviceStateBusy, false
|
||||
}
|
||||
return export, "", true
|
||||
}
|
||||
|
||||
// ConfirmImport broadcasts that an import is now active. The busy mark
|
||||
// was set during TryReserveForImport; this method only broadcasts so
|
||||
// clients see the state change.
|
||||
// ConfirmImport broadcasts that an import is now active.
|
||||
func (l *exportLedger) ConfirmImport(ctx context.Context) {
|
||||
l.BroadcastIfChanged(ctx)
|
||||
}
|
||||
|
||||
@@ -24,7 +24,7 @@ import (
|
||||
type ExportHost interface {
|
||||
Start(ctx context.Context) error
|
||||
Close() error
|
||||
Reconcile(ctx context.Context, isBusy func(busid string) bool) (snapshot map[string]Export, released []string, changed bool, err error)
|
||||
Reconcile(ctx context.Context, isBusy func(busid string) bool) (snapshot map[string]Export, released []string, err error)
|
||||
FinishImport(ctx context.Context, busid string) (released bool, err error)
|
||||
// Events returns a channel that fires when device topology changes.
|
||||
// Returning (nil, nil) means "no native event source; rely on
|
||||
|
||||
@@ -87,10 +87,10 @@ func (h *darwinExportHost) Events(ctx context.Context) (<-chan struct{}, error)
|
||||
return ch, nil
|
||||
}
|
||||
|
||||
func (h *darwinExportHost) Reconcile(ctx context.Context, isBusy func(busid string) bool) (map[string]Export, []string, bool, error) {
|
||||
func (h *darwinExportHost) Reconcile(ctx context.Context, isBusy func(busid string) bool) (map[string]Export, []string, error) {
|
||||
devices, err := h.ops.copyUSBHostDevices()
|
||||
if err != nil {
|
||||
return h.snapshotSelf(), nil, false, E.Cause(err, "enumerate IOUSBHost devices")
|
||||
return h.snapshotSelf(), nil, E.Cause(err, "enumerate IOUSBHost devices")
|
||||
}
|
||||
desired := make(map[string]darwinUSBHostDeviceInfo)
|
||||
for _, match := range h.matches {
|
||||
@@ -159,7 +159,6 @@ func (h *darwinExportHost) Reconcile(ctx context.Context, isBusy func(busid stri
|
||||
released = append(released, busid)
|
||||
}
|
||||
|
||||
changed := len(toAdd) > 0 || len(toRemove) > 0
|
||||
h.access.Lock()
|
||||
for _, busid := range toStale {
|
||||
exp, ok := h.exports[busid]
|
||||
@@ -167,7 +166,6 @@ func (h *darwinExportHost) Reconcile(ctx context.Context, isBusy func(busid stri
|
||||
continue
|
||||
}
|
||||
exp.stale = true
|
||||
changed = true
|
||||
}
|
||||
for _, exp := range toRemove {
|
||||
delete(h.exports, exp.busid)
|
||||
@@ -189,7 +187,7 @@ func (h *darwinExportHost) Reconcile(ctx context.Context, isBusy func(busid stri
|
||||
exp.device.Close()
|
||||
}
|
||||
}
|
||||
return out, released, changed, nil
|
||||
return out, released, nil
|
||||
}
|
||||
|
||||
func (h *darwinExportHost) FinishImport(ctx context.Context, busid string) (bool, error) {
|
||||
@@ -243,23 +241,20 @@ func (e *darwinExport) Snapshot(ctx context.Context, busy bool) ExportSnapshot {
|
||||
}
|
||||
}
|
||||
state := deviceStateAvailable
|
||||
reason := deviceStateAvailable
|
||||
if busy {
|
||||
state = deviceStateBusy
|
||||
reason = deviceStateBusy
|
||||
}
|
||||
return ExportSnapshot{
|
||||
Entry: e.entry,
|
||||
Backend: backendIDDarwinIOKit,
|
||||
StableID: stableID,
|
||||
State: state,
|
||||
StatusReason: reason,
|
||||
Entry: e.entry,
|
||||
Backend: backendIDDarwinIOKit,
|
||||
StableID: stableID,
|
||||
State: state,
|
||||
}
|
||||
}
|
||||
|
||||
func (e *darwinExport) LeaseCheck(ctx context.Context) (bool, string) {
|
||||
if e.stale {
|
||||
return false, deviceStateUnavailable
|
||||
return false, "capture released"
|
||||
}
|
||||
return true, ""
|
||||
}
|
||||
|
||||
@@ -131,10 +131,10 @@ func nextUEventListenerBackoff(current time.Duration) time.Duration {
|
||||
return next
|
||||
}
|
||||
|
||||
func (h *linuxExportHost) Reconcile(ctx context.Context, isBusy func(busid string) bool) (map[string]Export, []string, bool, error) {
|
||||
func (h *linuxExportHost) Reconcile(ctx context.Context, isBusy func(busid string) bool) (map[string]Export, []string, error) {
|
||||
devices, err := h.ops.listUSBDevices()
|
||||
if err != nil {
|
||||
return h.snapshotSelf(), nil, false, E.Cause(err, "enumerate usb devices")
|
||||
return h.snapshotSelf(), nil, E.Cause(err, "enumerate usb devices")
|
||||
}
|
||||
desired := make(map[string]sysfsDevice)
|
||||
present := make(map[string]struct{}, len(devices))
|
||||
@@ -165,19 +165,17 @@ func (h *linuxExportHost) Reconcile(ctx context.Context, isBusy func(busid strin
|
||||
}
|
||||
h.access.Unlock()
|
||||
|
||||
changed := false
|
||||
for busid, device := range desired {
|
||||
if _, ok := current[busid]; ok {
|
||||
continue
|
||||
}
|
||||
exp, bindErr := h.bindOne(&device)
|
||||
if bindErr != nil {
|
||||
return h.snapshotSelf(), nil, changed, E.Cause(bindErr, "bind ", busid)
|
||||
return h.snapshotSelf(), nil, E.Cause(bindErr, "bind ", busid)
|
||||
}
|
||||
h.access.Lock()
|
||||
h.exports[busid] = exp
|
||||
h.access.Unlock()
|
||||
changed = true
|
||||
}
|
||||
|
||||
var released []string
|
||||
@@ -194,10 +192,9 @@ func (h *linuxExportHost) Reconcile(ctx context.Context, isBusy func(busid strin
|
||||
delete(h.exports, busid)
|
||||
h.access.Unlock()
|
||||
released = append(released, busid)
|
||||
changed = true
|
||||
}
|
||||
|
||||
return h.snapshotSelf(), released, changed, nil
|
||||
return h.snapshotSelf(), released, nil
|
||||
}
|
||||
|
||||
func (h *linuxExportHost) FinishImport(ctx context.Context, busid string) (bool, error) {
|
||||
@@ -466,7 +463,7 @@ func (h *linuxImportHost) attachOnce(ctx context.Context, info DeviceInfoTruncat
|
||||
}
|
||||
err = h.ops.vhciAttach(port, handoff.kernelFD(), info.DevID(), info.Speed)
|
||||
if err != nil {
|
||||
h.trackPort(port, false)
|
||||
h.releasePort(port)
|
||||
if errors.Is(err, unix.EBUSY) {
|
||||
triedPorts[port] = struct{}{}
|
||||
continue
|
||||
@@ -493,16 +490,11 @@ func (h *linuxImportHost) reservePort(port int) bool {
|
||||
return true
|
||||
}
|
||||
|
||||
func (h *linuxImportHost) trackPort(port int, add bool) {
|
||||
func (h *linuxImportHost) releasePort(port int) {
|
||||
h.portsAccess.Lock()
|
||||
defer h.portsAccess.Unlock()
|
||||
if add {
|
||||
h.logger.Debug("reserve vhci port ", port)
|
||||
h.ports[port] = struct{}{}
|
||||
} else {
|
||||
h.logger.Debug("release vhci port ", port)
|
||||
delete(h.ports, port)
|
||||
}
|
||||
h.logger.Debug("release vhci port ", port)
|
||||
delete(h.ports, port)
|
||||
}
|
||||
|
||||
// linuxClientSession wraps kernelHandoffSession with vhci-port cleanup
|
||||
@@ -528,7 +520,7 @@ func (s *linuxClientSession) Close() error {
|
||||
s.closeOnce.Do(func() {
|
||||
detachErr := s.host.ops.vhciDetach(s.port)
|
||||
closeErr := s.handoff.Close()
|
||||
s.host.trackPort(s.port, false)
|
||||
s.host.releasePort(s.port)
|
||||
s.closeErr = E.Errors(detachErr, closeErr)
|
||||
})
|
||||
return s.closeErr
|
||||
|
||||
@@ -1000,9 +1000,8 @@ func TestServerReconcileExportsBindsMatchesAndSkipsHub(t *testing.T) {
|
||||
|
||||
host := newTestLinuxExportHost(t, []option.USBIPDeviceMatch{{VendorID: 0x1d6b, ProductID: 0x0002}}, ops)
|
||||
|
||||
_, _, changed, err := host.Reconcile(context.Background(), func(string) bool { return false })
|
||||
_, _, err := host.Reconcile(context.Background(), func(string) bool { return false })
|
||||
require.NoError(t, err)
|
||||
require.True(t, changed)
|
||||
require.Equal(t, []string{
|
||||
"unbind 1-1 usbhid",
|
||||
"match 1-1 add",
|
||||
@@ -1106,9 +1105,8 @@ func TestServerReconcileExportsSkipsVHCIDevices(t *testing.T) {
|
||||
|
||||
host := newTestLinuxExportHost(t, []option.USBIPDeviceMatch{{VendorID: 0x1d6b, ProductID: 0x0002}}, ops)
|
||||
|
||||
_, _, changed, err := host.Reconcile(context.Background(), func(string) bool { return false })
|
||||
_, _, err := host.Reconcile(context.Background(), func(string) bool { return false })
|
||||
require.NoError(t, err)
|
||||
require.True(t, changed)
|
||||
require.Equal(t, []string{
|
||||
"unbind 1-1 usb",
|
||||
"match 1-1",
|
||||
@@ -1155,9 +1153,8 @@ func TestServerReconcileExportsReleasesRemovedExports(t *testing.T) {
|
||||
host := newTestLinuxExportHost(t, nil, ops)
|
||||
setLinuxExport(host, &linuxExport{busid: "1-1", managed: true, originalDriver: "usbhid", ops: ops, logger: host.logger})
|
||||
|
||||
_, _, changed, err := host.Reconcile(context.Background(), func(string) bool { return false })
|
||||
_, _, err := host.Reconcile(context.Background(), func(string) bool { return false })
|
||||
require.NoError(t, err)
|
||||
require.True(t, changed)
|
||||
require.Empty(t, linuxExportSnapshot(host))
|
||||
require.Equal(t, []string{
|
||||
"sockfd 1-1",
|
||||
|
||||
@@ -138,7 +138,7 @@ func (s *ServerService) reconcileAndBroadcast(notify bool) error {
|
||||
if s.ctx != nil && s.ctx.Err() != nil {
|
||||
return nil
|
||||
}
|
||||
snapshot, released, _, err := s.host.Reconcile(s.ctx, s.ledger.IsBusy)
|
||||
snapshot, released, err := s.host.Reconcile(s.ctx, s.ledger.IsBusy)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
@@ -197,7 +197,7 @@ func (s *darwinServerDataSession) handleSubmit(command SubmitCommand) SubmitResp
|
||||
},
|
||||
StartFrame: command.StartFrame,
|
||||
NumberOfPackets: command.NumberOfPackets,
|
||||
IsoPackets: cloneIsoPackets(command.IsoPackets),
|
||||
IsoPackets: slices.Clone(command.IsoPackets),
|
||||
}
|
||||
buffer := command.Buffer
|
||||
if command.Header.Direction == USBIPDirIn && command.TransferBufferLength > 0 {
|
||||
@@ -300,12 +300,3 @@ func commandEndpoint(command SubmitCommand) uint8 {
|
||||
}
|
||||
return endpoint
|
||||
}
|
||||
|
||||
func cloneIsoPackets(in []IsoPacketDescriptor) []IsoPacketDescriptor {
|
||||
if len(in) == 0 {
|
||||
return nil
|
||||
}
|
||||
out := make([]IsoPacketDescriptor, len(in))
|
||||
copy(out, in)
|
||||
return out
|
||||
}
|
||||
|
||||
@@ -188,9 +188,8 @@ func TestDarwinExportHostEventTriggersReconcile(t *testing.T) {
|
||||
t.Fatal("timed out waiting for Events channel signal")
|
||||
}
|
||||
|
||||
snapshot, released, changed, err := host.Reconcile(ctx, func(string) bool { return false })
|
||||
snapshot, released, err := host.Reconcile(ctx, func(string) bool { return false })
|
||||
require.NoError(t, err)
|
||||
require.True(t, changed)
|
||||
require.Empty(t, released)
|
||||
require.Contains(t, snapshot, busid)
|
||||
}
|
||||
@@ -244,11 +243,10 @@ func TestDarwinExportHostReconcileMarksBusyMissingExportStale(t *testing.T) {
|
||||
entry: entry,
|
||||
}
|
||||
|
||||
snapshot, released, changed, err := host.Reconcile(context.Background(), func(b string) bool {
|
||||
snapshot, released, err := host.Reconcile(context.Background(), func(b string) bool {
|
||||
return b == busid
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.True(t, changed)
|
||||
require.Empty(t, released)
|
||||
require.NotContains(t, snapshot, busid)
|
||||
|
||||
@@ -291,11 +289,10 @@ func TestDarwinExportHostReconcileCapturesReplacementAfterStaleRelease(t *testin
|
||||
entry: oldEntry,
|
||||
}
|
||||
|
||||
snapshot, released, changed, err := host.Reconcile(context.Background(), func(b string) bool {
|
||||
snapshot, released, err := host.Reconcile(context.Background(), func(b string) bool {
|
||||
return b == busid
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.True(t, changed)
|
||||
require.Empty(t, released)
|
||||
require.NotContains(t, snapshot, busid)
|
||||
require.True(t, host.exports[busid].stale)
|
||||
@@ -305,9 +302,8 @@ func TestDarwinExportHostReconcileCapturesReplacementAfterStaleRelease(t *testin
|
||||
require.NoError(t, err)
|
||||
require.True(t, releasedFinish)
|
||||
|
||||
snapshot, released, changed, err = host.Reconcile(context.Background(), func(string) bool { return false })
|
||||
snapshot, released, err = host.Reconcile(context.Background(), func(string) bool { return false })
|
||||
require.NoError(t, err)
|
||||
require.True(t, changed)
|
||||
require.Empty(t, released)
|
||||
require.Contains(t, snapshot, busid)
|
||||
require.Equal(t, 1, opened)
|
||||
|
||||
Reference in New Issue
Block a user