Skip to content

qui-devices: Improve partition handling #310

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.

Sign up for GitHub

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

Open
noskb wants to merge 1 commit into
base: main
Choose a base branch
from
Open

qui-devices: Improve partition handling #310

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 55 additions & 1 deletion qui/devices/device_widget.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,6 @@ def device_removed(self, vm, _event, port):
else:
# we never knew the device anyway
return

microphone = self.devices.get("mic:dom0:mic:dom0:mic::m000000", None)

self.emit_notification(
Expand All @@ -338,6 +337,48 @@ def device_removed(self, vm, _event, port):
self.cameras_to_hide.remove(dev.parent)
del self.devices[dev_id]

# If the removed device has sub-devices, update their states
if not dev.has_children:
return

parent_dev = dev
to_delete: List[str] = []
to_update: Dict[
qubesadmin.device_protocol.DeviceInfo,
List[qui.devices.backend.VM],
] = {}

vm.devices[parent_dev.device_class].clear_cache()
exposed_devices = list(
vm.devices[parent_dev.device_class]
.get_exposed_devices()
)
for dev_id, dev in self.devices.items():
if dev.parent != parent_dev.port:
continue
for exposed_dev in exposed_devices:
if str(exposed_dev.port) != dev.port:
continue
to_update[exposed_dev] = [
attachment
for attachment in dev.attachments
if dev.attachments
]
potential_new_dev_id = (
backend.Device.id_from_device(exposed_dev)
)
if dev_id != potential_new_dev_id:
to_delete.append(dev.id_string)

for dev, attachments in to_update.items():
dev_id = backend.Device.id_from_device(dev)
self.devices[dev_id] = backend.Device(dev, self)
if attachments:
for attachment in attachments:
self.devices[dev_id].attachments.add(attachment)
for dev_id in to_delete:
del self.devices[dev_id]

def initialize_dev_data(self):
# list all devices
for domain in self.qapp.domains:
Expand Down Expand Up @@ -606,7 +647,20 @@ def device_attached(self, vm, _event, device, **_kwargs):
# we don't have access to VM state
return

if not isinstance(
device, qubesadmin.device_protocol.DeviceInfo
):
device.backend_domain.devices[device.devclass].clear_cache()
Copy link
Copy Markdown
Member

@marmarta marmarta Apr 23, 2026

Choose a reason for hiding this comment

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

I am somewhat worried by the amount of clear_cache here. One, this will potentially be quire slow, I think, but two, doesn't this point to an underlying issue in core-admin-client that should be fixed instead of working around it?

Copy link
Copy Markdown
Author

@noskb noskb Apr 23, 2026

Choose a reason for hiding this comment

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

I am somewhat worried by the amount of clear_cache here. One, this will potentially be quire slow, I think,

if the backend vm is holding onto a lot of block devices ?

but two, doesn't this point to an underlying issue in core-admin-client that should be fixed instead of working around it?

I agree. As @marmarek pointed out in the first comment as well, it needs to be fixed at the root.
However, since some time has passed since this message (QubesOS/qubes-issues#10180 (comment)) of yours and there has been no progress, I wrote a workaround to be used as a temporary stopgap until that is completed. (In other words, my brain finds it hard to tinker with APIs)

Copy link
Copy Markdown
Member

@marmarta marmarta Apr 24, 2026

Choose a reason for hiding this comment

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

I am somewhat worried by the amount of clear_cache here. One, this will potentially be quire slow, I think,

if the backend vm is holding onto a lot of block devices ?

yeah, for example in the default case of sys-usb, just having a bunch of USB sticks with multiple partitions each will cause the amount of calls to skyrocket.

In general, I am very worried about putting a slow workaround to an API bug in the frontend... It is ultimately @marmarek's call here.

Copy link
Copy Markdown
Author

@noskb noskb Apr 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Your concern is entirely justified. I appreciate you taking the time to wrote a review despite your busy workload.

devices = (
device.backend_domain
.devices[device.devclass]
.get_exposed_devices()
)
for dev in devices:
if dev.port_id == device.port_id:
device = dev
dev_id = backend.Device.id_from_device(device)

if dev_id not in self.devices:
self.devices[dev_id] = backend.Device(device, self)

Expand Down