Storage in R4.0 fixes/improvements/missing parts

[marmarek]

Most of those are just minor things, so lets collect them in a single ticket to not trash github-issues repo.

• qubes-lvm is already running as root, no need to use sudo internally; and BTW it isn't possible to allow non-root user managing LVM: besides access to various files, /dev/mapper/control driver have hardcoded check for CAP_SYS_ADMIN
• qubes-lvm should be as minimal/fast as possible - it is called multiple times during some time-critical actions (like DispVM startup); for example it should not load qubes.xml, nor even import qubes
• vm.storage.stop() should not be called in vm.shutdown() - VM is still running at that time (system inside is shutting down), even vm.kill() isn't good idea - VM can always shutdown itself (like when someone run poweroff inside); it needs to be done from some real shutdown event handler. Libvirt 2.2 is going to support something like this (/etc/libvirt/hooks/libxl), but for now all we have is block devices hotplug scripts. Libvirt 2.2 is going to be released somewhere in September, so we can wait for it.
• ThinPool do not implement resize method
• qvm-block (or any other tool) do not support resizing volumes; not sure if that should be qvm-block work, but surely it should be available somewhere
• qvm-block ls list internal VM volumes (even without --internal flag), if VM is created with non-default pool
• qubes-lvm (or any other storage utility) should not parse volumes content in any way; I see there unused function lvm_image_changed, which calls tune2fs on given volume
• VMs are attached to "origin" LVM volume, instead of snapshot created for performing changes
• volatile volumes may be created just before VM startup - no need to create them at VM creation - will be removed anyway
• ~~~handling extra volumes (not exposed by other VM, but created in some existing pool) is kind of broken: 1) no single API to create such volume (needs manual filling vm.storage.pool, vm.volumes etc), 2) loading such volumes from qubes.xml (mentioned in qubes.xml, but not in initial vm.volume_config) fails~~~ not supposed to work, devices API should be used instead
• LVM snapshots are mishandled in many ways:
  • "-back" volume is created as snapshot of "new" data, not the old one,
  • "-snap" volumes are inconsistently created/deleted (for example created on domain creation only to be removed just before VM start)
  • "-snap" volume used even when no snapshot on start was requested (volume.snap_on_start=False)
• qvm-block ls when called without particular VM, list empty metadata for internal volumes: first of all those volumes should be hidden by default (without -i flag), but even when listed, should have domain and volume name filled
• volumes are not updated when VM template is changed - vm.volumes['root'].source still point at old template's root.
• VM rename is broken in many ways:
  • volume.vid is updated, but volume.path is not (both file and lvm pools)
  • volume.source is not updated if source.vid is changed (like template rename)

I'll update this list with further issues when found.
/cc @kalkin

[marmarek]

As for qubes-lvm performance - half of its running time is used for just python startup + imports. For me it means it shouldn't be a separate script, but a function which calls appropriate tools (through sudo). This means it can't use python lvm API, but it looks it is used only for volume/pool existence checks, so not a big deal to rewrite it to parse lvdisplay -c (or similar) output.

[kalkin]

• qvm-block extend & lvm resize PR: Add qvm-block extend qubes-core-admin#52
• Volume add size property wrapper qubes-core-admin#54

[kalkin]

• qvm-block ls PR Fix qvm-block lists internal volumes on non default pools qubes-core-admin#53

[jpouellet]

• pools' import_data() must not naively return the existing backend unchanged and just dd over it because:
  1. A management VM could import a tiny filesystem to the beginning of the disk containing just a .bashrc which reads the rest of the raw blocks and exfiltrates them. This violates the "mgmt should be able to enforce policy but not read data" principle we are aiming for.
  2. Even if the writing VM is not malicious and writes the whole disk, conv=sparse means old blocks could remain in now-zero holes and cause problems. See [Contribution] qubes-incremental-backup-poc OR Wyng backup #858 (comment) for more info
  3. Importing a smaller-utilization filesystem over a larger-utilization one does not reclaim the blocks that are now unused but still allocated in the backend, causing higher long-term disk utilization.

[marmarek]

All points handled.