Use new API in the Devices Widget #267
Conversation
|
tests fail because of QubesOS/qubes-core-admin-client#362 |
|
(will fix pylint promptly) |
marmarta
changed the title
This comment was marked as resolved.
This comment was marked as resolved.
marmarta
force-pushed
the
device_widget_upgrade
branch
from
053f63f to
2703b5b
Compare
marmarta
changed the title
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025071903-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=2025061004-4.3&flavor=update
Failed tests10 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/142375#dependencies 10 fixed
Unstable testsPerformance TestsPerformance degradation:9 performance degradations
Remaining performance tests:63 tests
|
There was a problem hiding this comment.
When attaching a device, it issues a call with full port_id:device_id, but when detaches - it issues a call with * as device_id - why the difference? This is especially problematic when using sys-gui, as * is not allowed in the qrexec call argument. When issuing a call without device_id, simply use just the port_id (but that's probably in core-admin-client repo, not here).
qui/devices/backend.py
Outdated
| If it is not, add it. | ||
| """ | ||
| feature = self._vm.features.get(feature_name, "") | ||
| all_devs: List[str] = [f for f in feature.split(",") if f] |
There was a problem hiding this comment.
It will be safer to use space as a separator. While comma currently isn't part of device ID, it may be part of some other identifier (or maybe device id of some future class?). Space is safe, as it's forbidden also in qrexec argument, and device ID need to be compatible with that.
| for dev in self.devices.values(): | ||
| dev.devices_to_attach_with_me = [] | ||
| dev.hide_this_device = False | ||
| dev.show_children = True |
There was a problem hiding this comment.
This looks to clear info related to all domains, and yet with non-None vm it will re-populate it only for a given VM. So changing a feature on one VM will cause the widget to forget about settings for other VMs...
| [dev for dev in mic_feature.split(",") if dev] | ||
| ) | ||
|
|
||
| microphone.devices_to_attach_with_me = [] |
qui/devices/device_widget.py
Outdated
| self.devices[dev].devices_to_attach_with_me = [microphone] | ||
| microphone.devices_to_attach_with_me.append(self.devices[dev]) | ||
|
|
||
| self.parent_ids_to_hide = [] |
I think this is how the API works, see https://github.com/QubesOS/qubes-core-admin-client/blob/main/qubesadmin/tests/devices.py#L169 I don't think I should change it here, and I don't think I could without some painful acrobatics and working around the API... |
marmarta
force-pushed
the
device_widget_upgrade
branch
from
2703b5b to
96ddadd
Compare
|
I think I fixed all the problems, but this was not yet tested for hiding child devices (due to how last one we attached a usb stick to a test laptop it stopped booting) |
marmarta
force-pushed
the
device_widget_upgrade
branch
2 times, most recently
from
e0b775c to
783753c
Compare
|
rewrote some code to be faster and batch some slow operations together. the tests fail due to QubesOS/qubes-core-admin-client#362 |
- icons appropriate for category - show/hide child devices - connect with mic - open global config fixes QubesOS/qubes-issues#6811 fixes QubesOS/qubes-issues#8537
marmarta
force-pushed
the
device_widget_upgrade
branch
from
783753c to
3346912
Compare
|
(not a real change, added some issue references to commit messages) |
qui/devices/device_widget.py
Outdated
| def device_list_update(self, vm, _event, **_kwargs): | ||
|
|
||
| changed_devices: Dict[str, backend.Device] = {} | ||
| def _update_queue(self, vm, device, **_kwargs): |
There was a problem hiding this comment.
that "device" parameter is actually an event name (device-list-change:usb for example), not the device.
qui/devices/device_widget.py
Outdated
| asyncio.create_task(self.update_parents(vm)) | ||
| if device not in self.dev_update_queue: | ||
| self.dev_update_queue.add(device) | ||
| asyncio.create_task(self.update_assignments()) |
There was a problem hiding this comment.
shouldn't this pass the device class to the function (from the event name)? then you could refresh devices only of that type
|
Note to self: update qrexec policy for sys-gui to allow new features. |
All relevant information should happen in device-added and device-removed events.
marmarta
force-pushed
the
device_widget_upgrade
branch
from
098d1d4 to
3710003
Compare
|
requested changes done, ghost exorcised (hopefully) |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #267 +/- ##
==========================================
- Coverage 92.99% 92.94% -0.06%
==========================================
Files 64 64
Lines 13034 13063 +29
==========================================
+ Hits 12121 12141 +20
- Misses 913 922 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
fixes QubesOS/qubes-issues#6811
fixes QubesOS/qubes-issues#8537
fixes QubesOS/qubes-issues#7612