diff --git a/service/usbip/export_ledger.go b/service/usbip/export_ledger.go index 85b0d1a34..3012ac19e 100644 --- a/service/usbip/export_ledger.go +++ b/service/usbip/export_ledger.go @@ -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) } diff --git a/service/usbip/host.go b/service/usbip/host.go index d8040eb8a..f30033f24 100644 --- a/service/usbip/host.go +++ b/service/usbip/host.go @@ -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 diff --git a/service/usbip/host_darwin.go b/service/usbip/host_darwin.go index 29550ada4..6efa8abf3 100644 --- a/service/usbip/host_darwin.go +++ b/service/usbip/host_darwin.go @@ -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, "" } diff --git a/service/usbip/host_linux.go b/service/usbip/host_linux.go index 3e2dbbfa6..20080454e 100644 --- a/service/usbip/host_linux.go +++ b/service/usbip/host_linux.go @@ -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 diff --git a/service/usbip/linux_test.go b/service/usbip/linux_test.go index 5176484cf..b540d14ea 100644 --- a/service/usbip/linux_test.go +++ b/service/usbip/linux_test.go @@ -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", diff --git a/service/usbip/server.go b/service/usbip/server.go index 00b88a70e..cae436644 100644 --- a/service/usbip/server.go +++ b/service/usbip/server.go @@ -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 } diff --git a/service/usbip/server_darwin.go b/service/usbip/server_darwin.go index 02f24a4c4..92e8d6d9a 100644 --- a/service/usbip/server_darwin.go +++ b/service/usbip/server_darwin.go @@ -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 -} diff --git a/service/usbip/server_darwin_test.go b/service/usbip/server_darwin_test.go index 72826bfb3..ddd74c2ba 100644 --- a/service/usbip/server_darwin_test.go +++ b/service/usbip/server_darwin_test.go @@ -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)