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 and mention it in the documentation. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- docs/formatdomain.rst | 2 +- src/conf/schemas/domaincommon.rng | 1 + .../disk-network-ssh.x86_64-latest.args | 36 +++++++++++++++++++ tests/qemuxml2argvdata/disk-network-ssh.xml | 31 ++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 5 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/disk-network-ssh.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-network-ssh.xml diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 34e4906eb4..4222605ede 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -2709,7 +2709,7 @@ paravirtualized driver is specified via the ``disk`` element. ``network`` The ``protocol`` attribute specifies the protocol to access to the requested image. Possible values are "nbd", "iscsi", "rbd", "sheepdog", - "gluster", "vxhs", "nfs", "http", "https", "ftp", ftps", or "tftp". + "gluster", "vxhs", "nfs", "http", "https", "ftp", ftps", "tftp", or "ssh". For any ``protocol`` other than ``nbd`` an additional attribute ``name`` is mandatory to specify which volume/image will be used. diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index bb6b1058e6..b3836b2f61 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -2132,6 +2132,7 @@ <choice> <value>sheepdog</value> <value>tftp</value> + <value>ssh</value> </choice> </attribute> <attribute name="name"/> 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 32f1e8ce3b..9ed7152544 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1365,6 +1365,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.37.1

In future commits, we will optionally use nbdkit to serve some remote disk sources. This patch queries to see whether nbdkit is installed on the host and queries it for capabilities. These capabilities are stored in the qemu driver. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- po/POTFILES | 1 + src/qemu/meson.build | 1 + src/qemu/qemu_conf.h | 3 + src/qemu/qemu_driver.c | 3 + src/qemu/qemu_nbdkit.c | 226 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_nbdkit.h | 51 ++++++++++ 6 files changed, 285 insertions(+) create mode 100644 src/qemu/qemu_nbdkit.c create mode 100644 src/qemu/qemu_nbdkit.h diff --git a/po/POTFILES b/po/POTFILES index d32105f7d5..1fd92256d4 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -179,6 +179,7 @@ src/qemu/qemu_monitor.c src/qemu/qemu_monitor_json.c src/qemu/qemu_monitor_text.c src/qemu/qemu_namespace.c +src/qemu/qemu_nbdkit.c src/qemu/qemu_process.c src/qemu/qemu_qapi.c src/qemu/qemu_saveimage.c diff --git a/src/qemu/meson.build b/src/qemu/meson.build index 96952cc52d..101cf3591f 100644 --- a/src/qemu/meson.build +++ b/src/qemu/meson.build @@ -28,6 +28,7 @@ qemu_driver_sources = [ 'qemu_monitor_json.c', 'qemu_monitor_text.c', 'qemu_namespace.c', + 'qemu_nbdkit.c', 'qemu_process.c', 'qemu_qapi.c', 'qemu_saveimage.c', diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index c40c452f58..d05c715400 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -36,6 +36,7 @@ #include "virthreadpool.h" #include "locking/lock_manager.h" #include "qemu_capabilities.h" +#include "qemu_nbdkit.h" #include "virclosecallbacks.h" #include "virhostdev.h" #include "virfile.h" @@ -306,6 +307,8 @@ struct _virQEMUDriver { /* Immutable pointer, self-locking APIs */ virHashAtomic *migrationErrors; + + qemuNbdkitCaps *nbdkitCaps; }; virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 707f4cc1bb..943fa8621d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -832,6 +832,9 @@ qemuStateInitialize(bool privileged, defsecmodel))) goto error; + /* find whether nbdkit is available and query its capabilities */ + qemu_driver->nbdkitCaps = qemuQueryNbdkitCaps(); + /* 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. */ diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c new file mode 100644 index 0000000000..f55f68299f --- /dev/null +++ b/src/qemu/qemu_nbdkit.c @@ -0,0 +1,226 @@ +/* + * qemu_nbdkit.c: helpers for using nbdkit + * + * Copyright (C) 2021 Red Hat, Inc. + * + * 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 <glib.h> + +#include "vircommand.h" +#include "virerror.h" +#include "virlog.h" +#include "virpidfile.h" +#include "virsecureerase.h" +#include "virutil.h" +#include "qemu_block.h" +#include "qemu_conf.h" +#include "qemu_domain.h" +#include "qemu_driver.h" +#include "qemu_extdevice.h" +#include "qemu_nbdkit.h" +#include "qemu_security.h" + +#include <fcntl.h> + +#define VIR_FROM_THIS VIR_FROM_QEMU + +VIR_LOG_INIT("qemu.nbdkit"); + +VIR_ENUM_IMPL(qemuNbdkitCaps, + QEMU_NBDKIT_CAPS_LAST, + /* 0 */ + "plugin-curl", /* QEMU_NBDKIT_CAPS_PLUGIN_CURL */ + "plugin-ssh", /* QEMU_NBDKIT_CAPS_PLUGIN_SSH */ + "filter-readahead", /* QEMU_NBDKIT_CAPS_FILTER_READAHEAD */ +); + +struct _qemuNbdkitCaps { + GObject parent; + + char *path; + char *version; + + virBitmap *flags; +}; +G_DEFINE_TYPE(qemuNbdkitCaps, qemu_nbdkit_caps, G_TYPE_OBJECT); + + +static void +qemuNbdkitCheckCommandCap(qemuNbdkitCaps *nbdkit, + virCommand *cmd, + qemuNbdkitCapsFlags cap) +{ + if (virCommandRun(cmd, NULL) != 0) + return; + + VIR_DEBUG("Setting nbdkit capability %i", cap); + ignore_value(virBitmapSetBit(nbdkit->flags, cap)); +} + + +static void +qemuNbdkitQueryFilter(qemuNbdkitCaps *nbdkit, + const char *filter, + qemuNbdkitCapsFlags cap) +{ + g_autoptr(virCommand) cmd = virCommandNewArgList(nbdkit->path, + "--version", + NULL); + + virCommandAddArgPair(cmd, "--filter", filter); + + /* use null plugin to check for filter */ + virCommandAddArg(cmd, "null"); + + qemuNbdkitCheckCommandCap(nbdkit, cmd, cap); +} + + +static void +qemuNbdkitQueryPlugin(qemuNbdkitCaps *nbdkit, + const char *plugin, + qemuNbdkitCapsFlags cap) +{ + g_autoptr(virCommand) cmd = virCommandNewArgList(nbdkit->path, + plugin, + "--version", + NULL); + + qemuNbdkitCheckCommandCap(nbdkit, cmd, cap); +} + + +static void +qemuNbdkitCapsQueryPlugins(qemuNbdkitCaps *nbdkit) +{ + qemuNbdkitQueryPlugin(nbdkit, "curl", QEMU_NBDKIT_CAPS_PLUGIN_CURL); + qemuNbdkitQueryPlugin(nbdkit, "ssh", QEMU_NBDKIT_CAPS_PLUGIN_SSH); +} + + +static void +qemuNbdkitCapsQueryFilters(qemuNbdkitCaps *nbdkit) +{ + qemuNbdkitQueryFilter(nbdkit, "readahead", + QEMU_NBDKIT_CAPS_FILTER_READAHEAD); +} + + +static int +qemuNbdkitCapsQueryVersion(qemuNbdkitCaps *nbdkit) +{ + g_autoptr(virCommand) cmd = virCommandNewArgList(nbdkit->path, + "--version", + NULL); + + virCommandSetOutputBuffer(cmd, &nbdkit->version); + + if (virCommandRun(cmd, NULL) != 0) + return -1; + + VIR_DEBUG("Got nbdkit version %s", nbdkit->version); + return 0; +} + + +static void qemuNbdkitCapsFinalize(GObject *object) +{ + qemuNbdkitCaps *nbdkit = QEMU_NBDKIT_CAPS(object); + + g_clear_pointer(&nbdkit->path, g_free); + g_clear_pointer(&nbdkit->version, g_free); + g_clear_pointer(&nbdkit->flags, virBitmapFree); + + G_OBJECT_CLASS(qemu_nbdkit_caps_parent_class)->finalize(object); +} + + +void qemu_nbdkit_caps_init(qemuNbdkitCaps *caps) +{ + caps->flags = virBitmapNew(QEMU_NBDKIT_CAPS_LAST); + caps->version = NULL; +} + + +static void +qemu_nbdkit_caps_class_init(qemuNbdkitCapsClass *klass) +{ + GObjectClass *obj = G_OBJECT_CLASS(klass); + + obj->finalize = qemuNbdkitCapsFinalize; +} + + +qemuNbdkitCaps * +qemuNbdkitCapsNew(const char *path) +{ + qemuNbdkitCaps *caps = g_object_new(QEMU_TYPE_NBDKIT_CAPS, NULL); + caps->path = g_strdup(path); + + return caps; +} + + +static void +qemuNbdkitCapsQuery(qemuNbdkitCaps *caps) +{ + qemuNbdkitCapsQueryPlugins(caps); + qemuNbdkitCapsQueryFilters(caps); + qemuNbdkitCapsQueryVersion(caps); +} + + +qemuNbdkitCaps * +qemuQueryNbdkitCaps(void) +{ + qemuNbdkitCaps *caps = NULL; + g_autofree char *path = virFindFileInPath("nbdkit"); + + if (!path) + return NULL; + + if (!virFileIsExecutable(path)) { + virReportError(VIR_ERR_INTERNAL_ERROR, _("nbdkit '%s' is not executable"), + path); + return NULL; + } + + VIR_DEBUG("found nbdkit executable '%s'", path); + + caps = qemuNbdkitCapsNew(path); + qemuNbdkitCapsQuery(caps); + + return caps; +} + + +bool +qemuNbdkitCapsGet(qemuNbdkitCaps *nbdkitCaps, + qemuNbdkitCapsFlags flag) +{ + return virBitmapIsBitSet(nbdkitCaps->flags, flag); +} + + +void +qemuNbdkitCapsSet(qemuNbdkitCaps *nbdkitCaps, + qemuNbdkitCapsFlags flag) +{ + ignore_value(virBitmapSetBit(nbdkitCaps->flags, flag)); +} diff --git a/src/qemu/qemu_nbdkit.h b/src/qemu/qemu_nbdkit.h new file mode 100644 index 0000000000..4baf4d4d31 --- /dev/null +++ b/src/qemu/qemu_nbdkit.h @@ -0,0 +1,51 @@ +/* + * qemu_nbdkit.h: helpers for using nbdkit + * + * Copyright (C) 2021 Red Hat, Inc. + * + * 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 "internal.h" +#include "virbitmap.h" +#include "vircgroup.h" +#include "vircommand.h" +#include "virstorageobj.h" +#include "viruri.h" + +typedef struct _qemuNbdkitCaps qemuNbdkitCaps; + +typedef enum { + QEMU_NBDKIT_CAPS_PLUGIN_CURL, + QEMU_NBDKIT_CAPS_PLUGIN_SSH, + QEMU_NBDKIT_CAPS_FILTER_READAHEAD, + QEMU_NBDKIT_CAPS_LAST, +} qemuNbdkitCapsFlags; + +VIR_ENUM_DECL(qemuNbdkitCaps); + +qemuNbdkitCaps* qemuQueryNbdkitCaps(void); + +qemuNbdkitCaps* qemuNbdkitCapsNew(const char *path); + +bool qemuNbdkitCapsGet(qemuNbdkitCaps *nbdkitCaps, qemuNbdkitCapsFlags flag); + +void qemuNbdkitCapsSet(qemuNbdkitCaps *nbdkitCaps, qemuNbdkitCapsFlags flag); + +#define QEMU_TYPE_NBDKIT_CAPS qemu_nbdkit_caps_get_type() +G_DECLARE_FINAL_TYPE(qemuNbdkitCaps, qemu_nbdkit_caps, QEMU, NBDKIT_CAPS, GObject); -- 2.37.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@redhat.com> --- meson.build | 6 ++++++ meson_options.txt | 1 + src/qemu/qemu_nbdkit.c | 39 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+) diff --git a/meson.build b/meson.build index 0b3187ad88..2b0425f1f6 100644 --- a/meson.build +++ b/meson.build @@ -1744,6 +1744,12 @@ if not get_option('driver_qemu').disabled() qemu_dbus_daemon_path = '/usr/bin/dbus-daemon' endif conf.set_quoted('QEMU_DBUS_DAEMON', qemu_dbus_daemon_path) + + nbdkit_moddir = get_option('nbdkit_moddir') + if nbdkit_moddir == '' + nbdkit_moddir = libdir / 'nbdkit' + endif + conf.set_quoted('NBDKIT_MODDIR', nbdkit_moddir) endif endif diff --git a/meson_options.txt b/meson_options.txt index 5b43cdbd6b..382174bc03 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -71,6 +71,7 @@ option('driver_vbox', type: 'feature', value: 'auto', description: 'VirtualBox X option('vbox_xpcomc_dir', type: 'string', value: '', description: 'Location of directory containing VirtualBox XPCOMC library') option('driver_vmware', type: 'feature', value: 'auto', description: 'VMware driver') option('driver_vz', type: 'feature', value: 'auto', description: 'Virtuozzo driver') +option('nbdkit_moddir', type: 'string', value: '', description: 'set the directory where nbdkit modules are located') option('secdriver_apparmor', type: 'feature', value: 'auto', description: 'use AppArmor security driver') option('apparmor_profiles', type: 'feature', value: 'auto', description: 'install apparmor profiles') diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c index f55f68299f..0ea1b754cd 100644 --- a/src/qemu/qemu_nbdkit.c +++ b/src/qemu/qemu_nbdkit.c @@ -42,6 +42,9 @@ VIR_LOG_INIT("qemu.nbdkit"); +#define NBDKIT_PLUGINDIR NBDKIT_MODDIR "/plugins" +#define NBDKIT_FILTERDIR NBDKIT_MODDIR "/filters" + VIR_ENUM_IMPL(qemuNbdkitCaps, QEMU_NBDKIT_CAPS_LAST, /* 0 */ @@ -55,6 +58,11 @@ struct _qemuNbdkitCaps { char *path; char *version; + time_t ctime; + time_t libvirtCtime; + time_t pluginDirMtime; + time_t filterDirMtime; + unsigned int libvirtVersion; virBitmap *flags; }; @@ -177,9 +185,40 @@ qemuNbdkitCapsNew(const char *path) } +static time_t +getDirMtime(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; +} + + 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; + } else { + caps->ctime = st.st_ctime; + } + caps->filterDirMtime = getDirMtime(NBDKIT_FILTERDIR); + caps->pluginDirMtime = getDirMtime(NBDKIT_PLUGINDIR); + caps->libvirtCtime = virGetSelfLastChanged(); + caps->libvirtVersion = LIBVIR_VERSION_NUMBER; + qemuNbdkitCapsQueryPlugins(caps); qemuNbdkitCapsQueryFilters(caps); qemuNbdkitCapsQueryVersion(caps); -- 2.37.1

Since the libvirt documentation suggests to prefer GObject over virObject, and since virObject is a GObject, change virFileCache to allow GObjects as data. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/util/virfilecache.c | 15 +++++++++------ src/util/virfilecache.h | 2 +- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/util/virfilecache.c b/src/util/virfilecache.c index bad37c9f00..e37008990d 100644 --- a/src/util/virfilecache.c +++ b/src/util/virfilecache.c @@ -170,7 +170,8 @@ virFileCacheLoad(virFileCache *cache, *data = g_steal_pointer(&loadData); cleanup: - virObjectUnref(loadData); + if (loadData) + g_object_unref(loadData); return ret; } @@ -207,7 +208,7 @@ virFileCacheNewData(virFileCache *cache, return NULL; if (virFileCacheSave(cache, name, data) < 0) { - g_clear_pointer(&data, virObjectUnref); + g_clear_object(&data); } } @@ -239,7 +240,7 @@ virFileCacheNew(const char *dir, if (!(cache = virObjectNew(virFileCacheClass))) return NULL; - cache->table = virHashNew(virObjectUnref); + cache->table = virHashNew(g_object_unref); cache->dir = g_strdup(dir); @@ -270,7 +271,7 @@ virFileCacheValidate(virFileCache *cache, if (*data) { VIR_DEBUG("Caching data '%p' for '%s'", *data, name); if (virHashAddEntry(cache->table, name, *data) < 0) { - g_clear_pointer(data, virObjectUnref); + g_clear_pointer(data, g_object_unref); } } } @@ -300,7 +301,8 @@ virFileCacheLookup(virFileCache *cache, data = virHashLookup(cache->table, name); virFileCacheValidate(cache, name, &data); - virObjectRef(data); + if (data) + g_object_ref(data); virObjectUnlock(cache); return data; @@ -331,7 +333,8 @@ virFileCacheLookupByFunc(virFileCache *cache, data = virHashSearch(cache->table, iter, iterData, &name); virFileCacheValidate(cache, name, &data); - virObjectRef(data); + if (data) + g_object_ref(data); virObjectUnlock(cache); return data; diff --git a/src/util/virfilecache.h b/src/util/virfilecache.h index 81be8feef5..f0d220cc86 100644 --- a/src/util/virfilecache.h +++ b/src/util/virfilecache.h @@ -48,7 +48,7 @@ typedef bool * @priv: private data created together with cache * * Creates a new data based on the @name. The returned data must be - * an instance of virObject. + * an instance of GObject. * * Returns data object or NULL on error. */ -- 2.37.1

Preparatory step for caching nbdkit capabilities. This patch implements the newData and isValid virFileCacheHandlers callback functions. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/qemu/qemu_nbdkit.c | 91 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_nbdkit.h | 3 ++ 2 files changed, 94 insertions(+) diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c index 0ea1b754cd..3faa16674a 100644 --- a/src/qemu/qemu_nbdkit.c +++ b/src/qemu/qemu_nbdkit.c @@ -263,3 +263,94 @@ qemuNbdkitCapsSet(qemuNbdkitCaps *nbdkitCaps, { ignore_value(virBitmapSetBit(nbdkitCaps->flags, flag)); } + + +static bool +virNbkditCapsCheckModdir(const char *moddir, + time_t expectedMtime) +{ + time_t mtime = getDirMtime(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 4baf4d4d31..b65e76895b 100644 --- a/src/qemu/qemu_nbdkit.h +++ b/src/qemu/qemu_nbdkit.h @@ -25,6 +25,7 @@ #include "virbitmap.h" #include "vircgroup.h" #include "vircommand.h" +#include "virfilecache.h" #include "virstorageobj.h" #include "viruri.h" @@ -43,6 +44,8 @@ qemuNbdkitCaps* qemuQueryNbdkitCaps(void); qemuNbdkitCaps* qemuNbdkitCapsNew(const char *path); +virFileCache* qemuNbdkitCapsCacheNew(const char *cachedir); + bool qemuNbdkitCapsGet(qemuNbdkitCaps *nbdkitCaps, qemuNbdkitCapsFlags flag); void qemuNbdkitCapsSet(qemuNbdkitCaps *nbdkitCaps, qemuNbdkitCapsFlags flag); -- 2.37.1

Switch to using the virFileCache implementation for nbdkit capabilities so that we have persistent caching and re-load capabilities whenever something changes. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/qemu/qemu_conf.h | 2 +- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_nbdkit.c | 24 ------------------------ src/qemu/qemu_nbdkit.h | 2 -- 4 files changed, 2 insertions(+), 28 deletions(-) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index d05c715400..414bcfb8a2 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -308,7 +308,7 @@ struct _virQEMUDriver { /* Immutable pointer, self-locking APIs */ virHashAtomic *migrationErrors; - qemuNbdkitCaps *nbdkitCaps; + virFileCache *nbdkitCapsCache; }; virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 943fa8621d..dce3ba1a32 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -833,7 +833,7 @@ qemuStateInitialize(bool privileged, goto error; /* find whether nbdkit is available and query its capabilities */ - qemu_driver->nbdkitCaps = qemuQueryNbdkitCaps(); + 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 diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c index ac498b948c..fc83f80f1f 100644 --- a/src/qemu/qemu_nbdkit.c +++ b/src/qemu/qemu_nbdkit.c @@ -225,30 +225,6 @@ qemuNbdkitCapsQuery(qemuNbdkitCaps *caps) } -qemuNbdkitCaps * -qemuQueryNbdkitCaps(void) -{ - qemuNbdkitCaps *caps = NULL; - g_autofree char *path = virFindFileInPath("nbdkit"); - - if (!path) - return NULL; - - if (!virFileIsExecutable(path)) { - virReportError(VIR_ERR_INTERNAL_ERROR, _("nbdkit '%s' is not executable"), - path); - return NULL; - } - - VIR_DEBUG("found nbdkit executable '%s'", path); - - caps = qemuNbdkitCapsNew(path); - qemuNbdkitCapsQuery(caps); - - return caps; -} - - bool qemuNbdkitCapsGet(qemuNbdkitCaps *nbdkitCaps, qemuNbdkitCapsFlags flag) diff --git a/src/qemu/qemu_nbdkit.h b/src/qemu/qemu_nbdkit.h index b65e76895b..3e34403d90 100644 --- a/src/qemu/qemu_nbdkit.h +++ b/src/qemu/qemu_nbdkit.h @@ -40,8 +40,6 @@ typedef enum { VIR_ENUM_DECL(qemuNbdkitCaps); -qemuNbdkitCaps* qemuQueryNbdkitCaps(void); - qemuNbdkitCaps* qemuNbdkitCapsNew(const char *path); virFileCache* qemuNbdkitCapsCacheNew(const char *cachedir); -- 2.37.1

An object for storing information about a nbdkit process that is serving a specific virStorageSource. At the moment, this information is just stored in the private data of virStorageSource and not used at all. Future commits will use this data to actually start a nbdkit process. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/qemu/qemu_conf.c | 22 ++++++++++++ src/qemu/qemu_conf.h | 3 ++ src/qemu/qemu_domain.c | 29 +++++++++++++++ src/qemu/qemu_domain.h | 4 +++ src/qemu/qemu_nbdkit.c | 82 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_nbdkit.h | 24 +++++++++++++ 6 files changed, 164 insertions(+) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 3b75cdeb95..b5f451a8f9 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1571,3 +1571,25 @@ qemuGetMemoryBackingPath(virQEMUDriver *driver, *memPath = g_strdup_printf("%s/%s", domainPath, alias); return 0; } + +/* + * qemuGetNbdkitCaps: + * @driver: the qemu driver + * + * Gets the capabilities for Nbdkit for the specified driver. These can be used + * to determine whether a particular disk source can be served by nbdkit or + * not. + * + * Returns: a reference to qemuNbdkitCaps or NULL + */ +qemuNbdkitCaps* +qemuGetNbdkitCaps(virQEMUDriver *driver) +{ + if (!driver->nbdkitBinary) + driver->nbdkitBinary = virFindFileInPath("nbdkit"); + + if (driver->nbdkitBinary) + return virFileCacheLookup(driver->nbdkitCapsCache, driver->nbdkitBinary); + + return NULL; +} diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 414bcfb8a2..91212d6608 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -309,6 +309,7 @@ struct _virQEMUDriver { virHashAtomic *migrationErrors; virFileCache *nbdkitCapsCache; + char *nbdkitBinary; }; virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged, @@ -362,3 +363,5 @@ int qemuGetMemoryBackingPath(virQEMUDriver *driver, const virDomainDef *def, const char *alias, char **memPath); + +qemuNbdkitCaps* qemuGetNbdkitCaps(virQEMUDriver *driver); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index fe3ce023a4..5b3b2d3e9c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -819,6 +819,7 @@ qemuDomainStorageSourcePrivateDispose(void *obj) g_clear_pointer(&priv->encinfo, qemuDomainSecretInfoFree); g_clear_pointer(&priv->httpcookie, qemuDomainSecretInfoFree); g_clear_pointer(&priv->tlsKeySecret, qemuDomainSecretInfoFree); + g_clear_pointer(&priv->nbdkitProcess, qemuNbdkitProcessFree); } @@ -10013,6 +10014,32 @@ qemuDomainPrepareStorageSourceNFS(virStorageSource *src) } +/* qemuPrepareStorageSourceNbdkit: + * @src: source for a disk + * + * If src is an network source that is managed by nbdkit, prepare data so that + * nbdkit can be launched before the domain is started + * + * Returns true if nbdkit will be used for this source, + */ +static bool +qemuDomainPrepareStorageSourceNbdkit(virStorageSource *src, + virQEMUDriverConfig *cfg, + const char *alias, + qemuDomainObjPrivate *priv) +{ + g_autoptr(qemuNbdkitCaps) nbdkit = qemuGetNbdkitCaps(priv->driver); + if (!nbdkit) + return false; + + if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) + return false; + + return qemuNbdkitInitStorageSource(nbdkit, src, priv->libDir, + alias, cfg->user, cfg->group); +} + + /* qemuProcessPrepareStorageSourceTLS: * @source: source for a disk * @cfg: driver configuration @@ -10787,6 +10814,8 @@ qemuDomainPrepareStorageSourceBlockdevNodename(virDomainDiskDef *disk, if (qemuDomainPrepareStorageSourceNFS(src) < 0) return -1; + qemuDomainPrepareStorageSourceNbdkit(src, cfg, src->nodestorage, priv); + return 0; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 592ee9805b..563b0aa925 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -33,6 +33,7 @@ #include "qemu_conf.h" #include "qemu_capabilities.h" #include "qemu_migration_params.h" +#include "qemu_nbdkit.h" #include "qemu_slirp.h" #include "qemu_fd.h" #include "virchrdev.h" @@ -293,6 +294,9 @@ struct _qemuDomainStorageSourcePrivate { /* key for decrypting TLS certificate */ qemuDomainSecretInfo *tlsKeySecret; + + /* an nbdkit process for serving network storage sources */ + qemuNbdkitProcess *nbdkitProcess; }; virObject *qemuDomainStorageSourcePrivateNew(void); diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c index fc83f80f1f..4df8957196 100644 --- a/src/qemu/qemu_nbdkit.c +++ b/src/qemu/qemu_nbdkit.c @@ -579,3 +579,85 @@ qemuNbdkitCapsCacheNew(const char *cachedir) g_autofree char *dir = g_build_filename(cachedir, "nbdkitcapabilities", NULL); return virFileCacheNew(dir, "xml", &nbdkitCapsCacheHandlers); } + + +static qemuNbdkitProcess * +qemuNbdkitProcessNew(qemuNbdkitCaps *caps, + virStorageSource *source, + char *statedir, + const char *alias, + uid_t user, + gid_t group) +{ + qemuNbdkitProcess *proc = g_new0(qemuNbdkitProcess, 1); + g_autofree char *pidfile = g_strdup_printf("nbdkit-%s.pid", alias); + g_autofree char *socketfile = g_strdup_printf("nbdkit-%s.socket", alias); + + proc->caps = g_object_ref(caps); + /* weak reference -- source owns this object, so it will always outlive us */ + proc->source = source; + proc->user = user; + proc->group = group; + proc->pidfile = g_build_filename(statedir, pidfile, NULL); + proc->socketfile = g_build_filename(statedir, socketfile, NULL); + + return proc; +} + + +bool +qemuNbdkitInitStorageSource(qemuNbdkitCaps *caps, + virStorageSource *source, + char *statedir, + const char *alias, + uid_t user, + gid_t group) +{ + qemuDomainStorageSourcePrivate *srcPriv = qemuDomainStorageSourcePrivateFetch(source); + + if (srcPriv->nbdkitProcess) + return false; + + switch (source->protocol) { + 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: + if (!virBitmapIsBitSet(caps->flags, QEMU_NBDKIT_CAPS_PLUGIN_CURL)) + return false; + break; + case VIR_STORAGE_NET_PROTOCOL_SSH: + if (!virBitmapIsBitSet(caps->flags, QEMU_NBDKIT_CAPS_PLUGIN_SSH)) + return false; + break; + case VIR_STORAGE_NET_PROTOCOL_NONE: + case VIR_STORAGE_NET_PROTOCOL_NBD: + case VIR_STORAGE_NET_PROTOCOL_RBD: + case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: + case VIR_STORAGE_NET_PROTOCOL_GLUSTER: + case VIR_STORAGE_NET_PROTOCOL_ISCSI: + case VIR_STORAGE_NET_PROTOCOL_VXHS: + case VIR_STORAGE_NET_PROTOCOL_NFS: + case VIR_STORAGE_NET_PROTOCOL_LAST: + return false; + } + srcPriv->nbdkitProcess = qemuNbdkitProcessNew(caps, source, statedir, alias, user, group); + + return true; +} + + +void +qemuNbdkitProcessFree(qemuNbdkitProcess *proc) +{ + if (virProcessKillPainfully(proc->pid, true) < 0) + VIR_WARN("Unable to kill nbdkit process"); + + unlink(proc->pidfile); + g_clear_pointer(&proc->pidfile, g_free); + unlink(proc->socketfile); + g_clear_pointer(&proc->socketfile, g_free); + g_clear_object(&proc->caps); + g_free(proc); +} diff --git a/src/qemu/qemu_nbdkit.h b/src/qemu/qemu_nbdkit.h index 3e34403d90..24762cce8f 100644 --- a/src/qemu/qemu_nbdkit.h +++ b/src/qemu/qemu_nbdkit.h @@ -30,6 +30,7 @@ #include "viruri.h" typedef struct _qemuNbdkitCaps qemuNbdkitCaps; +typedef struct _qemuNbdkitProcess qemuNbdkitProcess; typedef enum { QEMU_NBDKIT_CAPS_PLUGIN_CURL, @@ -44,9 +45,32 @@ qemuNbdkitCaps* qemuNbdkitCapsNew(const char *path); virFileCache* qemuNbdkitCapsCacheNew(const char *cachedir); +bool qemuNbdkitInitStorageSource(qemuNbdkitCaps *nbdkitCaps, + virStorageSource *source, + char *statedir, + const char *alias, + uid_t user, + gid_t group + /*, char *selinux_label*/); + bool qemuNbdkitCapsGet(qemuNbdkitCaps *nbdkitCaps, qemuNbdkitCapsFlags flag); void qemuNbdkitCapsSet(qemuNbdkitCaps *nbdkitCaps, qemuNbdkitCapsFlags flag); #define QEMU_TYPE_NBDKIT_CAPS qemu_nbdkit_caps_get_type() G_DECLARE_FINAL_TYPE(qemuNbdkitCaps, qemu_nbdkit_caps, QEMU, NBDKIT_CAPS, GObject); + +struct _qemuNbdkitProcess { + qemuNbdkitCaps *caps; + virStorageSource *source; + + char *pidfile; + char *socketfile; + uid_t user; + gid_t group; + pid_t pid; +}; + +void qemuNbdkitProcessFree(qemuNbdkitProcess *proc); + +G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuNbdkitProcess, qemuNbdkitProcessFree); -- 2.37.1

Add some helper functions to build a virCommand object and run the nbdkit process for a given virStorageSource. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/qemu/qemu_nbdkit.c | 208 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_nbdkit.h | 8 ++ 2 files changed, 216 insertions(+) diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c index 4df8957196..e8f9acc17c 100644 --- a/src/qemu/qemu_nbdkit.c +++ b/src/qemu/qemu_nbdkit.c @@ -648,6 +648,156 @@ qemuNbdkitInitStorageSource(qemuNbdkitCaps *caps, } +static int +qemuNbdkitProcessBuildCommandCurl(qemuNbdkitProcess *proc, + virCommand *cmd) +{ + g_autoptr(virURI) uri = qemuBlockStorageSourceGetURI(proc->source); + g_autofree char *uristring = virURIFormat(uri); + + /* nbdkit plugin name */ + virCommandAddArg(cmd, "curl"); + virCommandAddArgPair(cmd, "protocols", + virStorageNetProtocolTypeToString(proc->source->protocol)); + virCommandAddArgPair(cmd, "url", uristring); + + if (proc->source->auth) { + g_autoptr(virConnect) conn = virGetConnectSecret(); + g_autofree uint8_t *secret = NULL; + size_t secretlen = 0; + g_autofree char *password = NULL; + int secrettype; + + virCommandAddArgPair(cmd, "user", + proc->source->auth->username); + + if ((secrettype = virSecretUsageTypeFromString(proc->source->auth->secrettype)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid secret type %s"), + proc->source->auth->secrettype); + return -1; + } + + if (virSecretGetSecretString(conn, + &proc->source->auth->seclookupdef, + secrettype, + &secret, + &secretlen) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to get auth secret for storage")); + return -1; + } + + /* ensure that the secret is a NULL-terminated string */ + password = g_strndup((char*)secret, secretlen); + + virCommandAddArgPair(cmd, "password", password); + + virSecureErase(secret, secretlen); + virSecureErase(password, secretlen); + } + + if (proc->source->ncookies > 0) + virCommandAddArgPair(cmd, "cookie", + qemuBlockStorageSourceGetCookieString(proc->source)); + + if (proc->source->sslverify == VIR_TRISTATE_BOOL_NO) { + virCommandAddArgPair(cmd, "sslverify", "false"); + } + + if (proc->source->timeout > 0) { + g_autofree char *timeout = g_strdup_printf("%llu", proc->source->timeout); + virCommandAddArgPair(cmd, "timeout", timeout); + } + + return 0; +} + + +static int +qemuNbdkitProcessBuildCommandSSH(qemuNbdkitProcess *proc, + virCommand *cmd) +{ + const char *user = NULL; + virStorageNetHostDef *host = &proc->source->hosts[0]; + g_autofree char *portstr = g_strdup_printf("%u", host->port); + + /* nbdkit plugin name */ + virCommandAddArg(cmd, "ssh"); + + virCommandAddArgPair(cmd, "host", host->name); + virCommandAddArgPair(cmd, "port", portstr); + virCommandAddArgPair(cmd, "path", proc->source->path); + + if (proc->source->auth) + user = proc->source->auth->username; + else if (proc->source->ssh_user) + user = proc->source->ssh_user; + + if (user) + virCommandAddArgPair(cmd, "user", user); + + if (proc->source->ssh_host_key_check_disabled) + virCommandAddArgPair(cmd, "verify-remote-host", "false"); + + return 0; +} + + +static virCommand * +qemuNbdkitProcessBuildCommand(qemuNbdkitProcess *proc) +{ + g_autoptr(virCommand) cmd = virCommandNewArgList(proc->caps->path, + "--exit-with-parent", + "--unix", + proc->socketfile, + "--foreground", + //"--selinux-label", + //selinux_label, + NULL); + + if (proc->source->readonly) + virCommandAddArg(cmd, "--readonly"); + + if (qemuNbdkitCapsGet(proc->caps, QEMU_NBDKIT_CAPS_FILTER_READAHEAD) && + proc->source->readahead > 0) + virCommandAddArgPair(cmd, "--filter", "readahead"); + + switch (proc->source->protocol) { + 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: + if (qemuNbdkitProcessBuildCommandCurl(proc, cmd) < 0) + return NULL; + break; + case VIR_STORAGE_NET_PROTOCOL_SSH: + if (qemuNbdkitProcessBuildCommandSSH(proc, cmd) < 0) + return NULL; + break; + + case VIR_STORAGE_NET_PROTOCOL_NONE: + case VIR_STORAGE_NET_PROTOCOL_NBD: + case VIR_STORAGE_NET_PROTOCOL_RBD: + case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: + case VIR_STORAGE_NET_PROTOCOL_GLUSTER: + case VIR_STORAGE_NET_PROTOCOL_ISCSI: + case VIR_STORAGE_NET_PROTOCOL_VXHS: + case VIR_STORAGE_NET_PROTOCOL_NFS: + case VIR_STORAGE_NET_PROTOCOL_LAST: + virReportError(VIR_ERR_NO_SUPPORT, + _("protocol '%s' is not supported by nbdkit"), + virStorageNetProtocolTypeToString(proc->source->protocol)); + return NULL; + } + + virCommandDaemonize(cmd); + + return g_steal_pointer(&cmd); +} + + void qemuNbdkitProcessFree(qemuNbdkitProcess *proc) { @@ -661,3 +811,61 @@ qemuNbdkitProcessFree(qemuNbdkitProcess *proc) g_clear_object(&proc->caps); g_free(proc); } + + +int +qemuNbdkitProcessStart(qemuNbdkitProcess *proc, + virDomainObj *vm, + virQEMUDriver *driver) +{ + g_autoptr(virCommand) cmd = NULL; + int rc; + int exitstatus = 0; + int cmdret = 0; + int errfd = -1; + + if (!(cmd = qemuNbdkitProcessBuildCommand(proc))) + return -1; + + virCommandSetErrorFD(cmd, &errfd); + virCommandSetPidFile(cmd, proc->pidfile); + + if (qemuExtDeviceLogCommand(driver, vm, cmd, "nbdkit") < 0) + goto error; + + if (qemuSecurityCommandRun(driver, vm, cmd, proc->user, proc->group, &exitstatus, &cmdret) < 0) + goto error; + + if (cmdret < 0 || exitstatus != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not start 'nbdkit'. exitstatus: %d"), exitstatus); + goto error; + } + + if ((rc = virPidFileReadPath(proc->pidfile, &proc->pid)) < 0) { + virReportSystemError(-rc, + _("Failed to read pidfile %s"), + proc->pidfile); + goto error; + } + + return 0; + + error: + if (errfd > 0) { + char errbuf[1024] = { 0 }; + if (read(errfd, errbuf, sizeof(errbuf) - 1) > 0) + VIR_WARN("nbdkit failed to start: %s", errbuf); + } + if (proc->pid) + virProcessKillPainfully(proc->pid, true); + unlink(proc->pidfile); + return -1; +} + + +int +qemuNbdkitProcessStop(qemuNbdkitProcess *proc) +{ + return virProcessKillPainfully(proc->pid, true); +} diff --git a/src/qemu/qemu_nbdkit.h b/src/qemu/qemu_nbdkit.h index 24762cce8f..07ae0eead4 100644 --- a/src/qemu/qemu_nbdkit.h +++ b/src/qemu/qemu_nbdkit.h @@ -71,6 +71,14 @@ struct _qemuNbdkitProcess { pid_t pid; }; +typedef struct _virQEMUDriver virQEMUDriver; + +int qemuNbdkitProcessStart(qemuNbdkitProcess *proc, + virDomainObj *vm, + virQEMUDriver *driver); + +int qemuNbdkitProcessStop(qemuNbdkitProcess *proc); + void qemuNbdkitProcessFree(qemuNbdkitProcess *proc); G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuNbdkitProcess, qemuNbdkitProcessFree); -- 2.37.1

Add new DO_TEST_CAPS_LATEST_NBDKIT macro to test xml2argv for various nbdkit capability scenarios. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- 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 e8f9acc17c..df3a691bff 100644 --- a/src/qemu/qemu_nbdkit.c +++ b/src/qemu/qemu_nbdkit.c @@ -260,10 +260,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; @@ -306,9 +312,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 9ed7152544..4890c3b01b 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -705,6 +705,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))) @@ -963,6 +971,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 6d3decdc16..386042aa79 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; @@ -1020,6 +1039,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; @@ -1037,6 +1062,8 @@ testQemuInfoClear(struct testQemuInfo *info) VIR_FREE(info->errfile); virObjectUnref(info->qemuCaps); g_clear_pointer(&info->args.fakeCaps, virObjectUnref); + g_clear_object(&info->nbdkitCaps); + g_clear_object(&info->args.fakeNbdkitCaps); } diff --git a/tests/testutilsqemu.h b/tests/testutilsqemu.h index 943958d02a..618837559c 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 "/usr/bin/nbdkit" enum { GIC_NONE = 0, @@ -52,6 +53,7 @@ typedef enum { ARG_CAPS_VER, ARG_CAPS_HOST_CPU_MODEL, ARG_HOST_OS, + ARG_NBDKIT_CAPS, ARG_END, } testQemuInfoArgName; @@ -82,6 +84,8 @@ struct testQemuArgs { bool newargs; virQEMUCaps *fakeCaps; bool fakeCapsUsed; + qemuNbdkitCaps *fakeNbdkitCaps; + bool fakeNbdkitCapsUsed; char *capsver; char *capsarch; qemuTestCPUDef capsHostCPUModel; @@ -96,6 +100,7 @@ struct testQemuInfo { char *outfile; char *errfile; virQEMUCaps *qemuCaps; + qemuNbdkitCaps *nbdkitCaps; const char *migrateFrom; int migrateFd; unsigned int flags; -- 2.37.1

We were testing the arguments that were being passed to qemu when a disk was being served by nbdkit, but the arguments used to start nbdkit itself were not testable. This adds a test to ensure that we're invoking nbdkit correctly for various disk source definitions. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/qemu/qemu_nbdkit.c | 4 +- src/qemu/qemu_nbdkitpriv.h | 31 +++ tests/meson.build | 1 + .../disk-cdrom-network.args.disk0 | 7 + .../disk-cdrom-network.args.disk1 | 9 + .../disk-cdrom-network.args.disk2 | 9 + .../disk-network-http.args.disk0 | 7 + .../disk-network-http.args.disk1 | 6 + .../disk-network-http.args.disk2 | 7 + .../disk-network-http.args.disk3 | 8 + ...work-source-curl-nbdkit-backing.args.disk0 | 8 + .../disk-network-source-curl.args.disk0 | 8 + .../disk-network-source-curl.args.disk1 | 8 + .../disk-network-source-curl.args.disk2 | 8 + .../disk-network-source-curl.args.disk3 | 7 + .../disk-network-source-curl.args.disk4 | 7 + .../disk-network-ssh.args.disk0 | 7 + tests/qemunbdkittest.c | 220 ++++++++++++++++++ 18 files changed, 361 insertions(+), 1 deletion(-) create mode 100644 src/qemu/qemu_nbdkitpriv.h create mode 100644 tests/qemunbdkitdata/disk-cdrom-network.args.disk0 create mode 100644 tests/qemunbdkitdata/disk-cdrom-network.args.disk1 create mode 100644 tests/qemunbdkitdata/disk-cdrom-network.args.disk2 create mode 100644 tests/qemunbdkitdata/disk-network-http.args.disk0 create mode 100644 tests/qemunbdkitdata/disk-network-http.args.disk1 create mode 100644 tests/qemunbdkitdata/disk-network-http.args.disk2 create mode 100644 tests/qemunbdkitdata/disk-network-http.args.disk3 create mode 100644 tests/qemunbdkitdata/disk-network-source-curl-nbdkit-backing.args.disk0 create mode 100644 tests/qemunbdkitdata/disk-network-source-curl.args.disk0 create mode 100644 tests/qemunbdkitdata/disk-network-source-curl.args.disk1 create mode 100644 tests/qemunbdkitdata/disk-network-source-curl.args.disk2 create mode 100644 tests/qemunbdkitdata/disk-network-source-curl.args.disk3 create mode 100644 tests/qemunbdkitdata/disk-network-source-curl.args.disk4 create mode 100644 tests/qemunbdkitdata/disk-network-ssh.args.disk0 create mode 100644 tests/qemunbdkittest.c diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c index 5a0c80def8..0ecf6c6537 100644 --- a/src/qemu/qemu_nbdkit.c +++ b/src/qemu/qemu_nbdkit.c @@ -34,6 +34,8 @@ #include "qemu_driver.h" #include "qemu_extdevice.h" #include "qemu_nbdkit.h" +#define LIBVIRT_QEMU_NBDKITPRIV_H_ALLOW +#include "qemu_nbdkitpriv.h" #include "qemu_security.h" #include <fcntl.h> @@ -779,7 +781,7 @@ qemuNbdkitProcessBuildCommandSSH(qemuNbdkitProcess *proc, } -static virCommand * +virCommand * qemuNbdkitProcessBuildCommand(qemuNbdkitProcess *proc) { g_autoptr(virCommand) cmd = virCommandNewArgList(proc->caps->path, diff --git a/src/qemu/qemu_nbdkitpriv.h b/src/qemu/qemu_nbdkitpriv.h new file mode 100644 index 0000000000..64f9bb99d8 --- /dev/null +++ b/src/qemu/qemu_nbdkitpriv.h @@ -0,0 +1,31 @@ +/* + * qemu_nbdkitpriv.h: exposing some functions for testing + * + * Copyright (C) 2021 Red Hat, Inc. + * + * 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/>. + * + */ + +#ifndef LIBVIRT_QEMU_NBDKITPRIV_H_ALLOW +# error "qemu_nbdkitpriv.h may only be included by qemu_nbdkit.c or test suites" +#endif /* LIBVIRT_QEMU_NBDKITPRIV_H_ALLOW */ + +#pragma once + +#include "qemu_nbdkit.h" + +virCommand * +qemuNbdkitProcessBuildCommand(qemuNbdkitProcess *proc); diff --git a/tests/meson.build b/tests/meson.build index d6b1bb2bf0..f1eddc2fd6 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -451,6 +451,7 @@ if conf.has('WITH_QEMU') { 'name': 'qemuvhostusertest', 'link_with': [ test_qemu_driver_lib ], 'link_whole': [ test_file_wrapper_lib ] }, { 'name': 'qemuxml2argvtest', 'link_with': [ test_qemu_driver_lib, test_utils_qemu_monitor_lib ], 'link_whole': [ test_utils_qemu_lib, test_file_wrapper_lib ] }, { 'name': 'qemuxml2xmltest', 'link_with': [ test_qemu_driver_lib ], 'link_whole': [ test_utils_qemu_lib, test_file_wrapper_lib ] }, + { 'name': 'qemunbdkittest', 'link_with': [ test_qemu_driver_lib ], 'link_whole': [ test_utils_qemu_lib, test_file_wrapper_lib ] }, ] endif diff --git a/tests/qemunbdkitdata/disk-cdrom-network.args.disk0 b/tests/qemunbdkitdata/disk-cdrom-network.args.disk0 new file mode 100644 index 0000000000..5f3a795ba0 --- /dev/null +++ b/tests/qemunbdkitdata/disk-cdrom-network.args.disk0 @@ -0,0 +1,7 @@ +nbdkit \ +--exit-with-parent \ +--unix /tmp/statedir-0/nbdkit-test-disk-0.socket \ +--foreground \ +--readonly curl \ +protocols=ftp \ +url=ftp://host.name:21/url/path/file.iso diff --git a/tests/qemunbdkitdata/disk-cdrom-network.args.disk1 b/tests/qemunbdkitdata/disk-cdrom-network.args.disk1 new file mode 100644 index 0000000000..257e331db8 --- /dev/null +++ b/tests/qemunbdkitdata/disk-cdrom-network.args.disk1 @@ -0,0 +1,9 @@ +nbdkit \ +--exit-with-parent \ +--unix /tmp/statedir-1/nbdkit-test-disk-1.socket \ +--foreground \ +--readonly curl \ +protocols=ftps \ +url=ftps://host.name:990/url/path/file.iso \ +user=testuser \ +password=iscsi-mycluster_myname-secret diff --git a/tests/qemunbdkitdata/disk-cdrom-network.args.disk2 b/tests/qemunbdkitdata/disk-cdrom-network.args.disk2 new file mode 100644 index 0000000000..f7879a9f24 --- /dev/null +++ b/tests/qemunbdkitdata/disk-cdrom-network.args.disk2 @@ -0,0 +1,9 @@ +nbdkit \ +--exit-with-parent \ +--unix /tmp/statedir-2/nbdkit-test-disk-2.socket \ +--foreground \ +--readonly curl \ +protocols=https \ +'url=https://host.name:443/url/path/file.iso?test=val' \ +user=testuser \ +password=iscsi-mycluster_myname-secret diff --git a/tests/qemunbdkitdata/disk-network-http.args.disk0 b/tests/qemunbdkitdata/disk-network-http.args.disk0 new file mode 100644 index 0000000000..fa8ef90cd1 --- /dev/null +++ b/tests/qemunbdkitdata/disk-network-http.args.disk0 @@ -0,0 +1,7 @@ +nbdkit \ +--exit-with-parent \ +--unix /tmp/statedir-0/nbdkit-test-disk-0.socket \ +--foreground curl \ +protocols=http \ +url=http://example.org:80/test.img \ +timeout=1234 diff --git a/tests/qemunbdkitdata/disk-network-http.args.disk1 b/tests/qemunbdkitdata/disk-network-http.args.disk1 new file mode 100644 index 0000000000..9bac3fe229 --- /dev/null +++ b/tests/qemunbdkitdata/disk-network-http.args.disk1 @@ -0,0 +1,6 @@ +nbdkit \ +--exit-with-parent \ +--unix /tmp/statedir-1/nbdkit-test-disk-1.socket \ +--foreground curl \ +protocols=https \ +url=https://example.org:443/test2.img diff --git a/tests/qemunbdkitdata/disk-network-http.args.disk2 b/tests/qemunbdkitdata/disk-network-http.args.disk2 new file mode 100644 index 0000000000..7286b684a8 --- /dev/null +++ b/tests/qemunbdkitdata/disk-network-http.args.disk2 @@ -0,0 +1,7 @@ +nbdkit \ +--exit-with-parent \ +--unix /tmp/statedir-2/nbdkit-test-disk-2.socket \ +--foreground curl \ +protocols=http \ +url=http://example.org:1234/test3.img \ +'cookie=test=testcookievalue; test2="blurb"' diff --git a/tests/qemunbdkitdata/disk-network-http.args.disk3 b/tests/qemunbdkitdata/disk-network-http.args.disk3 new file mode 100644 index 0000000000..da177c9e6d --- /dev/null +++ b/tests/qemunbdkitdata/disk-network-http.args.disk3 @@ -0,0 +1,8 @@ +nbdkit \ +--exit-with-parent \ +--unix /tmp/statedir-3/nbdkit-test-disk-3.socket \ +--foreground curl \ +protocols=https \ +'url=https://example.org:1234/test4.img?par=val&other=ble' \ +'cookie=test=testcookievalue; test2="blurb"' \ +sslverify=false diff --git a/tests/qemunbdkitdata/disk-network-source-curl-nbdkit-backing.args.disk0 b/tests/qemunbdkitdata/disk-network-source-curl-nbdkit-backing.args.disk0 new file mode 100644 index 0000000000..b13f5ed628 --- /dev/null +++ b/tests/qemunbdkitdata/disk-network-source-curl-nbdkit-backing.args.disk0 @@ -0,0 +1,8 @@ +nbdkit \ +--exit-with-parent \ +--unix /tmp/statedir-0/nbdkit-test-disk-0.socket \ +--foreground \ +--readonly curl \ +protocols=https \ +url=https://https.example.org:8443/path/to/disk1.qcow2 \ +'cookie=cookie1=cookievalue1; cookie2=cookievalue2' diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk0 b/tests/qemunbdkitdata/disk-network-source-curl.args.disk0 new file mode 100644 index 0000000000..6de42c626f --- /dev/null +++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk0 @@ -0,0 +1,8 @@ +nbdkit \ +--exit-with-parent \ +--unix /tmp/statedir-0/nbdkit-test-disk-0.socket \ +--foreground \ +--readonly curl \ +protocols=https \ +url=https://https.example.org:8443/path/to/disk1.iso \ +'cookie=cookie1=cookievalue1; cookie2=cookievalue2' diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk1 b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1 new file mode 100644 index 0000000000..9abc1578dd --- /dev/null +++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1 @@ -0,0 +1,8 @@ +nbdkit \ +--exit-with-parent \ +--unix /tmp/statedir-1/nbdkit-test-disk-1.socket \ +--foreground curl \ +protocols=https \ +'url=https://https.example.org:8443/path/to/disk5.iso?foo=bar' \ +'cookie=cookie1=cookievalue1; cookie2=cookievalue2' \ +sslverify=false diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk2 b/tests/qemunbdkitdata/disk-network-source-curl.args.disk2 new file mode 100644 index 0000000000..1ce11ce618 --- /dev/null +++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk2 @@ -0,0 +1,8 @@ +nbdkit \ +--exit-with-parent \ +--unix /tmp/statedir-2/nbdkit-test-disk-2.socket \ +--foreground \ +--readonly curl \ +protocols=http \ +url=http://http.example.org:8080/path/to/disk2.iso \ +'cookie=cookie1=cookievalue1; cookie2=cookievalue2; cookie3=cookievalue3' diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk3 b/tests/qemunbdkitdata/disk-network-source-curl.args.disk3 new file mode 100644 index 0000000000..bc28f04564 --- /dev/null +++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk3 @@ -0,0 +1,7 @@ +nbdkit \ +--exit-with-parent \ +--unix /tmp/statedir-3/nbdkit-test-disk-3.socket \ +--foreground \ +--readonly curl \ +protocols=ftp \ +url=ftp://ftp.example.org:20/path/to/disk3.iso diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk4 b/tests/qemunbdkitdata/disk-network-source-curl.args.disk4 new file mode 100644 index 0000000000..7c3cc711ae --- /dev/null +++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk4 @@ -0,0 +1,7 @@ +nbdkit \ +--exit-with-parent \ +--unix /tmp/statedir-4/nbdkit-test-disk-4.socket \ +--foreground \ +--readonly curl \ +protocols=ftps \ +url=ftps://ftps.example.org:22/path/to/disk4.iso diff --git a/tests/qemunbdkitdata/disk-network-ssh.args.disk0 b/tests/qemunbdkitdata/disk-network-ssh.args.disk0 new file mode 100644 index 0000000000..e0020dff6a --- /dev/null +++ b/tests/qemunbdkitdata/disk-network-ssh.args.disk0 @@ -0,0 +1,7 @@ +nbdkit \ +--exit-with-parent \ +--unix /tmp/statedir-0/nbdkit-test-disk-0.socket \ +--foreground ssh \ +host=example.org \ +port=2222 \ +path=test.img diff --git a/tests/qemunbdkittest.c b/tests/qemunbdkittest.c new file mode 100644 index 0000000000..c53601dac8 --- /dev/null +++ b/tests/qemunbdkittest.c @@ -0,0 +1,220 @@ +#include <config.h> + +#include "internal.h" +#include "testutils.h" +#include "testutilsqemu.h" +#include "qemu/qemu_domain.h" +#include "qemu/qemu_nbdkit.h" +#define LIBVIRT_QEMU_NBDKITPRIV_H_ALLOW +#include "qemu/qemu_nbdkitpriv.h" +#include "vircommand.h" +#define LIBVIRT_VIRCOMMANDPRIV_H_ALLOW +#include "vircommandpriv.h" +#include "virutil.h" +#include "virsecret.h" +#include "datatypes.h" + +#define VIR_FROM_THIS VIR_FROM_QEMU + +static virQEMUDriver driver; + + +/* Some mock implementations for testing */ +int +virSecretGetSecretString(virConnectPtr conn G_GNUC_UNUSED, + virSecretLookupTypeDef *seclookupdef, + virSecretUsageType secretUsageType, + uint8_t **secret, + size_t *secret_size) +{ + char uuidstr[VIR_UUID_BUFLEN]; + const char *secretname = NULL; + char *tmp = NULL; + + switch (seclookupdef->type) { + case VIR_SECRET_LOOKUP_TYPE_UUID: + virUUIDFormat(seclookupdef->u.uuid, uuidstr); + secretname = uuidstr; + break; + case VIR_SECRET_LOOKUP_TYPE_USAGE: + secretname = seclookupdef->u.usage; + break; + case VIR_SECRET_LOOKUP_TYPE_NONE: + case VIR_SECRET_LOOKUP_TYPE_LAST: + default: + virReportEnumRangeError(virSecretLookupType, seclookupdef->type); + return -1; + }; + + /* For testing, just generate a value for the secret that includes the type + * and the id of the secret */ + tmp = g_strdup_printf("%s-%s-secret", virSecretUsageTypeToString(secretUsageType), secretname); + *secret = (uint8_t*)tmp; + *secret_size = strlen(tmp) + 1; + + return 0; +} + +virConnectPtr virGetConnectSecret(void) +{ + return virGetConnect(); +} + +/* end of mock implementations */ + + +typedef struct { + const char *name; + char* infile; + char* outtemplate; + qemuNbdkitCaps *nbdkitcaps; +} TestInfo; + + +typedef enum { + NBDKIT_ARG_CAPS, + NBDKIT_ARG_END +} NbdkitArgName; + + +static void +testInfoSetPaths(TestInfo *info) +{ + info->infile = g_strdup_printf("%s/qemuxml2argvdata/%s.xml", + abs_srcdir, info->name); + info->outtemplate = g_strdup_printf("%s/qemunbdkitdata/%s", + abs_srcdir, info->name); +} + +static void +testInfoClear(TestInfo *info) +{ + g_free(info->infile); + g_free(info->outtemplate); + g_clear_object(&info->nbdkitcaps); +} + +static void +testInfoSetArgs(TestInfo *info, ...) +{ + va_list argptr; + NbdkitArgName argname; + unsigned int cap; + + va_start(argptr, info); + while ((argname = va_arg(argptr, NbdkitArgName)) != NBDKIT_ARG_END) { + switch (argname) { + case NBDKIT_ARG_CAPS: + while ((cap = va_arg(argptr, unsigned int)) < QEMU_NBDKIT_CAPS_LAST) + qemuNbdkitCapsSet(info->nbdkitcaps, cap); + break; + case NBDKIT_ARG_END: + default: + break; + } + } +} + + +static int +testNbdkit(const void *data) +{ + const TestInfo *info = data; + g_autoptr(virDomainDef) def = NULL; + size_t i; + int ret = 0; + + if (!virFileExists(info->infile)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Test input file '%s' is missing", info->infile); + return -1; + } + + if (!(def = virDomainDefParseFile(info->infile, driver.xmlopt, NULL, + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))) + return -1; + + for (i = 0; i < def->ndisks; i++) { + virDomainDiskDef *disk = def->disks[i]; + g_autofree char *statedir = g_strdup_printf("/tmp/statedir-%zi", i); + g_autofree char *alias = g_strdup_printf("test-disk-%zi", i); + g_autofree char *cmdfile = g_strdup_printf("%s.args.disk%zi", + info->outtemplate, i); + + if (qemuNbdkitInitStorageSource(info->nbdkitcaps, disk->src, statedir, + alias, 101, 101)) { + qemuDomainStorageSourcePrivate *srcPriv = + qemuDomainStorageSourcePrivateFetch(disk->src); + g_autoptr(virCommand) cmd = NULL; + g_autoptr(virCommandDryRunToken) dryRunToken = virCommandDryRunTokenNew(); + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + const char *actualCmdline = NULL; + + virCommandSetDryRun(dryRunToken, &buf, true, true, NULL, NULL); + cmd = qemuNbdkitProcessBuildCommand(srcPriv->nbdkitProcess); + + if (virCommandRun(cmd, NULL) < 0) { + ret = -1; + continue; + } + + if (!(actualCmdline = virBufferContentAndReset(&buf))) { + ret = -1; + continue; + } + + if (virTestCompareToFileFull(actualCmdline, cmdfile, false) < 0) { + ret = -1; + continue; + } + } else { + if (virFileExists(cmdfile)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + "qemuNbdkitInitStorageSource() was not expected to fail"); + ret = -1; + } + } + } + + return ret; +} + +static int +mymain(void) +{ + int ret = 0; + + if (qemuTestDriverInit(&driver) < 0) + return EXIT_FAILURE; + + +#define DO_TEST_FULL(_name, ...) \ + do { \ + TestInfo info = { \ + .name = _name, \ + .nbdkitcaps = qemuNbdkitCapsNew(TEST_NBDKIT_PATH), \ + }; \ + testInfoSetPaths(&info); \ + testInfoSetArgs(&info, __VA_ARGS__); \ + virTestRunLog(&ret, "nbdkit " _name, testNbdkit, &info); \ + testInfoClear(&info); \ + } while (0) + +#define DO_TEST(_name, ...) \ + DO_TEST_FULL(_name, NBDKIT_ARG_CAPS, __VA_ARGS__, QEMU_NBDKIT_CAPS_LAST, NBDKIT_ARG_END) + +#define DO_TEST_NOCAPS(_name) \ + DO_TEST_FULL(_name, NBDKIT_ARG_END) + + DO_TEST("disk-cdrom-network", QEMU_NBDKIT_CAPS_PLUGIN_CURL); + DO_TEST("disk-network-http", QEMU_NBDKIT_CAPS_PLUGIN_CURL); + DO_TEST("disk-network-source-curl-nbdkit-backing", QEMU_NBDKIT_CAPS_PLUGIN_CURL); + DO_TEST("disk-network-source-curl", QEMU_NBDKIT_CAPS_PLUGIN_CURL); + DO_TEST("disk-network-ssh", QEMU_NBDKIT_CAPS_PLUGIN_SSH); + + qemuTestDriverFree(&driver); + + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIR_TEST_MAIN(mymain) -- 2.37.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@redhat.com> --- build-aux/syntax-check.mk | 4 +- src/qemu/qemu_nbdkit.c | 162 ++++++++++++++++-- src/qemu/qemu_nbdkitpriv.h | 19 +- src/util/virutil.h | 2 +- .../disk-cdrom-network.args.disk1 | 2 +- .../disk-cdrom-network.args.disk1.pipe.45 | 1 + .../disk-cdrom-network.args.disk2 | 2 +- .../disk-cdrom-network.args.disk2.pipe.47 | 1 + .../disk-network-http.args.disk2 | 2 +- .../disk-network-http.args.disk2.pipe.45 | 1 + .../disk-network-http.args.disk3 | 2 +- .../disk-network-http.args.disk3.pipe.47 | 1 + ...work-source-curl-nbdkit-backing.args.disk0 | 2 +- ...rce-curl-nbdkit-backing.args.disk0.pipe.45 | 1 + .../disk-network-source-curl.args.disk0 | 2 +- ...isk-network-source-curl.args.disk0.pipe.45 | 1 + .../disk-network-source-curl.args.disk1 | 2 +- ...isk-network-source-curl.args.disk1.pipe.47 | 1 + .../disk-network-source-curl.args.disk2 | 2 +- ...isk-network-source-curl.args.disk2.pipe.49 | 1 + tests/qemunbdkittest.c | 57 +++++- 21 files changed, 238 insertions(+), 30 deletions(-) create mode 100644 tests/qemunbdkitdata/disk-cdrom-network.args.disk1.pipe.45 create mode 100644 tests/qemunbdkitdata/disk-cdrom-network.args.disk2.pipe.47 create mode 100644 tests/qemunbdkitdata/disk-network-http.args.disk2.pipe.45 create mode 100644 tests/qemunbdkitdata/disk-network-http.args.disk3.pipe.47 create mode 100644 tests/qemunbdkitdata/disk-network-source-curl-nbdkit-backing.args.disk0.pipe.45 create mode 100644 tests/qemunbdkitdata/disk-network-source-curl.args.disk0.pipe.45 create mode 100644 tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.47 create mode 100644 tests/qemunbdkitdata/disk-network-source-curl.args.disk2.pipe.49 diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index 649eb91acb..c0cf730d13 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -1363,10 +1363,10 @@ exclude_file_name_regexp--sc_prohibit_strdup = \ ^(docs/|examples/|tests/virnetserverclientmock.c|tests/commandhelper.c|tools/nss/libvirt_nss_(leases|macs)\.c$$) exclude_file_name_regexp--sc_prohibit_close = \ - (\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/vir(file|event)\.c|src/libvirt-stream\.c|tests/(vir.+mock\.c|commandhelper\.c|qemusecuritymock\.c)|tools/nss/libvirt_nss_(leases|macs)\.c)$$) + (\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/vir(file|event)\.c|src/libvirt-stream\.c|tests/(vir.+mock\.c|commandhelper\.c|qemusecuritymock\.c|qemunbdkittest\.c)|tools/nss/libvirt_nss_(leases|macs)\.c)$$) exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF = \ - (^tests/(nodedevmdevctl|virhostcpu|virpcitest|virstoragetest)data/|docs/js/.*\.js|docs/fonts/.*\.woff|\.diff|tests/virconfdata/no-newline\.conf$$) + (^tests/(nodedevmdevctl|virhostcpu|virpcitest|virstoragetest|qemunbdkit)data/|docs/js/.*\.js|docs/fonts/.*\.woff|\.diff|tests/virconfdata/no-newline\.conf$$) exclude_file_name_regexp--sc_prohibit_fork_wrappers = \ (^(src/(util/(vircommand|virdaemon)|lxc/lxc_controller)|tests/testutils)\.c$$) diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c index 0ecf6c6537..2b8e203d16 100644 --- a/src/qemu/qemu_nbdkit.c +++ b/src/qemu/qemu_nbdkit.c @@ -55,6 +55,76 @@ VIR_ENUM_IMPL(qemuNbdkitCaps, "filter-readahead", /* QEMU_NBDKIT_CAPS_FILTER_READAHEAD */ ); + +static void +nbdkitPipeItemFree(NbdkitPipeItem *item) +{ + if (item->buf) { + virSecureErase(item->buf, item->buflen); + g_free(item->buf); + } + + if (item->fd > 0) + VIR_FORCE_CLOSE(item->fd); + + g_free(item); +} + + +void nbdkitPipeDataFree(NbdkitPipeData *self) +{ + size_t i; + + if (!self) + return; + + for (i = 0; i < self->nitems; i++) { + nbdkitPipeItemFree(self->items[i]); + } + + g_free(self->items); + g_free(self); +} + + +static NbdkitPipeItem* +nbdkitPipeItemNew(int fd, void *data, int datalen) +{ + NbdkitPipeItem *d; + + if (!data || datalen == 0) + return NULL; + + d = g_new0(NbdkitPipeItem, 1); + d->fd = fd; + + if (datalen < 0) { + /* -1 indicates a null-terminated string */ + d->buf = g_strdup(data); + d->buflen = strlen(data); + } else { + d->buf = g_malloc(datalen); + memcpy(d->buf, data, datalen); + d->buflen = datalen; + } + + return d; +} + + +static int +nbdkitPipeDataWrite(NbdkitPipeItem *pipe) +{ + if (safewrite(pipe->fd, pipe->buf, pipe->buflen) < 0) { + virReportSystemError(errno, + _("failed to write data to pipe %i for nbdkit"), + pipe->fd); + return -1; + } + return 0; +} + + struct _qemuNbdkitCaps { GObject parent; @@ -71,6 +141,12 @@ struct _qemuNbdkitCaps { G_DEFINE_TYPE(qemuNbdkitCaps, qemu_nbdkit_caps, G_TYPE_OBJECT); +enum { + PIPE_FD_READ = 0, + PIPE_FD_WRITE = 1 +}; + + static void qemuNbdkitCheckCommandCap(qemuNbdkitCaps *nbdkit, virCommand *cmd, @@ -685,12 +761,36 @@ qemuNbdkitInitStorageSource(qemuNbdkitCaps *caps, } +static NbdkitPipeItem* +commandPassDataByPipe(virCommand *cmd, + const char *argName, + char *buf, + size_t buflen) +{ + int fds[2] = { -1, -1 }; + g_autofree char *fdfmt = NULL; + + if (virPipe(fds) < 0) + return NULL; + + /* 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", fds[PIPE_FD_READ]); + virCommandAddArgPair(cmd, argName, fdfmt); + virCommandPassFD(cmd, fds[PIPE_FD_READ], VIR_COMMAND_PASS_FD_CLOSE_PARENT); + + return nbdkitPipeItemNew(fds[PIPE_FD_WRITE], (char*)buf, buflen); +} + static int qemuNbdkitProcessBuildCommandCurl(qemuNbdkitProcess *proc, - virCommand *cmd) + virCommand *cmd, + NbdkitPipeData **pipeData) { g_autoptr(virURI) uri = qemuBlockStorageSourceGetURI(proc->source); g_autofree char *uristring = virURIFormat(uri); + g_autoptr(GPtrArray) pipes = + g_ptr_array_new_with_free_func((GDestroyNotify)nbdkitPipeDataFree); /* nbdkit plugin name */ virCommandAddArg(cmd, "curl"); @@ -702,8 +802,9 @@ 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; + NbdkitPipeItem *pipe = NULL; virCommandAddArgPair(cmd, "user", proc->source->auth->username); @@ -716,7 +817,7 @@ qemuNbdkitProcessBuildCommandCurl(qemuNbdkitProcess *proc, } if (virSecretGetSecretString(conn, - &proc->source->auth->seclookupdef, + &authdef->seclookupdef, secrettype, &secret, &secretlen) < 0) { @@ -725,18 +826,28 @@ qemuNbdkitProcessBuildCommandCurl(qemuNbdkitProcess *proc, return -1; } - /* ensure that the secret is a NULL-terminated string */ - password = g_strndup((char*)secret, secretlen); - - virCommandAddArgPair(cmd, "password", password); + if (!(pipe = commandPassDataByPipe(cmd, "password", (char*)secret, + secretlen))) { + virSecureErase(secret, secretlen); + return -1; + } virSecureErase(secret, secretlen); - virSecureErase(password, secretlen); + g_ptr_array_add(pipes, pipe); } - if (proc->source->ncookies > 0) - virCommandAddArgPair(cmd, "cookie", - qemuBlockStorageSourceGetCookieString(proc->source)); + /* Create a pipe to send the cookies to the nbdkit process. */ + if (proc->source->ncookies) { + NbdkitPipeItem *pipe = NULL; + g_autofree char *cookies = + qemuBlockStorageSourceGetCookieString(proc->source); + + if (!(pipe = commandPassDataByPipe(cmd, "cookie", cookies, -1))) { + return -1; + } + + g_ptr_array_add(pipes, pipe); + } if (proc->source->sslverify == VIR_TRISTATE_BOOL_NO) { virCommandAddArgPair(cmd, "sslverify", "false"); @@ -747,6 +858,10 @@ qemuNbdkitProcessBuildCommandCurl(qemuNbdkitProcess *proc, virCommandAddArgPair(cmd, "timeout", timeout); } + *pipeData = g_new0(NbdkitPipeData, 1); + (*pipeData)->nitems = pipes->len; + (*pipeData)->items = (NbdkitPipeItem**)g_ptr_array_free(g_steal_pointer(&pipes), false); + return 0; } @@ -781,8 +896,17 @@ qemuNbdkitProcessBuildCommandSSH(qemuNbdkitProcess *proc, } +/* Builds a nbdkit command for the given disk source. + * + * Some sensitive data should be not be passed to nbdkit via commandline, so + * this command may set up one or more pipes and pass the fd of these pipes to + * the nbdkit command. In that case, pipeData will return information about the + * pipes and the information that must be written to that pipe (via + * nbdkitPipeDataWrite()) after the command has been executed. The pipeData + * elements should be freed after writing. */ virCommand * -qemuNbdkitProcessBuildCommand(qemuNbdkitProcess *proc) +qemuNbdkitProcessBuildCommand(qemuNbdkitProcess *proc, + NbdkitPipeData **pipeData) { g_autoptr(virCommand) cmd = virCommandNewArgList(proc->caps->path, "--exit-with-parent", @@ -806,7 +930,7 @@ qemuNbdkitProcessBuildCommand(qemuNbdkitProcess *proc) case VIR_STORAGE_NET_PROTOCOL_FTP: case VIR_STORAGE_NET_PROTOCOL_FTPS: case VIR_STORAGE_NET_PROTOCOL_TFTP: - if (qemuNbdkitProcessBuildCommandCurl(proc, cmd) < 0) + if (qemuNbdkitProcessBuildCommandCurl(proc, cmd, pipeData) < 0) return NULL; break; case VIR_STORAGE_NET_PROTOCOL_SSH: @@ -869,8 +993,10 @@ qemuNbdkitProcessStart(qemuNbdkitProcess *proc, int exitstatus = 0; int cmdret = 0; int errfd = -1; + g_autoptr(NbdkitPipeData) pipes = NULL; + size_t i; - if (!(cmd = qemuNbdkitProcessBuildCommand(proc))) + if (!(cmd = qemuNbdkitProcessBuildCommand(proc, &pipes))) return -1; virCommandSetErrorFD(cmd, &errfd); @@ -888,6 +1014,14 @@ qemuNbdkitProcessStart(qemuNbdkitProcess *proc, goto error; } + if (pipes) { + for (i = 0; i < pipes->nitems; i++) { + NbdkitPipeItem *pipe = pipes->items[i]; + if (nbdkitPipeDataWrite(pipe) < 0) + goto error; + } + } + if ((rc = virPidFileReadPath(proc->pidfile, &proc->pid)) < 0) { virReportSystemError(-rc, _("Failed to read pidfile %s"), diff --git a/src/qemu/qemu_nbdkitpriv.h b/src/qemu/qemu_nbdkitpriv.h index 64f9bb99d8..a4cc61bb2c 100644 --- a/src/qemu/qemu_nbdkitpriv.h +++ b/src/qemu/qemu_nbdkitpriv.h @@ -27,5 +27,20 @@ #include "qemu_nbdkit.h" -virCommand * -qemuNbdkitProcessBuildCommand(qemuNbdkitProcess *proc); +typedef struct { + int fd; + void *buf; + size_t buflen; +} NbdkitPipeItem; + +typedef struct { + size_t nitems; + NbdkitPipeItem **items; +} NbdkitPipeData; + +void nbdkitPipeDataFree(NbdkitPipeData *pipedata); + +G_DEFINE_AUTOPTR_CLEANUP_FUNC(NbdkitPipeData, nbdkitPipeDataFree); + +virCommand* qemuNbdkitProcessBuildCommand(qemuNbdkitProcess *proc, + NbdkitPipeData **pipeData); diff --git a/src/util/virutil.h b/src/util/virutil.h index ab8511bf8d..094b399859 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -186,7 +186,7 @@ char *virGetPassword(void); * * Returns: -1 on error, 0 on success */ -int virPipe(int fds[2]); +int virPipe(int fds[2]) G_NO_INLINE; /* * virPipeQuiet: diff --git a/tests/qemunbdkitdata/disk-cdrom-network.args.disk1 b/tests/qemunbdkitdata/disk-cdrom-network.args.disk1 index 257e331db8..da9e507f1b 100644 --- a/tests/qemunbdkitdata/disk-cdrom-network.args.disk1 +++ b/tests/qemunbdkitdata/disk-cdrom-network.args.disk1 @@ -6,4 +6,4 @@ nbdkit \ protocols=ftps \ url=ftps://host.name:990/url/path/file.iso \ user=testuser \ -password=iscsi-mycluster_myname-secret +password=-44 diff --git a/tests/qemunbdkitdata/disk-cdrom-network.args.disk1.pipe.45 b/tests/qemunbdkitdata/disk-cdrom-network.args.disk1.pipe.45 new file mode 100644 index 0000000000..ccdd4033fc --- /dev/null +++ b/tests/qemunbdkitdata/disk-cdrom-network.args.disk1.pipe.45 @@ -0,0 +1 @@ +iscsi-mycluster_myname-secret \ No newline at end of file diff --git a/tests/qemunbdkitdata/disk-cdrom-network.args.disk2 b/tests/qemunbdkitdata/disk-cdrom-network.args.disk2 index f7879a9f24..0742b29853 100644 --- a/tests/qemunbdkitdata/disk-cdrom-network.args.disk2 +++ b/tests/qemunbdkitdata/disk-cdrom-network.args.disk2 @@ -6,4 +6,4 @@ nbdkit \ protocols=https \ 'url=https://host.name:443/url/path/file.iso?test=val' \ user=testuser \ -password=iscsi-mycluster_myname-secret +password=-46 diff --git a/tests/qemunbdkitdata/disk-cdrom-network.args.disk2.pipe.47 b/tests/qemunbdkitdata/disk-cdrom-network.args.disk2.pipe.47 new file mode 100644 index 0000000000..ccdd4033fc --- /dev/null +++ b/tests/qemunbdkitdata/disk-cdrom-network.args.disk2.pipe.47 @@ -0,0 +1 @@ +iscsi-mycluster_myname-secret \ No newline at end of file diff --git a/tests/qemunbdkitdata/disk-network-http.args.disk2 b/tests/qemunbdkitdata/disk-network-http.args.disk2 index 7286b684a8..767ea881d8 100644 --- a/tests/qemunbdkitdata/disk-network-http.args.disk2 +++ b/tests/qemunbdkitdata/disk-network-http.args.disk2 @@ -4,4 +4,4 @@ nbdkit \ --foreground curl \ protocols=http \ url=http://example.org:1234/test3.img \ -'cookie=test=testcookievalue; test2="blurb"' +cookie=-44 diff --git a/tests/qemunbdkitdata/disk-network-http.args.disk2.pipe.45 b/tests/qemunbdkitdata/disk-network-http.args.disk2.pipe.45 new file mode 100644 index 0000000000..2c42c95930 --- /dev/null +++ b/tests/qemunbdkitdata/disk-network-http.args.disk2.pipe.45 @@ -0,0 +1 @@ +test=testcookievalue; test2="blurb" \ No newline at end of file diff --git a/tests/qemunbdkitdata/disk-network-http.args.disk3 b/tests/qemunbdkitdata/disk-network-http.args.disk3 index da177c9e6d..30dfd15861 100644 --- a/tests/qemunbdkitdata/disk-network-http.args.disk3 +++ b/tests/qemunbdkitdata/disk-network-http.args.disk3 @@ -4,5 +4,5 @@ nbdkit \ --foreground curl \ protocols=https \ 'url=https://example.org:1234/test4.img?par=val&other=ble' \ -'cookie=test=testcookievalue; test2="blurb"' \ +cookie=-46 \ sslverify=false diff --git a/tests/qemunbdkitdata/disk-network-http.args.disk3.pipe.47 b/tests/qemunbdkitdata/disk-network-http.args.disk3.pipe.47 new file mode 100644 index 0000000000..2c42c95930 --- /dev/null +++ b/tests/qemunbdkitdata/disk-network-http.args.disk3.pipe.47 @@ -0,0 +1 @@ +test=testcookievalue; test2="blurb" \ No newline at end of file diff --git a/tests/qemunbdkitdata/disk-network-source-curl-nbdkit-backing.args.disk0 b/tests/qemunbdkitdata/disk-network-source-curl-nbdkit-backing.args.disk0 index b13f5ed628..d5ad545cdc 100644 --- a/tests/qemunbdkitdata/disk-network-source-curl-nbdkit-backing.args.disk0 +++ b/tests/qemunbdkitdata/disk-network-source-curl-nbdkit-backing.args.disk0 @@ -5,4 +5,4 @@ nbdkit \ --readonly curl \ protocols=https \ url=https://https.example.org:8443/path/to/disk1.qcow2 \ -'cookie=cookie1=cookievalue1; cookie2=cookievalue2' +cookie=-44 diff --git a/tests/qemunbdkitdata/disk-network-source-curl-nbdkit-backing.args.disk0.pipe.45 b/tests/qemunbdkitdata/disk-network-source-curl-nbdkit-backing.args.disk0.pipe.45 new file mode 100644 index 0000000000..20af4ae383 --- /dev/null +++ b/tests/qemunbdkitdata/disk-network-source-curl-nbdkit-backing.args.disk0.pipe.45 @@ -0,0 +1 @@ +cookie1=cookievalue1; cookie2=cookievalue2 \ No newline at end of file diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk0 b/tests/qemunbdkitdata/disk-network-source-curl.args.disk0 index 6de42c626f..3ea686e14f 100644 --- a/tests/qemunbdkitdata/disk-network-source-curl.args.disk0 +++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk0 @@ -5,4 +5,4 @@ nbdkit \ --readonly curl \ protocols=https \ url=https://https.example.org:8443/path/to/disk1.iso \ -'cookie=cookie1=cookievalue1; cookie2=cookievalue2' +cookie=-44 diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk0.pipe.45 b/tests/qemunbdkitdata/disk-network-source-curl.args.disk0.pipe.45 new file mode 100644 index 0000000000..20af4ae383 --- /dev/null +++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk0.pipe.45 @@ -0,0 +1 @@ +cookie1=cookievalue1; cookie2=cookievalue2 \ No newline at end of file diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk1 b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1 index 9abc1578dd..fb77794b56 100644 --- a/tests/qemunbdkitdata/disk-network-source-curl.args.disk1 +++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1 @@ -4,5 +4,5 @@ nbdkit \ --foreground curl \ protocols=https \ 'url=https://https.example.org:8443/path/to/disk5.iso?foo=bar' \ -'cookie=cookie1=cookievalue1; cookie2=cookievalue2' \ +cookie=-46 \ sslverify=false diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.47 b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.47 new file mode 100644 index 0000000000..20af4ae383 --- /dev/null +++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.47 @@ -0,0 +1 @@ +cookie1=cookievalue1; cookie2=cookievalue2 \ No newline at end of file diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk2 b/tests/qemunbdkitdata/disk-network-source-curl.args.disk2 index 1ce11ce618..eab66746ef 100644 --- a/tests/qemunbdkitdata/disk-network-source-curl.args.disk2 +++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk2 @@ -5,4 +5,4 @@ nbdkit \ --readonly curl \ protocols=http \ url=http://http.example.org:8080/path/to/disk2.iso \ -'cookie=cookie1=cookievalue1; cookie2=cookievalue2; cookie3=cookievalue3' +cookie=-48 diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk2.pipe.49 b/tests/qemunbdkitdata/disk-network-source-curl.args.disk2.pipe.49 new file mode 100644 index 0000000000..5c035e84c5 --- /dev/null +++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk2.pipe.49 @@ -0,0 +1 @@ +cookie1=cookievalue1; cookie2=cookievalue2; cookie3=cookievalue3 \ No newline at end of file diff --git a/tests/qemunbdkittest.c b/tests/qemunbdkittest.c index c53601dac8..a18567e9b4 100644 --- a/tests/qemunbdkittest.c +++ b/tests/qemunbdkittest.c @@ -13,6 +13,7 @@ #include "virutil.h" #include "virsecret.h" #include "datatypes.h" +#include "virmock.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -20,6 +21,41 @@ static virQEMUDriver driver; /* Some mock implementations for testing */ +#define PIPE_FD_START 44 +static int mockpipefd = PIPE_FD_START; +int +virPipe(int fds[2]) +{ + fds[0] = mockpipefd++; + fds[1] = mockpipefd++; + + return 0; +} + +static int (*real_close)(int fd); +static void +init_syms(void) +{ + VIR_MOCK_REAL_INIT(close); +} + +int +close(int fd) +{ + int ret; + + init_syms(); + + if (fd >= PIPE_FD_START) + ret = 0; + else + ret = real_close(fd); + + return ret; +} + + + int virSecretGetSecretString(virConnectPtr conn G_GNUC_UNUSED, virSecretLookupTypeDef *seclookupdef, @@ -124,6 +160,9 @@ testNbdkit(const void *data) size_t i; int ret = 0; + /* restart mock pipe fds so tests are consistent */ + mockpipefd = PIPE_FD_START; + if (!virFileExists(info->infile)) { virReportError(VIR_ERR_INTERNAL_ERROR, "Test input file '%s' is missing", info->infile); @@ -149,9 +188,11 @@ testNbdkit(const void *data) g_autoptr(virCommandDryRunToken) dryRunToken = virCommandDryRunTokenNew(); g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; const char *actualCmdline = NULL; + g_autoptr(NbdkitPipeData) pipes = NULL; + size_t j; virCommandSetDryRun(dryRunToken, &buf, true, true, NULL, NULL); - cmd = qemuNbdkitProcessBuildCommand(srcPriv->nbdkitProcess); + cmd = qemuNbdkitProcessBuildCommand(srcPriv->nbdkitProcess, &pipes); if (virCommandRun(cmd, NULL) < 0) { ret = -1; @@ -163,9 +204,19 @@ testNbdkit(const void *data) continue; } - if (virTestCompareToFileFull(actualCmdline, cmdfile, false) < 0) { + if (virTestCompareToFileFull(actualCmdline, cmdfile, false) < 0) ret = -1; - continue; + + if (pipes) { + for (j = 0; j < pipes->nitems; j++) { + NbdkitPipeItem *item = pipes->items[j]; + g_autofree char *pipefile = g_strdup_printf("%s.pipe.%i", + cmdfile, + item->fd); + + if (virTestCompareToFile(item->buf, pipefile) < 0) + ret = -1; + } } } else { if (virFileExists(cmdfile)) { -- 2.37.1

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- tests/qemunbdkitdata/disk-network-source-curl.args.1.pipe.1 | 1 + tests/qemunbdkitdata/disk-network-source-curl.args.disk1 | 4 +++- .../disk-network-source-curl.args.disk1.pipe.47 | 2 +- .../disk-network-source-curl.args.disk1.pipe.49 | 1 + tests/qemunbdkitdata/disk-network-source-curl.args.disk2 | 2 +- .../disk-network-source-curl.args.disk2.pipe.51 | 1 + .../disk-network-source-curl.x86_64-latest.args | 3 ++- tests/qemuxml2argvdata/disk-network-source-curl.xml | 3 +++ 8 files changed, 13 insertions(+), 4 deletions(-) create mode 100644 tests/qemunbdkitdata/disk-network-source-curl.args.1.pipe.1 create mode 100644 tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.49 create mode 100644 tests/qemunbdkitdata/disk-network-source-curl.args.disk2.pipe.51 diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.1.pipe.1 b/tests/qemunbdkitdata/disk-network-source-curl.args.1.pipe.1 new file mode 100644 index 0000000000..20af4ae383 --- /dev/null +++ b/tests/qemunbdkitdata/disk-network-source-curl.args.1.pipe.1 @@ -0,0 +1 @@ +cookie1=cookievalue1; cookie2=cookievalue2 \ No newline at end of file diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk1 b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1 index fb77794b56..7a5ebee46e 100644 --- a/tests/qemunbdkitdata/disk-network-source-curl.args.disk1 +++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1 @@ -4,5 +4,7 @@ nbdkit \ --foreground curl \ protocols=https \ 'url=https://https.example.org:8443/path/to/disk5.iso?foo=bar' \ -cookie=-46 \ +user=myname \ +password=-46 \ +cookie=-48 \ sslverify=false diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.47 b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.47 index 20af4ae383..ccdd4033fc 100644 --- a/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.47 +++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.47 @@ -1 +1 @@ -cookie1=cookievalue1; cookie2=cookievalue2 \ No newline at end of file +iscsi-mycluster_myname-secret \ No newline at end of file diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.49 b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.49 new file mode 100644 index 0000000000..20af4ae383 --- /dev/null +++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk1.pipe.49 @@ -0,0 +1 @@ +cookie1=cookievalue1; cookie2=cookievalue2 \ No newline at end of file diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk2 b/tests/qemunbdkitdata/disk-network-source-curl.args.disk2 index eab66746ef..00151a10b0 100644 --- a/tests/qemunbdkitdata/disk-network-source-curl.args.disk2 +++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk2 @@ -5,4 +5,4 @@ nbdkit \ --readonly curl \ protocols=http \ url=http://http.example.org:8080/path/to/disk2.iso \ -cookie=-48 +cookie=-50 diff --git a/tests/qemunbdkitdata/disk-network-source-curl.args.disk2.pipe.51 b/tests/qemunbdkitdata/disk-network-source-curl.args.disk2.pipe.51 new file mode 100644 index 0000000000..5c035e84c5 --- /dev/null +++ b/tests/qemunbdkitdata/disk-network-source-curl.args.disk2.pipe.51 @@ -0,0 +1 @@ +cookie1=cookievalue1; cookie2=cookievalue2; cookie3=cookievalue3 \ 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 index ec6dd13f6c..7f09e84227 100644 --- a/tests/qemuxml2argvdata/disk-network-source-curl.x86_64-latest.args +++ b/tests/qemuxml2argvdata/disk-network-source-curl.x86_64-latest.args @@ -33,9 +33,10 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ -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-storage-auth-secret0","data":"9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1","keyid":"masterKey0","iv":"AAECAwQFBgcICQoLDA0ODw==","format":"base64"}' \ -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 '{"driver":"https","url":"https://https.example.org:8443/path/to/disk5.iso?foo=bar","username":"myname","password-secret":"libvirt-4-storage-auth-secret0","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"}' \ diff --git a/tests/qemuxml2argvdata/disk-network-source-curl.xml b/tests/qemuxml2argvdata/disk-network-source-curl.xml index 1e50314abe..8c3982cd73 100644 --- a/tests/qemuxml2argvdata/disk-network-source-curl.xml +++ b/tests/qemuxml2argvdata/disk-network-source-curl.xml @@ -59,6 +59,9 @@ <encryption format='luks'> <secret type='passphrase' uuid='1148b693-0843-4cef-9f97-8feb4e1ae365'/> </encryption> + <auth username='myname'> + <secret type='iscsi' usage='mycluster_myname'/> + </auth> </source> <target dev='vde' bus='virtio'/> </disk> -- 2.37.1