[libvirt PATCHv2 09/10] qemu: build vhost-user-fs device command line

[Peter Krempa]

On Thu, Jan 23, 2020 at 18:46:10 +0100, Ján Tomko wrote:
> Format the 'vhost-user-fs' device on the QEMU command line.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1694166

Rather than posting this bugzilla link to a not very descriptive bug it
would be worthwhile to describe what the device actually does and how it
integrates with the daemon.

> 
> Signed-off-by: Ján Tomko <jtomko at redhat.com>
> ---
>  src/qemu/qemu_command.c                       | 47 ++++++++++++++++++-
>  ...vhost-user-fs-fd-memory.x86_64-latest.args | 38 +++++++++++++++
>  ...vhost-user-fs-hugepages.x86_64-latest.args | 46 ++++++++++++++++++
>  tests/qemuxml2argvtest.c                      |  3 ++
>  4 files changed, 132 insertions(+), 2 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/vhost-user-fs-fd-memory.x86_64-latest.args
>  create mode 100644 tests/qemuxml2argvdata/vhost-user-fs-hugepages.x86_64-latest.args
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 974c2b79af..31573860dd 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2581,6 +2581,47 @@ qemuBuildDisksCommandLine(virCommandPtr cmd,
>  }
>  
>  
> +static int
> +qemuBuildVHostUserFsCommandLine(virCommandPtr cmd,
> +                                virDomainFSDef *fs,
> +                                const virDomainDef *def G_GNUC_UNUSED,
> +                                qemuDomainObjPrivatePtr priv)
> +{
> +    g_autofree char *chardev_alias = NULL;
> +    g_auto(virBuffer) opt = VIR_BUFFER_INITIALIZER;
> +    const char *tag = fs->dst;
> +
> +    chardev_alias = g_strdup_printf("chr-vu-%s", fs->info.alias);

I don't like to see another alias generated in the command line
generator and then thrown away. Unfortunately it's still a common
practice.

> +
> +    virCommandAddArg(cmd, "-chardev");
> +    virBufferAddLit(&opt, "socket");
> +    virBufferAsprintf(&opt, ",id=%s", chardev_alias);
> +    virBufferEscapeString(&opt, ",path=%s", QEMU_DOMAIN_FS_PRIVATE(fs)->vhostuser_fs_sock);
> +    virCommandAddArg(cmd, virBufferContentAndReset(&opt));
> +
> +    virCommandAddArg(cmd, "-device");
> +
> +    if (qemuBuildVirtioDevStr(&opt, "vhost-user-fs", priv->qemuCaps,
> +                              VIR_DOMAIN_DEVICE_FS, fs) < 0)
> +        return -1;
> +
> +    virBufferAsprintf(&opt, ",chardev=%s", chardev_alias);
> +    if (fs->queue_size)
> +        virBufferAsprintf(&opt, ",queue-size=%llu", fs->queue_size);
> +    virBufferEscapeString(&opt, ",tag=%s", tag);

fs->dst is user-provided and not validated. It must be escaped on the
command line.

> +    if (fs->cache_size)
> +        virBufferAsprintf(&opt, ",cache-size=%lluK", fs->cache_size);

Hmm, so only cache size and queue is relevant for the device itself. I
think the options in the XML should reflect that.

I'll mention this in the XML schema patch.

> +    if (qemuBuildVirtioOptionsStr(&opt, fs->virtio, priv->qemuCaps) < 0)
> +        return -1;
> +
> +    if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, priv->qemuCaps) < 0)
> +        return -1;
> +
> +    virCommandAddArg(cmd, virBufferContentAndReset(&opt));
> +    return 0;
> +}
> +
> +
>  static char *
>  qemuBuildFSStr(virDomainFSDefPtr fs)
>  {
> @@ -2673,7 +2714,7 @@ static int
>  qemuBuildFilesystemCommandLine(virCommandPtr cmd,
>                                 const virDomainDef *def,
>                                 virQEMUCapsPtr qemuCaps,
> -                               qemuDomainObjPrivatePtr priv G_GNUC_UNUSED)
> +                               qemuDomainObjPrivatePtr priv)
>  {
>      size_t i;
>