Fix USB Gadget polling/writing to recover from errors #1035

IDisposable · 2025-12-02T01:33:56Z

Fixes #128

Summary

Revamped the polling and writing of the USB gadget to correct for dropped connections
Corrected the initialization order to ensure we run even if the USB is disconnected (and we're getting power from the y-splitter/POE)
Ensured that we restart the LED listening polling
If we exceed too many consecutive write errors, close and reopen the gadget
Extensive changes to logging to ensure we have complete context and honor the changing of log levels in the config.
Fixed the script to see all JETKVM_LOG_* log-level commands.
Handle the native C code error returns in the CGO wrapper
MORE TO COME

Remove duplicate logging. Moved the log helpers to internal/logging to share. Cleaned up usbGadget setup and tear down. Fixed active-session count mismatch on device screen. Cleaned up the usbGadget content comparisons. Enhance the usbgadget changeset management. Unmount images when shutting down. nmlite gateway add/remote are info, not warning. Made all the HID message structures loggable. Keep usbGadget Init from destroying the gadget. Clear the auto-release timers before cancelling the state poller. Made all gadget callbacks execute in their own goroutines and moved the USB state polling to after the callback setup. Attempt to reopen the KeyboardHidFile every time we check USB state and it's configured (will do nothing if already open). Made active session count atomic, without mutexes. Fixed tracing level check of HID Message Handler so Data is shown in Trace or Debug level. Fixed order of mount/ndb start/nbp stop/dismount to fix mount drive wonkiness. Protect the open of Keyboard HID file with mutex. Fix process for killing/copying/starting application Better logging Move ByteSlice to utils and added zerolog marshalling Exposed the UpdateConfigLogLevel Fixed the logging Updating the config would not change the logging until a reboot. Environment variables setting logging levels was broken. Caching the loggers prevent configuration changes from being effective because zerolog.Logger(s) are immutable. Added log level option to Advanced Settings menu

Also now can be logged as hex

Fix log level changes being reflected before a reboot Add tons of helpers for contextual logging Fix typo in relative mouse logging.

Made all call-backs go routines. Made listenKeyboardEvents callable as a go routine and returns when the keyboard file is closed/nil Cleaned up the auto-release to remove duplicate code. Moved to new logging helpers

Also restructured config_tx so it doesn't need copies of config data.

Also restructured config_tx so it doesn't need copies of config data. Fuller debug logging in hidrpc and make all reportHidRPC sends async. Pass up errors from handleHidRPCMessage. listenKeyboardEvents handles closed keyboard file and terminates loop. Excess write failures cause close/open cycle Adds Settings menu to UI to allow setting the log level

Add missing types for enhancing context Also cleaned up the syntax to be more compact Expose the zerolog.Event for levels Ported synctrace and timesync

IDisposable · 2025-12-04T06:52:34Z

internal/hidrpc/hidrpc.go

 	"github.com/jetkvm/kvm/internal/usbgadget"
 )

-// MessageType is the type of the HID RPC message


Moved this to message.go where it really makes more sense

internal/hidrpc/message.go

IDisposable · 2025-12-04T08:23:09Z

main.go

 	setProcTitle("ready")
+	mainLogger.Log().Msg("JetKVM ready")
+
+	go RunAutoUpdateCheck()


... no don't start up the AUP until we're fully ready...

IDisposable · 2025-12-04T08:23:53Z

main.go

-	//	if err != nil {
-	//		logger.Infof("Failed to unmount fuse: %v", err)
-	//	}
+	if err := rpcUnmountImage(); err != nil {


Have to unmount the drive or the UsbGadget gets wonky... and it takes reboot to get it back.

IDisposable · 2025-12-04T08:24:49Z

main.go

+		mainLogger.Warn().Err(err).Msg("failed to eject virtual media on shutdown")
+	}
+
+	gadget.Close()


Explicitly close the gadget so we don't have all the "setup" still pretending the devices exist to the client

IDisposable · 2025-12-04T08:25:19Z

ota.go

 	return nil
 }
+
+func RunAutoUpdateCheck() {


moved here from the main

IDisposable · 2025-12-04T08:30:45Z

usb.go

-	if err := gadget.OpenKeyboardHidFile(); err != nil {
-		usbLogger.Error().Err(err).Msg("failed to open keyboard hid file")
-	}
+	go func() {


... and the MAIN REASON we're here...

We need to do this last and we need to do two things... one, return the gadget to the main so it can .Close() it... and two.... automatically reopen the keyboard HID file (which restarts the listeners) whenever the USB goes "ready"... so effectively this loop is where we notice the plug-in and "resume/start" the handling correctly.

NOTE: the open is basically a NOP if it was already open... and this loop is what reopens it if we close it on some error... kicking things back off... so we can when too many writes timeout, or when too many reads (for the LED state) just close the whole gadget down and open 'er back up to get past them unplug/replug of the USB

IDisposable · 2025-12-04T08:32:14Z

usb_mass_storage.go

 		URL:    url,
 		Size:   n,
 	}
-	virtualMediaStateMutex.Unlock()


dangled lock if we hit any errors in this method... ugh.

IDisposable · 2025-12-04T08:33:33Z

usb_mass_storage.go

 	err := syscall.Statfs(imagesFolder, &stat)
 	if err != nil {
-		return nil, fmt.Errorf("failed to get storage stats: %v", err)
+		return nil, fmt.Errorf("failed to get storage stats: %w", err)


Errorf likes %w to know it should unwrap the error for more detail (but NOT Msgf)

IDisposable · 2025-12-04T08:34:25Z

webrtc.go


-	actionSessions++
-	return actionSessions
+func incrActiveSessions() int32 {


As noted... was seeing the session count get wonky... so just went atomic

Ported block_device/nbd Made the native CGO return distinct errors in set_edid Made the cgo_linux (go code) report C-level error return codes Use defer for server and listener cleanup. Try doing a timeboxed GracefulStop of the server

adamshiervani · 2025-12-08T20:38:35Z

Can you factor these out to their own PR:

Revamped the polling and writing of the USB gadget to correct for dropped connections

Corrected the initialization order to ensure we run even if the USB is disconnected (and we're getting power from the y-splitter/POE)

Ensured that we restart the LED listening polling

If we exceed too many consecutive write errors, close and reopen the gadget

IDisposable · 2025-12-11T11:23:05Z

Revamped the polling and writing of the USB gadget to correct for dropped connections

Corrected the initialization order to ensure we run even if the USB is disconnected (and we're getting power from the y-splitter/POE)

Ensured that we restart the LED listening polling

If we exceed too many consecutive write errors, close and reopen the gadget

It's going to be a bit... the logging changes were kind of invasive and required to actually find the bugs. I will extract the "functional" fixed when I get a bit more time.

Copilot

Pull request overview

Copilot reviewed 122 out of 123 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

internal/usbgadget/usbgadget.go:182

File handle may be writable as a result of data flow from a call to OpenFile and closing it may result in data loss upon failure, which is not handled explicitly.
internal/usbgadget/usbgadget.go:186
File handle may be writable as a result of data flow from a call to OpenFile and closing it may result in data loss upon failure, which is not handled explicitly.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

internal/usbgadget/hid_keyboard.go

IDisposable added 11 commits December 1, 2025 07:07

Pass all JETKVM_LOG_ environments to dev script

328b2d2

Move ByteSlice to general utils

b5cefcc

Also now can be logged as hex

Enhance logging

19d1ff9

Fix log level changes being reflected before a reboot Add tons of helpers for contextual logging Fix typo in relative mouse logging.

Fix failure to close & release keyboardStateCancel

2501154

Allow zerolog to serialize KeyboardState

0bea33c

Clear rollover after reporting key down state

44bbeca

Ignore invalid LED state bits

ef0d939

Hardening the hid_keyboard

7be804a

Made all call-backs go routines. Made listenKeyboardEvents callable as a go routine and returns when the keyboard file is closed/nil Cleaned up the auto-release to remove duplicate code. Moved to new logging helpers

Simplify tick handler

1f78665

Move to logging contexts

784af2c

Also restructured config_tx so it doesn't need copies of config data.

IDisposable force-pushed the fix/usb-restart branch 2 times, most recently from 3137b60 to bf436c5 Compare December 2, 2025 01:56

IDisposable force-pushed the fix/usb-restart branch 9 times, most recently from 4542b33 to 88f6b5c Compare December 3, 2025 18:24

Switched to fluent logging contexts

240e948

Add missing types for enhancing context Also cleaned up the syntax to be more compact Expose the zerolog.Event for levels Ported synctrace and timesync

IDisposable force-pushed the fix/usb-restart branch from 88f6b5c to 09eb984 Compare December 4, 2025 06:40

IDisposable requested a review from Copilot December 4, 2025 06:51

Copilot started reviewing on behalf of IDisposable December 4, 2025 06:52 View session

IDisposable commented Dec 4, 2025

View reviewed changes

Copilot finished reviewing on behalf of IDisposable December 4, 2025 06:52

IDisposable commented Dec 4, 2025

View reviewed changes

internal/hidrpc/message.go Outdated Show resolved Hide resolved

IDisposable commented Dec 4, 2025

View reviewed changes

ota.go

return nil

}

func RunAutoUpdateCheck() {

Copy link

Contributor Author

IDisposable Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved here from the main

IDisposable commented Dec 4, 2025

View reviewed changes

More modernizing logging

0a1f463

Ported block_device/nbd Made the native CGO return distinct errors in set_edid Made the cgo_linux (go code) report C-level error return codes Use defer for server and listener cleanup. Try doing a timeboxed GracefulStop of the server

IDisposable force-pushed the fix/usb-restart branch from 09eb984 to 0a1f463 Compare December 5, 2025 05:58

Merged in the recent fixes

ab10cea

IDisposable force-pushed the fix/usb-restart branch 2 times, most recently from a9d3809 to 272c0b0 Compare December 5, 2025 11:20

Make pion logger properly lazy

55b0820

IDisposable force-pushed the fix/usb-restart branch from 272c0b0 to 55b0820 Compare December 5, 2025 11:20

Switch to baseLogger for clarity

ec7c1b5

More cleanup

f8f6524

IDisposable force-pushed the fix/usb-restart branch from 8ecc02c to f8f6524 Compare December 11, 2025 10:09

IDisposable requested a review from Copilot December 11, 2025 11:20

IDisposable added component: ui dependencies Pull requests that update a dependency file labels Dec 11, 2025

Copilot started reviewing on behalf of IDisposable December 11, 2025 11:21 View session

IDisposable marked this pull request as ready for review December 11, 2025 11:23

Copilot AI reviewed Dec 11, 2025

View reviewed changes

internal/usbgadget/hid_keyboard.go Show resolved Hide resolved

Merge branch 'dev' into fix/usb-restart

4705dde

IDisposable force-pushed the fix/usb-restart branch from d50d832 to 4705dde Compare December 11, 2025 11:31

IDisposable added 2 commits December 11, 2025 09:17

More cleanup

73511fe

Fix copilot complaints

f5b6bb6

Fix USB Gadget polling/writing to recover from errors #1035

Are you sure you want to change the base?

Fix USB Gadget polling/writing to recover from errors #1035

Conversation

IDisposable commented Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamshiervani commented Dec 8, 2025

Uh oh!

IDisposable commented Dec 11, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

IDisposable commented Dec 2, 2025 •

edited

Loading