stable-id: use description as an alias for persistent device

[easydozen]

QubesOS/qubes-issues/issues/2272

[codecov]

Codecov Report

Merging #388 (7ad955b) into master (d3d6b9d) will decrease coverage by 0.02%.
The diff coverage is 33.33%.

@@            Coverage Diff             @@
##           master     #388      +/-   ##
==========================================
- Coverage   65.89%   65.86%   -0.03%     
==========================================
  Files          53       53              
  Lines        9926     9935       +9     
==========================================
+ Hits         6541     6544       +3     
- Misses       3385     3391       +6     

Flag	Coverage Δ
unittests	65.86% <33.33%> (-0.03%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
qubes/vm/__init__.py	81.17% <33.33%> (-1.76%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d3d6b9d...7ad955b. Read the comment docs.

[marmarek]

I'm afraid it isn't going to work. At the qubes.xml load time (qubesd service start), the backend domain most likely is not started yet, which means the description is not yet available.
The alias resolution needs to happen later - when the description (if using that as an alias) is already available. I think a better place would be when backend announces the device (device-list-change:... event, although it is quite clumsy - QubesOS/qubes-issues#4626) and maybe on domain start?

Another issue is ident uniqueness - this change basically overrides ident value. But the ident is still used in quite a lot of places, at the very least as a part of the key in qubes.devices.PersistentCollection. This means, you cannot connect two different devices with the same ident - which normally makes sense. But with this change, you potentially would do that. For example sys-usb:sda with alias "ABC" and sys-usb:sda with alias "DEF" - if both devices are present, then aliases should resolve to actual (unique) devices. But if one is missing, it will stay at sys-usb:sda, which may conflict with the other one - either failing the VM load, or loosing info about one of those devices.

I think a proper solution at the qubesd API would be an "alias" attribute as an alternative to ident, not an extra property, and changed consistently across the code base. Including safeguards like treating unresolved aliases as UnknownDevice (as it is done for unresolved ident), preventing setting both "ident" and "alias", detecting duplicated aliases etc.
But I'm not sure how that should influence admin API - perhaps it could stay the way it is (an "alias" option)?

[marmarek]

This is a bad idea - initially events are disabled, because events handler can assume VM object being fully initialized - which isn't the case here yet.

[easydozen]

OK, Now I see why this issue stalled for so long.
I'm almost losing hope to avoid invasive changes, but please, help me in checking another one idea that looks like simple solution. What could go wrong if we will extend qubesdb path and instead of implementing new 'alias' entity will write device UUID/label/path, depending on udev rules at the backend side?
For example qubes-block-devices/sda -> qubes-block-devices/ae7f5c7d-a8ed-42ff-b81c-32a350411e26
If there is no applicable device info or qubesdb path already exist then we can use default fallback ident.

[marmarek]

Yes, something like this can work, I think.

One thing is detecting whether two devices aren't in fact the same device. Consider one device attached as sys-usb:sda and another as sys-usb:ae7f5c7d-a8ed-42ff-b81c-32a350411e26. You need to ensure they won't be attached at the same time (which BTW is currently broken QubesOS/qubes-issues#4692).
I think qubes core needs to be aware of the actual device path, which may even be a must to properly attach the device (I'm not sure if /dev/disk/by-uuid/* paths will work here). This part should be quite easy - just add another entry to the device description in qubesdb with the actual device name and use that when constructing libvirt xml fragment.

In fact, it may be a good idea to expose the same device multiple times using various identifiers (device name, UUID, something else) and explicitly mark them as aliases for the same device (see above). The UI will need to be aware of that (displaying all of them may be confusing), but on the other hand, it will give some more flexibility. IMO the part "this device is in fact an alias for another device" is worth a property in the base DeviceInfo class, not just something block-specific.
I think "connect sys-usb:sda, whatever that is" is still a perfectly valid use case. This is even more appealing for USB device IDs, which are basically port numbers - you can say "connect whatever I've plugged to the first port on the left to VM X".

And some related rambling:

Another thing is UUID stability. It is normally is retrieved from filesystem on the block device, so if you re-format it, the UUID will change. This could still work, but you need to be sure to not detach device in such case automatically (or not pretend it's not attached). Likely loading UUID only at the device initial discovery time and not update it later will work. It shouldn't be a problem, but I want to mention it explicitly.

One minor issue is UX one - long device identifiers will make both CLI and GUI quite ugly. I'm not sure what's the best option here, but also IMO it doesn't invalidate your proposed change.

Also, note the qrexec argument length limit (relevant if you do qvm-block from non-dom0 mgmt vm) is quite short in Qubes 4.0. But this isn't an issue in Qubes 4.1 anymore.