There was support in the code for parsing protocol='ssh' on network disk sources, but it was not present in the xml schema. Add this to the schema. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/conf/schemas/domaincommon.rng | 1 + tests/qemublocktest.c | 2 +- ...w2-invalid.json => network-ssh-qcow2.json} | 0 ...cow2-invalid.xml => network-ssh-qcow2.xml} | 0 .../disk-network-ssh.x86_64-latest.args | 36 +++++++++++++++++++ tests/qemuxml2argvdata/disk-network-ssh.xml | 31 ++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 7 files changed, 70 insertions(+), 1 deletion(-) rename tests/qemublocktestdata/imagecreate/{network-ssh-qcow2-invalid.json => network-ssh-qcow2.json} (100%) rename tests/qemublocktestdata/imagecreate/{network-ssh-qcow2-invalid.xml => network-ssh-qcow2.xml} (100%) create mode 100644 tests/qemuxml2argvdata/disk-network-ssh.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-network-ssh.xml diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index a57dd212ab..f41de89af9 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -2167,6 +2167,7 @@ <choice> <value>sheepdog</value> <value>tftp</value> + <value>ssh</value> </choice> </attribute> <attribute name="name"/> diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 010b52f4b3..1c1013d4d9 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -1211,7 +1211,7 @@ mymain(void) TEST_IMAGE_CREATE("network-gluster-qcow2", NULL); TEST_IMAGE_CREATE("network-rbd-qcow2", NULL); - TEST_IMAGE_CREATE("network-ssh-qcow2-invalid", NULL); + TEST_IMAGE_CREATE("network-ssh-qcow2", NULL); #define TEST_BITMAP_DETECT(testname) \ do { \ diff --git a/tests/qemublocktestdata/imagecreate/network-ssh-qcow2-invalid.json b/tests/qemublocktestdata/imagecreate/network-ssh-qcow2.json similarity index 100% rename from tests/qemublocktestdata/imagecreate/network-ssh-qcow2-invalid.json rename to tests/qemublocktestdata/imagecreate/network-ssh-qcow2.json diff --git a/tests/qemublocktestdata/imagecreate/network-ssh-qcow2-invalid.xml b/tests/qemublocktestdata/imagecreate/network-ssh-qcow2.xml similarity index 100% rename from tests/qemublocktestdata/imagecreate/network-ssh-qcow2-invalid.xml rename to tests/qemublocktestdata/imagecreate/network-ssh-qcow2.xml diff --git a/tests/qemuxml2argvdata/disk-network-ssh.x86_64-latest.args b/tests/qemuxml2argvdata/disk-network-ssh.x86_64-latest.args new file mode 100644 index 0000000000..045474724b --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-ssh.x86_64-latest.args @@ -0,0 +1,36 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram \ +-accel kvm \ +-cpu qemu64 \ +-m 214 \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ +-blockdev '{"driver":"ssh","path":"test.img","server":{"host":"example.org","port":"2222"},"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \ +-device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x2","drive":"libvirt-1-format","id":"virtio-disk0","bootindex":1}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/disk-network-ssh.xml b/tests/qemuxml2argvdata/disk-network-ssh.xml new file mode 100644 index 0000000000..355add4fea --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-ssh.xml @@ -0,0 +1,31 @@ +<domain type='kvm'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='ssh' name='test.img'> + <host name='example.org' port='2222'/> + <timeout seconds='1234'/> + <readahead size='1024'/> + </source> + <target dev='vda' bus='virtio'/> + </disk> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ae0afb6f1c..8df6aaa575 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1301,6 +1301,7 @@ mymain(void) DO_TEST_CAPS_LATEST("disk-network-tlsx509-nbd-hostname"); DO_TEST_CAPS_VER("disk-network-tlsx509-vxhs", "5.0.0"); DO_TEST_CAPS_LATEST("disk-network-http"); + DO_TEST_CAPS_LATEST("disk-network-ssh"); driver.config->vxhsTLS = 0; VIR_FREE(driver.config->vxhsTLSx509certdir); DO_TEST_CAPS_LATEST("disk-no-boot"); -- 2.39.1

In order to add caching of the nbdkit capabilities, we will need to compare against file modification times, etc. So look up this information when creating the nbdkit caps. Add a nbdkit_moddir build option to allow the builder to specify the location to look for nbdkit plugins and filters. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_nbdkit.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c index 9ff293698d..486df8c161 100644 --- a/src/qemu/qemu_nbdkit.c +++ b/src/qemu/qemu_nbdkit.c @@ -20,6 +20,7 @@ #include <config.h> #include <glib.h> +#include "configmake.h" #include "vircommand.h" #include "virerror.h" #include "virlog.h" @@ -39,6 +40,10 @@ VIR_LOG_INIT("qemu.nbdkit"); +#define NBDKIT_MODDIR LIBDIR "/nbdkit" +#define NBDKIT_PLUGINDIR NBDKIT_MODDIR "/plugins" +#define NBDKIT_FILTERDIR NBDKIT_MODDIR "/filters" + VIR_ENUM_IMPL(qemuNbdkitCaps, QEMU_NBDKIT_CAPS_LAST, /* 0 */ @@ -52,6 +57,11 @@ struct _qemuNbdkitCaps { char *path; char *version; + time_t ctime; + time_t libvirtCtime; + time_t pluginDirMtime; + time_t filterDirMtime; + unsigned int libvirtVersion; virBitmap *flags; }; @@ -176,9 +186,41 @@ qemuNbdkitCapsNew(const char *path) } +static time_t +qemuNbdkitGetDirMtime(const char *moddir) +{ + struct stat st; + + if (stat(moddir, &st) < 0) { + VIR_DEBUG("Failed to stat nbdkit module directory '%s': %s", + moddir, + g_strerror(errno)); + return 0; + } + + return st.st_mtime; +} + + G_GNUC_UNUSED static void qemuNbdkitCapsQuery(qemuNbdkitCaps *caps) { + struct stat st; + + if (stat(caps->path, &st) < 0) { + VIR_DEBUG("Failed to stat nbdkit binary '%s': %s", + caps->path, + g_strerror(errno)); + caps->ctime = 0; + return; + } + + caps->ctime = st.st_ctime; + caps->filterDirMtime = qemuNbdkitGetDirMtime(NBDKIT_FILTERDIR); + caps->pluginDirMtime = qemuNbdkitGetDirMtime(NBDKIT_PLUGINDIR); + caps->libvirtCtime = virGetSelfLastChanged(); + caps->libvirtVersion = LIBVIR_VERSION_NUMBER; + qemuNbdkitCapsQueryPlugins(caps); qemuNbdkitCapsQueryFilters(caps); qemuNbdkitCapsQueryVersion(caps); -- 2.39.1

Preparatory step for caching nbdkit capabilities. This patch implements the newData and isValid virFileCacheHandlers callback functions. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_nbdkit.c | 89 +++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_nbdkit.h | 4 ++ 2 files changed, 92 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c index 486df8c161..cd6fe41910 100644 --- a/src/qemu/qemu_nbdkit.c +++ b/src/qemu/qemu_nbdkit.c @@ -202,7 +202,7 @@ qemuNbdkitGetDirMtime(const char *moddir) } -G_GNUC_UNUSED static void +static void qemuNbdkitCapsQuery(qemuNbdkitCaps *caps) { struct stat st; @@ -241,3 +241,90 @@ qemuNbdkitCapsSet(qemuNbdkitCaps *nbdkitCaps, { ignore_value(virBitmapSetBit(nbdkitCaps->flags, flag)); } + + +static bool +virNbkditCapsCheckModdir(const char *moddir, + time_t expectedMtime) +{ + time_t mtime = qemuNbdkitGetDirMtime(moddir); + + if (mtime != expectedMtime) { + VIR_DEBUG("Outdated capabilities for nbdkit: module directory '%s' changed (%lld vs %lld)", + moddir, (long long)mtime, (long long)expectedMtime); + return false; + } + return true; +} + + +static bool +virNbdkitCapsIsValid(void *data, + void *privData G_GNUC_UNUSED) +{ + qemuNbdkitCaps *nbdkitCaps = data; + struct stat st; + + if (!nbdkitCaps->path) + return true; + + if (!virNbkditCapsCheckModdir(NBDKIT_PLUGINDIR, nbdkitCaps->pluginDirMtime)) + return false; + if (!virNbkditCapsCheckModdir(NBDKIT_FILTERDIR, nbdkitCaps->filterDirMtime)) + return false; + + if (nbdkitCaps->libvirtCtime != virGetSelfLastChanged() || + nbdkitCaps->libvirtVersion != LIBVIR_VERSION_NUMBER) { + VIR_DEBUG("Outdated capabilities for '%s': libvirt changed (%lld vs %lld, %lu vs %lu)", + nbdkitCaps->path, + (long long)nbdkitCaps->libvirtCtime, + (long long)virGetSelfLastChanged(), + (unsigned long)nbdkitCaps->libvirtVersion, + (unsigned long)LIBVIR_VERSION_NUMBER); + return false; + } + + if (stat(nbdkitCaps->path, &st) < 0) { + VIR_DEBUG("Failed to stat nbdkit binary '%s': %s", + nbdkitCaps->path, + g_strerror(errno)); + return false; + } + + if (st.st_ctime != nbdkitCaps->ctime) { + VIR_DEBUG("Outdated capabilities for '%s': nbdkit binary changed (%lld vs %lld)", + nbdkitCaps->path, + (long long)st.st_ctime, (long long)nbdkitCaps->ctime); + return false; + } + + return true; +} + + +static void* +virNbdkitCapsNewData(const char *binary, + void *privData G_GNUC_UNUSED) +{ + qemuNbdkitCaps *caps = qemuNbdkitCapsNew(binary); + qemuNbdkitCapsQuery(caps); + + return caps; +} + + +virFileCacheHandlers nbdkitCapsCacheHandlers = { + .isValid = virNbdkitCapsIsValid, + .newData = virNbdkitCapsNewData, + .loadFile = NULL, + .saveFile = NULL, + .privFree = NULL, +}; + + +virFileCache* +qemuNbdkitCapsCacheNew(const char *cachedir) +{ + g_autofree char *dir = g_build_filename(cachedir, "nbdkitcapabilities", NULL); + return virFileCacheNew(dir, "xml", &nbdkitCapsCacheHandlers); +} diff --git a/src/qemu/qemu_nbdkit.h b/src/qemu/qemu_nbdkit.h index e191e1fdb4..4aba7c8455 100644 --- a/src/qemu/qemu_nbdkit.h +++ b/src/qemu/qemu_nbdkit.h @@ -21,6 +21,7 @@ #include "internal.h" #include "virenum.h" +#include "virfilecache.h" typedef struct _qemuNbdkitCaps qemuNbdkitCaps; @@ -38,6 +39,9 @@ VIR_ENUM_DECL(qemuNbdkitCaps); qemuNbdkitCaps * qemuNbdkitCapsNew(const char *path); +virFileCache * +qemuNbdkitCapsCacheNew(const char *cachedir); + bool qemuNbdkitCapsGet(qemuNbdkitCaps *nbdkitCaps, qemuNbdkitCapsFlags flag); -- 2.39.1

Add the virFileCache implementation for nbdkit capabilities to the qemu driver. This allows us to determine whether nbdkit is installed and which plugins are supported. it also has persistent caching and the capabilities are re-queried whenever something changes. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_conf.h | 3 +++ src/qemu/qemu_driver.c | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 4f610d86a1..a44985fb8b 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -318,6 +318,9 @@ struct _virQEMUDriver { /* Immutable pointer, self-locking APIs */ virHashAtomic *migrationErrors; + + /* Immutable pointer, self-locking APIs */ + virFileCache *nbdkitCapsCache; }; virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6154fe9bfe..6ef4ae2c56 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -845,6 +845,8 @@ qemuStateInitialize(bool privileged, defsecmodel))) goto error; + qemu_driver->nbdkitCapsCache = qemuNbdkitCapsCacheNew(cfg->cacheDir); + /* If hugetlbfs is present, then we need to create a sub-directory within * it, since we can't assume the root mount point has permissions that * will let our spawned QEMU instances use it. */ @@ -1078,6 +1080,7 @@ qemuStateCleanup(void) VIR_FREE(qemu_driver->qemuImgBinary); virObjectUnref(qemu_driver->domains); virThreadPoolFree(qemu_driver->workerPool); + virObjectUnref(qemu_driver->nbdkitCapsCache); if (qemu_driver->lockFD != -1) virPidFileRelease(qemu_driver->config->stateDir, "driver", qemu_driver->lockFD); -- 2.39.1

Rather than hard-coding the nbdkit module directory, query the nbdkit binary for the location to these directories. nbdkit provides a --dump-config optiont that outputs this information and can be easily parsed. We can also get the version from this output rather than executing `nbdkit --version` separately. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_nbdkit.c | 77 ++++++++++++++++++++++++++++++++---------- 1 file changed, 60 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c index 7dcb61d9a6..840710862e 100644 --- a/src/qemu/qemu_nbdkit.c +++ b/src/qemu/qemu_nbdkit.c @@ -20,7 +20,6 @@ #include <config.h> #include <glib.h> -#include "configmake.h" #include "vircommand.h" #include "virerror.h" #include "virlog.h" @@ -40,10 +39,6 @@ VIR_LOG_INIT("qemu.nbdkit"); -#define NBDKIT_MODDIR LIBDIR "/nbdkit" -#define NBDKIT_PLUGINDIR NBDKIT_MODDIR "/plugins" -#define NBDKIT_FILTERDIR NBDKIT_MODDIR "/filters" - VIR_ENUM_IMPL(qemuNbdkitCaps, QEMU_NBDKIT_CAPS_LAST, /* 0 */ @@ -57,6 +52,9 @@ struct _qemuNbdkitCaps { char *path; char *version; + char *filterDir; + char *pluginDir; + time_t ctime; time_t libvirtCtime; time_t pluginDirMtime; @@ -130,18 +128,47 @@ qemuNbdkitCapsQueryFilters(qemuNbdkitCaps *nbdkit) static int -qemuNbdkitCapsQueryVersion(qemuNbdkitCaps *nbdkit) +qemuNbdkitCapsQueryBuildConfig(qemuNbdkitCaps *nbdkit) { + size_t i; + g_autofree char *output = NULL; + g_auto(GStrv) lines = NULL; + const char *line; g_autoptr(virCommand) cmd = virCommandNewArgList(nbdkit->path, - "--version", + "--dump-config", NULL); - virCommandSetOutputBuffer(cmd, &nbdkit->version); + virCommandSetOutputBuffer(cmd, &output); if (virCommandRun(cmd, NULL) != 0) return -1; - VIR_DEBUG("Got nbdkit version %s", nbdkit->version); + lines = g_strsplit(output, "\n", 0); + if (!lines) + return -1; + + for (i = 0; (line = lines[i]); i++) { + const char *key; + const char *val; + char *p; + + p = strchr(line, '='); + if (!p) + continue; + + *p = '\0'; + key = line; + val = p + 1; + + VIR_DEBUG("Got nbdkit config value %s=%s", key, val); + + if (STREQ(key, "version")) + nbdkit->version = g_strdup(val); + else if (STREQ(key, "filterdir")) + nbdkit->filterDir = g_strdup(val); + else if (STREQ(key, "plugindir")) + nbdkit->pluginDir = g_strdup(val); + } return 0; } @@ -153,6 +180,8 @@ qemuNbdkitCapsFinalize(GObject *object) g_clear_pointer(&nbdkit->path, g_free); g_clear_pointer(&nbdkit->version, g_free); + g_clear_pointer(&nbdkit->filterDir, g_free); + g_clear_pointer(&nbdkit->pluginDir, g_free); g_clear_pointer(&nbdkit->flags, virBitmapFree); G_OBJECT_CLASS(qemu_nbdkit_caps_parent_class)->finalize(object); @@ -215,15 +244,15 @@ qemuNbdkitCapsQuery(qemuNbdkitCaps *caps) return; } + qemuNbdkitCapsQueryBuildConfig(caps); + qemuNbdkitCapsQueryPlugins(caps); + qemuNbdkitCapsQueryFilters(caps); + caps->ctime = st.st_ctime; - caps->filterDirMtime = qemuNbdkitGetDirMtime(NBDKIT_FILTERDIR); - caps->pluginDirMtime = qemuNbdkitGetDirMtime(NBDKIT_PLUGINDIR); + caps->filterDirMtime = qemuNbdkitGetDirMtime(caps->filterDir); + caps->pluginDirMtime = qemuNbdkitGetDirMtime(caps->pluginDir); caps->libvirtCtime = virGetSelfLastChanged(); caps->libvirtVersion = LIBVIR_VERSION_NUMBER; - - qemuNbdkitCapsQueryPlugins(caps); - qemuNbdkitCapsQueryFilters(caps); - qemuNbdkitCapsQueryVersion(caps); } @@ -268,9 +297,9 @@ virNbdkitCapsIsValid(void *data, if (!nbdkitCaps->path) return true; - if (!virNbkditCapsCheckModdir(NBDKIT_PLUGINDIR, nbdkitCaps->pluginDirMtime)) + if (!virNbkditCapsCheckModdir(nbdkitCaps->pluginDir, nbdkitCaps->pluginDirMtime)) return false; - if (!virNbkditCapsCheckModdir(NBDKIT_FILTERDIR, nbdkitCaps->filterDirMtime)) + if (!virNbkditCapsCheckModdir(nbdkitCaps->filterDir, nbdkitCaps->filterDirMtime)) return false; if (nbdkitCaps->libvirtCtime != virGetSelfLastChanged() || @@ -422,12 +451,22 @@ qemuNbdkitCapsLoadCache(qemuNbdkitCaps *nbdkitCaps, } nbdkitCaps->ctime = (time_t)l; + if ((nbdkitCaps->pluginDir = virXPathString("string(./plugindir)", ctxt)) == NULL) { + VIR_DEBUG("missing plugindir in nbdkit capabilities cache"); + return -1; + } + if (virXPathLongLong("string(./plugindirmtime)", ctxt, &l) < 0) { VIR_DEBUG("missing plugindirmtime in nbdkit capabilities XML"); return -1; } nbdkitCaps->pluginDirMtime = (time_t)l; + if ((nbdkitCaps->filterDir = virXPathString("string(./filterdir)", ctxt)) == NULL) { + VIR_DEBUG("missing filterdir in nbdkit capabilities cache"); + return -1; + } + if (virXPathLongLong("string(./filterdirmtime)", ctxt, &l) < 0) { VIR_DEBUG("missing filterdirmtime in nbdkit capabilities XML"); return -1; @@ -483,8 +522,12 @@ qemuNbdkitCapsFormatCache(qemuNbdkitCaps *nbdkitCaps) nbdkitCaps->path); virBufferAsprintf(&buf, "<nbdkitctime>%lu</nbdkitctime>\n", nbdkitCaps->ctime); + virBufferEscapeString(&buf, "<plugindir>%s</plugindir>\n", + nbdkitCaps->pluginDir); virBufferAsprintf(&buf, "<plugindirmtime>%lu</plugindirmtime>\n", nbdkitCaps->pluginDirMtime); + virBufferEscapeString(&buf, "<filterdir>%s</filterdir>\n", + nbdkitCaps->filterDir); virBufferAsprintf(&buf, "<filterdirmtime>%lu</filterdirmtime>\n", nbdkitCaps->filterDirMtime); virBufferAsprintf(&buf, "<selfctime>%lu</selfctime>\n", -- 2.39.1

The only use of this function always passes QEMU_DOMAIN_LOG_CONTEXT_MODE_START. The other enum values are never used anywhere in the code. Remove them. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 26 +++++++++++--------------- src/qemu/qemu_domain.h | 9 +-------- src/qemu/qemu_process.c | 3 +-- 3 files changed, 13 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 15fb3d5b1a..4c882840b2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6878,8 +6878,7 @@ void qemuDomainObjCheckNetTaint(virQEMUDriver *driver, qemuDomainLogContext *qemuDomainLogContextNew(virQEMUDriver *driver, - virDomainObj *vm, - qemuDomainLogContextMode mode) + virDomainObj *vm) { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); qemuDomainLogContext *ctxt = QEMU_DOMAIN_LOG_CONTEXT(g_object_new(QEMU_TYPE_DOMAIN_LOG_CONTEXT, NULL)); @@ -6920,25 +6919,22 @@ qemuDomainLogContext *qemuDomainLogContextNew(virQEMUDriver *driver, /* For unprivileged startup we must truncate the file since * we can't rely on logrotate. We don't use O_TRUNC since * it is better for SELinux policy if we truncate afterwards */ - if (mode == QEMU_DOMAIN_LOG_CONTEXT_MODE_START && - !driver->privileged && + if (!driver->privileged && ftruncate(ctxt->writefd, 0) < 0) { virReportSystemError(errno, _("failed to truncate %s"), ctxt->path); goto error; } - if (mode == QEMU_DOMAIN_LOG_CONTEXT_MODE_START) { - if ((ctxt->readfd = open(ctxt->path, O_RDONLY)) < 0) { - virReportSystemError(errno, _("failed to open logfile %s"), - ctxt->path); - goto error; - } - if (virSetCloseExec(ctxt->readfd) < 0) { - virReportSystemError(errno, _("failed to set close-on-exec flag on %s"), - ctxt->path); - goto error; - } + if ((ctxt->readfd = open(ctxt->path, O_RDONLY)) < 0) { + virReportSystemError(errno, _("failed to open logfile %s"), + ctxt->path); + goto error; + } + if (virSetCloseExec(ctxt->readfd) < 0) { + virReportSystemError(errno, _("failed to set close-on-exec flag on %s"), + ctxt->path); + goto error; } if ((ctxt->pos = lseek(ctxt->writefd, 0, SEEK_END)) < 0) { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index c0bf646680..e39419e7af 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -659,15 +659,8 @@ void qemuDomainObjCheckNetTaint(virQEMUDriver *driver, virDomainNetDef *net, qemuDomainLogContext *logCtxt); -typedef enum { - QEMU_DOMAIN_LOG_CONTEXT_MODE_START, - QEMU_DOMAIN_LOG_CONTEXT_MODE_ATTACH, - QEMU_DOMAIN_LOG_CONTEXT_MODE_STOP, -} qemuDomainLogContextMode; - qemuDomainLogContext *qemuDomainLogContextNew(virQEMUDriver *driver, - virDomainObj *vm, - qemuDomainLogContextMode mode); + virDomainObj *vm); int qemuDomainLogContextWrite(qemuDomainLogContext *ctxt, const char *fmt, ...) G_GNUC_PRINTF(2, 3); ssize_t qemuDomainLogContextRead(qemuDomainLogContext *ctxt, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d4499c6f84..039624c87b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7669,8 +7669,7 @@ qemuProcessLaunch(virConnectPtr conn, hookData.cfg = cfg; VIR_DEBUG("Creating domain log file"); - if (!(logCtxt = qemuDomainLogContextNew(driver, vm, - QEMU_DOMAIN_LOG_CONTEXT_MODE_START))) { + if (!(logCtxt = qemuDomainLogContextNew(driver, vm))) { virLastErrorPrefixMessage("%s", _("can't connect to virtlogd")); goto cleanup; } -- 2.39.1

This will allow us to use it for nbdkit logging in upcoming commits. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- po/POTFILES | 1 + src/qemu/meson.build | 1 + src/qemu/qemu_domain.c | 247 ++-------------------------------- src/qemu/qemu_domain.h | 29 +--- src/qemu/qemu_logcontext.c | 264 +++++++++++++++++++++++++++++++++++++ src/qemu/qemu_logcontext.h | 38 ++++++ src/qemu/qemu_process.c | 44 +++---- 7 files changed, 347 insertions(+), 277 deletions(-) create mode 100644 src/qemu/qemu_logcontext.c create mode 100644 src/qemu/qemu_logcontext.h diff --git a/po/POTFILES b/po/POTFILES index 175b3e9fe3..43950b388b 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -173,6 +173,7 @@ src/qemu/qemu_hostdev.c src/qemu/qemu_hotplug.c src/qemu/qemu_interface.c src/qemu/qemu_interop_config.c +src/qemu/qemu_logcontext.c src/qemu/qemu_migration.c src/qemu/qemu_migration_cookie.c src/qemu/qemu_migration_params.c diff --git a/src/qemu/meson.build b/src/qemu/meson.build index 9be6996195..6d7a1bfbb0 100644 --- a/src/qemu/meson.build +++ b/src/qemu/meson.build @@ -21,6 +21,7 @@ qemu_driver_sources = [ 'qemu_hotplug.c', 'qemu_interface.c', 'qemu_interop_config.c', + 'qemu_logcontext.c', 'qemu_migration.c', 'qemu_migration_cookie.c', 'qemu_migration_params.c', diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ae1d0af6f4..c979fe0c85 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -454,21 +454,8 @@ qemuDomainObjFromDomain(virDomainPtr domain) } -struct _qemuDomainLogContext { - GObject parent; - - int writefd; - int readfd; /* Only used if manager == NULL */ - off_t pos; - ino_t inode; /* Only used if manager != NULL */ - char *path; - virLogManager *manager; -}; - -G_DEFINE_TYPE(qemuDomainLogContext, qemu_domain_log_context, G_TYPE_OBJECT); static virClass *qemuDomainSaveCookieClass; -static void qemuDomainLogContextFinalize(GObject *obj); static void qemuDomainSaveCookieDispose(void *obj); @@ -481,32 +468,8 @@ qemuDomainOnceInit(void) return 0; } -static void qemu_domain_log_context_init(qemuDomainLogContext *logctxt G_GNUC_UNUSED) -{ -} - -static void qemu_domain_log_context_class_init(qemuDomainLogContextClass *klass) -{ - GObjectClass *obj = G_OBJECT_CLASS(klass); - - obj->finalize = qemuDomainLogContextFinalize; -} - VIR_ONCE_GLOBAL_INIT(qemuDomain); -static void -qemuDomainLogContextFinalize(GObject *object) -{ - qemuDomainLogContext *ctxt = QEMU_DOMAIN_LOG_CONTEXT(object); - VIR_DEBUG("ctxt=%p", ctxt); - - virLogManagerFree(ctxt->manager); - VIR_FREE(ctxt->path); - VIR_FORCE_CLOSE(ctxt->writefd); - VIR_FORCE_CLOSE(ctxt->readfd); - G_OBJECT_CLASS(qemu_domain_log_context_parent_class)->finalize(object); -} - /* qemuDomainGetMasterKeyFilePath: * @libDir: Directory path to domain lib files * @@ -6642,7 +6605,7 @@ qemuDomainDefFormatLive(virQEMUDriver *driver, void qemuDomainObjTaint(virQEMUDriver *driver, virDomainObj *obj, virDomainTaintFlags taint, - qemuDomainLogContext *logCtxt) + qemuLogContext *logCtxt) { qemuDomainObjTaintMsg(driver, obj, taint, logCtxt, NULL); qemuDomainSaveStatus(obj); @@ -6651,7 +6614,7 @@ void qemuDomainObjTaint(virQEMUDriver *driver, void qemuDomainObjTaintMsg(virQEMUDriver *driver, virDomainObj *obj, virDomainTaintFlags taint, - qemuDomainLogContext *logCtxt, + qemuLogContext *logCtxt, const char *fmt, ...) { virErrorPtr orig_err = NULL; @@ -6704,12 +6667,12 @@ void qemuDomainObjTaintMsg(virQEMUDriver *driver, goto cleanup; if (logCtxt) { - rc = qemuDomainLogContextWrite(logCtxt, - "%s: Domain id=%d is tainted: %s%s%s%s\n", - timestamp, - obj->def->id, - virDomainTaintTypeToString(taint), - extraprefix, extramsg, extrasuffix); + rc = qemuLogContextWrite(logCtxt, + "%s: Domain id=%d is tainted: %s%s%s%s\n", + timestamp, + obj->def->id, + virDomainTaintTypeToString(taint), + extraprefix, extramsg, extrasuffix); } else { rc = qemuDomainLogAppendMessage(driver, obj, "%s: Domain id=%d is tainted: %s%s%s%s\n", @@ -6729,7 +6692,7 @@ void qemuDomainObjTaintMsg(virQEMUDriver *driver, static void qemuDomainObjCheckMachineTaint(virQEMUDriver *driver, virDomainObj *obj, - qemuDomainLogContext *logCtxt) + qemuLogContext *logCtxt) { qemuDomainObjPrivate *priv = obj->privateData; virQEMUCaps *qemuCaps = priv->qemuCaps; @@ -6747,7 +6710,7 @@ qemuDomainObjCheckMachineTaint(virQEMUDriver *driver, static void qemuDomainObjCheckCPUTaint(virQEMUDriver *driver, virDomainObj *obj, - qemuDomainLogContext *logCtxt, + qemuLogContext *logCtxt, bool incomingMigration) { qemuDomainObjPrivate *priv = obj->privateData; @@ -6779,7 +6742,7 @@ qemuDomainObjCheckCPUTaint(virQEMUDriver *driver, void qemuDomainObjCheckTaint(virQEMUDriver *driver, virDomainObj *obj, - qemuDomainLogContext *logCtxt, + qemuLogContext *logCtxt, bool incomingMigration) { size_t i; @@ -6835,7 +6798,7 @@ void qemuDomainObjCheckTaint(virQEMUDriver *driver, void qemuDomainObjCheckDiskTaint(virQEMUDriver *driver, virDomainObj *obj, virDomainDiskDef *disk, - qemuDomainLogContext *logCtxt) + qemuLogContext *logCtxt) { if (disk->rawio == VIR_TRISTATE_BOOL_YES) qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_HIGH_PRIVILEGES, @@ -6852,7 +6815,7 @@ void qemuDomainObjCheckDiskTaint(virQEMUDriver *driver, void qemuDomainObjCheckHostdevTaint(virQEMUDriver *driver, virDomainObj *obj, virDomainHostdevDef *hostdev, - qemuDomainLogContext *logCtxt) + qemuLogContext *logCtxt) { if (!virHostdevIsSCSIDevice(hostdev)) return; @@ -6865,7 +6828,7 @@ void qemuDomainObjCheckHostdevTaint(virQEMUDriver *driver, void qemuDomainObjCheckNetTaint(virQEMUDriver *driver, virDomainObj *obj, virDomainNetDef *net, - qemuDomainLogContext *logCtxt) + qemuLogContext *logCtxt) { /* script is only useful for NET_TYPE_ETHERNET (qemu) and * NET_TYPE_BRIDGE (xen), but could be (incorrectly) specified for @@ -6877,163 +6840,6 @@ void qemuDomainObjCheckNetTaint(virQEMUDriver *driver, } -qemuDomainLogContext *qemuDomainLogContextNew(virQEMUDriver *driver, - virDomainObj *vm, - const char *basename) -{ - g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - qemuDomainLogContext *ctxt = QEMU_DOMAIN_LOG_CONTEXT(g_object_new(QEMU_TYPE_DOMAIN_LOG_CONTEXT, NULL)); - - VIR_DEBUG("Context new %p stdioLogD=%d", ctxt, cfg->stdioLogD); - ctxt->writefd = -1; - ctxt->readfd = -1; - - ctxt->path = g_strdup_printf("%s/%s.log", cfg->logDir, basename); - - if (cfg->stdioLogD) { - ctxt->manager = virLogManagerNew(driver->privileged); - if (!ctxt->manager) - goto error; - - ctxt->writefd = virLogManagerDomainOpenLogFile(ctxt->manager, - "qemu", - vm->def->uuid, - vm->def->name, - ctxt->path, - 0, - &ctxt->inode, - &ctxt->pos); - if (ctxt->writefd < 0) - goto error; - } else { - if ((ctxt->writefd = open(ctxt->path, O_WRONLY | O_CREAT | O_APPEND, S_IRUSR | S_IWUSR)) < 0) { - virReportSystemError(errno, _("failed to create logfile %s"), - ctxt->path); - goto error; - } - if (virSetCloseExec(ctxt->writefd) < 0) { - virReportSystemError(errno, _("failed to set close-on-exec flag on %s"), - ctxt->path); - goto error; - } - - /* For unprivileged startup we must truncate the file since - * we can't rely on logrotate. We don't use O_TRUNC since - * it is better for SELinux policy if we truncate afterwards */ - if (!driver->privileged && - ftruncate(ctxt->writefd, 0) < 0) { - virReportSystemError(errno, _("failed to truncate %s"), - ctxt->path); - goto error; - } - - if ((ctxt->readfd = open(ctxt->path, O_RDONLY)) < 0) { - virReportSystemError(errno, _("failed to open logfile %s"), - ctxt->path); - goto error; - } - if (virSetCloseExec(ctxt->readfd) < 0) { - virReportSystemError(errno, _("failed to set close-on-exec flag on %s"), - ctxt->path); - goto error; - } - - if ((ctxt->pos = lseek(ctxt->writefd, 0, SEEK_END)) < 0) { - virReportSystemError(errno, _("failed to seek in log file %s"), - ctxt->path); - goto error; - } - } - - return ctxt; - - error: - g_clear_object(&ctxt); - return NULL; -} - - -int qemuDomainLogContextWrite(qemuDomainLogContext *ctxt, - const char *fmt, ...) -{ - va_list argptr; - g_autofree char *message = NULL; - int ret = -1; - - va_start(argptr, fmt); - - message = g_strdup_vprintf(fmt, argptr); - if (!ctxt->manager && - lseek(ctxt->writefd, 0, SEEK_END) < 0) { - virReportSystemError(errno, "%s", - _("Unable to seek to end of domain logfile")); - goto cleanup; - } - if (safewrite(ctxt->writefd, message, strlen(message)) < 0) { - virReportSystemError(errno, "%s", - _("Unable to write to domain logfile")); - goto cleanup; - } - - ret = 0; - - cleanup: - va_end(argptr); - return ret; -} - - -ssize_t qemuDomainLogContextRead(qemuDomainLogContext *ctxt, - char **msg) -{ - char *buf; - size_t buflen; - - VIR_DEBUG("Context read %p manager=%p inode=%llu pos=%llu", - ctxt, ctxt->manager, - (unsigned long long)ctxt->inode, - (unsigned long long)ctxt->pos); - - if (ctxt->manager) { - buf = virLogManagerDomainReadLogFile(ctxt->manager, - ctxt->path, - ctxt->inode, - ctxt->pos, - 1024 * 128, - 0); - if (!buf) - return -1; - buflen = strlen(buf); - } else { - ssize_t got; - - buflen = 1024 * 128; - - /* Best effort jump to start of messages */ - ignore_value(lseek(ctxt->readfd, ctxt->pos, SEEK_SET)); - - buf = g_new0(char, buflen); - - got = saferead(ctxt->readfd, buf, buflen - 1); - if (got < 0) { - VIR_FREE(buf); - virReportSystemError(errno, "%s", - _("Unable to read from log file")); - return -1; - } - - buf[got] = '\0'; - - buf = g_renew(char, buf, got + 1); - buflen = got; - } - - *msg = buf; - - return buflen; -} - - /** * qemuDomainLogAppendMessage: * @@ -7091,31 +6897,6 @@ qemuDomainLogAppendMessage(virQEMUDriver *driver, } -int qemuDomainLogContextGetWriteFD(qemuDomainLogContext *ctxt) -{ - return ctxt->writefd; -} - - -void qemuDomainLogContextMarkPosition(qemuDomainLogContext *ctxt) -{ - if (ctxt->manager) - virLogManagerDomainGetLogFilePosition(ctxt->manager, - ctxt->path, - 0, - &ctxt->inode, - &ctxt->pos); - else - ctxt->pos = lseek(ctxt->writefd, 0, SEEK_END); -} - - -virLogManager *qemuDomainLogContextGetManager(qemuDomainLogContext *ctxt) -{ - return ctxt->manager; -} - - /* Locate an appropriate 'qemu-img' binary. */ const char * qemuFindQemuImgBinary(virQEMUDriver *driver) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 37eeb2b18d..2b60d4ece2 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -32,13 +32,13 @@ #include "qemu_domainjob.h" #include "qemu_conf.h" #include "qemu_capabilities.h" +#include "qemu_logcontext.h" #include "qemu_migration_params.h" #include "qemu_nbdkit.h" #include "qemu_slirp.h" #include "qemu_fd.h" #include "virchrdev.h" #include "virobject.h" -#include "logging/log_manager.h" #include "virdomainmomentobjlist.h" #include "virenum.h" #include "vireventthread.h" @@ -475,9 +475,6 @@ struct qemuProcessEvent { void qemuProcessEventFree(struct qemuProcessEvent *event); -#define QEMU_TYPE_DOMAIN_LOG_CONTEXT qemu_domain_log_context_get_type() -G_DECLARE_FINAL_TYPE(qemuDomainLogContext, qemu_domain_log_context, QEMU, DOMAIN_LOG_CONTEXT, GObject); - typedef struct _qemuDomainSaveCookie qemuDomainSaveCookie; struct _qemuDomainSaveCookie { virObject parent; @@ -630,12 +627,12 @@ char *qemuDomainDefFormatLive(virQEMUDriver *driver, void qemuDomainObjTaint(virQEMUDriver *driver, virDomainObj *obj, virDomainTaintFlags taint, - qemuDomainLogContext *logCtxt); + qemuLogContext *logCtxt); void qemuDomainObjTaintMsg(virQEMUDriver *driver, virDomainObj *obj, virDomainTaintFlags taint, - qemuDomainLogContext *logCtxt, + qemuLogContext *logCtxt, const char *msg, ...) G_GNUC_PRINTF(5, 6); @@ -644,32 +641,20 @@ char **qemuDomainObjGetTainting(virQEMUDriver *driver, void qemuDomainObjCheckTaint(virQEMUDriver *driver, virDomainObj *obj, - qemuDomainLogContext *logCtxt, + qemuLogContext *logCtxt, bool incomingMigration); void qemuDomainObjCheckDiskTaint(virQEMUDriver *driver, virDomainObj *obj, virDomainDiskDef *disk, - qemuDomainLogContext *logCtxt); + qemuLogContext *logCtxt); void qemuDomainObjCheckHostdevTaint(virQEMUDriver *driver, virDomainObj *obj, virDomainHostdevDef *disk, - qemuDomainLogContext *logCtxt); + qemuLogContext *logCtxt); void qemuDomainObjCheckNetTaint(virQEMUDriver *driver, virDomainObj *obj, virDomainNetDef *net, - qemuDomainLogContext *logCtxt); - -qemuDomainLogContext *qemuDomainLogContextNew(virQEMUDriver *driver, - virDomainObj *vm, - const char *basename); -int qemuDomainLogContextWrite(qemuDomainLogContext *ctxt, - const char *fmt, ...) G_GNUC_PRINTF(2, 3); -ssize_t qemuDomainLogContextRead(qemuDomainLogContext *ctxt, - char **msg); -int qemuDomainLogContextGetWriteFD(qemuDomainLogContext *ctxt); -void qemuDomainLogContextMarkPosition(qemuDomainLogContext *ctxt); - -virLogManager *qemuDomainLogContextGetManager(qemuDomainLogContext *ctxt); + qemuLogContext *logCtxt); int qemuDomainLogAppendMessage(virQEMUDriver *driver, virDomainObj *vm, diff --git a/src/qemu/qemu_logcontext.c b/src/qemu/qemu_logcontext.c new file mode 100644 index 0000000000..6ad0beeeae --- /dev/null +++ b/src/qemu/qemu_logcontext.c @@ -0,0 +1,264 @@ +/* + * qemu_logcontext.c: QEMU log context + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + */ + +#include <config.h> + +#include "qemu_logcontext.h" +#include "viralloc.h" +#include "virlog.h" +#include "virutil.h" + +#include <fcntl.h> + +#define VIR_FROM_THIS VIR_FROM_QEMU + +VIR_LOG_INIT("qemu.qemu_logcontext"); + + +struct _qemuLogContext { + GObject parent; + + int writefd; + int readfd; /* Only used if manager == NULL */ + off_t pos; + ino_t inode; /* Only used if manager != NULL */ + char *path; + virLogManager *manager; +}; + +G_DEFINE_TYPE(qemuLogContext, qemu_log_context, G_TYPE_OBJECT); + +static void +qemuLogContextFinalize(GObject *obj); + + +static void +qemu_log_context_init(qemuLogContext *logctxt G_GNUC_UNUSED) +{ +} + + +static void +qemu_log_context_class_init(qemuLogContextClass *klass) +{ + GObjectClass *obj = G_OBJECT_CLASS(klass); + + obj->finalize = qemuLogContextFinalize; +} + + +static void +qemuLogContextFinalize(GObject *object) +{ + qemuLogContext *ctxt = QEMU_LOG_CONTEXT(object); + VIR_DEBUG("ctxt=%p", ctxt); + + virLogManagerFree(ctxt->manager); + VIR_FREE(ctxt->path); + VIR_FORCE_CLOSE(ctxt->writefd); + VIR_FORCE_CLOSE(ctxt->readfd); + G_OBJECT_CLASS(qemu_log_context_parent_class)->finalize(object); +} + + +qemuLogContext * +qemuLogContextNew(virQEMUDriver *driver, + virDomainObj *vm, + const char *basename) +{ + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + qemuLogContext *ctxt = QEMU_LOG_CONTEXT(g_object_new(QEMU_TYPE_LOG_CONTEXT, NULL)); + + VIR_DEBUG("Context new %p stdioLogD=%d", ctxt, cfg->stdioLogD); + ctxt->writefd = -1; + ctxt->readfd = -1; + + ctxt->path = g_strdup_printf("%s/%s.log", cfg->logDir, basename); + + if (cfg->stdioLogD) { + ctxt->manager = virLogManagerNew(driver->privileged); + if (!ctxt->manager) + goto error; + + ctxt->writefd = virLogManagerDomainOpenLogFile(ctxt->manager, + "qemu", + vm->def->uuid, + vm->def->name, + ctxt->path, + 0, + &ctxt->inode, + &ctxt->pos); + if (ctxt->writefd < 0) + goto error; + } else { + if ((ctxt->writefd = open(ctxt->path, O_WRONLY | O_CREAT | O_APPEND, S_IRUSR | S_IWUSR)) < 0) { + virReportSystemError(errno, _("failed to create logfile %s"), + ctxt->path); + goto error; + } + if (virSetCloseExec(ctxt->writefd) < 0) { + virReportSystemError(errno, _("failed to set close-on-exec flag on %s"), + ctxt->path); + goto error; + } + + /* For unprivileged startup we must truncate the file since + * we can't rely on logrotate. We don't use O_TRUNC since + * it is better for SELinux policy if we truncate afterwards */ + if (!driver->privileged && + ftruncate(ctxt->writefd, 0) < 0) { + virReportSystemError(errno, _("failed to truncate %s"), + ctxt->path); + goto error; + } + + if ((ctxt->readfd = open(ctxt->path, O_RDONLY)) < 0) { + virReportSystemError(errno, _("failed to open logfile %s"), + ctxt->path); + goto error; + } + if (virSetCloseExec(ctxt->readfd) < 0) { + virReportSystemError(errno, _("failed to set close-on-exec flag on %s"), + ctxt->path); + goto error; + } + + if ((ctxt->pos = lseek(ctxt->writefd, 0, SEEK_END)) < 0) { + virReportSystemError(errno, _("failed to seek in log file %s"), + ctxt->path); + goto error; + } + } + + return ctxt; + + error: + g_clear_object(&ctxt); + return NULL; +} + + +int +qemuLogContextWrite(qemuLogContext *ctxt, + const char *fmt, ...) +{ + va_list argptr; + g_autofree char *message = NULL; + int ret = -1; + + va_start(argptr, fmt); + + message = g_strdup_vprintf(fmt, argptr); + if (!ctxt->manager && + lseek(ctxt->writefd, 0, SEEK_END) < 0) { + virReportSystemError(errno, "%s", + _("Unable to seek to end of domain logfile")); + goto cleanup; + } + if (safewrite(ctxt->writefd, message, strlen(message)) < 0) { + virReportSystemError(errno, "%s", + _("Unable to write to domain logfile")); + goto cleanup; + } + + ret = 0; + + cleanup: + va_end(argptr); + return ret; +} + + +ssize_t +qemuLogContextRead(qemuLogContext *ctxt, + char **msg) +{ + char *buf; + size_t buflen; + + VIR_DEBUG("Context read %p manager=%p inode=%llu pos=%llu", + ctxt, ctxt->manager, + (unsigned long long)ctxt->inode, + (unsigned long long)ctxt->pos); + + if (ctxt->manager) { + buf = virLogManagerDomainReadLogFile(ctxt->manager, + ctxt->path, + ctxt->inode, + ctxt->pos, + 1024 * 128, + 0); + if (!buf) + return -1; + buflen = strlen(buf); + } else { + ssize_t got; + + buflen = 1024 * 128; + + /* Best effort jump to start of messages */ + ignore_value(lseek(ctxt->readfd, ctxt->pos, SEEK_SET)); + + buf = g_new0(char, buflen); + + got = saferead(ctxt->readfd, buf, buflen - 1); + if (got < 0) { + VIR_FREE(buf); + virReportSystemError(errno, "%s", + _("Unable to read from log file")); + return -1; + } + + buf[got] = '\0'; + + buf = g_renew(char, buf, got + 1); + buflen = got; + } + + *msg = buf; + + return buflen; +} + + +int +qemuLogContextGetWriteFD(qemuLogContext *ctxt) +{ + return ctxt->writefd; +} + + +void +qemuLogContextMarkPosition(qemuLogContext *ctxt) +{ + if (ctxt->manager) + virLogManagerDomainGetLogFilePosition(ctxt->manager, + ctxt->path, + 0, + &ctxt->inode, + &ctxt->pos); + else + ctxt->pos = lseek(ctxt->writefd, 0, SEEK_END); +} + + +virLogManager * +qemuLogContextGetManager(qemuLogContext *ctxt) +{ + return ctxt->manager; +} diff --git a/src/qemu/qemu_logcontext.h b/src/qemu/qemu_logcontext.h new file mode 100644 index 0000000000..c6dbf3cb84 --- /dev/null +++ b/src/qemu/qemu_logcontext.h @@ -0,0 +1,38 @@ +/* + * qemu_logcontext.h: QEMU log context + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + */ + +#pragma once + +#include <glib-object.h> +#include "qemu_conf.h" +#include "logging/log_manager.h" + +#define QEMU_TYPE_LOG_CONTEXT qemu_log_context_get_type() +G_DECLARE_FINAL_TYPE(qemuLogContext, qemu_log_context, QEMU, LOG_CONTEXT, GObject); + +qemuLogContext *qemuLogContextNew(virQEMUDriver *driver, + virDomainObj *vm, + const char *basename); +int qemuLogContextWrite(qemuLogContext *ctxt, + const char *fmt, ...) G_GNUC_PRINTF(2, 3); +ssize_t qemuLogContextRead(qemuLogContext *ctxt, + char **msg); +int qemuLogContextGetWriteFD(qemuLogContext *ctxt); +void qemuLogContextMarkPosition(qemuLogContext *ctxt); + +virLogManager *qemuLogContextGetManager(qemuLogContext *ctxt); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a1a5c9ed6c..6e02967871 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1813,7 +1813,7 @@ qemuProcessMonitorReportLogError(qemuMonitor *mon, static void qemuProcessMonitorLogFree(void *opaque) { - qemuDomainLogContext *logCtxt = opaque; + qemuLogContext *logCtxt = opaque; g_clear_object(&logCtxt); } @@ -1839,7 +1839,7 @@ static int qemuConnectMonitor(virQEMUDriver *driver, virDomainObj *vm, int asyncJob, - qemuDomainLogContext *logCtxt, + qemuLogContext *logCtxt, bool reconnect) { qemuDomainObjPrivate *priv = vm->privateData; @@ -1903,7 +1903,7 @@ qemuConnectMonitor(virQEMUDriver *driver, * Returns 0 on success or -1 on error */ static int -qemuProcessReadLog(qemuDomainLogContext *logCtxt, +qemuProcessReadLog(qemuLogContext *logCtxt, char **msg, size_t max) { @@ -1913,7 +1913,7 @@ qemuProcessReadLog(qemuDomainLogContext *logCtxt, char *filter_next; size_t skip; - if ((got = qemuDomainLogContextRead(logCtxt, &buf)) < 0) + if ((got = qemuLogContextRead(logCtxt, &buf)) < 0) return -1; /* Filter out debug messages from intermediate libvirt process */ @@ -1956,7 +1956,7 @@ qemuProcessReadLog(qemuDomainLogContext *logCtxt, static int -qemuProcessReportLogError(qemuDomainLogContext *logCtxt, +qemuProcessReportLogError(qemuLogContext *logCtxt, const char *msgprefix) { g_autofree char *logmsg = NULL; @@ -1981,7 +1981,7 @@ qemuProcessMonitorReportLogError(qemuMonitor *mon G_GNUC_UNUSED, const char *msg, void *opaque) { - qemuDomainLogContext *logCtxt = opaque; + qemuLogContext *logCtxt = opaque; qemuProcessReportLogError(logCtxt, msg); } @@ -2283,7 +2283,7 @@ static int qemuProcessWaitForMonitor(virQEMUDriver *driver, virDomainObj *vm, int asyncJob, - qemuDomainLogContext *logCtxt) + qemuLogContext *logCtxt) { int ret = -1; g_autoptr(GHashTable) info = NULL; @@ -4636,7 +4636,7 @@ static void qemuLogOperation(virDomainObj *vm, const char *msg, virCommand *cmd, - qemuDomainLogContext *logCtxt) + qemuLogContext *logCtxt) { g_autofree char *timestamp = NULL; qemuDomainObjPrivate *priv = vm->privateData; @@ -4650,20 +4650,20 @@ qemuLogOperation(virDomainObj *vm, if ((timestamp = virTimeStringNow()) == NULL) return; - if (qemuDomainLogContextWrite(logCtxt, - "%s: %s %s, qemu version: %d.%d.%d%s, kernel: %s, hostname: %s\n", - timestamp, msg, VIR_LOG_VERSION_STRING, - (qemuVersion / 1000000) % 1000, - (qemuVersion / 1000) % 1000, - qemuVersion % 1000, - NULLSTR_EMPTY(package), - uts.release, - NULLSTR_EMPTY(hostname)) < 0) + if (qemuLogContextWrite(logCtxt, + "%s: %s %s, qemu version: %d.%d.%d%s, kernel: %s, hostname: %s\n", + timestamp, msg, VIR_LOG_VERSION_STRING, + (qemuVersion / 1000000) % 1000, + (qemuVersion / 1000) % 1000, + qemuVersion % 1000, + NULLSTR_EMPTY(package), + uts.release, + NULLSTR_EMPTY(hostname)) < 0) return; if (cmd) { g_autofree char *args = virCommandToString(cmd, true); - qemuDomainLogContextWrite(logCtxt, "%s\n", args); + qemuLogContextWrite(logCtxt, "%s\n", args); } } @@ -7619,7 +7619,7 @@ qemuProcessLaunch(virConnectPtr conn, int ret = -1; int rv; int logfile = -1; - g_autoptr(qemuDomainLogContext) logCtxt = NULL; + g_autoptr(qemuLogContext) logCtxt = NULL; qemuDomainObjPrivate *priv = vm->privateData; g_autoptr(virCommand) cmd = NULL; struct qemuProcessHookData hookData; @@ -7669,11 +7669,11 @@ qemuProcessLaunch(virConnectPtr conn, hookData.cfg = cfg; VIR_DEBUG("Creating domain log file"); - if (!(logCtxt = qemuDomainLogContextNew(driver, vm, vm->def->name))) { + if (!(logCtxt = qemuLogContextNew(driver, vm, vm->def->name))) { virLastErrorPrefixMessage("%s", _("can't connect to virtlogd")); goto cleanup; } - logfile = qemuDomainLogContextGetWriteFD(logCtxt); + logfile = qemuLogContextGetWriteFD(logCtxt); if (qemuProcessGenID(vm, flags) < 0) goto cleanup; @@ -7709,7 +7709,7 @@ qemuProcessLaunch(virConnectPtr conn, qemuDomainObjCheckTaint(driver, vm, logCtxt, incoming != NULL); - qemuDomainLogContextMarkPosition(logCtxt); + qemuLogContextMarkPosition(logCtxt); if (qemuProcessEnableDomainNamespaces(driver, vm) < 0) goto cleanup; -- 2.39.1

On Tue, Feb 14, 2023 at 11:08:01 -0600, Jonathon Jongsma wrote: You've got the quotes wrong here compared to the place where you took it from I wonder how that happened as it looks like code movement.

log stderr and stdout from nbdkit into its own log so that nbdkit-related issues can be debugged more easily. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; --- src/qemu/qemu_nbdkit.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c index 5abccf1536..f2ff9552a3 100644 --- a/src/qemu/qemu_nbdkit.c +++ b/src/qemu/qemu_nbdkit.c @@ -850,15 +850,28 @@ qemuNbdkitProcessStart(qemuNbdkitProcess *proc, int rc; int exitstatus = 0; int cmdret = 0; - g_autofree char *errbuf = NULL; virTimeBackOffVar timebackoff; bool socketCreated = false; + g_autofree char *basename = g_strdup_printf("%s-nbdkit-%i", vm->def->name, proc->source->id); + int logfd = -1; + g_autoptr(qemuLogContext) logContext = NULL; + g_autofree char *errmsg = NULL; + g_autoptr(virURI) uri = NULL; + g_autofree char *uristring = NULL; if (!(cmd = qemuNbdkitProcessBuildCommand(proc))) return -1; + if (!(logContext = qemuLogContextNew(driver, vm, basename))) { + virLastErrorPrefixMessage("%s", _("can't connect to virtlogd")); + return -1; + } + + logfd = qemuLogContextGetWriteFD(logContext); + VIR_DEBUG("starting nbdkit process for %s", proc->source->nodestorage); - virCommandSetErrorBuffer(cmd, &errbuf); + virCommandSetErrorFD(cmd, &logfd); + virCommandSetOutputFD(cmd, &logfd); virCommandSetPidFile(cmd, proc->pidfile); if (qemuExtDeviceLogCommand(driver, vm, cmd, "nbdkit") < 0) @@ -902,9 +915,16 @@ qemuNbdkitProcessStart(qemuNbdkitProcess *proc, return 0; error: - if (errbuf) - virReportError(VIR_ERR_OPERATION_FAILED, - _("nbdkit failed to start and reported: %s"), errbuf); + if (qemuLogContextReadFiltered(logContext, &errmsg, 1024) < 0) + VIR_WARN("Unable to read from nbdkit log"); + + if ((uri = qemuBlockStorageSourceGetURI(proc->source))) + uristring = virURIFormat(uri); + + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to connect to nbdkit for '%s': %s"), + NULLSTR(uristring), NULLSTR(errmsg)); + qemuNbdkitProcessStop(proc); return -1; } -- 2.39.1

Add new DO_TEST_CAPS_LATEST_NBDKIT macro to test xml2argv for various nbdkit capability scenarios. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_nbdkit.c | 20 +++++++++++++++++--- tests/qemuxml2argvtest.c | 11 +++++++++++ tests/testutilsqemu.c | 27 +++++++++++++++++++++++++++ tests/testutilsqemu.h | 5 +++++ 4 files changed, 60 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c index f2ff9552a3..c79c9ec6cd 100644 --- a/src/qemu/qemu_nbdkit.c +++ b/src/qemu/qemu_nbdkit.c @@ -291,10 +291,16 @@ virNbkditCapsCheckModdir(const char *moddir, static bool virNbdkitCapsIsValid(void *data, - void *privData G_GNUC_UNUSED) + void *privData) { qemuNbdkitCaps *nbdkitCaps = data; struct stat st; + /* when run under test, we will use privData as a signal to indicate that + * we shouldn't touch the filesystem */ + bool skipValidation = (privData != NULL); + + if (skipValidation) + return true; if (!nbdkitCaps->path) return true; @@ -335,9 +341,17 @@ virNbdkitCapsIsValid(void *data, static void* virNbdkitCapsNewData(const char *binary, - void *privData G_GNUC_UNUSED) + void *privData) { - qemuNbdkitCaps *caps = qemuNbdkitCapsNew(binary); + /* when run under test, we will use privData as a signal to indicate that + * we shouldn't touch the filesystem */ + bool skipNewData = (privData != NULL); + qemuNbdkitCaps *caps = NULL; + + if (skipNewData) + return NULL; + + caps = qemuNbdkitCapsNew(binary); qemuNbdkitCapsQuery(caps); return caps; diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 8df6aaa575..f2f7467102 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -671,6 +671,14 @@ testCompareXMLToArgv(const void *data) if (rc < 0) goto cleanup; + if (info->nbdkitCaps) { + if (virFileCacheInsertData(driver.nbdkitCapsCache, TEST_NBDKIT_PATH, + g_object_ref(info->nbdkitCaps)) < 0) { + g_object_unref(info->nbdkitCaps); + goto cleanup; + } + } + if (info->migrateFrom && !(migrateURI = qemuMigrationDstGetURI(info->migrateFrom, info->migrateFd))) @@ -933,6 +941,9 @@ mymain(void) # define DO_TEST_CAPS_ARCH_VER(name, arch, ver) \ DO_TEST_CAPS_ARCH_VER_FULL(name, arch, ver, ARG_END) +# define DO_TEST_CAPS_LATEST_NBDKIT(name, ...) \ + DO_TEST_CAPS_ARCH_LATEST_FULL(name, "x86_64", ARG_NBDKIT_CAPS, __VA_ARGS__, QEMU_NBDKIT_CAPS_LAST, ARG_END) + # define DO_TEST_CAPS_LATEST(name) \ DO_TEST_CAPS_ARCH_LATEST(name, "x86_64") diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 396803c40b..24fc068058 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -131,6 +131,10 @@ virFindFileInPath(const char *file) return g_strdup_printf("/usr/bin/%s", file); } + if (g_str_equal(file, "nbdkit")) { + return g_strdup(TEST_NBDKIT_PATH); + } + /* Nothing in tests should be relying on real files * in host OS, so we return NULL to try to force * an error in such a case @@ -422,6 +426,7 @@ void qemuTestDriverFree(virQEMUDriver *driver) virObjectUnref(driver->caps); virObjectUnref(driver->config); virObjectUnref(driver->securityManager); + g_clear_object(&driver->nbdkitCapsCache); virCPUDefFree(cpuDefault); virCPUDefFree(cpuHaswell); @@ -665,6 +670,12 @@ int qemuTestDriverInit(virQEMUDriver *driver) if (!driver->qemuCapsCache) goto error; + driver->nbdkitCapsCache = qemuNbdkitCapsCacheNew("/dev/null"); + /* the nbdkitCapsCache just interprets the presence of a non-null private + * data pointer as a signal to skip cache validation. This prevents the + * cache from trying to validate the plugindir mtime, etc during test */ + virFileCacheSetPriv(driver->nbdkitCapsCache, GUINT_TO_POINTER(1)); + driver->xmlopt = virQEMUDriverCreateXMLConf(driver, "none"); if (!driver->xmlopt) goto error; @@ -885,6 +896,7 @@ testQemuInfoSetArgs(struct testQemuInfo *info, info->conf = conf; info->args.newargs = true; + info->args.fakeNbdkitCaps = qemuNbdkitCapsNew(TEST_NBDKIT_PATH); va_start(argptr, conf); while ((argname = va_arg(argptr, testQemuInfoArgName)) != ARG_END) { @@ -896,6 +908,13 @@ testQemuInfoSetArgs(struct testQemuInfo *info, virQEMUCapsSet(info->args.fakeCaps, flag); break; + case ARG_NBDKIT_CAPS: + info->args.fakeNbdkitCapsUsed = true; + + while ((flag = va_arg(argptr, int)) < QEMU_NBDKIT_CAPS_LAST) + qemuNbdkitCapsSet(info->args.fakeNbdkitCaps, flag); + break; + case ARG_GIC: info->args.gic = va_arg(argptr, int); break; @@ -1052,6 +1071,12 @@ testQemuInfoInitArgs(struct testQemuInfo *info) info->qemuCaps = g_steal_pointer(&info->args.fakeCaps); } + if (info->args.fakeNbdkitCapsUsed) + info->nbdkitCaps = g_steal_pointer(&info->args.fakeNbdkitCaps); + else + /* empty caps */ + info->nbdkitCaps = qemuNbdkitCapsNew(TEST_NBDKIT_PATH); + if (info->args.gic != GIC_NONE && testQemuCapsSetGIC(info->qemuCaps, info->args.gic) < 0) return -1; @@ -1070,6 +1095,8 @@ testQemuInfoClear(struct testQemuInfo *info) virObjectUnref(info->qemuCaps); g_clear_pointer(&info->args.fakeCaps, virObjectUnref); g_clear_pointer(&info->args.fds, g_hash_table_unref); + g_clear_object(&info->nbdkitCaps); + g_clear_object(&info->args.fakeNbdkitCaps); } diff --git a/tests/testutilsqemu.h b/tests/testutilsqemu.h index 51c072cb13..8d246ec4ac 100644 --- a/tests/testutilsqemu.h +++ b/tests/testutilsqemu.h @@ -28,6 +28,7 @@ # define TEST_TPM_ENV_VAR "VIR_TEST_MOCK_FAKE_TPM_VERSION" # define TPM_VER_1_2 "1.2" # define TPM_VER_2_0 "2.0" +# define TEST_NBDKIT_PATH "/fakebindir/nbdkit" enum { GIC_NONE = 0, @@ -53,6 +54,7 @@ typedef enum { ARG_CAPS_HOST_CPU_MODEL, ARG_HOST_OS, ARG_FD_GROUP, /* name, nfds, fd[0], ... fd[n-1] */ + ARG_NBDKIT_CAPS, ARG_END, } testQemuInfoArgName; @@ -83,6 +85,8 @@ struct testQemuArgs { bool newargs; virQEMUCaps *fakeCaps; bool fakeCapsUsed; + qemuNbdkitCaps *fakeNbdkitCaps; + bool fakeNbdkitCapsUsed; char *capsver; char *capsarch; qemuTestCPUDef capsHostCPUModel; @@ -98,6 +102,7 @@ struct testQemuInfo { char *outfile; char *errfile; virQEMUCaps *qemuCaps; + qemuNbdkitCaps *nbdkitCaps; const char *migrateFrom; int migrateFd; unsigned int flags; -- 2.39.1

Add xml to the private data for a disk source to represent the nbdkit process so that the state can be re-created if the libvirt daemon is restarted. Format: <nbdkit> <pidfile>/path/to/nbdkit.pid</pidfile> <socketfile>/path/to/nbdkit.socket</socketfile> </nbdkit> Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 52 +++++++++++++++++ src/qemu/qemu_nbdkit.c | 71 +++++++++++++++++++++++ src/qemu/qemu_nbdkit.h | 8 +++ src/qemu/qemu_process.c | 6 ++ tests/qemustatusxml2xmldata/modern-in.xml | 4 ++ 5 files changed, 141 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 099ce2ab9a..781b80b46d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1936,6 +1936,33 @@ qemuStorageSourcePrivateDataAssignSecinfo(qemuDomainSecretInfo **secinfo, } +static int +qemuStorageSourcePrivateDataParseNbdkit(xmlNodePtr node, + xmlXPathContextPtr ctxt, + virStorageSource *src) +{ + g_autofree char *pidfile = NULL; + g_autofree char *socketfile = NULL; + VIR_XPATH_NODE_AUTORESTORE(ctxt); + + ctxt->node = node; + + if (!(pidfile = virXPathString("string(./pidfile)", ctxt))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing nbdkit pidfile")); + return -1; + } + + if (!(socketfile = virXPathString("string(./socketfile)", ctxt))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing nbdkit socketfile")); + return -1; + } + + qemuNbdkitReconnectStorageSource(src, pidfile, socketfile); + + return 0; +} + + static int qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt, virStorageSource *src) @@ -1948,6 +1975,7 @@ qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt, g_autofree char *thresholdEventWithIndex = NULL; bool fdsetPresent = false; unsigned int fdSetID; + xmlNodePtr nbdkitnode = NULL; src->nodestorage = virXPathString("string(./nodenames/nodename[@type='storage']/@name)", ctxt); src->nodeformat = virXPathString("string(./nodenames/nodename[@type='format']/@name)", ctxt); @@ -1996,6 +2024,10 @@ qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt, virTristateBoolTypeFromString(thresholdEventWithIndex) == VIR_TRISTATE_BOOL_YES) src->thresholdEventWithIndex = true; + if ((nbdkitnode = virXPathNode("nbdkit", ctxt))) { + if (qemuStorageSourcePrivateDataParseNbdkit(nbdkitnode, ctxt, src) < 0) + return -1; + } return 0; } @@ -2013,6 +2045,23 @@ qemuStorageSourcePrivateDataFormatSecinfo(virBuffer *buf, } +static void +qemuStorageSourcePrivateDataFormatNbdkit(qemuNbdkitProcess *nbdkit, + virBuffer *buf) +{ + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); + + if (!nbdkit) + return; + + virBufferEscapeString(&childBuf, "<pidfile>%s</pidfile>\n", + nbdkit->pidfile); + virBufferEscapeString(&childBuf, "<socketfile>%s</socketfile>\n", + nbdkit->socketfile); + virXMLFormatElement(buf, "nbdkit", NULL, &childBuf); +} + + static int qemuStorageSourcePrivateDataFormat(virStorageSource *src, virBuffer *buf) @@ -2059,6 +2108,9 @@ qemuStorageSourcePrivateDataFormat(virStorageSource *src, if (src->thresholdEventWithIndex) virBufferAddLit(buf, "<thresholdEvent indexUsed='yes'/>\n"); + if (srcPriv) + qemuStorageSourcePrivateDataFormatNbdkit(srcPriv->nbdkitProcess, buf); + return 0; } diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c index c79c9ec6cd..5f0c0c23b5 100644 --- a/src/qemu/qemu_nbdkit.c +++ b/src/qemu/qemu_nbdkit.c @@ -628,6 +628,77 @@ qemuNbdkitProcessNew(virStorageSource *source, return nbdkit; } +/** + * qemuNbdkitReconnectStorageSource: + * @source: a storage source + * @pidfile: a pidfile for an nbdkit process + * @socketfile: the socket file associated with the nbdkit process + * + * This function constructs a new qemuNbdkitProcess object with the given values for @pidfile and + * @socketfile and stores it in @source. This is intended to be called when the libvirt daemon is + * restarted and tries to reconnect to all currently-running domains. Since this function is called + * from the code that parses the current daemon state, it should not perform any filesystem + * operations, or anything else that might fail. Additional initialization will be done later by + * calling qemuNbdkitStorageSourceManageProcess(). + */ +void +qemuNbdkitReconnectStorageSource(virStorageSource *source, + const char *pidfile, + const char *socketfile) +{ + qemuDomainStorageSourcePrivate *srcpriv = qemuDomainStorageSourcePrivateFetch(source); + + if (srcpriv->nbdkitProcess) { + VIR_WARN("source already has an nbdkit process"); + return; + } + + srcpriv->nbdkitProcess = qemuNbdkitProcessNew(source, pidfile, socketfile); +} + + +static void +qemuNbdkitStorageSourceManageProcessOne(virStorageSource *source) +{ + qemuDomainStorageSourcePrivate *srcpriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(source); + qemuNbdkitProcess *proc; + + if (!srcpriv) + return; + + proc = srcpriv->nbdkitProcess; + + if (!proc) + return; + + if (proc->pid <= 0) { + if (virPidFileReadPath(proc->pidfile, &proc->pid) < 0) { + VIR_WARN("Unable to read pidfile '%s'", proc->pidfile); + return; + } + } + + if (virProcessKill(proc->pid, 0) < 0) + VIR_WARN("nbdkit process %i is not alive", proc->pid); +} + +/** + * qemuNbdkitStorageSourceManageProcess: + * @source: a storage source + * @vm: the vm that owns this storage source + * + * This function re-enables monitoring of any nbdkit processes associated with the backing chain of + * @source. It is intended to be called after libvirt restarts and has loaded its current state from + * disk and is attempting to re-connect to active domains. + */ +void +qemuNbdkitStorageSourceManageProcess(virStorageSource *source) +{ + virStorageSource *backing; + for (backing = source; backing != NULL; backing = backing->backingStore) + qemuNbdkitStorageSourceManageProcessOne(backing); +} + bool qemuNbdkitInitStorageSource(qemuNbdkitCaps *caps, diff --git a/src/qemu/qemu_nbdkit.h b/src/qemu/qemu_nbdkit.h index ccd418b7d3..7e2aeed4eb 100644 --- a/src/qemu/qemu_nbdkit.h +++ b/src/qemu/qemu_nbdkit.h @@ -54,6 +54,14 @@ qemuNbdkitInitStorageSource(qemuNbdkitCaps *nbdkitCaps, uid_t user, gid_t group); +void +qemuNbdkitReconnectStorageSource(virStorageSource *source, + const char *pidfile, + const char *socketfile); + +void +qemuNbdkitStorageSourceManageProcess(virStorageSource *src); + bool qemuNbdkitCapsGet(qemuNbdkitCaps *nbdkitCaps, qemuNbdkitCapsFlags flag); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a61c3192fd..d0b797575d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -9035,6 +9035,12 @@ qemuProcessReconnect(void *opaque) } } + for (i = 0; i < obj->def->ndisks; i++) + qemuNbdkitStorageSourceManageProcess(obj->def->disks[i]->src); + + if (obj->def->os.loader && obj->def->os.loader->nvram) + qemuNbdkitStorageSourceManageProcess(obj->def->os.loader->nvram); + /* update domain state XML with possibly updated state in virDomainObj */ if (virDomainObjSave(obj, driver->xmlopt, cfg->stateDir) < 0) goto error; diff --git a/tests/qemustatusxml2xmldata/modern-in.xml b/tests/qemustatusxml2xmldata/modern-in.xml index cdab1d7178..6efb52fb14 100644 --- a/tests/qemustatusxml2xmldata/modern-in.xml +++ b/tests/qemustatusxml2xmldata/modern-in.xml @@ -345,6 +345,10 @@ <fdset type='storage' id='1337'/> </fdsets> <thresholdEvent indexUsed='yes'/> + <nbdkit> + <pidfile>/path/to/nbdkit.pid</pidfile> + <socketfile>/path/to/nbdkit.socket</socketfile> + </nbdkit> </privateData> </source> <backingStore/> -- 2.39.1

All users of virCommandSetSendBuffer() are using it to send sensitive data to a child process. So, since these buffers contain sensitive information, clear it with virSecureErase(). Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; --- src/util/vircommand.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 0917bc9cfb..aef63efb26 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -54,6 +54,7 @@ #include "virpidfile.h" #include "virprocess.h" #include "virbuffer.h" +#include "virsecureerase.h" #include "virthread.h" #include "virstring.h" @@ -1672,6 +1673,7 @@ virCommandFreeSendBuffers(virCommand *cmd) for (i = 0; i < virCommandGetNumSendBuffers(cmd); i++) { VIR_FORCE_CLOSE(cmd->sendBuffers[i].fd); + virSecureErase(cmd->sendBuffers[i].buffer, cmd->sendBuffers[i].buflen); VIR_FREE(cmd->sendBuffers[i].buffer); } VIR_FREE(cmd->sendBuffers); -- 2.39.1

Rather than passing passwords and cookies (which could contain passwords) to nbdkit via commandline arguments, use the alternate format that nbdkit supports where we can specify a file descriptor which nbdkit will read to get the password or cookies. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; --- src/qemu/qemu_nbdkit.c | 53 +++++++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c index 5f0c0c23b5..b17f9e772f 100644 --- a/src/qemu/qemu_nbdkit.c +++ b/src/qemu/qemu_nbdkit.c @@ -754,6 +754,29 @@ qemuNbdkitInitStorageSource(qemuNbdkitCaps *caps, } +static int +qemuNbdkitCommandPassDataByPipe(virCommand *cmd, + const char *argName, + unsigned char **buf, + size_t buflen) +{ + g_autofree char *fdfmt = NULL; + int fd = virCommandSetSendBuffer(cmd, buf, buflen); + + if (fd < 0) + return -1; + + /* some nbdkit arguments accept a variation where nbdkit will read the data + * from a file descriptor, e.g. password=-FD */ + fdfmt = g_strdup_printf("-%i", fd); + virCommandAddArgPair(cmd, argName, fdfmt); + + virCommandDoAsyncIO(cmd); + + return 0; +} + + static int qemuNbdkitProcessBuildCommandCurl(qemuNbdkitProcess *proc, virCommand *cmd) @@ -776,7 +799,6 @@ qemuNbdkitProcessBuildCommandCurl(qemuNbdkitProcess *proc, g_autoptr(virConnect) conn = virGetConnectSecret(); g_autofree uint8_t *secret = NULL; size_t secretlen = 0; - g_autofree char *password = NULL; int secrettype; virStorageAuthDef *authdef = proc->source->auth; @@ -800,26 +822,19 @@ qemuNbdkitProcessBuildCommandCurl(qemuNbdkitProcess *proc, return -1; } - /* ensure that the secret is a NULL-terminated string */ - password = g_strndup((char*)secret, secretlen); - virSecureErase(secret, secretlen); - - /* for now, just report an error rather than passing the password in - * cleartext on the commandline */ - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Password not yet supported for nbdkit sources")); - - virSecureEraseString(password); - - return -1; + if (qemuNbdkitCommandPassDataByPipe(cmd, "password", + &secret, secretlen) < 0) + return -1; } - if (proc->source->ncookies > 0) { - /* for now, just report an error rather than passing cookies in - * cleartext on the commandline */ - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cookies not yet supported for nbdkit sources")); - return -1; + /* Create a pipe to send the cookies to the nbdkit process. */ + if (proc->source->ncookies) { + g_autofree char *cookies = qemuBlockStorageSourceGetCookieString(proc->source); + + if (qemuNbdkitCommandPassDataByPipe(cmd, "cookie", + (unsigned char**)&cookies, + strlen(cookies)) < 0) + return -1; } if (proc->source->sslverify == VIR_TRISTATE_BOOL_NO) { -- 2.39.1

For virStorageSource objects that contain an nbdkitProcess, start that nbdkit process to serve that network drive and then pass the nbdkit socket to qemu rather than sending the network url to qemu directly. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; --- src/qemu/qemu_block.c | 162 +++++++++++------- src/qemu/qemu_domain.c | 13 +- src/qemu/qemu_extdevice.c | 56 ++++++ src/qemu/qemu_hotplug.c | 7 + src/qemu/qemu_nbdkit.c | 63 +++++++ src/qemu/qemu_nbdkit.h | 13 ++ ...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 + ...rce-curl-nbdkit-backing.x86_64-latest.args | 38 ++++ ...isk-network-source-curl-nbdkit-backing.xml | 45 +++++ ...work-source-curl-nbdkit.x86_64-latest.args | 50 ++++++ .../disk-network-source-curl-nbdkit.xml | 1 + ...isk-network-source-curl.x86_64-latest.args | 53 ++++++ .../disk-network-source-curl.xml | 71 ++++++++ ...disk-network-ssh-nbdkit.x86_64-latest.args | 36 ++++ .../disk-network-ssh-nbdkit.xml | 1 + tests/qemuxml2argvtest.c | 6 + 19 files changed, 638 insertions(+), 66 deletions(-) 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-backing.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-network-source-curl-nbdkit-backing.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 create mode 100644 tests/qemuxml2argvdata/disk-network-ssh-nbdkit.x86_64-latest.args create mode 120000 tests/qemuxml2argvdata/disk-network-ssh-nbdkit.xml diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 5e700eff99..125470f5e2 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -438,6 +438,32 @@ qemuBlockStorageSourceGetCURLProps(virStorageSource *src, } +static virJSONValue * +qemuBlockStorageSourceGetNbdkitProps(virStorageSource *src) +{ + qemuDomainStorageSourcePrivate *srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); + virJSONValue *ret = NULL; + g_autoptr(virJSONValue) serverprops = NULL; + virStorageNetHostDef host = { .transport = VIR_STORAGE_NET_HOST_TRANS_UNIX }; + + /* srcPriv->nbdkitProcess will already be initialized if we can use nbdkit + * to proxy this storage source */ + if (!(srcPriv && srcPriv->nbdkitProcess)) + return NULL; + + host.socket = srcPriv->nbdkitProcess->socketfile; + serverprops = qemuBlockStorageSourceBuildJSONSocketAddress(&host); + + if (!serverprops) + return NULL; + + if (virJSONValueObjectAdd(&ret, "a:server", &serverprops, NULL) < 0) + return NULL; + + return ret; +} + + static virJSONValue * qemuBlockStorageSourceGetISCSIProps(virStorageSource *src, bool onlytarget) @@ -875,69 +901,75 @@ qemuBlockStorageSourceGetBackendProps(virStorageSource *src, return NULL; case VIR_STORAGE_TYPE_NETWORK: - switch ((virStorageNetProtocol) src->protocol) { - case VIR_STORAGE_NET_PROTOCOL_GLUSTER: - driver = "gluster"; - if (!(fileprops = qemuBlockStorageSourceGetGlusterProps(src, onlytarget))) - return NULL; - break; - - case VIR_STORAGE_NET_PROTOCOL_VXHS: - driver = "vxhs"; - if (!(fileprops = qemuBlockStorageSourceGetVxHSProps(src, onlytarget))) - return NULL; - break; - - case VIR_STORAGE_NET_PROTOCOL_HTTP: - case VIR_STORAGE_NET_PROTOCOL_HTTPS: - case VIR_STORAGE_NET_PROTOCOL_FTP: - case VIR_STORAGE_NET_PROTOCOL_FTPS: - case VIR_STORAGE_NET_PROTOCOL_TFTP: - driver = virStorageNetProtocolTypeToString(src->protocol); - if (!(fileprops = qemuBlockStorageSourceGetCURLProps(src, onlytarget))) - return NULL; - break; - - case VIR_STORAGE_NET_PROTOCOL_ISCSI: - driver = "iscsi"; - if (!(fileprops = qemuBlockStorageSourceGetISCSIProps(src, onlytarget))) - return NULL; - break; - - case VIR_STORAGE_NET_PROTOCOL_NBD: + /* prefer using nbdkit for sources that are supported */ + if ((fileprops = qemuBlockStorageSourceGetNbdkitProps(src))) { driver = "nbd"; - if (!(fileprops = qemuBlockStorageSourceGetNBDProps(src, onlytarget))) - return NULL; - break; - - case VIR_STORAGE_NET_PROTOCOL_RBD: - driver = "rbd"; - if (!(fileprops = qemuBlockStorageSourceGetRBDProps(src, onlytarget))) - return NULL; - break; - - case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: - driver = "sheepdog"; - if (!(fileprops = qemuBlockStorageSourceGetSheepdogProps(src))) - return NULL; - break; - - case VIR_STORAGE_NET_PROTOCOL_SSH: - driver = "ssh"; - if (!(fileprops = qemuBlockStorageSourceGetSshProps(src))) - return NULL; break; - - case VIR_STORAGE_NET_PROTOCOL_NFS: - driver = "nfs"; - if (!(fileprops = qemuBlockStorageSourceGetNFSProps(src))) - return NULL; - break; - - case VIR_STORAGE_NET_PROTOCOL_NONE: - case VIR_STORAGE_NET_PROTOCOL_LAST: - virReportEnumRangeError(virStorageNetProtocol, src->protocol); - return NULL; + } else { + switch ((virStorageNetProtocol) src->protocol) { + case VIR_STORAGE_NET_PROTOCOL_GLUSTER: + driver = "gluster"; + if (!(fileprops = qemuBlockStorageSourceGetGlusterProps(src, onlytarget))) + return NULL; + break; + + case VIR_STORAGE_NET_PROTOCOL_VXHS: + driver = "vxhs"; + if (!(fileprops = qemuBlockStorageSourceGetVxHSProps(src, onlytarget))) + return NULL; + break; + + case VIR_STORAGE_NET_PROTOCOL_HTTP: + case VIR_STORAGE_NET_PROTOCOL_HTTPS: + case VIR_STORAGE_NET_PROTOCOL_FTP: + case VIR_STORAGE_NET_PROTOCOL_FTPS: + case VIR_STORAGE_NET_PROTOCOL_TFTP: + driver = virStorageNetProtocolTypeToString(src->protocol); + if (!(fileprops = qemuBlockStorageSourceGetCURLProps(src, onlytarget))) + return NULL; + break; + + case VIR_STORAGE_NET_PROTOCOL_ISCSI: + driver = "iscsi"; + if (!(fileprops = qemuBlockStorageSourceGetISCSIProps(src, onlytarget))) + return NULL; + break; + + case VIR_STORAGE_NET_PROTOCOL_NBD: + driver = "nbd"; + if (!(fileprops = qemuBlockStorageSourceGetNBDProps(src, onlytarget))) + return NULL; + break; + + case VIR_STORAGE_NET_PROTOCOL_RBD: + driver = "rbd"; + if (!(fileprops = qemuBlockStorageSourceGetRBDProps(src, onlytarget))) + return NULL; + break; + + case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: + driver = "sheepdog"; + if (!(fileprops = qemuBlockStorageSourceGetSheepdogProps(src))) + return NULL; + break; + + case VIR_STORAGE_NET_PROTOCOL_SSH: + driver = "ssh"; + if (!(fileprops = qemuBlockStorageSourceGetSshProps(src))) + return NULL; + break; + + case VIR_STORAGE_NET_PROTOCOL_NFS: + driver = "nfs"; + if (!(fileprops = qemuBlockStorageSourceGetNFSProps(src))) + return NULL; + break; + + case VIR_STORAGE_NET_PROTOCOL_NONE: + case VIR_STORAGE_NET_PROTOCOL_LAST: + virReportEnumRangeError(virStorageNetProtocol, src->protocol); + return NULL; + } } break; } @@ -2227,6 +2259,7 @@ qemuBlockStorageSourceCreateGetStorageProps(virStorageSource *src, g_autoptr(virJSONValue) location = NULL; const char *driver = NULL; const char *filename = NULL; + qemuDomainStorageSourcePrivate *srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); switch (actualType) { case VIR_STORAGE_TYPE_FILE: @@ -2255,6 +2288,13 @@ qemuBlockStorageSourceCreateGetStorageProps(virStorageSource *src, break; case VIR_STORAGE_NET_PROTOCOL_SSH: + if (srcPriv->nbdkitProcess) { + /* disk creation not yet supported with nbdkit, and even if it + * was supported, it would not be done with blockdev-create + * props */ + return 0; + } + driver = "ssh"; if (!(location = qemuBlockStorageSourceGetSshProps(src))) return -1; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 781b80b46d..ef1789062d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -10880,9 +10880,14 @@ qemuDomainPrepareStorageSourceBlockdevNodename(virDomainDiskDef *disk, if (qemuDomainSecretStorageSourcePrepareEncryption(priv, src, src->nodeformat) < 0) return -1; - if (qemuDomainSecretStorageSourcePrepareAuth(priv, src, - src->nodestorage) < 0) - return -1; + + if (!qemuDomainPrepareStorageSourceNbdkit(src, cfg, src->nodestorage, priv)) { + /* If we're using nbdkit to serve the storage source, we don't pass + * authentication secrets to qemu, but will pass them to nbdkit instead */ + if (qemuDomainSecretStorageSourcePrepareAuth(priv, src, + src->nodestorage) < 0) + return -1; + } if (qemuDomainPrepareStorageSourcePR(src, priv, src->nodestorage) < 0) return -1; @@ -10897,8 +10902,6 @@ qemuDomainPrepareStorageSourceBlockdevNodename(virDomainDiskDef *disk, if (qemuDomainPrepareStorageSourceFDs(src, priv) < 0) return -1; - qemuDomainPrepareStorageSourceNbdkit(src, cfg, src->nodestorage, priv); - return 0; } diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index fdefe59215..13604904a0 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -232,6 +232,17 @@ qemuExtDevicesStart(virQEMUDriver *driver, return -1; } + for (i = 0; i < def->ndisks; i++) { + virDomainDiskDef *disk = def->disks[i]; + if (qemuNbdkitStartStorageSource(driver, vm, disk->src) < 0) + return -1; + } + + if (def->os.loader && def->os.loader->nvram) { + if (qemuNbdkitStartStorageSource(driver, vm, def->os.loader->nvram) < 0) + return -1; + } + return 0; } @@ -283,6 +294,14 @@ qemuExtDevicesStop(virQEMUDriver *driver, fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) qemuVirtioFSStop(driver, vm, fs); } + + for (i = 0; i < def->ndisks; i++) { + virDomainDiskDef *disk = def->disks[i]; + qemuNbdkitStopStorageSource(disk->src); + } + + if (def->os.loader && def->os.loader->nvram) + qemuNbdkitStopStorageSource(def->os.loader->nvram); } @@ -308,10 +327,36 @@ qemuExtDevicesHasDevice(virDomainDef *def) return true; } + for (i = 0; i < def->ndisks; i++) { + qemuDomainStorageSourcePrivate *priv = + QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(def->disks[i]->src); + if (priv->nbdkitProcess) + return true; + } + + return false; } +/* recursively setup nbdkit cgroups for backing chain of src */ +static int qemuExtDevicesSetupCgroupNbdkit(virStorageSource *src, + virCgroup *cgroup) +{ + qemuDomainStorageSourcePrivate *priv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); + + if (src->backingStore) + if (qemuExtDevicesSetupCgroupNbdkit(src->backingStore, cgroup) < 0) + return -1; + + if (priv && priv->nbdkitProcess && + qemuNbdkitProcessSetupCgroup(priv->nbdkitProcess, cgroup) < 0) + return -1; + + return 0; +} + + int qemuExtDevicesSetupCgroup(virQEMUDriver *driver, virDomainObj *vm, @@ -351,6 +396,17 @@ qemuExtDevicesSetupCgroup(virQEMUDriver *driver, return -1; } + for (i = 0; i < def->ndisks; i++) { + virDomainDiskDef *disk = def->disks[i]; + if (qemuExtDevicesSetupCgroupNbdkit(disk->src, cgroup) < 0) + return -1; + } + + if (def->os.loader && def->os.loader->nvram) { + if (qemuExtDevicesSetupCgroupNbdkit(def->os.loader->nvram, cgroup) < 0) + return -1; + } + for (i = 0; i < def->nfss; i++) { virDomainFSDef *fs = def->fss[i]; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index c490e2b97a..113bd5436c 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -993,6 +993,9 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriver *driver, if (qemuHotplugAttachManagedPR(vm, disk->src, VIR_ASYNC_JOB_NONE) < 0) goto cleanup; + if (qemuNbdkitStartStorageSource(driver, vm, disk->src) < 0) + goto cleanup; + ret = qemuDomainAttachDiskGeneric(vm, disk, VIR_ASYNC_JOB_NONE); virDomainAuditDisk(vm, NULL, disk->src, "attach", ret == 0); @@ -1015,6 +1018,8 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriver *driver, if (virStorageSourceChainHasManagedPR(disk->src)) ignore_value(qemuHotplugRemoveManagedPR(vm, VIR_ASYNC_JOB_NONE)); + + qemuNbdkitStopStorageSource(disk->src); } qemuDomainSecretDiskDestroy(disk); qemuDomainCleanupStorageSourceFD(disk->src); @@ -4332,6 +4337,8 @@ qemuDomainRemoveDiskDevice(virQEMUDriver *driver, qemuHotplugRemoveManagedPR(vm, VIR_ASYNC_JOB_NONE) < 0) goto cleanup; + qemuNbdkitStopStorageSource(disk->src); + if (disk->transient) { VIR_DEBUG("Removing transient overlay '%s' of disk '%s'", disk->src->path, disk->dst); diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c index b17f9e772f..dcdd38d119 100644 --- a/src/qemu/qemu_nbdkit.c +++ b/src/qemu/qemu_nbdkit.c @@ -754,6 +754,61 @@ qemuNbdkitInitStorageSource(qemuNbdkitCaps *caps, } +static int +qemuNbdkitStartStorageSourceOne(virQEMUDriver *driver, + virDomainObj *vm, + virStorageSource *src) +{ + qemuDomainStorageSourcePrivate *priv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); + + if (priv && priv->nbdkitProcess && + qemuNbdkitProcessStart(priv->nbdkitProcess, vm, driver) < 0) + return -1; + + return 0; +} + + +/* recursively start nbdkit for backing chain of src */ +int +qemuNbdkitStartStorageSource(virQEMUDriver *driver, + virDomainObj *vm, + virStorageSource *src) +{ + virStorageSource *backing; + + for (backing = src->backingStore; backing != NULL; backing = backing->backingStore) + if (qemuNbdkitStartStorageSourceOne(driver, vm, backing) < 0) + return -1; + + return qemuNbdkitStartStorageSourceOne(driver, vm, src); +} + + +static void +qemuNbdkitStopStorageSourceOne(virStorageSource *src) +{ + qemuDomainStorageSourcePrivate *priv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); + + if (priv && priv->nbdkitProcess && + qemuNbdkitProcessStop(priv->nbdkitProcess) < 0) + VIR_WARN("Unable to stop nbdkit for storage source '%s'", src->nodestorage); +} + + +/* recursively stop nbdkit processes for backing chain of src */ +void +qemuNbdkitStopStorageSource(virStorageSource *src) +{ + virStorageSource *backing; + + qemuNbdkitStopStorageSourceOne(src); + + for (backing = src->backingStore; backing != NULL; backing = backing->backingStore) + qemuNbdkitStopStorageSourceOne(backing); +} + + static int qemuNbdkitCommandPassDataByPipe(virCommand *cmd, const char *argName, @@ -941,6 +996,14 @@ qemuNbdkitProcessFree(qemuNbdkitProcess *proc) } +int +qemuNbdkitProcessSetupCgroup(qemuNbdkitProcess *proc, + virCgroup *cgroup) +{ + return virCgroupAddProcess(cgroup, proc->pid); +} + + int qemuNbdkitProcessStart(qemuNbdkitProcess *proc, virDomainObj *vm, diff --git a/src/qemu/qemu_nbdkit.h b/src/qemu/qemu_nbdkit.h index 7e2aeed4eb..36a2219d82 100644 --- a/src/qemu/qemu_nbdkit.h +++ b/src/qemu/qemu_nbdkit.h @@ -21,6 +21,7 @@ #include "internal.h" #include "storage_source_conf.h" +#include "vircgroup.h" #include "virenum.h" #include "virfilecache.h" @@ -59,6 +60,14 @@ qemuNbdkitReconnectStorageSource(virStorageSource *source, const char *pidfile, const char *socketfile); +int +qemuNbdkitStartStorageSource(virQEMUDriver *driver, + virDomainObj *vm, + virStorageSource *src); + +void +qemuNbdkitStopStorageSource(virStorageSource *src); + void qemuNbdkitStorageSourceManageProcess(virStorageSource *src); @@ -95,4 +104,8 @@ qemuNbdkitProcessStop(qemuNbdkitProcess *proc); void qemuNbdkitProcessFree(qemuNbdkitProcess *proc); +int +qemuNbdkitProcessSetupCgroup(qemuNbdkitProcess *proc, + virCgroup *cgroup); + G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuNbdkitProcess, qemuNbdkitProcessFree); diff --git a/tests/qemuxml2argvdata/disk-cdrom-network-nbdkit.x86_64-latest.args b/tests/qemuxml2argvdata/disk-cdrom-network-nbdkit.x86_64-latest.args new file mode 100644 index 0000000000..eec7ef2af7 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-cdrom-network-nbdkit.x86_64-latest.args @@ -0,0 +1,42 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram \ +-accel kvm \ +-cpu qemu64 \ +-m 1024 \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":1073741824}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ +-blockdev '{"driver":"nbd","server":{"type":"unix","path":"/tmp/lib/domain--1-QEMUGuest1/nbdkit-libvirt-3-storage.socket"},"node-name":"libvirt-3-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-3-format","read-only":true,"driver":"raw","file":"libvirt-3-storage"}' \ +-device '{"driver":"ide-cd","bus":"ide.0","unit":0,"drive":"libvirt-3-format","id":"ide0-0-0","bootindex":1}' \ +-blockdev '{"driver":"nbd","server":{"type":"unix","path":"/tmp/lib/domain--1-QEMUGuest1/nbdkit-libvirt-2-storage.socket"},"node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-2-format","read-only":true,"driver":"raw","file":"libvirt-2-storage"}' \ +-device '{"driver":"ide-cd","bus":"ide.0","unit":1,"drive":"libvirt-2-format","id":"ide0-0-1"}' \ +-blockdev '{"driver":"nbd","server":{"type":"unix","path":"/tmp/lib/domain--1-QEMUGuest1/nbdkit-libvirt-1-storage.socket"},"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":true,"driver":"raw","file":"libvirt-1-storage"}' \ +-device '{"driver":"ide-cd","bus":"ide.1","unit":0,"drive":"libvirt-1-format","id":"ide0-1-0"}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x2"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/disk-cdrom-network-nbdkit.xml b/tests/qemuxml2argvdata/disk-cdrom-network-nbdkit.xml new file mode 120000 index 0000000000..55f677546f --- /dev/null +++ b/tests/qemuxml2argvdata/disk-cdrom-network-nbdkit.xml @@ -0,0 +1 @@ +disk-cdrom-network.xml \ No newline at end of file diff --git a/tests/qemuxml2argvdata/disk-network-http-nbdkit.x86_64-latest.args b/tests/qemuxml2argvdata/disk-network-http-nbdkit.x86_64-latest.args new file mode 100644 index 0000000000..25d476d3ce --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-http-nbdkit.x86_64-latest.args @@ -0,0 +1,45 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram \ +-accel kvm \ +-cpu qemu64 \ +-m 214 \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ +-blockdev '{"driver":"nbd","server":{"type":"unix","path":"/tmp/lib/domain--1-QEMUGuest1/nbdkit-libvirt-4-storage.socket"},"node-name":"libvirt-4-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-4-format","read-only":false,"driver":"raw","file":"libvirt-4-storage"}' \ +-device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x2","drive":"libvirt-4-format","id":"virtio-disk0","bootindex":1}' \ +-blockdev '{"driver":"nbd","server":{"type":"unix","path":"/tmp/lib/domain--1-QEMUGuest1/nbdkit-libvirt-3-storage.socket"},"node-name":"libvirt-3-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-3-format","read-only":false,"driver":"raw","file":"libvirt-3-storage"}' \ +-device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x3","drive":"libvirt-3-format","id":"virtio-disk1"}' \ +-blockdev '{"driver":"nbd","server":{"type":"unix","path":"/tmp/lib/domain--1-QEMUGuest1/nbdkit-libvirt-2-storage.socket"},"node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-2-format","read-only":false,"driver":"raw","file":"libvirt-2-storage"}' \ +-device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x4","drive":"libvirt-2-format","id":"virtio-disk2"}' \ +-blockdev '{"driver":"nbd","server":{"type":"unix","path":"/tmp/lib/domain--1-QEMUGuest1/nbdkit-libvirt-1-storage.socket"},"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \ +-device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x5","drive":"libvirt-1-format","id":"virtio-disk3"}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/disk-network-http-nbdkit.xml b/tests/qemuxml2argvdata/disk-network-http-nbdkit.xml new file mode 120000 index 0000000000..6a05204e8a --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-http-nbdkit.xml @@ -0,0 +1 @@ +disk-network-http.xml \ No newline at end of file diff --git a/tests/qemuxml2argvdata/disk-network-source-curl-nbdkit-backing.x86_64-latest.args b/tests/qemuxml2argvdata/disk-network-source-curl-nbdkit-backing.x86_64-latest.args new file mode 100644 index 0000000000..98cfcd219a --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-source-curl-nbdkit-backing.x86_64-latest.args @@ -0,0 +1,38 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram \ +-accel tcg \ +-cpu qemu64 \ +-m 214 \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ +-blockdev '{"driver":"nbd","server":{"type":"unix","path":"/tmp/lib/domain--1-QEMUGuest1/nbdkit-libvirt-2-storage.socket"},"node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-2-format","read-only":true,"driver":"qcow2","file":"libvirt-2-storage"}' \ +-blockdev '{"driver":"nbd","server":{"type":"unix","path":"/tmp/lib/domain--1-QEMUGuest1/nbdkit-libvirt-1-storage.socket"},"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":true,"driver":"qcow2","file":"libvirt-1-storage","backing":"libvirt-2-format"}' \ +-device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x2","drive":"libvirt-1-format","id":"virtio-disk0","bootindex":1}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/disk-network-source-curl-nbdkit-backing.xml b/tests/qemuxml2argvdata/disk-network-source-curl-nbdkit-backing.xml new file mode 100644 index 0000000000..37a30fcbd6 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-source-curl-nbdkit-backing.xml @@ -0,0 +1,45 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='network' device='disk'> + <driver name='qemu' type='qcow2'/> + <source protocol='https' name='path/to/disk1.qcow2'> + <host name='https.example.org' port='8443'/> + <cookies> + <cookie name='cookie1'>cookievalue1</cookie> + <cookie name='cookie2'>cookievalue2</cookie> + </cookies> + </source> + <backingStore type='network'> + <format type='qcow2'/> + <source protocol='https' name='path/to/backing.qcow2'> + <host name='https.example2.org' port='8444'/> + <cookies> + <cookie name='cookie3'>cookievalue3</cookie> + <cookie name='cookie4'>cookievalue4</cookie> + </cookies> + </source> + </backingStore> + <target dev='vda' bus='virtio'/> + <readonly/> + </disk> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/disk-network-source-curl-nbdkit.x86_64-latest.args b/tests/qemuxml2argvdata/disk-network-source-curl-nbdkit.x86_64-latest.args new file mode 100644 index 0000000000..ec193bb10a --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-source-curl-nbdkit.x86_64-latest.args @@ -0,0 +1,50 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram \ +-accel tcg \ +-cpu qemu64 \ +-m 214 \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ +-device '{"driver":"ahci","id":"sata0","bus":"pci.0","addr":"0x2"}' \ +-blockdev '{"driver":"nbd","server":{"type":"unix","path":"/tmp/lib/domain--1-QEMUGuest1/nbdkit-libvirt-5-storage.socket"},"node-name":"libvirt-5-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-5-format","read-only":true,"driver":"raw","file":"libvirt-5-storage"}' \ +-device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x3","drive":"libvirt-5-format","id":"virtio-disk0","bootindex":1}' \ +-object '{"qom-type":"secret","id":"libvirt-4-format-encryption-secret0","data":"9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1","keyid":"masterKey0","iv":"AAECAwQFBgcICQoLDA0ODw==","format":"base64"}' \ +-blockdev '{"driver":"nbd","server":{"type":"unix","path":"/tmp/lib/domain--1-QEMUGuest1/nbdkit-libvirt-4-storage.socket"},"node-name":"libvirt-4-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-4-format","read-only":false,"driver":"luks","key-secret":"libvirt-4-format-encryption-secret0","file":"libvirt-4-storage"}' \ +-device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x4","drive":"libvirt-4-format","id":"virtio-disk4"}' \ +-blockdev '{"driver":"nbd","server":{"type":"unix","path":"/tmp/lib/domain--1-QEMUGuest1/nbdkit-libvirt-3-storage.socket"},"node-name":"libvirt-3-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-3-format","read-only":true,"driver":"raw","file":"libvirt-3-storage"}' \ +-device '{"driver":"ide-cd","bus":"sata0.1","drive":"libvirt-3-format","id":"sata0-0-1"}' \ +-blockdev '{"driver":"nbd","server":{"type":"unix","path":"/tmp/lib/domain--1-QEMUGuest1/nbdkit-libvirt-2-storage.socket"},"node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-2-format","read-only":true,"driver":"raw","file":"libvirt-2-storage"}' \ +-device '{"driver":"ide-cd","bus":"sata0.2","drive":"libvirt-2-format","id":"sata0-0-2"}' \ +-blockdev '{"driver":"nbd","server":{"type":"unix","path":"/tmp/lib/domain--1-QEMUGuest1/nbdkit-libvirt-1-storage.socket"},"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":true,"driver":"raw","file":"libvirt-1-storage"}' \ +-device '{"driver":"ide-cd","bus":"sata0.3","drive":"libvirt-1-format","id":"sata0-0-3"}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/disk-network-source-curl-nbdkit.xml b/tests/qemuxml2argvdata/disk-network-source-curl-nbdkit.xml new file mode 120000 index 0000000000..4a1e40bd70 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-source-curl-nbdkit.xml @@ -0,0 +1 @@ +disk-network-source-curl.xml \ No newline at end of file diff --git a/tests/qemuxml2argvdata/disk-network-source-curl.x86_64-latest.args b/tests/qemuxml2argvdata/disk-network-source-curl.x86_64-latest.args new file mode 100644 index 0000000000..ec6dd13f6c --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-source-curl.x86_64-latest.args @@ -0,0 +1,53 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram \ +-accel tcg \ +-cpu qemu64 \ +-m 214 \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ +-device '{"driver":"ahci","id":"sata0","bus":"pci.0","addr":"0x2"}' \ +-object '{"qom-type":"secret","id":"libvirt-5-storage-httpcookie-secret0","data":"BUU0KmnWfonHdjzhYhwVQZ5iTI1KweTJ22q8XWUVoBCVu1z70reDuczPBIabZtC3","keyid":"masterKey0","iv":"AAECAwQFBgcICQoLDA0ODw==","format":"base64"}' \ +-blockdev '{"driver":"https","url":"https://https.example.org:8443/path/to/disk1.iso","cookie-secret":"libvirt-5-storage-httpcookie-secret0","node-name":"libvirt-5-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-5-format","read-only":true,"driver":"raw","file":"libvirt-5-storage"}' \ +-device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x3","drive":"libvirt-5-format","id":"virtio-disk0","bootindex":1}' \ +-object '{"qom-type":"secret","id":"libvirt-4-format-encryption-secret0","data":"9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1","keyid":"masterKey0","iv":"AAECAwQFBgcICQoLDA0ODw==","format":"base64"}' \ +-object '{"qom-type":"secret","id":"libvirt-4-storage-httpcookie-secret0","data":"BUU0KmnWfonHdjzhYhwVQZ5iTI1KweTJ22q8XWUVoBCVu1z70reDuczPBIabZtC3","keyid":"masterKey0","iv":"AAECAwQFBgcICQoLDA0ODw==","format":"base64"}' \ +-blockdev '{"driver":"https","url":"https://https.example.org:8443/path/to/disk5.iso?foo=bar","sslverify":false,"cookie-secret":"libvirt-4-storage-httpcookie-secret0","node-name":"libvirt-4-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-4-format","read-only":false,"driver":"luks","key-secret":"libvirt-4-format-encryption-secret0","file":"libvirt-4-storage"}' \ +-device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x4","drive":"libvirt-4-format","id":"virtio-disk4"}' \ +-object '{"qom-type":"secret","id":"libvirt-3-storage-httpcookie-secret0","data":"BUU0KmnWfonHdjzhYhwVQZ5iTI1KweTJ22q8XWUVoBBv7TuTgTkyAyOPpC2P5qLbOIypLoHpppjz+u5O+X8oT+jA1m7q/OJQ8dk2EFD5c0A=","keyid":"masterKey0","iv":"AAECAwQFBgcICQoLDA0ODw==","format":"base64"}' \ +-blockdev '{"driver":"http","url":"http://http.example.org:8080/path/to/disk2.iso","cookie-secret":"libvirt-3-storage-httpcookie-secret0","node-name":"libvirt-3-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-3-format","read-only":true,"driver":"raw","file":"libvirt-3-storage"}' \ +-device '{"driver":"ide-cd","bus":"sata0.1","drive":"libvirt-3-format","id":"sata0-0-1"}' \ +-blockdev '{"driver":"ftp","url":"ftp://ftp.example.org:20/path/to/disk3.iso","node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-2-format","read-only":true,"driver":"raw","file":"libvirt-2-storage"}' \ +-device '{"driver":"ide-cd","bus":"sata0.2","drive":"libvirt-2-format","id":"sata0-0-2"}' \ +-blockdev '{"driver":"ftps","url":"ftps://ftps.example.org:22/path/to/disk4.iso","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":true,"driver":"raw","file":"libvirt-1-storage"}' \ +-device '{"driver":"ide-cd","bus":"sata0.3","drive":"libvirt-1-format","id":"sata0-0-3"}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/disk-network-source-curl.xml b/tests/qemuxml2argvdata/disk-network-source-curl.xml new file mode 100644 index 0000000000..1e50314abe --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-source-curl.xml @@ -0,0 +1,71 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='network' device='disk'> + <source protocol='https' name='path/to/disk1.iso'> + <host name='https.example.org' port='8443'/> + <cookies> + <cookie name='cookie1'>cookievalue1</cookie> + <cookie name='cookie2'>cookievalue2</cookie> + </cookies> + </source> + <target dev='vda' bus='virtio'/> + <readonly/> + </disk> + <disk type='network' device='cdrom'> + <source protocol='http' name='path/to/disk2.iso'> + <host name='http.example.org' port='8080'/> + <cookies> + <cookie name='cookie1'>cookievalue1</cookie> + <cookie name='cookie2'>cookievalue2</cookie> + <cookie name='cookie3'>cookievalue3</cookie> + </cookies> + </source> + <target dev='hdb' bus='sata'/> + </disk> + <disk type='network' device='cdrom'> + <source protocol='ftp' name='path/to/disk3.iso'> + <host name='ftp.example.org' port='20'/> + </source> + <target dev='hdc' bus='sata'/> + </disk> + <disk type='network' device='cdrom'> + <source protocol='ftps' name='path/to/disk4.iso'> + <host name='ftps.example.org' port='22'/> + </source> + <target dev='hdd' bus='sata'/> + </disk> + <disk type='network' device='disk'> + <source protocol='https' name='path/to/disk5.iso' query='foo=bar'> + <host name='https.example.org' port='8443'/> + <ssl verify='no'/> + <cookies> + <cookie name='cookie1'>cookievalue1</cookie> + <cookie name='cookie2'>cookievalue2</cookie> + </cookies> + <encryption format='luks'> + <secret type='passphrase' uuid='1148b693-0843-4cef-9f97-8feb4e1ae365'/> + </encryption> + </source> + <target dev='vde' bus='virtio'/> + </disk> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/disk-network-ssh-nbdkit.x86_64-latest.args b/tests/qemuxml2argvdata/disk-network-ssh-nbdkit.x86_64-latest.args new file mode 100644 index 0000000000..e22ba095b1 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-ssh-nbdkit.x86_64-latest.args @@ -0,0 +1,36 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram \ +-accel kvm \ +-cpu qemu64 \ +-m 214 \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ +-blockdev '{"driver":"nbd","server":{"type":"unix","path":"/tmp/lib/domain--1-QEMUGuest1/nbdkit-libvirt-1-storage.socket"},"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \ +-device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x2","drive":"libvirt-1-format","id":"virtio-disk0","bootindex":1}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/disk-network-ssh-nbdkit.xml b/tests/qemuxml2argvdata/disk-network-ssh-nbdkit.xml new file mode 120000 index 0000000000..b0589bdfb5 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-ssh-nbdkit.xml @@ -0,0 +1 @@ +disk-network-ssh.xml \ No newline at end of file diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index f2f7467102..6b9785b32d 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1262,6 +1262,7 @@ mymain(void) DO_TEST_CAPS_LATEST("disk-cdrom-empty-network-invalid"); DO_TEST_CAPS_LATEST("disk-cdrom-bus-other"); DO_TEST_CAPS_LATEST("disk-cdrom-network"); + DO_TEST_CAPS_LATEST_NBDKIT("disk-cdrom-network-nbdkit", QEMU_NBDKIT_CAPS_PLUGIN_CURL); DO_TEST_CAPS_LATEST("disk-cdrom-tray"); DO_TEST_CAPS_LATEST("disk-floppy"); DO_TEST_CAPS_LATEST("disk-floppy-q35"); @@ -1302,6 +1303,9 @@ mymain(void) /* qemu-6.0 is the last qemu version supporting sheepdog */ DO_TEST_CAPS_VER("disk-network-sheepdog", "6.0.0"); DO_TEST_CAPS_LATEST("disk-network-source-auth"); + DO_TEST_CAPS_LATEST("disk-network-source-curl"); + DO_TEST_CAPS_LATEST_NBDKIT("disk-network-source-curl-nbdkit", QEMU_NBDKIT_CAPS_PLUGIN_CURL); + DO_TEST_CAPS_LATEST_NBDKIT("disk-network-source-curl-nbdkit-backing", QEMU_NBDKIT_CAPS_PLUGIN_CURL); DO_TEST_CAPS_LATEST("disk-network-nfs"); driver.config->vxhsTLS = 1; driver.config->nbdTLSx509secretUUID = g_strdup("6fd3f62d-9fe7-4a4e-a869-7acd6376d8ea"); @@ -1312,7 +1316,9 @@ mymain(void) DO_TEST_CAPS_LATEST("disk-network-tlsx509-nbd-hostname"); DO_TEST_CAPS_VER("disk-network-tlsx509-vxhs", "5.0.0"); DO_TEST_CAPS_LATEST("disk-network-http"); + DO_TEST_CAPS_LATEST_NBDKIT("disk-network-http-nbdkit", QEMU_NBDKIT_CAPS_PLUGIN_CURL); DO_TEST_CAPS_LATEST("disk-network-ssh"); + DO_TEST_CAPS_LATEST_NBDKIT("disk-network-ssh-nbdkit", QEMU_NBDKIT_CAPS_PLUGIN_SSH); driver.config->vxhsTLS = 0; VIR_FREE(driver.config->vxhsTLSx509certdir); DO_TEST_CAPS_LATEST("disk-no-boot"); -- 2.39.1

On 2/16/23 9:55 AM, Peter Krempa wrote: I just figured that logically the backing source should always be available to the upper layers, so I would try to be consistent and always set them up that way. But I agree that this is not really necessary here and makes the code less concise, so I'll change them.

Add a private function to peek at the list of send buffers in virCommand so that it is testable Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + src/util/vircommand.c | 17 +++++++++-------- src/util/vircommand.h | 8 ++++++++ src/util/vircommandpriv.h | 4 ++++ 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 97c3d86217..b73449fcad 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2097,6 +2097,7 @@ virCommandNewArgs; virCommandNewVAList; virCommandNonblockingFDs; virCommandPassFD; +virCommandPeekSendBuffers; virCommandRawStatus; virCommandRequireHandshake; virCommandRun; diff --git a/src/util/vircommand.c b/src/util/vircommand.c index aef63efb26..1ef1411436 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -78,14 +78,6 @@ struct _virCommandFD { unsigned int flags; }; -typedef struct _virCommandSendBuffer virCommandSendBuffer; -struct _virCommandSendBuffer { - int fd; - unsigned char *buffer; - size_t buflen; - off_t offset; -}; - struct _virCommand { int has_error; /* 0 on success, -1 on error */ @@ -3454,3 +3446,12 @@ virCommandSetRunAmong(virCommand *cmd, cmd->schedCore = pid; } + +void +virCommandPeekSendBuffers(virCommand *cmd, + virCommandSendBuffer **buffers, + int *nbuffers) +{ + *buffers = cmd->sendBuffers; + *nbuffers = cmd->numSendBuffers; +} diff --git a/src/util/vircommand.h b/src/util/vircommand.h index e0002103b6..0f14d14e49 100644 --- a/src/util/vircommand.h +++ b/src/util/vircommand.h @@ -24,6 +24,14 @@ #include "internal.h" #include "virbuffer.h" +typedef struct _virCommandSendBuffer virCommandSendBuffer; +struct _virCommandSendBuffer { + int fd; + unsigned char *buffer; + size_t buflen; + off_t offset; +}; + typedef struct _virCommand virCommand; /* This will execute in the context of the first child diff --git a/src/util/vircommandpriv.h b/src/util/vircommandpriv.h index ff17fa5ded..d579810bb5 100644 --- a/src/util/vircommandpriv.h +++ b/src/util/vircommandpriv.h @@ -47,3 +47,7 @@ void virCommandSetDryRun(virCommandDryRunToken *tok, bool bufCommandStripPath, virCommandDryRunCallback cb, void *opaque); + +void virCommandPeekSendBuffers(virCommand *cmd, + virCommandSendBuffer **buffers, + int *nbuffers); -- 2.39.1

Adds the ability to monitor the nbdkit process so that we can take action in case the child exits unexpectedly. When the nbdkit process exits, we pause the vm, restart nbdkit, and then resume the vm. This allows the vm to continue working in the event of a nbdkit failure. Eventually we may want to generalize this functionality since we may need something similar for e.g. qemu-storage-daemon, etc. The process is monitored with the pidfd_open() syscall if it exists (since linux 5.3). Otherwise it resorts to checking whether the process is alive once a second. The one-second time period was chosen somewhat arbitrarily. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; --- meson.build | 7 ++ src/qemu/qemu_nbdkit.c | 147 ++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_nbdkit.h | 4 +- src/qemu/qemu_process.c | 4 +- 4 files changed, 155 insertions(+), 7 deletions(-) diff --git a/meson.build b/meson.build index e498b49be4..048b15ff71 100644 --- a/meson.build +++ b/meson.build @@ -645,6 +645,13 @@ symbols = [ [ 'sched.h', 'cpu_set_t' ], ] +if host_machine.system() == 'linux' + symbols += [ + # process management + [ 'sys/syscall.h', 'SYS_pidfd_open' ], + ] +endif + foreach symbol : symbols if cc.has_header_symbol(symbol[0], symbol[1], args: '-D_GNU_SOURCE', prefix: symbol.get(2, '')) conf.set(&#39;WITH_DECL_@0(a)&#39;.format(symbol[1].to_upper()), 1) diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c index e6d3232a1e..74e2ceee19 100644 --- a/src/qemu/qemu_nbdkit.c +++ b/src/qemu/qemu_nbdkit.c @@ -19,9 +19,11 @@ #include <config.h> #include <glib.h> +#include <sys/syscall.h> #include "vircommand.h" #include "virerror.h" +#include "virevent.h" #include "virlog.h" #include "virpidfile.h" #include "virsecureerase.h" @@ -35,6 +37,7 @@ #include "qemu_nbdkit.h" #define LIBVIRT_QEMU_NBDKITPRIV_H_ALLOW #include "qemu_nbdkitpriv.h" +#include "qemu_process.h" #include "qemu_security.h" #include <fcntl.h> @@ -613,6 +616,123 @@ qemuNbdkitCapsCacheNew(const char *cachedir) } +static void +qemuNbdkitProcessRestart(qemuNbdkitProcess *proc, + virDomainObj *vm) +{ + qemuDomainObjPrivate *vmpriv = vm->privateData; + virQEMUDriver *driver = vmpriv->driver; + + /* clean up resources associated with process */ + qemuNbdkitProcessStop(proc); + + if (qemuNbdkitProcessStart(proc, vm, driver) < 0) + VIR_WARN("Unable to restart nbkdit process"); +} + + +typedef struct { + qemuNbdkitProcess *proc; + virDomainObj *vm; +} qemuNbdkitProcessEventData; + + +static qemuNbdkitProcessEventData* +qemuNbdkitProcessEventDataNew(qemuNbdkitProcess *proc, + virDomainObj *vm) +{ + qemuNbdkitProcessEventData *d = g_new(qemuNbdkitProcessEventData, 1); + d->proc = proc; + d->vm = virObjectRef(vm); + return d; +} + + +static void +qemuNbdkitProcessEventDataFree(qemuNbdkitProcessEventData *d) +{ + virObjectUnref(d->vm); + g_free(d); +} + + +#if WITH_DECL_SYS_PIDFD_OPEN +static void +qemuNbdkitProcessPidfdCb(int watch G_GNUC_UNUSED, + int fd, + int events G_GNUC_UNUSED, + void *opaque) +{ + qemuNbdkitProcessEventData *d = opaque; + + VIR_FORCE_CLOSE(fd); + VIR_DEBUG("nbdkit process %i died", d->proc->pid); + qemuNbdkitProcessRestart(d->proc, d->vm); +} +#else +static void +qemuNbdkitProcessTimeoutCb(int timer G_GNUC_UNUSED, + void *opaque) +{ + qemuNbdkitProcessEventData *d = opaque; + + if (virProcessKill(d->proc->pid, 0) < 0) { + VIR_DEBUG("nbdkit process %i died", d->proc->pid); + qemuNbdkitProcessRestart(d->proc, d->vm); + } +} +#endif /* WITH_DECL_SYS_PIDFD_OPEN */ + + +static int +qemuNbdkitProcessStartMonitor(qemuNbdkitProcess *proc, + virDomainObj *vm) +{ +#if WITH_DECL_SYS_PIDFD_OPEN + int pidfd; +#endif + +#if WITH_DECL_SYS_PIDFD_OPEN + pidfd = syscall(SYS_pidfd_open, proc->pid, 0); + if (pidfd < 0) + return -1; + + proc->eventwatch = virEventAddHandle(pidfd, + VIR_EVENT_HANDLE_READABLE, + qemuNbdkitProcessPidfdCb, + qemuNbdkitProcessEventDataNew(proc, vm), + (virFreeCallback)qemuNbdkitProcessEventDataFree); +#else + /* fall back to checking once a second */ + proc->eventwatch = virEventAddTimeout(1000, + qemuNbdkitProcessTimeoutCb, + qemuNbdkitProcessEventDataNew(proc, vm), + (virFreeCallback)qemuNbdkitProcessEventDataFree); +#endif /* WITH_DECL_SYS_PIDFD_OPEN */ + + if (proc->eventwatch < 0) + return -1; + + VIR_DEBUG("Monitoring nbdkit process %i for exit", proc->pid); + + return 0; +} + + +static void +qemuNbdkitProcessStopMonitor(qemuNbdkitProcess *proc) +{ + if (proc->eventwatch > 0) { +#if WITH_DECL_SYS_PIDFD_OPEN + virEventRemoveHandle(proc->eventwatch); +#else + virEventRemoveTimeout(proc->eventwatch); +#endif /* WITH_DECL_SYS_PIDFD_OPEN */ + proc->eventwatch = 0; + } +} + + static qemuNbdkitProcess * qemuNbdkitProcessNew(virStorageSource *source, const char *pidfile, @@ -660,9 +780,11 @@ qemuNbdkitReconnectStorageSource(virStorageSource *source, static void -qemuNbdkitStorageSourceManageProcessOne(virStorageSource *source) +qemuNbdkitStorageSourceManageProcessOne(virStorageSource *source, + virDomainObj *vm) { qemuDomainStorageSourcePrivate *srcpriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(source); + qemuDomainObjPrivate *vmpriv = vm->privateData; qemuNbdkitProcess *proc; if (!srcpriv) @@ -673,6 +795,9 @@ qemuNbdkitStorageSourceManageProcessOne(virStorageSource *source) if (!proc) return; + if (!proc->caps) + proc->caps = qemuGetNbdkitCaps(vmpriv->driver); + if (proc->pid <= 0) { if (virPidFileReadPath(proc->pidfile, &proc->pid) < 0) { VIR_WARN("Unable to read pidfile '%s'", proc->pidfile); @@ -680,8 +805,14 @@ qemuNbdkitStorageSourceManageProcessOne(virStorageSource *source) } } - if (virProcessKill(proc->pid, 0) < 0) + if (virProcessKill(proc->pid, 0) < 0) { VIR_WARN("nbdkit process %i is not alive", proc->pid); + qemuNbdkitProcessRestart(proc, vm); + return; + } + + if (qemuNbdkitProcessStartMonitor(proc, vm) < 0) + VIR_WARN("unable monitor nbdkit process"); } /** @@ -694,11 +825,12 @@ qemuNbdkitStorageSourceManageProcessOne(virStorageSource *source) * disk and is attempting to re-connect to active domains. */ void -qemuNbdkitStorageSourceManageProcess(virStorageSource *source) +qemuNbdkitStorageSourceManageProcess(virStorageSource *source, + virDomainObj *vm) { virStorageSource *backing; for (backing = source; backing != NULL; backing = backing->backingStore) - qemuNbdkitStorageSourceManageProcessOne(backing); + qemuNbdkitStorageSourceManageProcessOne(backing, vm); } @@ -991,6 +1123,8 @@ qemuNbdkitProcessBuildCommand(qemuNbdkitProcess *proc) void qemuNbdkitProcessFree(qemuNbdkitProcess *proc) { + qemuNbdkitProcessStopMonitor(proc); + g_clear_pointer(&proc->pidfile, g_free); g_clear_pointer(&proc->socketfile, g_free); g_clear_object(&proc->caps); @@ -1077,6 +1211,9 @@ qemuNbdkitProcessStart(qemuNbdkitProcess *proc, goto error; } + if (qemuNbdkitProcessStartMonitor(proc, vm) < 0) + goto error; + return 0; error: @@ -1098,6 +1235,8 @@ qemuNbdkitProcessStart(qemuNbdkitProcess *proc, int qemuNbdkitProcessStop(qemuNbdkitProcess *proc) { + qemuNbdkitProcessStopMonitor(proc); + if (proc->pid < 0) return 0; diff --git a/src/qemu/qemu_nbdkit.h b/src/qemu/qemu_nbdkit.h index 36a2219d82..326f3d5920 100644 --- a/src/qemu/qemu_nbdkit.h +++ b/src/qemu/qemu_nbdkit.h @@ -69,7 +69,8 @@ void qemuNbdkitStopStorageSource(virStorageSource *src); void -qemuNbdkitStorageSourceManageProcess(virStorageSource *src); +qemuNbdkitStorageSourceManageProcess(virStorageSource *src, + virDomainObj *vm); bool qemuNbdkitCapsGet(qemuNbdkitCaps *nbdkitCaps, @@ -91,6 +92,7 @@ struct _qemuNbdkitProcess { uid_t user; gid_t group; pid_t pid; + int eventwatch; }; int diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d0b797575d..390f1c2862 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -9036,10 +9036,10 @@ qemuProcessReconnect(void *opaque) } for (i = 0; i < obj->def->ndisks; i++) - qemuNbdkitStorageSourceManageProcess(obj->def->disks[i]->src); + qemuNbdkitStorageSourceManageProcess(obj->def->disks[i]->src, obj); if (obj->def->os.loader && obj->def->os.loader->nvram) - qemuNbdkitStorageSourceManageProcess(obj->def->os.loader->nvram); + qemuNbdkitStorageSourceManageProcess(obj->def->os.loader->nvram, obj); /* update domain state XML with possibly updated state in virDomainObj */ if (virDomainObjSave(obj, driver->xmlopt, cfg->stateDir) < 0) -- 2.39.1

On Tue, Feb 14, 2023 at 11:08:14 -0600, Jonathon Jongsma wrote: This paragraph doesn't really state what to put into 'type' for ssh as 'ceph' and 'iscsi' are only mentioned. For 'ssh' we need a 'ssh' type. This will also tie nicely in case you want to do ssh key authentication with encrypted key, where you can pass the key via a different type 'ssh-key' or something like that to unlock the key.

In order to make ssh disks usable, we need to be able to validate a remote host. To do this, add a <knownHosts> xml element for ssh disks to allow the user to specify a location for a file that contains known host keys. Implementation to follow. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; --- docs/formatdomain.rst | 6 ++++++ src/conf/schemas/domaincommon.rng | 11 +++++++++++ 2 files changed, 17 insertions(+) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index bf071255c5..d5ad5d80b0 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -2953,6 +2953,12 @@ paravirtualized driver is specified via the ``disk`` element. If the reconnect feature is enabled, accepts ``yes`` and ``no`` ``timeout`` The amount of seconds after which hypervisor tries to reconnect. + ``knownHosts`` + For storage accessed via the ``ssh`` protocol, this element configures a + path to a file containing a list of known ssh hosts to be used to verify + the remote host. The location of the file is specified via the ``path`` + attribute. + :since:`Since 9.1.0` For a "file" or "volume" disk type which represents a cdrom or floppy (the diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index bc582e450e..f38f1f3ff1 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -2160,6 +2160,14 @@ </element> </define> + <define name="diskSourceNetworkProtocolSSHHostVerify"> + <element name="knownHosts"> + <attribute name="path"> + <ref name="absFilePath"/> + </attribute> + </element> + </define> + <define name="diskSourceNetworkProtocolSSH"> <element name="source"> <interleave> @@ -2175,6 +2183,9 @@ <ref name="encryption"/> </optional> <ref name="diskSourceNetworkProtocolPropsCommon"/> + <optional> + <ref name="diskSourceNetworkProtocolSSHHostVerify"/> + </optional> <optional> <ref name="diskAuth"/> </optional> -- 2.39.1

On 2/16/23 10:45 AM, Peter Krempa wrote: It expects a prefilled known hosts file. Here's what it says in the manpage for nbdkit-ssh-plugin: Known hosts The SSH server’s host key is checked at connection time, and must be present and correct in the local "known hosts" file. If you have never connected to the SSH server before then the connection will usually fail. You can: • connect to the server first using ssh(1) so you can manually accept the host key, or • provide the host key in an alternate file which you specify using the "known-hosts" option, or • set verify-remote-host=false on the command line. This latter option is dangerous because it allows a MITM attack to be conducted against you.

For ssh disks that are served by nbdkit, use the configured value for knownHosts and pass it to the nbdkit process. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; --- src/conf/domain_conf.c | 8 ++++++++ src/conf/storage_source_conf.c | 1 + src/conf/storage_source_conf.h | 2 ++ src/qemu/qemu_nbdkit.c | 3 +++ tests/qemunbdkitdata/disk-network-ssh-password.args.disk0 | 3 ++- tests/qemunbdkitdata/disk-network-ssh.args.disk0 | 3 ++- tests/qemuxml2argvdata/disk-network-ssh-password.xml | 1 + tests/qemuxml2argvdata/disk-network-ssh.xml | 1 + 8 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a5578324b9..cb9d01dc6d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7214,6 +7214,11 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node, return -1; } } + if (src->protocol == VIR_STORAGE_NET_PROTOCOL_SSH && + (tmpnode = virXPathNode("./knownHosts", ctxt))) { + if (!(src->ssh_known_hosts_file = virXMLPropStringRequired(tmpnode, "path"))) + return -1; + } return 0; } @@ -22091,6 +22096,9 @@ virDomainDiskSourceFormatNetwork(virBuffer *attrBuf, if (src->timeout) virBufferAsprintf(childBuf, "<timeout seconds='%llu'/>\n", src->timeout); + + if (src->protocol == VIR_STORAGE_NET_PROTOCOL_SSH && src->ssh_known_hosts_file) + virBufferAsprintf(childBuf, "<knownHosts path='%s'/>\n", src->ssh_known_hosts_file); } diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c index cecd7e811e..5d60c46cfc 100644 --- a/src/conf/storage_source_conf.c +++ b/src/conf/storage_source_conf.c @@ -1167,6 +1167,7 @@ virStorageSourceClear(virStorageSource *def) VIR_FREE(def->tlsHostname); VIR_FREE(def->ssh_user); + VIR_FREE(def->ssh_known_hosts_file); VIR_FREE(def->nfs_user); VIR_FREE(def->nfs_group); diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h index 14a6825d54..a2d8b1f8bd 100644 --- a/src/conf/storage_source_conf.h +++ b/src/conf/storage_source_conf.h @@ -405,6 +405,8 @@ struct _virStorageSource { /* these must not be used apart from formatting the output JSON in the qemu driver */ char *ssh_user; bool ssh_host_key_check_disabled; + /* additional ssh variables */ + char *ssh_known_hosts_file; /* nfs_user and nfs_group store the strings passed in by the user for NFS params. * nfs_uid and nfs_gid represent the converted/looked up ID numbers which are used diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c index 14e31ec186..dbbe71944f 100644 --- a/src/qemu/qemu_nbdkit.c +++ b/src/qemu/qemu_nbdkit.c @@ -1078,6 +1078,9 @@ qemuNbdkitProcessBuildCommandSSH(qemuNbdkitProcess *proc, if (proc->source->ssh_host_key_check_disabled) virCommandAddArgPair(cmd, "verify-remote-host", "false"); + if (proc->source->ssh_known_hosts_file) + virCommandAddArgPair(cmd, "known-hosts", proc->source->ssh_known_hosts_file); + return 0; } diff --git a/tests/qemunbdkitdata/disk-network-ssh-password.args.disk0 b/tests/qemunbdkitdata/disk-network-ssh-password.args.disk0 index 30711f7f07..ee2d7c3343 100644 --- a/tests/qemunbdkitdata/disk-network-ssh-password.args.disk0 +++ b/tests/qemunbdkitdata/disk-network-ssh-password.args.disk0 @@ -5,4 +5,5 @@ host=example.org \ port=2222 \ path=test2.img \ user=testuser \ -password=-777 +password=-777 \ +known-hosts=/path/to/knownhosts diff --git a/tests/qemunbdkitdata/disk-network-ssh.args.disk0 b/tests/qemunbdkitdata/disk-network-ssh.args.disk0 index c04dc8bb03..481b218936 100644 --- a/tests/qemunbdkitdata/disk-network-ssh.args.disk0 +++ b/tests/qemunbdkitdata/disk-network-ssh.args.disk0 @@ -3,4 +3,5 @@ nbdkit \ --foreground ssh \ host=example.org \ port=2222 \ -path=test.img +path=test.img \ +known-hosts=/path/to/ssh_known_hosts diff --git a/tests/qemuxml2argvdata/disk-network-ssh-password.xml b/tests/qemuxml2argvdata/disk-network-ssh-password.xml index 266acb761f..bdb4cf6e35 100644 --- a/tests/qemuxml2argvdata/disk-network-ssh-password.xml +++ b/tests/qemuxml2argvdata/disk-network-ssh-password.xml @@ -22,6 +22,7 @@ <auth username='testuser'> <secret type='iscsi' usage='mycluster_myname'/> </auth> + <knownHosts path='/path/to/knownhosts'/> </source> <target dev='vda' bus='virtio'/> </disk> diff --git a/tests/qemuxml2argvdata/disk-network-ssh.xml b/tests/qemuxml2argvdata/disk-network-ssh.xml index 355add4fea..a3aeca0c99 100644 --- a/tests/qemuxml2argvdata/disk-network-ssh.xml +++ b/tests/qemuxml2argvdata/disk-network-ssh.xml @@ -19,6 +19,7 @@ <host name='example.org' port='2222'/> <timeout seconds='1234'/> <readahead size='1024'/> + <knownHosts path="/path/to/ssh_known_hosts"/> </source> <target dev='vda' bus='virtio'/> </disk> -- 2.39.1

On Tue, Feb 14, 2023 at 11:08:18 -0600, Jonathon Jongsma wrote: Reword this so that it says that the ssh key can be used with an agent even when it is not protected. Or in fact promote the agent first and mention that a password-less key can be used without an agent with the keyfile option. And tweak the schema to allow agent without keyfile. Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt;

On Tue, Feb 14, 2023 at 11:08:19 -0600, Jonathon Jongsma wrote: Why is 'keyfile' mandatory ... ... if you can use the agent? In case agent is in use I expect that the key is added to the agent by the user and then the socket is passed to nbdkit to do the auth. nbdkit in that case has no need to look at the keyfile. Similarly selinux labelling may be the problem. But we can theoretically instruct the user via docs to use one agent per VM. We then can label the socket instead.