-
-
Notifications
You must be signed in to change notification settings - Fork 41
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
device identity #217
device identity #217
Conversation
only dependency ( |
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 will also today test it on a test laptop for a more meaningful comments on the UI
qui/devices/backend.py
Outdated
@@ -231,8 +233,9 @@ def attach_to_vm(self, vm: VM): | |||
Perform attachment to provided VM. | |||
""" | |||
try: | |||
assignment = qubesadmin.device_protocol.DeviceAssignment( | |||
self.backend_domain, self.id_string) | |||
assignment = DeviceAssignment(VirtualDevice(Port( |
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 think this needs a bit better documentation - this is a pretty opaque way of making a device assignment and a comment would be nice here :)
@@ -124,7 +124,8 @@ def device_list_update(self, vm, _event, **_kwargs): | |||
try: | |||
for devclass in DEV_TYPES: | |||
for device in vm.devices[devclass]: | |||
changed_devices[str(device)] = backend.Device(device, self) | |||
changed_devices[str(device.port)] = backend.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.
if the variable changed_devices is now a dict that maps port to device, that should be reflected in the name or at least in comments
@@ -172,7 +173,8 @@ def initialize_dev_data(self): | |||
for devclass in DEV_TYPES: | |||
try: | |||
for device in domain.devices[devclass]: | |||
self.devices[str(device)] = backend.Device(device, self) | |||
self.devices[str(device.port)] = backend.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.
same here - it's very non-obvious that devices are indexed by port
So, I found a double bug. To reproduce:
Result:
after I restarted qubesd, the domain started but the device did not get attached. I also can't seem to ever get the confirmation popup. |
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2024111612-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=2024091704-4.3&flavor=update
Failed tests5 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/112766#dependencies 201 fixed
Unstable tests
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #217 +/- ##
=======================================
Coverage 93.72% 93.72%
=======================================
Files 57 57
Lines 10845 10845
=======================================
Hits 10165 10165
Misses 680 680
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
implements QubesOS/qubes-issues/issues/9325