-
-
Notifications
You must be signed in to change notification settings - Fork 44
Show "List USB Devices" option if sys-usb exists #275
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
PipelineRetryFailed |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #275 +/- ##
==========================================
- Coverage 93.09% 93.02% -0.08%
==========================================
Files 64 64
Lines 13269 13269
==========================================
- Hits 12353 12343 -10
- Misses 916 926 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
eccb827
to
0029bdd
Compare
I started to try it today, seems to solve the issue |
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025091804-4.3&flavor=pull-requests Test run included the following:
New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025081011-4.3&flavor=update
Failed tests29 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/149225#dependencies 82 fixed
Unstable testsPerformance TestsPerformance degradation:12 performance degradations
Remaining performance tests:161 tests
|
qui/devices/device_widget.py
Outdated
wrapped_vm = backend.VM(vm) | ||
if wrapped_vm == self.sysusb: | ||
self.sysusb.is_running = False | ||
if _is_usbvm(vm): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it wouldn't be faster if we cached the result of this function in the wrapped_vm object. Mostly because iterating over devices can sometimes be very slow. There's a couple of places where you run this, and I think it might work for all of them to turn this into a property on the wrapped_vm object?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it wouldn't be faster if we cached the result of this function in the wrapped_vm object. Mostly because iterating over devices can sometimes be very slow. There's a couple of places where you run this, and I think it might work for all of them to turn this into a property on the wrapped_vm object?
This helper function is used mainly in 4 places:
- At the Qui Widget initialization. It is necessary to check all (non-running) qubes anyways.
- When PCI device is assigned to qube (and only if the qube is not already a dorman USBVM). Caching will not help here.
- When PCI device is unassigned from a qube. Again, caching will not help here since it is necessary to assure if the qube has still a USB PCI controller attached to it.
- At qube shutdown. This is the only place which would benefit from caching. I will look to see how to implement it. Maybe it would be better to do it as a property for
wrapped_vm
object. Or maybe anotheractive_usbvms
set would be better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is ready. I resolved this by having two sets of active and dormant usbvms.
0029bdd
to
6b3ef85
Compare
064c9bb
to
230f2a9
Compare
719c013
to
541718f
Compare
qui/devices/device_widget.py
Outdated
# assigned. Cheers! | ||
return | ||
|
||
def pci_action(self, vm, _event, _device, **_kwargs): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The device
arg is a keyword argument, so it cannot have a different name. But since you are not using it, you can simply omit it (**_kwargs
will get it).
The way it's now fails with:
Sep 16 21:00:21 dom0 widget-wrapper[19607]: Failed to handle event: sys-usb-clone, device-assign:pci, {'device': dom0+00_0d.0:0x8086:0x9a13::p0c0330, 'options': "{'no-strict-reset': 'True'}"}
Sep 16 21:00:21 dom0 widget-wrapper[19607]: Traceback (most recent call last):
Sep 16 21:00:21 dom0 widget-wrapper[19607]: File "/usr/lib/python3.13/site-packages/qubesadmin/events/__init__.py", line 278, in handle
Sep 16 21:00:21 dom0 widget-wrapper[19607]: handler(subject, event, **kwargs)
Sep 16 21:00:21 dom0 widget-wrapper[19607]: ~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^
Sep 16 21:00:21 dom0 widget-wrapper[19607]: TypeError: DevicesTray.pci_action() missing 1 required positional argument: '_device'
Sep 16 21:00:21 dom0 widget-wrapper[19607]: Failed to handle event: sys-usb-clone, device-assign:pci, {'device': dom0+00_0d.2:0x8086:0x9a1b::p0c0340, 'options': "{'no-strict-reset': 'True'}"}
Sep 16 21:00:21 dom0 widget-wrapper[19607]: Traceback (most recent call last):
Sep 16 21:00:21 dom0 widget-wrapper[19607]: File "/usr/lib/python3.13/site-packages/qubesadmin/events/__init__.py", line 278, in handle
Sep 16 21:00:21 dom0 widget-wrapper[19607]: handler(subject, event, **kwargs)
Sep 16 21:00:21 dom0 widget-wrapper[19607]: ~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^
Sep 16 21:00:21 dom0 widget-wrapper[19607]: TypeError: DevicesTray.pci_action() missing 1 required positional argument: '_device'
Sep 16 21:00:21 dom0 widget-wrapper[19607]: Failed to handle event: sys-usb-clone, device-assign:pci, {'device': dom0+00_0d.3:0x8086:0x9a1d::p0c0340, 'options': "{'no-strict-reset': 'True'}"}
Sep 16 21:00:21 dom0 widget-wrapper[19607]: Traceback (most recent call last):
Sep 16 21:00:21 dom0 widget-wrapper[19607]: File "/usr/lib/python3.13/site-packages/qubesadmin/events/__init__.py", line 278, in handle
Sep 16 21:00:21 dom0 widget-wrapper[19607]: handler(subject, event, **kwargs)
Sep 16 21:00:21 dom0 widget-wrapper[19607]: ~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^
Sep 16 21:00:21 dom0 widget-wrapper[19607]: TypeError: DevicesTray.pci_action() missing 1 required positional argument: '_device'
Sep 16 21:00:21 dom0 widget-wrapper[19607]: Failed to handle event: sys-usb-clone, device-assign:pci, {'device': dom0+00_14.0:0x8086:0xa0ed::p0c0330, 'options': "{'no-strict-reset': 'True'}"}
Sep 16 21:00:21 dom0 widget-wrapper[19607]: Traceback (most recent call last):
Sep 16 21:00:21 dom0 widget-wrapper[19607]: File "/usr/lib/python3.13/site-packages/qubesadmin/events/__init__.py", line 278, in handle
Sep 16 21:00:21 dom0 widget-wrapper[19607]: handler(subject, event, **kwargs)
Sep 16 21:00:21 dom0 widget-wrapper[19607]: ~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^
Sep 16 21:00:21 dom0 widget-wrapper[19607]: TypeError: DevicesTray.pci_action() missing 1 required positional argument: '_device'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
cf0df31
to
fb275e9
Compare
black complains (this repo is an exception that has lines of 88 chars - see README) |
other than black, it's okay now |
Also show individual menu items if user has more than one usbvm resolves: QubesOS/qubes-issues#10131 resolves: QubesOS/qubes-issues#10202 resolves: QubesOS/qubes-issues#10216
fb275e9
to
09bb5ab
Compare
This is also done. I wonder why the CI check for black is allowed to fail. If it is important, it should not be allowed to fail IMHO. |
resolves: QubesOS/qubes-issues#10131
resolves: QubesOS/qubes-issues#10202
resolves: QubesOS/qubes-issues#10216