[libvirt PATCH 0/6] implement vhost-user-blk support
Pavel Hrdina (6): qemu_alias: introduce qemuDomainGetVhostUserAlias helper qemu_validate: move and refactor qemuValidateDomainDefVirtioFSSharedMemory docs: introduces new vhostuser disk type conf: implement support for vhostuser disk qemu_capabilities: introduce vhost-user-blk capability qemu: implement vhost-user-blk support docs/formatdomain.rst | 32 ++- docs/schemas/domaincommon.rng | 19 ++ src/conf/domain_conf.c | 85 ++++++++ src/conf/domain_validate.c | 187 ++++++++++++++++++ src/conf/storage_source_conf.c | 3 + src/conf/storage_source_conf.h | 4 + src/libxl/xen_xl.c | 1 + src/qemu/qemu_alias.c | 6 + src/qemu/qemu_alias.h | 2 + src/qemu/qemu_block.c | 20 ++ src/qemu/qemu_block.h | 5 + src/qemu/qemu_capabilities.c | 4 + src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_command.c | 100 ++++++++-- src/qemu/qemu_command.h | 8 + src/qemu/qemu_domain.c | 5 + src/qemu/qemu_hotplug.c | 5 +- src/qemu/qemu_migration.c | 2 + src/qemu/qemu_snapshot.c | 4 + src/qemu/qemu_validate.c | 112 ++++++----- src/storage_file/storage_source.c | 1 + .../caps_2.12.0.aarch64.xml | 1 + .../caps_2.12.0.ppc64.xml | 1 + .../caps_2.12.0.s390x.xml | 1 + .../caps_2.12.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 1 + .../caps_3.0.0.riscv32.xml | 1 + .../caps_3.0.0.riscv64.xml | 1 + .../qemucapabilitiesdata/caps_3.0.0.s390x.xml | 1 + .../caps_3.0.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_3.1.0.ppc64.xml | 1 + .../caps_3.1.0.x86_64.xml | 1 + .../caps_4.0.0.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 1 + .../caps_4.0.0.riscv32.xml | 1 + .../caps_4.0.0.riscv64.xml | 1 + .../qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 + .../caps_4.0.0.x86_64.xml | 1 + .../caps_4.1.0.x86_64.xml | 1 + .../caps_4.2.0.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 + .../qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 + .../caps_4.2.0.x86_64.xml | 1 + .../caps_5.0.0.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 + .../caps_5.0.0.riscv64.xml | 1 + .../caps_5.0.0.x86_64.xml | 1 + .../caps_5.1.0.x86_64.xml | 1 + .../caps_5.2.0.aarch64.xml | 1 + .../qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 + .../caps_5.2.0.riscv64.xml | 1 + .../qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 + .../caps_5.2.0.x86_64.xml | 1 + .../caps_6.0.0.x86_64.xml | 1 + .../disk-vhostuser.x86_64-latest.args | 41 ++++ tests/qemuxml2argvdata/disk-vhostuser.xml | 29 +++ tests/qemuxml2argvtest.c | 1 + .../disk-vhostuser.x86_64-latest.xml | 48 +++++ tests/qemuxml2xmltest.c | 1 + 59 files changed, 699 insertions(+), 62 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-vhostuser.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-vhostuser.xml create mode 100644 tests/qemuxml2xmloutdata/disk-vhostuser.x86_64-latest.xml -- 2.29.2
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_alias.c | 6 ++++++ src/qemu/qemu_alias.h | 2 ++ src/qemu/qemu_command.c | 8 +++++--- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index b3492d6e85..57b56b9ded 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -868,3 +868,9 @@ qemuDomainGetDBusVMStateAlias(void) { return "dbus-vmstate0"; } + +char * +qemuDomainGetVhostUserAlias(const char *devalias) +{ + return g_strdup_printf("chr-vu-%s", devalias); +} diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h index 239747beb1..7c5b1ec929 100644 --- a/src/qemu/qemu_alias.h +++ b/src/qemu/qemu_alias.h @@ -97,3 +97,5 @@ const char *qemuDomainGetManagedPRAlias(void); char *qemuDomainGetUnmanagedPRAlias(const char *parentalias); const char *qemuDomainGetDBusVMStateAlias(void); + +char *qemuDomainGetVhostUserAlias(const char *devalias); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f613aa0201..5101e5d74c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2172,7 +2172,7 @@ qemuBuildVHostUserFsCommandLine(virCommandPtr cmd, g_autofree char *chardev_alias = NULL; g_auto(virBuffer) opt = VIR_BUFFER_INITIALIZER; - chardev_alias = g_strdup_printf("chr-vu-%s", fs->info.alias); + chardev_alias = qemuDomainGetVhostUserAlias(fs->info.alias); virCommandAddArg(cmd, "-chardev"); virBufferAddLit(&opt, "socket"); @@ -4166,9 +4166,10 @@ qemuBuildDeviceVideoStr(const virDomainDef *def, virBufferAsprintf(&buf, ",max_outputs=%u", video->heads); } } else if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) { + g_autofree char *alias = qemuDomainGetVhostUserAlias(video->info.alias); if (video->heads) virBufferAsprintf(&buf, ",max_outputs=%u", video->heads); - virBufferAsprintf(&buf, ",chardev=chr-vu-%s", video->info.alias); + virBufferAsprintf(&buf, ",chardev=%s", alias); } else if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO) { if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_GPU_MAX_OUTPUTS)) { if (video->heads) @@ -4288,6 +4289,7 @@ qemuBuildVhostUserChardevStr(const char *alias, int *fd, virCommandPtr cmd) { + g_autofree char *chardev_alias = qemuDomainGetVhostUserAlias(alias); char *chardev = NULL; if (*fd == -1) { @@ -4296,7 +4298,7 @@ qemuBuildVhostUserChardevStr(const char *alias, return NULL; } - chardev = g_strdup_printf("socket,id=chr-vu-%s,fd=%d", alias, *fd); + chardev = g_strdup_printf("socket,id=%s,fd=%d", chardev_alias, *fd); virCommandPassFD(cmd, *fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); *fd = -1; -- 2.29.2
On a Tuesday in 2021, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_alias.c | 6 ++++++ src/qemu/qemu_alias.h | 2 ++ src/qemu/qemu_command.c | 8 +++++--- 3 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index b3492d6e85..57b56b9ded 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -868,3 +868,9 @@ qemuDomainGetDBusVMStateAlias(void) { return "dbus-vmstate0"; } + +char * +qemuDomainGetVhostUserAlias(const char *devalias)
Something like VhostUserChrAlias would be more fitting. But that's just internal naming that can be changed later.
+{ + return g_strdup_printf("chr-vu-%s", devalias); +}
Make the function reusable by other vhost-user based devices. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_validate.c | 99 +++++++++++++++++++++------------------- 1 file changed, 51 insertions(+), 48 deletions(-) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 88f4344df0..96fc7d4118 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1380,6 +1380,54 @@ qemuValidateDomainVirtioOptions(const virDomainVirtioOptions *virtio, } +static int +qemuValidateDomainDefVhostUserRequireSharedMemory(const virDomainDef *def, + const char *name, + virQEMUCapsPtr qemuCaps) +{ + const char *defaultRAMId = virQEMUCapsGetMachineDefaultRAMid(qemuCaps, + def->virtType, + def->os.machine); + size_t numa_nodes = virDomainNumaGetNodeCount(def->numa); + size_t i; + + if (numa_nodes == 0 && + !(defaultRAMId && def->mem.access == VIR_DOMAIN_MEMORY_ACCESS_SHARED)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("'%s' requires shared memory"), name); + return -1; + } + + for (i = 0; i < numa_nodes; i++) { + virDomainMemoryAccess node_access = + virDomainNumaGetNodeMemoryAccessMode(def->numa, i); + + switch (node_access) { + case VIR_DOMAIN_MEMORY_ACCESS_DEFAULT: + if (def->mem.access != VIR_DOMAIN_MEMORY_ACCESS_SHARED) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("'%s' requires shared memory"), name); + return -1; + } + break; + case VIR_DOMAIN_MEMORY_ACCESS_SHARED: + break; + case VIR_DOMAIN_MEMORY_ACCESS_PRIVATE: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("'%s' requires shared memory"), name); + return -1; + + case VIR_DOMAIN_MEMORY_ACCESS_LAST: + default: + virReportEnumRangeError(virDomainMemoryAccess, node_access); + return -1; + + } + } + return 0; +} + + static int qemuValidateDomainDeviceDefNetwork(const virDomainNetDef *net, virQEMUCapsPtr qemuCaps) @@ -3947,53 +3995,6 @@ qemuValidateDomainDeviceDefGraphics(const virDomainGraphicsDef *graphics, } -static int -qemuValidateDomainDefVirtioFSSharedMemory(const virDomainDef *def, - virQEMUCapsPtr qemuCaps) -{ - const char *defaultRAMId = virQEMUCapsGetMachineDefaultRAMid(qemuCaps, - def->virtType, - def->os.machine); - size_t numa_nodes = virDomainNumaGetNodeCount(def->numa); - size_t i; - - if (numa_nodes == 0 && - !(defaultRAMId && def->mem.access == VIR_DOMAIN_MEMORY_ACCESS_SHARED)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("virtiofs requires shared memory")); - return -1; - } - - for (i = 0; i < numa_nodes; i++) { - virDomainMemoryAccess node_access = - virDomainNumaGetNodeMemoryAccessMode(def->numa, i); - - switch (node_access) { - case VIR_DOMAIN_MEMORY_ACCESS_DEFAULT: - if (def->mem.access != VIR_DOMAIN_MEMORY_ACCESS_SHARED) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("virtiofs requires shared memory")); - return -1; - } - break; - case VIR_DOMAIN_MEMORY_ACCESS_SHARED: - break; - case VIR_DOMAIN_MEMORY_ACCESS_PRIVATE: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("virtiofs requires shared memory")); - return -1; - - case VIR_DOMAIN_MEMORY_ACCESS_LAST: - default: - virReportEnumRangeError(virDomainMemoryAccess, node_access); - return -1; - - } - } - return 0; -} - - static int qemuValidateDomainDeviceDefFS(virDomainFSDefPtr fs, const virDomainDef *def, @@ -4091,8 +4092,10 @@ qemuValidateDomainDeviceDefFS(virDomainFSDefPtr fs, _("virtiofs does not support fmode and dmode")); return -1; } - if (qemuValidateDomainDefVirtioFSSharedMemory(def, qemuCaps) < 0) + if (qemuValidateDomainDefVhostUserRequireSharedMemory(def, "virtiofs", + qemuCaps) < 0) { return -1; + } if (fs->info.bootIndex && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VHOST_USER_FS_BOOTINDEX)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", -- 2.29.2
On Tue, Feb 02, 2021 at 16:04:08 +0100, Pavel Hrdina wrote:
Make the function reusable by other vhost-user based devices.
Since reusability is the goal ...
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_validate.c | 99 +++++++++++++++++++++------------------- 1 file changed, 51 insertions(+), 48 deletions(-)
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 88f4344df0..96fc7d4118 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1380,6 +1380,54 @@ qemuValidateDomainVirtioOptions(const virDomainVirtioOptions *virtio, }
+static int +qemuValidateDomainDefVhostUserRequireSharedMemory(const virDomainDef *def, + const char *name, + virQEMUCapsPtr qemuCaps)
... please document the semantics of the function.
+{ + const char *defaultRAMId = virQEMUCapsGetMachineDefaultRAMid(qemuCaps, + def->virtType,
On Wed, Feb 03, 2021 at 08:45:52AM +0100, Peter Krempa wrote:
On Tue, Feb 02, 2021 at 16:04:08 +0100, Pavel Hrdina wrote:
Make the function reusable by other vhost-user based devices.
Since reusability is the goal ...
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_validate.c | 99 +++++++++++++++++++++------------------- 1 file changed, 51 insertions(+), 48 deletions(-)
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 88f4344df0..96fc7d4118 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1380,6 +1380,54 @@ qemuValidateDomainVirtioOptions(const virDomainVirtioOptions *virtio, }
+static int +qemuValidateDomainDefVhostUserRequireSharedMemory(const virDomainDef *def, + const char *name, + virQEMUCapsPtr qemuCaps)
... please document the semantics of the function.
Is this reasonable documentation: +/** + * qemuValidateDomainDefVhostUserRequireSharedMemory: + * @def: VM definition + * @name: name of the attribute/element + * @qemuCaps: capabilities of QEMU binary + * + * Check if the VM definition contains any form of shared memory + * which is required by vhost-user devices to operate properly. + * + * On success returns 0, on error returns -1 and reports proper error + * message. + */ Pavel
On Wed, Feb 03, 2021 at 10:10:11 +0100, Pavel Hrdina wrote:
On Wed, Feb 03, 2021 at 08:45:52AM +0100, Peter Krempa wrote:
On Tue, Feb 02, 2021 at 16:04:08 +0100, Pavel Hrdina wrote:
Make the function reusable by other vhost-user based devices.
Since reusability is the goal ...
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_validate.c | 99 +++++++++++++++++++++------------------- 1 file changed, 51 insertions(+), 48 deletions(-)
[...]
Is this reasonable documentation:
+/** + * qemuValidateDomainDefVhostUserRequireSharedMemory: + * @def: VM definition + * @name: name of the attribute/element + * @qemuCaps: capabilities of QEMU binary + * + * Check if the VM definition contains any form of shared memory + * which is required by vhost-user devices to operate properly. + * + * On success returns 0, on error returns -1 and reports proper error + * message.
Yes, that sounds good.
<disk type='vhostuser' device='disk'> <driver name='qemu' type='raw'/> <source type='unix' path='/tmp/vhost-blk.sock' mode='client'> <reconnect enabled='yes' timeout='10'/> </source> <target dev='vda' bus='virtio'/> </disk> Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.rst | 32 ++++++++++++++++++++++++++++++-- docs/schemas/domaincommon.rng | 19 +++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index a09868bed5..99f5cad571 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -2419,6 +2419,13 @@ paravirtualized driver is specified via the ``disk`` element. </source> <target dev='vde' bus='virtio'/> </disk> + <disk type='vhostuser' device='disk'> + <driver name='qemu' type='raw'/> + <source type='unix' path='/tmp/vhost-blk.sock' mode='client'> + <reconnect enabled='yes' timeout='10'/> + </source> + <target dev='vdf' bus='virtio'/> + </disk> </devices> ... @@ -2429,8 +2436,8 @@ paravirtualized driver is specified via the ``disk`` element. ``type`` Valid values are "file", "block", "dir" ( :since:`since 0.7.5` ), "network" ( :since:`since 0.8.7` ), or "volume" ( :since:`since 1.0.5` ), - or "nvme" ( :since:`since 6.0.0` ) and refer to the underlying source for - the disk. :since:`Since 0.0.3` + or "nvme" ( :since:`since 6.0.0` ), or "vhostuser" ( :since:`since 7.1.0` ) + and refer to the underlying source for the disk. :since:`Since 0.0.3` ``device`` Indicates how the disk is to be exposed to the guest OS. Possible values for this attribute are "floppy", "disk", "cdrom", and "lun", defaulting to @@ -2581,6 +2588,18 @@ paravirtualized driver is specified via the ``disk`` element. is not involved (compared to passing say ``/dev/nvme0n1`` via ``<disk type='block'>`` and therefore lower latencies can be achieved. + ``vhostuser`` + Enables the hypervisor to connect to another process using vhost-user + protocol. Requires shared memory configured for the VM, for more details + see ``access`` mode for `memoryBacking <#elementsMemoryBacking>` element. + + The ``source`` element has following mandatory attributes: + + ``type`` + The type of char device. Currently only ``unix`` type is supported. + ``path`` + Path to the unix socket to be used as disk source. + With "file", "block", and "volume", one or more optional sub-elements ``seclabel``, `described below <#seclabel>`__ (and :since:`since 0.9.9` ), can be used to override the domain security labeling policy for just that @@ -2703,6 +2722,15 @@ paravirtualized driver is specified via the ``disk`` element. of these attributes is omitted, then that field is assumed to be the default value for the current system. If both ``user`` and ``group`` are intended to be default, then the entire element may be omitted. + ``reconnect`` + For disk type ``vhostuser`` configures reconnect timeout if the connection + is lost. It has two mandatory attributes: + + ``enabled`` + If the reconnect feature is enabled, accepts ``yes`` and ``no`` + ``timeout`` + The amount of seconds after which hypervisor tries to reconnect. + For a "file" or "volume" disk type which represents a cdrom or floppy (the ``device`` attribute), it is possible to define policy what to do with the diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 7bab818bc9..98d5cee8c0 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1645,6 +1645,7 @@ <ref name="diskSourceNetwork"/> <ref name="diskSourceVolume"/> <ref name="diskSourceNvme"/> + <ref name="diskSourceVhostUser"/> </choice> </define> @@ -2155,6 +2156,24 @@ </optional> </define> + <define name="diskSourceVhostUser"> + <attribute name="type"> + <value>vhostuser</value> + </attribute> + <element name="source"> + <attribute name="type"> + <value>unix</value> + </attribute> + <attribute name="path"> + <ref name="absFilePath"/> + </attribute> + <optional> + <ref name="reconnect"/> + </optional> + <empty/> + </element> + </define> + <define name="diskTarget"> <data type="string"> <param name="pattern">(ioemu:)?(fd|hd|sd|vd|xvd|ubd)[a-zA-Z0-9_]+</param> -- 2.29.2
On Tue, Feb 02, 2021 at 16:04:09 +0100, Pavel Hrdina wrote:
<disk type='vhostuser' device='disk'> <driver name='qemu' type='raw'/> <source type='unix' path='/tmp/vhost-blk.sock' mode='client'> <reconnect enabled='yes' timeout='10'/> </source> <target dev='vda' bus='virtio'/> </disk>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.rst | 32 ++++++++++++++++++++++++++++++-- docs/schemas/domaincommon.rng | 19 +++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-)
[...]
@@ -2581,6 +2588,18 @@ paravirtualized driver is specified via the ``disk`` element. is not involved (compared to passing say ``/dev/nvme0n1`` via ``<disk type='block'>`` and therefore lower latencies can be achieved.
+ ``vhostuser`` + Enables the hypervisor to connect to another process using vhost-user + protocol. Requires shared memory configured for the VM, for more details + see ``access`` mode for `memoryBacking <#elementsMemoryBacking>` element.
Here we are missing a lot of the caveats of a vhostuser backed disk. A blanked disclaimer such as: "Note that the vhost server replaces both the disk frontend and backend thus almost all of the disk properties can't be configured via the <disk> XML for this disk type. Additionally features such as blockjobs, incremental backups and snapshots are not supported for this disk type." Should do it.
On Tue, Feb 02, 2021 at 16:04:09 +0100, Pavel Hrdina wrote:
<disk type='vhostuser' device='disk'> <driver name='qemu' type='raw'/> <source type='unix' path='/tmp/vhost-blk.sock' mode='client'> <reconnect enabled='yes' timeout='10'/> </source> <target dev='vda' bus='virtio'/> </disk>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.rst | 32 ++++++++++++++++++++++++++++++-- docs/schemas/domaincommon.rng | 19 +++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-)
Sorry for two replies, I've trimmed out too much in my first reply and didn't want to bother to undo it.
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index a09868bed5..99f5cad571 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -2419,6 +2419,13 @@ paravirtualized driver is specified via the ``disk`` element. </source> <target dev='vde' bus='virtio'/> </disk> + <disk type='vhostuser' device='disk'> + <driver name='qemu' type='raw'/> + <source type='unix' path='/tmp/vhost-blk.sock' mode='client'>
mode='client' is an example here ...
+ <reconnect enabled='yes' timeout='10'/> + </source> + <target dev='vdf' bus='virtio'/> + </disk> </devices> ...
[...]
@@ -2581,6 +2588,18 @@ paravirtualized driver is specified via the ``disk`` element. is not involved (compared to passing say ``/dev/nvme0n1`` via ``<disk type='block'>`` and therefore lower latencies can be achieved.
+ ``vhostuser`` + Enables the hypervisor to connect to another process using vhost-user + protocol. Requires shared memory configured for the VM, for more details + see ``access`` mode for `memoryBacking <#elementsMemoryBacking>` element. + + The ``source`` element has following mandatory attributes: + + ``type`` + The type of char device. Currently only ``unix`` type is supported. + ``path`` + Path to the unix socket to be used as disk source.
... but isn't mentioned here, ...
With "file", "block", and "volume", one or more optional sub-elements ``seclabel``, `described below <#seclabel>`__ (and :since:`since 0.9.9` ), can be used to override the domain security labeling policy for just that
[...]
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 7bab818bc9..98d5cee8c0 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng
[...]
@@ -2155,6 +2156,24 @@ </optional> </define>
+ <define name="diskSourceVhostUser"> + <attribute name="type"> + <value>vhostuser</value> + </attribute> + <element name="source"> + <attribute name="type"> + <value>unix</value> + </attribute> + <attribute name="path"> + <ref name="absFilePath"/> + </attribute>
... nor here.
+ <optional> + <ref name="reconnect"/> + </optional> + <empty/> + </element> + </define> + <define name="diskTarget"> <data type="string"> <param name="pattern">(ioemu:)?(fd|hd|sd|vd|xvd|ubd)[a-zA-Z0-9_]+</param> -- 2.29.2
On Wed, Feb 03, 2021 at 10:21:37AM +0100, Peter Krempa wrote:
On Tue, Feb 02, 2021 at 16:04:09 +0100, Pavel Hrdina wrote:
<disk type='vhostuser' device='disk'> <driver name='qemu' type='raw'/> <source type='unix' path='/tmp/vhost-blk.sock' mode='client'> <reconnect enabled='yes' timeout='10'/> </source> <target dev='vda' bus='virtio'/> </disk>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.rst | 32 ++++++++++++++++++++++++++++++-- docs/schemas/domaincommon.rng | 19 +++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-)
Sorry for two replies, I've trimmed out too much in my first reply and didn't want to bother to undo it.
NP, I'll add the comment to documentation from the first reply, sounds good.
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index a09868bed5..99f5cad571 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -2419,6 +2419,13 @@ paravirtualized driver is specified via the ``disk`` element. </source> <target dev='vde' bus='virtio'/> </disk> + <disk type='vhostuser' device='disk'> + <driver name='qemu' type='raw'/> + <source type='unix' path='/tmp/vhost-blk.sock' mode='client'>
mode='client' is an example here ...
Nice catch, I'll remove it as this was part of my first approach. Pavel
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- We need to find a better way to validate different combinations of XML elements and attributes. src/conf/domain_conf.c | 85 ++++++++ src/conf/domain_validate.c | 187 ++++++++++++++++++ src/conf/storage_source_conf.c | 3 + src/conf/storage_source_conf.h | 4 + src/libxl/xen_xl.c | 1 + src/qemu/qemu_block.c | 4 + src/qemu/qemu_command.c | 1 + src/qemu/qemu_migration.c | 2 + src/qemu/qemu_snapshot.c | 4 + src/storage_file/storage_source.c | 1 + tests/qemuxml2argvdata/disk-vhostuser.xml | 29 +++ .../disk-vhostuser.x86_64-latest.xml | 48 +++++ tests/qemuxml2xmltest.c | 1 + 13 files changed, 370 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-vhostuser.xml create mode 100644 tests/qemuxml2xmloutdata/disk-vhostuser.x86_64-latest.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 97fa841bff..43552c36c3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5228,6 +5228,11 @@ virDomainDiskDefPostParse(virDomainDiskDefPtr disk, disk->src->nvme->managed = VIR_TRISTATE_BOOL_YES; } + if (disk->src->type == VIR_STORAGE_TYPE_VHOST_USER && + disk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT) { + disk->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE; + } + if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && virDomainDiskDefAssignAddress(xmlopt, disk, def) < 0) { return -1; @@ -8361,6 +8366,55 @@ virDomainDiskSourceNVMeParse(xmlNodePtr node, } +static int +virDomainChrSourceReconnectDefParseXML(virDomainChrSourceReconnectDefPtr def, + xmlNodePtr node, + xmlXPathContextPtr ctxt); + + +static int +virDomainDiskSourceVHostUserParse(xmlNodePtr node, + virStorageSourcePtr src, + virDomainXMLOptionPtr xmlopt, + xmlXPathContextPtr ctxt) +{ + g_autofree char *type = virXMLPropString(node, "type"); + g_autofree char *path = virXMLPropString(node, "path"); + + if (!type) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing 'type' attribute for vhostuser disk source")); + return -1; + } + + if (STRNEQ(type, "unix")) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid 'type' attribute for vhostuser disk source")); + return -1; + } + + if (!path) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing 'path' attribute for vhostuser disk source")); + return -1; + } + + if (!(src->vhostuser = virDomainChrSourceDefNew(xmlopt))) + return -1; + + src->vhostuser->type = virDomainChrTypeFromString(type); + src->vhostuser->data.nix.path = g_steal_pointer(&path); + + if (virDomainChrSourceReconnectDefParseXML(&src->vhostuser->data.nix.reconnect, + node, + ctxt) < 0) { + return -1; + } + + return 0; +} + + static int virDomainDiskSourcePRParse(xmlNodePtr node, xmlXPathContextPtr ctxt, @@ -8512,6 +8566,10 @@ virDomainStorageSourceParse(xmlNodePtr node, if (virDomainDiskSourceNVMeParse(node, ctxt, src) < 0) return -1; break; + case VIR_STORAGE_TYPE_VHOST_USER: + if (virDomainDiskSourceVHostUserParse(node, src, xmlopt, ctxt) < 0) + return -1; + break; case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -23939,6 +23997,23 @@ virDomainDiskSourceNVMeFormat(virBufferPtr attrBuf, } +static void +virDomainChrSourceReconnectDefFormat(virBufferPtr buf, + virDomainChrSourceReconnectDefPtr def); + + +static void +virDomainDiskSourceVhostuserFormat(virBufferPtr attrBuf, + virBufferPtr childBuf, + virDomainChrSourceDefPtr vhostuser) +{ + virBufferAddLit(attrBuf, " type='unix'"); + virBufferAsprintf(attrBuf, " path='%s'", vhostuser->data.nix.path); + + virDomainChrSourceReconnectDefFormat(childBuf, &vhostuser->data.nix.reconnect); +} + + static int virDomainDiskSourceFormatPrivateData(virBufferPtr buf, virStorageSourcePtr src, @@ -23989,6 +24064,12 @@ virDomainDiskSourceFormatSlices(virBufferPtr buf, } +static void +virDomainChrSourceDefFormat(virBufferPtr buf, + virDomainChrSourceDefPtr def, + unsigned int flags); + + /** * virDomainDiskSourceFormat: * @buf: output buffer @@ -24052,6 +24133,10 @@ virDomainDiskSourceFormat(virBufferPtr buf, virDomainDiskSourceNVMeFormat(&attrBuf, &childBuf, src->nvme); break; + case VIR_STORAGE_TYPE_VHOST_USER: + virDomainDiskSourceVhostuserFormat(&attrBuf, &childBuf, src->vhostuser); + break; + case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index c56b03ff3a..c60300e750 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -22,6 +22,7 @@ #include "domain_validate.h" #include "domain_conf.h" +#include "snapshot_conf.h" #include "virconftypes.h" #include "virlog.h" #include "virutil.h" @@ -254,6 +255,187 @@ virDomainCheckVirtioOptionsAreAbsent(virDomainVirtioOptionsPtr virtio) } +static int +virDomainDiskVhostUserValidate(const virDomainDiskDef *disk) +{ + if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("vhostuser disk supports only virtio bus")); + return -1; + } + + if (disk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_NONE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only snapshot=no is supported with vhostuser disk")); + return -1; + } + + /* Unsupported drive attributes */ + + if (disk->cachemode != VIR_DOMAIN_DISK_CACHE_DEFAULT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("cache is not supported with vhostuser disk")); + return -1; + } + + if (disk->error_policy || disk->rerror_policy) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("error_policy is not supported with vhostuser disk")); + return -1; + } + + if (disk->iomode) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("io is not supported with vhostuser disk")); + return -1; + } + + if (disk->ioeventfd != VIR_TRISTATE_SWITCH_ABSENT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("ioeventfd is not supported with vhostuser disk")); + return -1; + } + + if (disk->copy_on_read) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("copy_on_read is not supported with vhostuser disk")); + return -1; + } + + if (disk->discard) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("discard is not supported with vhostuser disk")); + return -1; + } + + if (disk->iothread) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("iothread is not supported with vhostuser disk")); + return -1; + } + + if (disk->detect_zeroes) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("detect_zeroes is not supported with vhostuser disk")); + return -1; + } + + if (disk->src->metadataCacheMaxSize > 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("metadata_cache is not supported with vhostuser disk")); + return -1; + } + + /* Unsupported driver elements */ + + if (disk->virtio) { + if (disk->virtio->iommu != VIR_TRISTATE_SWITCH_ABSENT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("iommu is not supported with vhostuser disk")); + return -1; + } + + if (disk->virtio->ats != VIR_TRISTATE_SWITCH_ABSENT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("ats is not supported with vhostuser disk")); + return -1; + } + + if (disk->virtio->packed != VIR_TRISTATE_SWITCH_ABSENT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("packed is not supported with vhostuser disk")); + return -1; + } + } + + /* Unsupported disk elements */ + + if (virDomainBlockIoTuneInfoHasAny(&disk->blkdeviotune)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("iotune is not supported with vhostuser disk")); + return -1; + } + + if (disk->src->backingStore) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("backingStore is not supported with vhostuser disk")); + return -1; + } + + if (disk->src->encryption) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("encryption is not supported with vhostuser disk")); + return -1; + } + + if (disk->src->readonly) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("readonly is not supported with vhostuser disk")); + return -1; + } + + if (disk->src->shared) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("shareable is not supported with vhostuser disk")); + return -1; + } + + if (disk->transient) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("transient is not supported with vhostuser disk")); + return -1; + } + + if (disk->serial) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("serial is not supported with vhostuser disk")); + return -1; + } + + if (disk->wwn) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("wwn is not supported with vhostuser disk")); + return -1; + } + + if (disk->vendor) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("vendor is not supported with vhostuser disk")); + return -1; + } + + if (disk->product) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("product is not supported with vhostuser disk")); + return -1; + } + + if (disk->src->auth) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("auth is not supported with vhostuser disk")); + return -1; + } + + if (disk->geometry.cylinders > 0 || + disk->geometry.heads > 0 || + disk->geometry.sectors > 0 || + disk->geometry.trans != VIR_DOMAIN_DISK_TRANS_DEFAULT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("geometry is not supported with vhostuser disk")); + return -1; + } + + if (disk->blockio.logical_block_size > 0 || + disk->blockio.physical_block_size > 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("blockio is not supported with vhostuser disk")); + return -1; + } + + return 0; +} + + #define VENDOR_LEN 8 #define PRODUCT_LEN 16 @@ -329,6 +511,11 @@ virDomainDiskDefValidate(const virDomainDef *def, } } + if (disk->src->type == VIR_STORAGE_TYPE_VHOST_USER && + virDomainDiskVhostUserValidate(disk) < 0) { + return -1; + } + for (next = disk->src; next; next = next->backingStore) { if (virSecurityDeviceLabelDefValidate(next->seclabels, next->nseclabels, diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c index 3eaf05fe52..7706bbd8da 100644 --- a/src/conf/storage_source_conf.c +++ b/src/conf/storage_source_conf.c @@ -46,6 +46,7 @@ VIR_ENUM_IMPL(virStorage, "network", "volume", "nvme", + "vhostuser" ); @@ -1035,6 +1036,7 @@ virStorageSourceIsLocalStorage(const virStorageSource *src) /* While NVMe disks are local, they are not accessible via src->path. * Therefore, we have to return false here. */ case VIR_STORAGE_TYPE_NVME: + case VIR_STORAGE_TYPE_VHOST_USER: case VIR_STORAGE_TYPE_LAST: case VIR_STORAGE_TYPE_NONE: return false; @@ -1215,6 +1217,7 @@ virStorageSourceIsRelative(virStorageSourcePtr src) case VIR_STORAGE_TYPE_NETWORK: case VIR_STORAGE_TYPE_VOLUME: case VIR_STORAGE_TYPE_NVME: + case VIR_STORAGE_TYPE_VHOST_USER: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: return false; diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h index e66ccdedef..f42bb1c67d 100644 --- a/src/conf/storage_source_conf.h +++ b/src/conf/storage_source_conf.h @@ -23,6 +23,7 @@ #include "storage_encryption_conf.h" #include "virbitmap.h" +#include "virconftypes.h" #include "virenum.h" #include "virobject.h" #include "virpci.h" @@ -41,6 +42,7 @@ typedef enum { VIR_STORAGE_TYPE_NETWORK, VIR_STORAGE_TYPE_VOLUME, VIR_STORAGE_TYPE_NVME, + VIR_STORAGE_TYPE_VHOST_USER, VIR_STORAGE_TYPE_LAST } virStorageType; @@ -300,6 +302,8 @@ struct _virStorageSource { virStorageSourceNVMeDefPtr nvme; /* type == VIR_STORAGE_TYPE_NVME */ + virDomainChrSourceDefPtr vhostuser; /* type == VIR_STORAGE_TYPE_VHOST_USER */ + virStorageSourceInitiatorDef initiator; virObjectPtr privateData; diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c index 6dcba43fe0..941832ce4e 100644 --- a/src/libxl/xen_xl.c +++ b/src/libxl/xen_xl.c @@ -1668,6 +1668,7 @@ xenFormatXLDiskSrc(virStorageSourcePtr src, char **srcstr) case VIR_STORAGE_TYPE_VOLUME: case VIR_STORAGE_TYPE_NVME: + case VIR_STORAGE_TYPE_VHOST_USER: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: break; diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 6456100170..f6e81a7503 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1173,6 +1173,9 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src, return NULL; break; + case VIR_STORAGE_TYPE_VHOST_USER: + break; + case VIR_STORAGE_TYPE_VOLUME: virReportError(VIR_ERR_INTERNAL_ERROR, _("storage source pool '%s' volume '%s' is not translated"), @@ -2599,6 +2602,7 @@ qemuBlockStorageSourceCreateGetStorageProps(virStorageSourcePtr src, case VIR_STORAGE_TYPE_DIR: case VIR_STORAGE_TYPE_VOLUME: case VIR_STORAGE_TYPE_NVME: + case VIR_STORAGE_TYPE_VHOST_USER: return 0; case VIR_STORAGE_TYPE_NONE: diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5101e5d74c..3e652e18b7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1085,6 +1085,7 @@ qemuGetDriveSourceString(virStorageSourcePtr src, case VIR_STORAGE_TYPE_VOLUME: case VIR_STORAGE_TYPE_NVME: + case VIR_STORAGE_TYPE_VHOST_USER: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: break; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 0adfdb9351..f44d31c971 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -238,6 +238,7 @@ qemuMigrationDstPrecreateDisk(virConnectPtr *conn, case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_DIR: case VIR_STORAGE_TYPE_NVME: + case VIR_STORAGE_TYPE_VHOST_USER: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1479,6 +1480,7 @@ qemuMigrationSrcIsSafe(virDomainDefPtr def, unsafe = true; break; + case VIR_STORAGE_TYPE_VHOST_USER: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_DIR: diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 39445738a2..115c2fc91b 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -428,6 +428,7 @@ qemuSnapshotPrepareDiskExternalInactive(virDomainSnapshotDiskDefPtr snapdisk, case VIR_STORAGE_TYPE_DIR: case VIR_STORAGE_TYPE_VOLUME: case VIR_STORAGE_TYPE_NVME: + case VIR_STORAGE_TYPE_VHOST_USER: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -445,6 +446,7 @@ qemuSnapshotPrepareDiskExternalInactive(virDomainSnapshotDiskDefPtr snapdisk, case VIR_STORAGE_TYPE_DIR: case VIR_STORAGE_TYPE_VOLUME: case VIR_STORAGE_TYPE_NVME: + case VIR_STORAGE_TYPE_VHOST_USER: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -518,6 +520,7 @@ qemuSnapshotPrepareDiskExternalActive(virDomainObjPtr vm, case VIR_STORAGE_TYPE_DIR: case VIR_STORAGE_TYPE_VOLUME: case VIR_STORAGE_TYPE_NVME: + case VIR_STORAGE_TYPE_VHOST_USER: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -648,6 +651,7 @@ qemuSnapshotPrepareDiskInternal(virDomainDiskDefPtr disk, case VIR_STORAGE_TYPE_DIR: case VIR_STORAGE_TYPE_VOLUME: case VIR_STORAGE_TYPE_NVME: + case VIR_STORAGE_TYPE_VHOST_USER: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/storage_file/storage_source.c b/src/storage_file/storage_source.c index 4b46cc4e84..ffe150a9b0 100644 --- a/src/storage_file/storage_source.c +++ b/src/storage_file/storage_source.c @@ -545,6 +545,7 @@ virStorageSourceUpdatePhysicalSize(virStorageSourcePtr src, /* We shouldn't get VOLUME, but the switch requires all cases */ case VIR_STORAGE_TYPE_VOLUME: case VIR_STORAGE_TYPE_NVME: + case VIR_STORAGE_TYPE_VHOST_USER: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: return -1; diff --git a/tests/qemuxml2argvdata/disk-vhostuser.xml b/tests/qemuxml2argvdata/disk-vhostuser.xml new file mode 100644 index 0000000000..48883ff2db --- /dev/null +++ b/tests/qemuxml2argvdata/disk-vhostuser.xml @@ -0,0 +1,29 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <memoryBacking> + <source type='memfd'/> + <access mode='shared'/> + </memoryBacking> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='vhostuser' device='disk'> + <driver name='qemu' type='raw'/> + <source type='unix' path='/tmp/vhost1.sock'/> + <target dev='vda' bus='virtio'/> + </disk> + <disk type='vhostuser' device='disk'> + <driver name='qemu' type='raw'/> + <source type='unix' path='/tmp/vhost1.sock'> + <reconnect enabled='yes' timeout='10'/> + </source> + <target dev='vdb' bus='virtio'/> + </disk> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/disk-vhostuser.x86_64-latest.xml b/tests/qemuxml2xmloutdata/disk-vhostuser.x86_64-latest.xml new file mode 100644 index 0000000000..774876916d --- /dev/null +++ b/tests/qemuxml2xmloutdata/disk-vhostuser.x86_64-latest.xml @@ -0,0 +1,48 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <memoryBacking> + <source type='memfd'/> + <access mode='shared'/> + </memoryBacking> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <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='vhostuser' device='disk' snapshot='no'> + <driver name='qemu' type='raw'/> + <source type='unix' path='/tmp/vhost1.sock'/> + <target dev='vda' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </disk> + <disk type='vhostuser' device='disk' snapshot='no'> + <driver name='qemu' type='raw'/> + <source type='unix' path='/tmp/vhost1.sock'> + <reconnect enabled='yes' timeout='10'/> + </source> + <target dev='vdb' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </disk> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 50dd970789..d9aec27565 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -307,6 +307,7 @@ mymain(void) DO_TEST("disk-network-tlsx509-nbd", NONE); DO_TEST("disk-network-tlsx509-vxhs", NONE); DO_TEST("disk-nvme", QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_QCOW2_LUKS); + DO_TEST_CAPS_LATEST("disk-vhostuser"); DO_TEST_CAPS_LATEST("disk-scsi"); DO_TEST("disk-virtio-scsi-reservations", QEMU_CAPS_VIRTIO_SCSI, -- 2.29.2
On Tue, Feb 02, 2021 at 16:04:10 +0100, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> ---
We need to find a better way to validate different combinations of XML elements and attributes.
src/conf/domain_conf.c | 85 ++++++++ src/conf/domain_validate.c | 187 ++++++++++++++++++ src/conf/storage_source_conf.c | 3 + src/conf/storage_source_conf.h | 4 + src/libxl/xen_xl.c | 1 + src/qemu/qemu_block.c | 4 + src/qemu/qemu_command.c | 1 + src/qemu/qemu_migration.c | 2 + src/qemu/qemu_snapshot.c | 4 + src/storage_file/storage_source.c | 1 + tests/qemuxml2argvdata/disk-vhostuser.xml | 29 +++ .../disk-vhostuser.x86_64-latest.xml | 48 +++++ tests/qemuxml2xmltest.c | 1 + 13 files changed, 370 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-vhostuser.xml create mode 100644 tests/qemuxml2xmloutdata/disk-vhostuser.x86_64-latest.xml
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 97fa841bff..43552c36c3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5228,6 +5228,11 @@ virDomainDiskDefPostParse(virDomainDiskDefPtr disk, disk->src->nvme->managed = VIR_TRISTATE_BOOL_YES; }
/* vhost-user doesn't allow us to snapshot, disable snapshots by default */
+ if (disk->src->type == VIR_STORAGE_TYPE_VHOST_USER && + disk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT) { + disk->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE; + } + if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && virDomainDiskDefAssignAddress(xmlopt, disk, def) < 0) { return -1; @@ -8361,6 +8366,55 @@ virDomainDiskSourceNVMeParse(xmlNodePtr node, }
+static int +virDomainChrSourceReconnectDefParseXML(virDomainChrSourceReconnectDefPtr def, + xmlNodePtr node, + xmlXPathContextPtr ctxt);
[... see below note on forward decls.]
+ + +static int +virDomainDiskSourceVHostUserParse(xmlNodePtr node, + virStorageSourcePtr src, + virDomainXMLOptionPtr xmlopt, + xmlXPathContextPtr ctxt) +{ + g_autofree char *type = virXMLPropString(node, "type"); + g_autofree char *path = virXMLPropString(node, "path"); + + if (!type) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing 'type' attribute for vhostuser disk source")); + return -1; + } + + if (STRNEQ(type, "unix")) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid 'type' attribute for vhostuser disk source")); + return -1; + } + + if (!path) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing 'path' attribute for vhostuser disk source")); + return -1; + } + + if (!(src->vhostuser = virDomainChrSourceDefNew(xmlopt))) + return -1; + + src->vhostuser->type = virDomainChrTypeFromString(type); + src->vhostuser->data.nix.path = g_steal_pointer(&path);
'path' doesn't really need a temp variable.
+ + if (virDomainChrSourceReconnectDefParseXML(&src->vhostuser->data.nix.reconnect, + node, + ctxt) < 0) { + return -1; + } + + return 0; +}
[...]
@@ -23939,6 +23997,23 @@ virDomainDiskSourceNVMeFormat(virBufferPtr attrBuf, }
+static void +virDomainChrSourceReconnectDefFormat(virBufferPtr buf, + virDomainChrSourceReconnectDefPtr def); + +
[...]
@@ -23989,6 +24064,12 @@ virDomainDiskSourceFormatSlices(virBufferPtr buf, }
+static void +virDomainChrSourceDefFormat(virBufferPtr buf, + virDomainChrSourceDefPtr def, + unsigned int flags); + +
Preferably put the forward declarations at the top of the file. [...]
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index c56b03ff3a..c60300e750 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c
[...]
@@ -254,6 +255,187 @@ virDomainCheckVirtioOptionsAreAbsent(virDomainVirtioOptionsPtr virtio) }
+static int +virDomainDiskVhostUserValidate(const virDomainDiskDef *disk) +{ + if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("vhostuser disk supports only virtio bus")); + return -1; + } + + if (disk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_NONE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only snapshot=no is supported with vhostuser disk")); + return -1; + } + + /* Unsupported drive attributes */
s/drive/dirver/
+ + if (disk->cachemode != VIR_DOMAIN_DISK_CACHE_DEFAULT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("cache is not supported with vhostuser disk")); + return -1; + } + + if (disk->error_policy || disk->rerror_policy) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("error_policy is not supported with vhostuser disk")); + return -1; + } + + if (disk->iomode) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("io is not supported with vhostuser disk")); + return -1; + } + + if (disk->ioeventfd != VIR_TRISTATE_SWITCH_ABSENT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("ioeventfd is not supported with vhostuser disk")); + return -1; + } + + if (disk->copy_on_read) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("copy_on_read is not supported with vhostuser disk")); + return -1; + } + + if (disk->discard) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("discard is not supported with vhostuser disk")); + return -1; + } + + if (disk->iothread) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("iothread is not supported with vhostuser disk")); + return -1; + } + + if (disk->detect_zeroes) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("detect_zeroes is not supported with vhostuser disk")); + return -1; + } + + if (disk->src->metadataCacheMaxSize > 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("metadata_cache is not supported with vhostuser disk")); + return -1; + } + + /* Unsupported driver elements */
s/driver/virtio/ ?
+ + if (disk->virtio) { + if (disk->virtio->iommu != VIR_TRISTATE_SWITCH_ABSENT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("iommu is not supported with vhostuser disk")); + return -1; + } + + if (disk->virtio->ats != VIR_TRISTATE_SWITCH_ABSENT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("ats is not supported with vhostuser disk")); + return -1; + } + + if (disk->virtio->packed != VIR_TRISTATE_SWITCH_ABSENT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("packed is not supported with vhostuser disk")); + return -1; + } + }
[...]
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 6456100170..f6e81a7503 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1173,6 +1173,9 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src, return NULL; break;
+ case VIR_STORAGE_TYPE_VHOST_USER: + break;
This case must return NULL and an error per API contract of the function.
+ case VIR_STORAGE_TYPE_VOLUME: virReportError(VIR_ERR_INTERNAL_ERROR, _("storage source pool '%s' volume '%s' is not translated"),
On Wed, Feb 03, 2021 at 10:27:01AM +0100, Peter Krempa wrote:
On Tue, Feb 02, 2021 at 16:04:10 +0100, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> ---
We need to find a better way to validate different combinations of XML elements and attributes.
src/conf/domain_conf.c | 85 ++++++++ src/conf/domain_validate.c | 187 ++++++++++++++++++ src/conf/storage_source_conf.c | 3 + src/conf/storage_source_conf.h | 4 + src/libxl/xen_xl.c | 1 + src/qemu/qemu_block.c | 4 + src/qemu/qemu_command.c | 1 + src/qemu/qemu_migration.c | 2 + src/qemu/qemu_snapshot.c | 4 + src/storage_file/storage_source.c | 1 + tests/qemuxml2argvdata/disk-vhostuser.xml | 29 +++ .../disk-vhostuser.x86_64-latest.xml | 48 +++++ tests/qemuxml2xmltest.c | 1 + 13 files changed, 370 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-vhostuser.xml create mode 100644 tests/qemuxml2xmloutdata/disk-vhostuser.x86_64-latest.xml
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 97fa841bff..43552c36c3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5228,6 +5228,11 @@ virDomainDiskDefPostParse(virDomainDiskDefPtr disk, disk->src->nvme->managed = VIR_TRISTATE_BOOL_YES; }
/* vhost-user doesn't allow us to snapshot, disable snapshots by default */
+ if (disk->src->type == VIR_STORAGE_TYPE_VHOST_USER && + disk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT) { + disk->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE; + } + if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && virDomainDiskDefAssignAddress(xmlopt, disk, def) < 0) { return -1; @@ -8361,6 +8366,55 @@ virDomainDiskSourceNVMeParse(xmlNodePtr node, }
+static int +virDomainChrSourceReconnectDefParseXML(virDomainChrSourceReconnectDefPtr def, + xmlNodePtr node, + xmlXPathContextPtr ctxt);
[... see below note on forward decls.]
+ + +static int +virDomainDiskSourceVHostUserParse(xmlNodePtr node, + virStorageSourcePtr src, + virDomainXMLOptionPtr xmlopt, + xmlXPathContextPtr ctxt) +{ + g_autofree char *type = virXMLPropString(node, "type"); + g_autofree char *path = virXMLPropString(node, "path"); + + if (!type) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing 'type' attribute for vhostuser disk source")); + return -1; + } + + if (STRNEQ(type, "unix")) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid 'type' attribute for vhostuser disk source")); + return -1; + } + + if (!path) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing 'path' attribute for vhostuser disk source")); + return -1; + } + + if (!(src->vhostuser = virDomainChrSourceDefNew(xmlopt))) + return -1; + + src->vhostuser->type = virDomainChrTypeFromString(type); + src->vhostuser->data.nix.path = g_steal_pointer(&path);
'path' doesn't really need a temp variable.
True, but IMHO it makes the code more readable. Without the variable it would look like this: g_autofree char *type = virXMLPropString(node, "type"); if (!type) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing 'type' attribute for vhostuser disk source")); return -1; } if (STRNEQ(type, "unix")) { virReportError(VIR_ERR_XML_ERROR, "%s", _("invalid 'type' attribute for vhostuser disk source")); return -1; } if (!(src->vhostuser = virDomainChrSourceDefNew(xmlopt))) return -1; src->vhostuser->type = virDomainChrTypeFromString(type); src->vhostuser->data.nix.path = virXMLPropString(node, "path"); if (!src->vhostuser->data.nix.path) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing 'path' attribute for vhostuser disk source")); return -1; } if (virDomainChrSourceReconnectDefParseXML(&src->vhostuser->data.nix.reconnect, node, ctxt) < 0) { return -1; } return 0; which is mixing the checks and assignment together.
+ + if (virDomainChrSourceReconnectDefParseXML(&src->vhostuser->data.nix.reconnect, + node, + ctxt) < 0) { + return -1; + } + + return 0; +}
[...]
@@ -23939,6 +23997,23 @@ virDomainDiskSourceNVMeFormat(virBufferPtr attrBuf, }
+static void +virDomainChrSourceReconnectDefFormat(virBufferPtr buf, + virDomainChrSourceReconnectDefPtr def); + +
[...]
@@ -23989,6 +24064,12 @@ virDomainDiskSourceFormatSlices(virBufferPtr buf, }
+static void +virDomainChrSourceDefFormat(virBufferPtr buf, + virDomainChrSourceDefPtr def, + unsigned int flags); + +
Preferably put the forward declarations at the top of the file.
[...]
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index c56b03ff3a..c60300e750 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c
[...]
@@ -254,6 +255,187 @@ virDomainCheckVirtioOptionsAreAbsent(virDomainVirtioOptionsPtr virtio) }
+static int +virDomainDiskVhostUserValidate(const virDomainDiskDef *disk) +{ + if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("vhostuser disk supports only virtio bus")); + return -1; + } + + if (disk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_NONE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only snapshot=no is supported with vhostuser disk")); + return -1; + } + + /* Unsupported drive attributes */
s/drive/dirver/
Fixed, thanks.
+ + if (disk->cachemode != VIR_DOMAIN_DISK_CACHE_DEFAULT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("cache is not supported with vhostuser disk")); + return -1; + } + + if (disk->error_policy || disk->rerror_policy) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("error_policy is not supported with vhostuser disk")); + return -1; + } + + if (disk->iomode) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("io is not supported with vhostuser disk")); + return -1; + } + + if (disk->ioeventfd != VIR_TRISTATE_SWITCH_ABSENT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("ioeventfd is not supported with vhostuser disk")); + return -1; + } + + if (disk->copy_on_read) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("copy_on_read is not supported with vhostuser disk")); + return -1; + } + + if (disk->discard) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("discard is not supported with vhostuser disk")); + return -1; + } + + if (disk->iothread) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("iothread is not supported with vhostuser disk")); + return -1; + } + + if (disk->detect_zeroes) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("detect_zeroes is not supported with vhostuser disk")); + return -1; + } + + if (disk->src->metadataCacheMaxSize > 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("metadata_cache is not supported with vhostuser disk")); + return -1; + } + + /* Unsupported driver elements */
s/driver/virtio/ ?
This was future proofing the comment :) currently there is only single child element <virtio>. So I would be OK with both versions: /* Unsupported driver child elements */ /* Unsupported virtio element */
+ + if (disk->virtio) { + if (disk->virtio->iommu != VIR_TRISTATE_SWITCH_ABSENT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("iommu is not supported with vhostuser disk")); + return -1; + } + + if (disk->virtio->ats != VIR_TRISTATE_SWITCH_ABSENT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("ats is not supported with vhostuser disk")); + return -1; + } + + if (disk->virtio->packed != VIR_TRISTATE_SWITCH_ABSENT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("packed is not supported with vhostuser disk")); + return -1; + } + }
[...]
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 6456100170..f6e81a7503 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1173,6 +1173,9 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src, return NULL; break;
+ case VIR_STORAGE_TYPE_VHOST_USER: + break;
This case must return NULL and an error per API contract of the function.
Fixed. It should never happen but I agree that we should make sure to error out if it happens. Pavel
+ case VIR_STORAGE_TYPE_VOLUME: virReportError(VIR_ERR_INTERNAL_ERROR, _("storage source pool '%s' volume '%s' is not translated"),
On Wed, Feb 03, 2021 at 10:51:23 +0100, Pavel Hrdina wrote:
On Wed, Feb 03, 2021 at 10:27:01AM +0100, Peter Krempa wrote:
On Tue, Feb 02, 2021 at 16:04:10 +0100, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> ---
[...]
+ + src->vhostuser->type = virDomainChrTypeFromString(type); + src->vhostuser->data.nix.path = g_steal_pointer(&path);
'path' doesn't really need a temp variable.
True, but IMHO it makes the code more readable. Without the variable it would look like this:
g_autofree char *type = virXMLPropString(node, "type");
[...]
which is mixing the checks and assignment together.
Agreed, keep it as is.
+
[...]
+ /* Unsupported driver elements */
s/driver/virtio/ ?
This was future proofing the comment :) currently there is only single child element <virtio>. So I would be OK with both versions:
/* Unsupported driver child elements */
You'd have to move the image cache setting here, since that's also an subelement.
/* Unsupported virtio element */
This is okay
[...]
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 6456100170..f6e81a7503 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1173,6 +1173,9 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src, return NULL; break;
+ case VIR_STORAGE_TYPE_VHOST_USER: + break;
This case must return NULL and an error per API contract of the function.
Fixed. It should never happen but I agree that we should make sure to error out if it happens.
Adding the error also prevents potential crash in such case, since 'fileprops' would still be NULL, but code after that adding the common props expects it to be already allocated.
On Wed, Feb 03, 2021 at 11:08:41AM +0100, Peter Krempa wrote:
On Wed, Feb 03, 2021 at 10:51:23 +0100, Pavel Hrdina wrote:
On Wed, Feb 03, 2021 at 10:27:01AM +0100, Peter Krempa wrote:
On Tue, Feb 02, 2021 at 16:04:10 +0100, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> ---
[...]
+ + src->vhostuser->type = virDomainChrTypeFromString(type); + src->vhostuser->data.nix.path = g_steal_pointer(&path);
'path' doesn't really need a temp variable.
True, but IMHO it makes the code more readable. Without the variable it would look like this:
g_autofree char *type = virXMLPropString(node, "type");
[...]
which is mixing the checks and assignment together.
Agreed, keep it as is.
+
[...]
+ /* Unsupported driver elements */
s/driver/virtio/ ?
This was future proofing the comment :) currently there is only single child element <virtio>. So I would be OK with both versions:
/* Unsupported driver child elements */
You'd have to move the image cache setting here, since that's also an subelement.
Somehow I missed that the 'metadata_cache' is also an element. I'll move it and keep this comment.
/* Unsupported virtio element */
This is okay
[...]
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 6456100170..f6e81a7503 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1173,6 +1173,9 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src, return NULL; break;
+ case VIR_STORAGE_TYPE_VHOST_USER: + break;
This case must return NULL and an error per API contract of the function.
Fixed. It should never happen but I agree that we should make sure to error out if it happens.
Adding the error also prevents potential crash in such case, since 'fileprops' would still be NULL, but code after that adding the common props expects it to be already allocated.
Right, another reason. Fixed for v2. Thanks
On Tue, Feb 02, 2021 at 16:04:10 +0100, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> ---
We need to find a better way to validate different combinations of XML elements and attributes.
src/conf/domain_conf.c | 85 ++++++++ src/conf/domain_validate.c | 187 ++++++++++++++++++ src/conf/storage_source_conf.c | 3 + src/conf/storage_source_conf.h | 4 + src/libxl/xen_xl.c | 1 + src/qemu/qemu_block.c | 4 + src/qemu/qemu_command.c | 1 + src/qemu/qemu_migration.c | 2 + src/qemu/qemu_snapshot.c | 4 + src/storage_file/storage_source.c | 1 + tests/qemuxml2argvdata/disk-vhostuser.xml | 29 +++ .../disk-vhostuser.x86_64-latest.xml | 48 +++++ tests/qemuxml2xmltest.c | 1 + 13 files changed, 370 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-vhostuser.xml create mode 100644 tests/qemuxml2xmloutdata/disk-vhostuser.x86_64-latest.xml
[...]
diff --git a/tests/qemuxml2argvdata/disk-vhostuser.xml b/tests/qemuxml2argvdata/disk-vhostuser.xml new file mode 100644 index 0000000000..48883ff2db --- /dev/null +++ b/tests/qemuxml2argvdata/disk-vhostuser.xml @@ -0,0 +1,29 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <memoryBacking> + <source type='memfd'/> + <access mode='shared'/> + </memoryBacking> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='vhostuser' device='disk'> + <driver name='qemu' type='raw'/> + <source type='unix' path='/tmp/vhost1.sock'/> + <target dev='vda' bus='virtio'/> + </disk> + <disk type='vhostuser' device='disk'> + <driver name='qemu' type='raw'/> + <source type='unix' path='/tmp/vhost1.sock'> + <reconnect enabled='yes' timeout='10'/> + </source> + <target dev='vdb' bus='virtio'/>
Please add boot order to one of the disks at least.
+ </disk> + </devices> +</domain>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_capabilities.c | 4 ++++ src/qemu/qemu_capabilities.h | 3 +++ tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_3.0.0.riscv32.xml | 1 + tests/qemucapabilitiesdata/caps_3.0.0.riscv64.xml | 1 + tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml | 1 + tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml | 1 + tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 1 + 35 files changed, 40 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 4a1bf9b6fe..aa54647a8c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -611,6 +611,9 @@ VIR_ENUM_IMPL(virQEMUCaps, "am53c974", "virtio-pmem-pci", "vhost-user-fs.bootindex", + + /* 390 */ + "vhost-user-blk", ); @@ -1328,6 +1331,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "dc390", QEMU_CAPS_SCSI_DC390 }, { "am53c974", QEMU_CAPS_SCSI_AM53C974 }, { "virtio-pmem-pci", QEMU_CAPS_DEVICE_VIRTIO_PMEM_PCI }, + { "vhost-user-blk", QEMU_CAPS_DEVICE_VHOST_USER_BLK }, }; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index cd95103652..81a1d926a8 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -592,6 +592,9 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_DEVICE_VIRTIO_PMEM_PCI, /* -device virtio-pmem-pci */ QEMU_CAPS_VHOST_USER_FS_BOOTINDEX, /* vhost-user-fs.bootindex */ + /* 390 */ + QEMU_CAPS_DEVICE_VHOST_USER_BLK, /* -device vhost-user-blk */ + QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml index a4574f70f6..3ff1bf3ff8 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml @@ -166,6 +166,7 @@ <flag name='fsdev.createmode'/> <flag name='dc390'/> <flag name='am53c974'/> + <flag name='vhost-user-blk'/> <version>2012000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700289</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml index ab5ab06084..9311bf66db 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml @@ -167,6 +167,7 @@ <flag name='fsdev.createmode'/> <flag name='dc390'/> <flag name='am53c974'/> + <flag name='vhost-user-blk'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900289</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml index 588ccc58e4..63c7e38f40 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml @@ -133,6 +133,7 @@ <flag name='migration-param.downtime'/> <flag name='migration-param.xbzrle-cache-size'/> <flag name='fsdev.createmode'/> + <flag name='vhost-user-blk'/> <version>2012000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100289</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml index 13b61fcfe5..df0ce08da6 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml @@ -210,6 +210,7 @@ <flag name='fsdev.createmode'/> <flag name='dc390'/> <flag name='am53c974'/> + <flag name='vhost-user-blk'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100289</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml index c92bb5f6a3..dc0e8d637e 100644 --- a/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml @@ -169,6 +169,7 @@ <flag name='fsdev.createmode'/> <flag name='dc390'/> <flag name='am53c974'/> + <flag name='vhost-user-blk'/> <version>2012050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900239</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.riscv32.xml b/tests/qemucapabilitiesdata/caps_3.0.0.riscv32.xml index d68c785583..cbc185fc3a 100644 --- a/tests/qemucapabilitiesdata/caps_3.0.0.riscv32.xml +++ b/tests/qemucapabilitiesdata/caps_3.0.0.riscv32.xml @@ -103,6 +103,7 @@ <flag name='migration-param.downtime'/> <flag name='migration-param.xbzrle-cache-size'/> <flag name='fsdev.createmode'/> + <flag name='vhost-user-blk'/> <version>3000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.riscv64.xml b/tests/qemucapabilitiesdata/caps_3.0.0.riscv64.xml index 3dd3ec87e5..aa803ccbd6 100644 --- a/tests/qemucapabilitiesdata/caps_3.0.0.riscv64.xml +++ b/tests/qemucapabilitiesdata/caps_3.0.0.riscv64.xml @@ -103,6 +103,7 @@ <flag name='migration-param.downtime'/> <flag name='migration-param.xbzrle-cache-size'/> <flag name='fsdev.createmode'/> + <flag name='vhost-user-blk'/> <version>3000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml b/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml index 649104ccbd..cdb767bbf7 100644 --- a/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml @@ -136,6 +136,7 @@ <flag name='migration-param.downtime'/> <flag name='migration-param.xbzrle-cache-size'/> <flag name='fsdev.createmode'/> + <flag name='vhost-user-blk'/> <version>3000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100239</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml index d7f1d6cd84..2ea912eaad 100644 --- a/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml @@ -216,6 +216,7 @@ <flag name='fsdev.createmode'/> <flag name='dc390'/> <flag name='am53c974'/> + <flag name='vhost-user-blk'/> <version>3000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100239</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml index f4b4566ea8..b05f16983c 100644 --- a/tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml @@ -174,6 +174,7 @@ <flag name='fsdev.createmode'/> <flag name='dc390'/> <flag name='am53c974'/> + <flag name='vhost-user-blk'/> <version>3000091</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900240</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml index b8391f1353..8a892a5da3 100644 --- a/tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml @@ -219,6 +219,7 @@ <flag name='fsdev.createmode'/> <flag name='dc390'/> <flag name='am53c974'/> + <flag name='vhost-user-blk'/> <version>3000092</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100240</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml index 4722557eaf..c28ada94fb 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml @@ -180,6 +180,7 @@ <flag name='fsdev.createmode'/> <flag name='dc390'/> <flag name='am53c974'/> + <flag name='vhost-user-blk'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700240</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml index 6f549902ca..a15edd87de 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml @@ -188,6 +188,7 @@ <flag name='fsdev.createmode'/> <flag name='dc390'/> <flag name='am53c974'/> + <flag name='vhost-user-blk'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900240</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml index b1dc08eb4d..de2b578b82 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml @@ -181,6 +181,7 @@ <flag name='fsdev.createmode'/> <flag name='dc390'/> <flag name='am53c974'/> + <flag name='vhost-user-blk'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml index babb8fb8ab..754ad6db53 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml @@ -181,6 +181,7 @@ <flag name='fsdev.createmode'/> <flag name='dc390'/> <flag name='am53c974'/> + <flag name='vhost-user-blk'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml index 5a15848f88..4a10deea01 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml @@ -144,6 +144,7 @@ <flag name='migration-param.downtime'/> <flag name='migration-param.xbzrle-cache-size'/> <flag name='fsdev.createmode'/> + <flag name='vhost-user-blk'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100240</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml index 43b70ccc94..c580d29374 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml @@ -225,6 +225,7 @@ <flag name='fsdev.createmode'/> <flag name='dc390'/> <flag name='am53c974'/> + <flag name='vhost-user-blk'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100240</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml index 28a4b0ede0..93b223aab2 100644 --- a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml @@ -232,6 +232,7 @@ <flag name='dc390'/> <flag name='am53c974'/> <flag name='virtio-pmem-pci'/> + <flag name='vhost-user-blk'/> <version>4001000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100241</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml index 77fdc73415..4c149e79bb 100644 --- a/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml @@ -193,6 +193,7 @@ <flag name='fsdev.createmode'/> <flag name='dc390'/> <flag name='am53c974'/> + <flag name='vhost-user-blk'/> <version>4001050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml index 15eaac77a6..20cb49a508 100644 --- a/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml @@ -194,6 +194,7 @@ <flag name='fsdev.createmode'/> <flag name='dc390'/> <flag name='am53c974'/> + <flag name='vhost-user-blk'/> <version>4001050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml b/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml index 42a7cca50a..41db85be6b 100644 --- a/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml @@ -155,6 +155,7 @@ <flag name='migration-param.xbzrle-cache-size'/> <flag name='blockdev-hostdev-scsi'/> <flag name='fsdev.createmode'/> + <flag name='vhost-user-blk'/> <version>4002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml index e150741f11..f663c5ca4c 100644 --- a/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml @@ -241,6 +241,7 @@ <flag name='dc390'/> <flag name='am53c974'/> <flag name='virtio-pmem-pci'/> + <flag name='vhost-user-blk'/> <version>4002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml index d584642bff..b9963bbd7e 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml @@ -203,6 +203,7 @@ <flag name='fsdev.createmode'/> <flag name='dc390'/> <flag name='am53c974'/> + <flag name='vhost-user-blk'/> <version>5000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700241</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml index 596bccd70a..46edacd44b 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml @@ -212,6 +212,7 @@ <flag name='fsdev.createmode'/> <flag name='dc390'/> <flag name='am53c974'/> + <flag name='vhost-user-blk'/> <version>5000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900241</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml index eb760f2911..496b75da20 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml @@ -199,6 +199,7 @@ <flag name='fsdev.createmode'/> <flag name='dc390'/> <flag name='am53c974'/> + <flag name='vhost-user-blk'/> <version>5000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml index 7c56d110f4..a6d2785e55 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml @@ -248,6 +248,7 @@ <flag name='dc390'/> <flag name='am53c974'/> <flag name='virtio-pmem-pci'/> + <flag name='vhost-user-blk'/> <version>5000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100241</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml index 07466093c9..136b5892d8 100644 --- a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml @@ -250,6 +250,7 @@ <flag name='dc390'/> <flag name='am53c974'/> <flag name='virtio-pmem-pci'/> + <flag name='vhost-user-blk'/> <version>5001000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml index cac9b40528..6b40141f15 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml @@ -207,6 +207,7 @@ <flag name='fsdev.createmode'/> <flag name='dc390'/> <flag name='am53c974'/> + <flag name='vhost-user-blk'/> <version>5002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml index e92201ad43..298139cdd7 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml @@ -214,6 +214,7 @@ <flag name='fsdev.createmode'/> <flag name='dc390'/> <flag name='am53c974'/> + <flag name='vhost-user-blk'/> <version>5002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml b/tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml index bee7f547c7..0457018c93 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml @@ -201,6 +201,7 @@ <flag name='fsdev.createmode'/> <flag name='dc390'/> <flag name='am53c974'/> + <flag name='vhost-user-blk'/> <version>5002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml b/tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml index 15e7ee84c6..b59404230c 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml @@ -162,6 +162,7 @@ <flag name='block-export-add'/> <flag name='netdev.vhost-vdpa'/> <flag name='fsdev.createmode'/> + <flag name='vhost-user-blk'/> <version>5002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml index dea2ff4b54..4d6f02aae2 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml @@ -251,6 +251,7 @@ <flag name='dc390'/> <flag name='am53c974'/> <flag name='virtio-pmem-pci'/> + <flag name='vhost-user-blk'/> <version>5002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml index f988357c44..6f24d80c6b 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml @@ -252,6 +252,7 @@ <flag name='am53c974'/> <flag name='virtio-pmem-pci'/> <flag name='vhost-user-fs.bootindex'/> + <flag name='vhost-user-blk'/> <version>5002050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100242</microcodeVersion> -- 2.29.2
Implements QEMU support for vhost-user-blk together with live hotplug/unplug. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_block.c | 16 ++++ src/qemu/qemu_block.h | 5 + src/qemu/qemu_command.c | 91 +++++++++++++++++-- src/qemu/qemu_command.h | 8 ++ src/qemu/qemu_domain.c | 5 + src/qemu/qemu_hotplug.c | 5 +- src/qemu/qemu_validate.c | 13 +++ .../disk-vhostuser.x86_64-latest.args | 41 +++++++++ tests/qemuxml2argvtest.c | 1 + 9 files changed, 176 insertions(+), 9 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-vhostuser.x86_64-latest.args diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index f6e81a7503..dd7858b6cc 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1640,6 +1640,8 @@ qemuBlockStorageSourceAttachDataFree(qemuBlockStorageSourceAttachDataPtr data) VIR_FREE(data->httpcookiesecretAlias); VIR_FREE(data->driveCmd); VIR_FREE(data->driveAlias); + VIR_FREE(data->chardevAlias); + VIR_FREE(data->chardevCmd); VIR_FREE(data); } @@ -1813,6 +1815,13 @@ qemuBlockStorageSourceAttachApply(qemuMonitorPtr mon, data->driveAdded = true; } + if (data->chardevDef) { + if (qemuMonitorAttachCharDev(mon, data->chardevAlias, data->chardevDef) < 0) + return -1; + + data->chardevAdded = true; + } + return 0; } @@ -1835,6 +1844,13 @@ qemuBlockStorageSourceAttachRollback(qemuMonitorPtr mon, virErrorPreserveLast(&orig_err); + if (data->chardevAdded) { + if (qemuMonitorDetachCharDev(mon, data->chardevAlias) < 0) { + VIR_WARN("Unable to remove chardev %s after failed " "qemuMonitorAddDevice", + data->chardevAlias); + } + } + if (data->driveAdded) { if (qemuMonitorDriveDel(mon, data->driveAlias) < 0) VIR_WARN("Unable to remove drive %s (%s) after failed " diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index 8f2a05f46a..d2efcfc3c0 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -99,6 +99,11 @@ struct qemuBlockStorageSourceAttachData { char *driveAlias; bool driveAdded; + virDomainChrSourceDefPtr chardevDef; + char *chardevAlias; + char *chardevCmd; + bool chardevAdded; + virJSONValuePtr authsecretProps; char *authsecretAlias; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3e652e18b7..059126cfeb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1712,9 +1712,16 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, break; case VIR_DOMAIN_DISK_BUS_VIRTIO: - if (qemuBuildVirtioDevStr(&opt, "virtio-blk", qemuCaps, - VIR_DOMAIN_DEVICE_DISK, disk) < 0) { - return NULL; + if (virStorageSourceGetActualType(disk->src) == VIR_STORAGE_TYPE_VHOST_USER) { + if (qemuBuildVirtioDevStr(&opt, "vhost-user-blk", qemuCaps, + VIR_DOMAIN_DEVICE_DISK, disk) < 0) { + return NULL; + } + } else { + if (qemuBuildVirtioDevStr(&opt, "virtio-blk", qemuCaps, + VIR_DOMAIN_DEVICE_DISK, disk) < 0) { + return NULL; + } } if (disk->iothread) @@ -1773,11 +1780,17 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, virQEMUCapsGet(qemuCaps, QEMU_CAPS_DISK_SHARE_RW)) virBufferAddLit(&opt, ",share-rw=on"); - if (qemuDomainDiskGetBackendAlias(disk, qemuCaps, &backendAlias) < 0) - return NULL; + if (virStorageSourceGetActualType(disk->src) == VIR_STORAGE_TYPE_VHOST_USER) { + backendAlias = qemuDomainGetVhostUserAlias(disk->info.alias); - if (backendAlias) - virBufferAsprintf(&opt, ",drive=%s", backendAlias); + virBufferAsprintf(&opt, ",chardev=%s", backendAlias); + } else { + if (qemuDomainDiskGetBackendAlias(disk, qemuCaps, &backendAlias) < 0) + return NULL; + + if (backendAlias) + virBufferAsprintf(&opt, ",drive=%s", backendAlias); + } virBufferAsprintf(&opt, ",id=%s", disk->info.alias); if (bootindex) @@ -1987,6 +2000,9 @@ qemuBuildBlockStorageSourceAttachDataCommandline(virCommandPtr cmd, if (data->driveCmd) virCommandAddArgList(cmd, "-drive", data->driveCmd, NULL); + if (data->chardevCmd) + virCommandAddArgList(cmd, "-chardev", data->chardevCmd, NULL); + if (data->storageProps) { if (!(tmp = virJSONValueToString(data->storageProps, false))) return -1; @@ -2025,7 +2041,10 @@ qemuBuildDiskSourceCommandLine(virCommandPtr cmd, g_autofree char *copyOnReadPropsStr = NULL; size_t i; - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV) && + if (virStorageSourceGetActualType(disk->src) == VIR_STORAGE_TYPE_VHOST_USER) { + if (!(data = qemuBuildStorageSourceChainAttachPrepareChardev(disk))) + return -1; + } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV) && !qemuDiskBusIsSD(disk->bus)) { if (virStorageSourceIsEmpty(disk->src)) return 0; @@ -10291,6 +10310,38 @@ qemuBuildStorageSourceAttachPrepareDrive(virDomainDiskDefPtr disk, } +/** + * qemuBuildStorageSourceAttachPrepareChardev: + * @src: disk source to prepare + * + * Prepare qemuBlockStorageSourceAttachDataPtr for use with -chardev. + */ +qemuBlockStorageSourceAttachDataPtr +qemuBuildStorageSourceAttachPrepareChardev(virDomainDiskDefPtr disk) +{ + g_autoptr(qemuBlockStorageSourceAttachData) data = NULL; + g_auto(virBuffer) chardev = VIR_BUFFER_INITIALIZER; + + data = g_new0(qemuBlockStorageSourceAttachData, 1); + + data->chardevDef = disk->src->vhostuser; + data->chardevAlias = qemuDomainGetVhostUserAlias(disk->info.alias); + + virBufferAddLit(&chardev, "socket"); + virBufferAsprintf(&chardev, ",id=%s", data->chardevAlias); + virBufferAddLit(&chardev, ",path="); + virQEMUBuildBufferEscapeComma(&chardev, disk->src->vhostuser->data.nix.path); + + qemuBuildChrChardevReconnectStr(&chardev, + &disk->src->vhostuser->data.nix.reconnect); + + if (!(data->chardevCmd = virBufferContentAndReset(&chardev))) + return NULL; + + return g_steal_pointer(&data); +} + + /** * qemuBuildStorageSourceAttachPrepareCommon: * @src: storage source @@ -10373,6 +10424,30 @@ qemuBuildStorageSourceChainAttachPrepareDrive(virDomainDiskDefPtr disk, } +/** + * qemuBuildStorageSourceChainAttachPrepareChardev: + * @src: disk definition + * + * Prepares qemuBlockStorageSourceChainDataPtr for attaching @disk via -drive. + */ +qemuBlockStorageSourceChainDataPtr +qemuBuildStorageSourceChainAttachPrepareChardev(virDomainDiskDefPtr disk) +{ + g_autoptr(qemuBlockStorageSourceAttachData) elem = NULL; + g_autoptr(qemuBlockStorageSourceChainData) data = NULL; + + data = g_new0(qemuBlockStorageSourceChainData, 1); + + if (!(elem = qemuBuildStorageSourceAttachPrepareChardev(disk))) + return NULL; + + if (VIR_APPEND_ELEMENT(data->srcdata, data->nsrcdata, elem) < 0) + return NULL; + + return g_steal_pointer(&data); +} + + static int qemuBuildStorageSourceChainAttachPrepareBlockdevOne(qemuBlockStorageSourceChainData *data, virStorageSourcePtr src, diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 3cfe6ff3e9..a33fbf6f4e 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -115,6 +115,10 @@ bool qemuDiskBusIsSD(int bus); qemuBlockStorageSourceAttachDataPtr qemuBuildStorageSourceAttachPrepareDrive(virDomainDiskDefPtr disk, virQEMUCapsPtr qemuCaps); + +qemuBlockStorageSourceAttachDataPtr +qemuBuildStorageSourceAttachPrepareChardev(virDomainDiskDefPtr disk); + int qemuBuildStorageSourceAttachPrepareCommon(virStorageSourcePtr src, qemuBlockStorageSourceAttachDataPtr data, @@ -126,6 +130,10 @@ qemuBuildStorageSourceChainAttachPrepareDrive(virDomainDiskDefPtr disk, virQEMUCapsPtr qemuCaps); +qemuBlockStorageSourceChainDataPtr +qemuBuildStorageSourceChainAttachPrepareChardev(virDomainDiskDefPtr disk); + + qemuBlockStorageSourceChainDataPtr qemuBuildStorageSourceChainAttachPrepareBlockdev(virStorageSourcePtr top, virQEMUCapsPtr qemuCaps); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0c078a9388..aa6d539610 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -10581,6 +10581,11 @@ qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk, disk->src->format = VIR_STORAGE_FILE_RAW; } + /* Nothing else to prepare as it will use -chardev instead + * of -blockdev/-drive option. */ + if (disk->src->type == VIR_STORAGE_TYPE_VHOST_USER) + return 0; + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV) && !qemuDiskBusIsSD(disk->bus)) { if (qemuDomainPrepareDiskSourceBlockdev(disk, priv, cfg) < 0) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 882e5d2384..dd6ba80cd0 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -714,7 +714,10 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0) goto cleanup; - if (blockdev) { + if (virStorageSourceGetActualType(disk->src) == VIR_STORAGE_TYPE_VHOST_USER) { + if (!(data = qemuBuildStorageSourceChainAttachPrepareChardev(disk))) + goto cleanup; + } else if (blockdev) { if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON) { if (!(corProps = qemuBlockStorageGetCopyOnReadProps(disk))) goto cleanup; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 96fc7d4118..f0e518a0f1 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2944,6 +2944,19 @@ qemuValidateDomainDeviceDefDisk(const virDomainDiskDef *disk, return -1; } + if (disk->src->type == VIR_STORAGE_TYPE_VHOST_USER) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VHOST_USER_BLK)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("vhostuser disk is not supported with this QEMU binary")); + return -1; + } + + if (qemuValidateDomainDefVhostUserRequireSharedMemory(def, "vhostuser", + qemuCaps) < 0) { + return -1; + } + } + return 0; } diff --git a/tests/qemuxml2argvdata/disk-vhostuser.x86_64-latest.args b/tests/qemuxml2argvdata/disk-vhostuser.x86_64-latest.args new file mode 100644 index 0000000000..b24b2c0b4f --- /dev/null +++ b/tests/qemuxml2argvdata/disk-vhostuser.x86_64-latest.args @@ -0,0 +1,41 @@ +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 \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off,memory-backend=pc.ram \ +-cpu qemu64 \ +-m 214 \ +-object memory-backend-memfd,id=pc.ram,share=yes,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,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-chardev socket,id=chr-vu-virtio-disk0,path=/tmp/vhost1.sock \ +-device vhost-user-blk-pci,bus=pci.0,addr=0x2,chardev=chr-vu-virtio-disk0,\ +id=virtio-disk0,bootindex=1 \ +-chardev socket,id=chr-vu-virtio-disk1,path=/tmp/vhost1.sock,reconnect=10 \ +-device vhost-user-blk-pci,bus=pci.0,addr=0x3,chardev=chr-vu-virtio-disk1,\ +id=virtio-disk1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index c5d82ac72e..0f29e166e3 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1267,6 +1267,7 @@ mymain(void) VIR_FREE(driver.config->vxhsTLSx509certdir); DO_TEST("disk-no-boot", NONE); DO_TEST_CAPS_LATEST("disk-nvme"); + DO_TEST_CAPS_LATEST("disk-vhostuser"); DO_TEST_PARSE_ERROR("disk-device-lun-type-invalid", QEMU_CAPS_VIRTIO_SCSI); DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-attaching-partition-nosupport"); -- 2.29.2
On Tue, Feb 02, 2021 at 16:04:12 +0100, Pavel Hrdina wrote:
Implements QEMU support for vhost-user-blk together with live hotplug/unplug.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_block.c | 16 ++++ src/qemu/qemu_block.h | 5 + src/qemu/qemu_command.c | 91 +++++++++++++++++-- src/qemu/qemu_command.h | 8 ++ src/qemu/qemu_domain.c | 5 + src/qemu/qemu_hotplug.c | 5 +- src/qemu/qemu_validate.c | 13 +++ .../disk-vhostuser.x86_64-latest.args | 41 +++++++++ tests/qemuxml2argvtest.c | 1 + 9 files changed, 176 insertions(+), 9 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-vhostuser.x86_64-latest.args
[...]
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3e652e18b7..059126cfeb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1712,9 +1712,16 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, break;
case VIR_DOMAIN_DISK_BUS_VIRTIO: - if (qemuBuildVirtioDevStr(&opt, "virtio-blk", qemuCaps, - VIR_DOMAIN_DEVICE_DISK, disk) < 0) { - return NULL; + if (virStorageSourceGetActualType(disk->src) == VIR_STORAGE_TYPE_VHOST_USER) { + if (qemuBuildVirtioDevStr(&opt, "vhost-user-blk", qemuCaps, + VIR_DOMAIN_DEVICE_DISK, disk) < 0) { + return NULL; + } + } else { + if (qemuBuildVirtioDevStr(&opt, "virtio-blk", qemuCaps, + VIR_DOMAIN_DEVICE_DISK, disk) < 0) { + return NULL; + }
Preferably, add a temporary string holding either "vhost-user-blk" or "virtio-blk" and call qemuBuildVirtioDevStr just once, since all other arguments are the same.
}
if (disk->iothread)
[...]
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0c078a9388..aa6d539610 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -10581,6 +10581,11 @@ qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk, disk->src->format = VIR_STORAGE_FILE_RAW; }
+ /* Nothing else to prepare as it will use -chardev instead + * of -blockdev/-drive option. */ + if (disk->src->type == VIR_STORAGE_TYPE_VHOST_USER) + return 0;
The code above sets 'format' for storage pool backed disks, but since format isn't something we'd be setting for vhost-user this exemption should be at the beginning of the function.
+ if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV) && !qemuDiskBusIsSD(disk->bus)) { if (qemuDomainPrepareDiskSourceBlockdev(disk, priv, cfg) < 0)
Missing stuff from this commit: - hot-unplug support - see qemuDomainRemoveDiskDevice where the chardev isn't deleted - blockjob lockout - qemuDomainDiskBlockJobIsSupported - block iotune lockout - possible better lockout for qemuDomainBlockPeek - it will barf either that the file is not 'raw' or that it can't be initialized - possible better lockout for qemuDomainSetBlockThreshold - "threshold currently can't be set for block device '%s'" <- it will never be possible - lockout for qemuDomainGetBlockInfo - lockout for qemuDomainBlockResize - lockout for qemuDomainBlockStats(Flags) - handling in bulk stats (qemuDomainGetStatsBlockExportDisk ?)
On Wed, Feb 03, 2021 at 11:04:04AM +0100, Peter Krempa wrote:
On Tue, Feb 02, 2021 at 16:04:12 +0100, Pavel Hrdina wrote:
Implements QEMU support for vhost-user-blk together with live hotplug/unplug.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_block.c | 16 ++++ src/qemu/qemu_block.h | 5 + src/qemu/qemu_command.c | 91 +++++++++++++++++-- src/qemu/qemu_command.h | 8 ++ src/qemu/qemu_domain.c | 5 + src/qemu/qemu_hotplug.c | 5 +- src/qemu/qemu_validate.c | 13 +++ .../disk-vhostuser.x86_64-latest.args | 41 +++++++++ tests/qemuxml2argvtest.c | 1 + 9 files changed, 176 insertions(+), 9 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-vhostuser.x86_64-latest.args
[...]
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3e652e18b7..059126cfeb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1712,9 +1712,16 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, break;
case VIR_DOMAIN_DISK_BUS_VIRTIO: - if (qemuBuildVirtioDevStr(&opt, "virtio-blk", qemuCaps, - VIR_DOMAIN_DEVICE_DISK, disk) < 0) { - return NULL; + if (virStorageSourceGetActualType(disk->src) == VIR_STORAGE_TYPE_VHOST_USER) { + if (qemuBuildVirtioDevStr(&opt, "vhost-user-blk", qemuCaps, + VIR_DOMAIN_DEVICE_DISK, disk) < 0) { + return NULL; + } + } else { + if (qemuBuildVirtioDevStr(&opt, "virtio-blk", qemuCaps, + VIR_DOMAIN_DEVICE_DISK, disk) < 0) { + return NULL; + }
Preferably, add a temporary string holding either "vhost-user-blk" or "virtio-blk" and call qemuBuildVirtioDevStr just once, since all other arguments are the same.
This would require adding one level of nesting because of the temporary variable or I would have to define the variable at the top of the function. Not sure if it makes it better.
}
if (disk->iothread)
[...]
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0c078a9388..aa6d539610 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -10581,6 +10581,11 @@ qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk, disk->src->format = VIR_STORAGE_FILE_RAW; }
+ /* Nothing else to prepare as it will use -chardev instead + * of -blockdev/-drive option. */ + if (disk->src->type == VIR_STORAGE_TYPE_VHOST_USER) + return 0;
The code above sets 'format' for storage pool backed disks, but since format isn't something we'd be setting for vhost-user this exemption should be at the beginning of the function.
Fixed.
+ if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV) && !qemuDiskBusIsSD(disk->bus)) { if (qemuDomainPrepareDiskSourceBlockdev(disk, priv, cfg) < 0)
Missing stuff from this commit:
- hot-unplug support - see qemuDomainRemoveDiskDevice where the chardev isn't deleted - blockjob lockout - qemuDomainDiskBlockJobIsSupported - block iotune lockout - possible better lockout for qemuDomainBlockPeek - it will barf either that the file is not 'raw' or that it can't be initialized - possible better lockout for qemuDomainSetBlockThreshold - "threshold currently can't be set for block device '%s'" <- it will never be possible - lockout for qemuDomainGetBlockInfo - lockout for qemuDomainBlockResize - lockout for qemuDomainBlockStats(Flags) - handling in bulk stats (qemuDomainGetStatsBlockExportDisk ?)
Well, that's a lot of missing things, thanks for the pointers, I'll look into it.
On a Tuesday in 2021, Pavel Hrdina wrote:
Pavel Hrdina (6): qemu_alias: introduce qemuDomainGetVhostUserAlias helper qemu_validate: move and refactor qemuValidateDomainDefVirtioFSSharedMemory
docs: introduces new vhostuser disk type conf: implement support for vhostuser disk
Typically the schema changes are introduced with the parser, since the addition of the XML file also runs it through virschematest.
qemu_capabilities: introduce vhost-user-blk capability qemu: implement vhost-user-blk support
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
On Tue, Feb 02, 2021 at 16:04:06 +0100, Pavel Hrdina wrote:
Pavel Hrdina (6): qemu_alias: introduce qemuDomainGetVhostUserAlias helper qemu_validate: move and refactor qemuValidateDomainDefVirtioFSSharedMemory docs: introduces new vhostuser disk type conf: implement support for vhostuser disk qemu_capabilities: introduce vhost-user-blk capability
For the above commits you can use: Reviewed-by: Peter Krempa <pkrempa@redhat.com> after applying my feedback.
qemu: implement vhost-user-blk support
This one has a bit too much to add though.
participants (3)
-
Ján Tomko -
Pavel Hrdina -
Peter Krempa