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@xxxxxxxxxx> > --- > 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; >
