On 6/22/22 23:26, Jonathon Jongsma wrote: > Hi guys, > > I've been working on adding support for nbdkit to libvirt for network > storage sources like http and ssh. See > https://bugzilla.redhat.com/show_bug.cgi?id=2016527 for more > information, but the summary is that RHEL does not want to ship the qemu > storage plugins for curl and ssh. Handling them outside of the qemu > process provides several advantages such as reduced attack surface and > stability. > > I have something that works for me, but as I have not dealt with the > storage stuff much before, I have a feeling that I'm missing some > things. > > A quick summary of the code: > - at startup I query to see whether nbdkit exists on the host and if > so, I query which plugins/filters are installed. This is stored as > qemuNbdkitCaps on the qemu driver > - When the driver prepares the domain, we go through each disk source > and determine whether the nbdkit capabilities allow us to support > this disk via nbdkit, and if so, we allocate a qemuNbdkitProcess > object and stash it in the private data of the virStorageSource. > - The presence or absence of this qemuNbdkitProcess data then indicates > whether this disk will be served to qemu indirectly via nbdkit or not > - When we launch the qemuProcess, as part of the "external device > start" step, I launch a ndkit process for each disk that is supported > by nbdkit. I also optionally fork a child process to communicate > authentication details and cookies to the nbdkit process via a unix > socket. > - for devices which are served by the ndkit process, I change the qemu > commandline in the following ways: > - I no longer pass auth/cookie secrets to qemu (those are handled by > nbdkit) > - I replace the actual network URL of the remote disk source with the > path to the nbdkit unix socket > > Known shortcomings > - I don't yet re-query for nbdkit / nbdkit caps, so need to restart libvirt to > pick up newly-installed nbdkit or additional capabilities There are couple of ways to solve this: 1) mimic what we do for dnsmasq: basically, when initializing the bridge driver we query dnsmasq for caps networkStateInitialize() -> dnsmasqCapsNewFromBinary() and when needing the caps for real (i.e. spawning dnsmasq when starting a network) we forcefully refresh the caps: networkStartDhcpDaemon() -> networkDnsmasqCapsRefresh(). 2) use full blown virFileCache() machinery which reacts to mtime of given binary. An argument against going with 1) is that while spawning an extra process (dnsmasq) when starting a network is acceptable (a network is not started that often), spawning an extra process for each disk (or even an image in backingchain) might be too expensive. > - testing is pretty limited at the moment > - selinux not working yet > - creating disks isn't supported, though Rich has added some support > for that upstream in the nbdkit ssh plugin. > > I'd appreciate feedback on what i've got so far. > > Jonathon Jongsma (3): > docs: clarify 'readahead' and 'timeout' for disks > schema: Be more flexible for diskSourceNetworkProtocolPropsCommon > WIP: use nbdkit for remote disk sources > > docs/formatdomain.rst | 10 +- > include/libvirt/virterror.h | 1 + > po/POTFILES | 1 + > src/conf/schemas/domaincommon.rng | 34 +- > src/qemu/meson.build | 1 + > src/qemu/qemu_block.c | 64 +- > src/qemu/qemu_block.h | 1 + > src/qemu/qemu_command.c | 26 +- > src/qemu/qemu_conf.c | 19 + > src/qemu/qemu_conf.h | 5 + > src/qemu/qemu_domain.c | 110 ++- > src/qemu/qemu_domain.h | 5 + > src/qemu/qemu_driver.c | 4 +- > src/qemu/qemu_extdevice.c | 25 + > src/qemu/qemu_nbdkit.c | 629 ++++++++++++++++++ > src/qemu/qemu_nbdkit.h | 89 +++ > src/qemu/qemu_validate.c | 22 +- > src/qemu/qemu_validate.h | 4 +- > src/util/virerror.c | 1 + > tests/qemublocktest.c | 8 +- > tests/qemustatusxml2xmldata/modern-in.xml | 1 - > ...sk-cdrom-network-nbdkit.x86_64-latest.args | 42 ++ > .../disk-cdrom-network-nbdkit.xml | 1 + > ...isk-network-http-nbdkit.x86_64-latest.args | 45 ++ > .../disk-network-http-nbdkit.xml | 1 + > ...work-source-curl-nbdkit.x86_64-latest.args | 49 ++ > .../disk-network-source-curl-nbdkit.xml | 1 + > ...isk-network-source-curl.x86_64-latest.args | 53 ++ > .../disk-network-source-curl.xml | 71 ++ > tests/qemuxml2argvtest.c | 12 + > tests/testutilsqemu.c | 16 + > tests/testutilsqemu.h | 4 + > 32 files changed, 1302 insertions(+), 53 deletions(-) > create mode 100644 src/qemu/qemu_nbdkit.c > create mode 100644 src/qemu/qemu_nbdkit.h > create mode 100644 tests/qemuxml2argvdata/disk-cdrom-network-nbdkit.x86_64-latest.args > create mode 120000 tests/qemuxml2argvdata/disk-cdrom-network-nbdkit.xml > create mode 100644 tests/qemuxml2argvdata/disk-network-http-nbdkit.x86_64-latest.args > create mode 120000 tests/qemuxml2argvdata/disk-network-http-nbdkit.xml > create mode 100644 tests/qemuxml2argvdata/disk-network-source-curl-nbdkit.x86_64-latest.args > create mode 120000 tests/qemuxml2argvdata/disk-network-source-curl-nbdkit.xml > create mode 100644 tests/qemuxml2argvdata/disk-network-source-curl.x86_64-latest.args > create mode 100644 tests/qemuxml2argvdata/disk-network-source-curl.xml >