From 0a6edefe53d3a50653085179bd5e31ae90787a02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=B8=96=E7=95=8C?= Date: Tue, 12 May 2026 21:59:06 +0800 Subject: [PATCH] usbip: drop dead code from host-seam refactor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- service/usbip/export_ledger.go | 40 +++++++++-------------------- service/usbip/host.go | 2 +- service/usbip/host_darwin.go | 21 ++++++--------- service/usbip/host_linux.go | 26 +++++++------------ service/usbip/linux_test.go | 9 +++---- service/usbip/server.go | 2 +- service/usbip/server_darwin.go | 11 +------- service/usbip/server_darwin_test.go | 12 +++------ 8 files changed, 39 insertions(+), 84 deletions(-) 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)