[libvirt PATCH 00/16] qemu: implement virtiofs hot(un)plug (virtiofs epopee)
https://bugzilla.redhat.com/show_bug.cgi?id=1897708 Ján Tomko (16): qemu: vhost-user-fs: format alias on the command line conf: define cleanup func for virDomainChrSourceDef qemu: domain: introduce qemuDomainGetVHostUserFSSocketPath qemu: domain: introduce qemuDomainGetVHostUserChrSourceDef tests: qemuxml2argvtest: fix path to virtiofs socket qemu: vhost-user-fs: separate building of chardev string qemu: vhost-user-fs: separate building of device string qemu: do not put virtiofs socket in private data qemu: remove private data from virDomainFSDef qemu: alias: prepare qemuAssignDeviceFSAlias for disjunct ranges qemu: vhost-user-fs: build extdevice for zpci qemu: export vhost-user-fs-related functions qemu: store logCtxt in domain private data qemu: use priv->logCtxt in qemuProcessLaunch qemu: implement virtiofs hotplug qemu: implement virtiofs hotunplug src/conf/domain_conf.c | 24 +++ src/conf/domain_conf.h | 4 + src/libvirt_private.syms | 1 + src/qemu/qemu_alias.c | 20 +- src/qemu/qemu_alias.h | 4 + src/qemu/qemu_command.c | 81 +++++--- src/qemu/qemu_command.h | 6 + src/qemu/qemu_domain.c | 64 +++---- src/qemu/qemu_domain.h | 30 +-- src/qemu/qemu_driver.c | 9 +- src/qemu/qemu_extdevice.c | 6 +- src/qemu/qemu_hotplug.c | 177 +++++++++++++++++- src/qemu/qemu_hotplug.h | 4 + src/qemu/qemu_process.c | 21 ++- src/qemu/qemu_virtiofs.c | 14 +- ...vhost-user-fs-fd-memory.x86_64-latest.args | 4 +- ...vhost-user-fs-hugepages.x86_64-latest.args | 4 +- tests/qemuxml2argvtest.c | 12 -- 18 files changed, 360 insertions(+), 125 deletions(-) -- 2.31.1
The commit adding the vhost-user-fs device forgot to format the device's alias on the command line. Thankfully it was not needed yet because virtiofs migration is not yet supported, but it will be needed in the future to allow hot(un)plug. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 1 + .../qemuxml2argvdata/vhost-user-fs-fd-memory.x86_64-latest.args | 2 +- .../qemuxml2argvdata/vhost-user-fs-hugepages.x86_64-latest.args | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 08f6d735f8..90c8022b07 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2057,6 +2057,7 @@ qemuBuildVHostUserFsCommandLine(virCommand *cmd, VIR_DOMAIN_DEVICE_FS, fs) < 0) return -1; + virBufferAsprintf(&opt, ",id=%s", fs->info.alias); virBufferAsprintf(&opt, ",chardev=%s", chardev_alias); if (fs->queue_size) virBufferAsprintf(&opt, ",queue-size=%llu", fs->queue_size); diff --git a/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.x86_64-latest.args b/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.x86_64-latest.args index 6311f8f65e..7586a8edbf 100644 --- a/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.x86_64-latest.args +++ b/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.x86_64-latest.args @@ -28,7 +28,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-guest/.config \ -no-acpi \ -boot strict=on \ -chardev socket,id=chr-vu-fs0,path=/tmp/lib/domain--1-guest/fs0.vhost-fs.sock \ --device vhost-user-fs-pci,chardev=chr-vu-fs0,queue-size=1024,tag=mount_tag,bus=pci.0,addr=0x2 \ +-device vhost-user-fs-pci,id=fs0,chardev=chr-vu-fs0,queue-size=1024,tag=mount_tag,bus=pci.0,addr=0x2 \ -audiodev id=audio1,driver=none \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ -msg timestamp=on diff --git a/tests/qemuxml2argvdata/vhost-user-fs-hugepages.x86_64-latest.args b/tests/qemuxml2argvdata/vhost-user-fs-hugepages.x86_64-latest.args index 58570592eb..290d0b9e2f 100644 --- a/tests/qemuxml2argvdata/vhost-user-fs-hugepages.x86_64-latest.args +++ b/tests/qemuxml2argvdata/vhost-user-fs-hugepages.x86_64-latest.args @@ -34,7 +34,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-guest/.config \ -blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"qcow2","file":"libvirt-1-storage"}' \ -device virtio-blk-pci,bus=pci.4,addr=0x0,drive=libvirt-1-format,id=virtio-disk0,bootindex=1 \ -chardev socket,id=chr-vu-fs0,path=/tmp/lib/domain--1-guest/fs0.vhost-fs.sock \ --device vhost-user-fs-pci,chardev=chr-vu-fs0,tag=mount_tag,bootindex=2,bus=pci.1,addr=0x0 \ +-device vhost-user-fs-pci,id=fs0,chardev=chr-vu-fs0,tag=mount_tag,bootindex=2,bus=pci.1,addr=0x0 \ -audiodev id=audio1,driver=none \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ -msg timestamp=on -- 2.31.1
On Wed, Oct 06, 2021 at 09:15:07 +0200, Ján Tomko wrote:
The commit adding the vhost-user-fs device forgot to format the device's alias on the command line.
Thankfully it was not needed yet because virtiofs migration is not yet supported, but it will be needed in the future to allow hot(un)plug.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 1 + .../qemuxml2argvdata/vhost-user-fs-fd-memory.x86_64-latest.args | 2 +- .../qemuxml2argvdata/vhost-user-fs-hugepages.x86_64-latest.args | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e054c1508e..c23c233184 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1284,6 +1284,7 @@ struct _virDomainChrSourceDef { size_t nseclabels; virSecurityDeviceLabelDef **seclabels; }; +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainChrSourceDef, virObjectUnref); /* A complete character device, both host and domain views. */ struct _virDomainChrDef { @@ -3300,6 +3301,7 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainNetDef, virDomainNetDefFree); void virDomainSmartcardDefFree(virDomainSmartcardDef *def); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainSmartcardDef, virDomainSmartcardDefFree); void virDomainChrDefFree(virDomainChrDef *def); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainChrDef, virDomainChrDefFree); int virDomainChrSourceDefCopy(virDomainChrSourceDef *dest, virDomainChrSourceDef *src); void virDomainSoundCodecDefFree(virDomainSoundCodecDef *def); -- 2.31.1
On Wed, Oct 06, 2021 at 09:15:08 +0200, Ján Tomko wrote: It's defined also for 'virDomainChrDef'
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.h | 2 ++ 1 file changed, 2 insertions(+)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Intended as a replacement for qemuVirtioFSCreateSocketFilename, to be used outside of qemu_virtiofs.c Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_domain.c | 10 ++++++++++ src/qemu/qemu_domain.h | 4 ++++ 2 files changed, 14 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a755f8678e..cb6aaecdee 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11440,3 +11440,13 @@ qemuDomainNamePathsCleanup(virQEMUDriverConfig *cfg, return 0; } + + +char * +qemuDomainGetVHostUserFSSocketPath(qemuDomainObjPrivate *priv, + const virDomainFSDef *fs) +{ + if (fs->sock) + return g_strdup(fs->sock); + return virFileBuildPath(priv->libDir, fs->info.alias, "-fs.sock"); +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 64f92988b7..aad076dd4c 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -1043,3 +1043,7 @@ int qemuDomainNamePathsCleanup(virQEMUDriverConfig *cfg, const char *name, bool bestEffort); + +char * +qemuDomainGetVHostUserFSSocketPath(qemuDomainObjPrivate *priv, + const virDomainFSDef *fs); -- 2.31.1
On Wed, Oct 06, 2021 at 09:15:09 +0200, Ján Tomko wrote:
Intended as a replacement for qemuVirtioFSCreateSocketFilename, to be used outside of qemu_virtiofs.c
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_domain.c | 10 ++++++++++ src/qemu/qemu_domain.h | 4 ++++ 2 files changed, 14 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a755f8678e..cb6aaecdee 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11440,3 +11440,13 @@ qemuDomainNamePathsCleanup(virQEMUDriverConfig *cfg,
return 0; } + + +char * +qemuDomainGetVHostUserFSSocketPath(qemuDomainObjPrivate *priv, + const virDomainFSDef *fs) +{ + if (fs->sock) + return g_strdup(fs->sock);
Empty line here please.
+ return virFileBuildPath(priv->libDir, fs->info.alias, "-fs.sock"); +}
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
A function that creates a chardev source with the appropriate socket path set. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_domain.c | 13 +++++++++++++ src/qemu/qemu_domain.h | 4 ++++ 2 files changed, 17 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index cb6aaecdee..5f467760c3 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11450,3 +11450,16 @@ qemuDomainGetVHostUserFSSocketPath(qemuDomainObjPrivate *priv, return g_strdup(fs->sock); return virFileBuildPath(priv->libDir, fs->info.alias, "-fs.sock"); } + + +virDomainChrSourceDef * +qemuDomainGetVHostUserChrSourceDef(qemuDomainObjPrivate *priv, + const virDomainFSDef *fs) +{ + virDomainChrSourceDef *src = virDomainChrSourceDefNew(priv->driver->xmlopt); + + src->type = VIR_DOMAIN_CHR_TYPE_UNIX; + src->data.nix.path = qemuDomainGetVHostUserFSSocketPath(priv, fs); + + return src; +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index aad076dd4c..59cb703c16 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -1047,3 +1047,7 @@ qemuDomainNamePathsCleanup(virQEMUDriverConfig *cfg, char * qemuDomainGetVHostUserFSSocketPath(qemuDomainObjPrivate *priv, const virDomainFSDef *fs); + +virDomainChrSourceDef * +qemuDomainGetVHostUserChrSourceDef(qemuDomainObjPrivate *priv, + const virDomainFSDef *fs); -- 2.31.1
On Wed, Oct 06, 2021 at 09:15:10 +0200, Ján Tomko wrote:
A function that creates a chardev source with the appropriate socket path set.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_domain.c | 13 +++++++++++++ src/qemu/qemu_domain.h | 4 ++++ 2 files changed, 17 insertions(+)
This function is called only from 'qemuDomainAttachFSDevice' thus it doesn't really feel it's worth having as a separate helper. Consider inlining it into the patch actually needing it. Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The mocked path in the test suite is not in sync with what libvirtd generates. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- .../qemuxml2argvdata/vhost-user-fs-fd-memory.x86_64-latest.args | 2 +- .../qemuxml2argvdata/vhost-user-fs-hugepages.x86_64-latest.args | 2 +- tests/qemuxml2argvtest.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.x86_64-latest.args b/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.x86_64-latest.args index 7586a8edbf..8adab6da81 100644 --- a/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.x86_64-latest.args +++ b/tests/qemuxml2argvdata/vhost-user-fs-fd-memory.x86_64-latest.args @@ -27,7 +27,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-guest/.config \ -no-shutdown \ -no-acpi \ -boot strict=on \ --chardev socket,id=chr-vu-fs0,path=/tmp/lib/domain--1-guest/fs0.vhost-fs.sock \ +-chardev socket,id=chr-vu-fs0,path=/tmp/lib/domain--1-guest/fs0-fs.sock \ -device vhost-user-fs-pci,id=fs0,chardev=chr-vu-fs0,queue-size=1024,tag=mount_tag,bus=pci.0,addr=0x2 \ -audiodev id=audio1,driver=none \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ diff --git a/tests/qemuxml2argvdata/vhost-user-fs-hugepages.x86_64-latest.args b/tests/qemuxml2argvdata/vhost-user-fs-hugepages.x86_64-latest.args index 290d0b9e2f..4158456744 100644 --- a/tests/qemuxml2argvdata/vhost-user-fs-hugepages.x86_64-latest.args +++ b/tests/qemuxml2argvdata/vhost-user-fs-hugepages.x86_64-latest.args @@ -33,7 +33,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-guest/.config \ -blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/guest.qcow2","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ -blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"qcow2","file":"libvirt-1-storage"}' \ -device virtio-blk-pci,bus=pci.4,addr=0x0,drive=libvirt-1-format,id=virtio-disk0,bootindex=1 \ --chardev socket,id=chr-vu-fs0,path=/tmp/lib/domain--1-guest/fs0.vhost-fs.sock \ +-chardev socket,id=chr-vu-fs0,path=/tmp/lib/domain--1-guest/fs0-fs.sock \ -device vhost-user-fs-pci,id=fs0,chardev=chr-vu-fs0,tag=mount_tag,bootindex=2,bus=pci.1,addr=0x0 \ -audiodev id=audio1,driver=none \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 94aaa2f53e..e35380293c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -444,7 +444,7 @@ testCompareXMLToArgvCreateArgs(virQEMUDriver *drv, QEMU_DOMAIN_FS_PRIVATE(fs)->vhostuser_fs_sock) continue; - s = g_strdup_printf("/tmp/lib/domain--1-guest/fs%zu.vhost-fs.sock", i); + s = g_strdup_printf("/tmp/lib/domain--1-guest/fs%zu-fs.sock", i); QEMU_DOMAIN_FS_PRIVATE(fs)->vhostuser_fs_sock = s; } -- 2.31.1
On Wed, Oct 06, 2021 at 09:15:11 +0200, Ján Tomko wrote:
The mocked path in the test suite is not in sync with what libvirtd generates.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- .../qemuxml2argvdata/vhost-user-fs-fd-memory.x86_64-latest.args | 2 +- .../qemuxml2argvdata/vhost-user-fs-hugepages.x86_64-latest.args | 2 +- tests/qemuxml2argvtest.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 90c8022b07..7e76e188c1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2033,6 +2033,20 @@ qemuBuildDisksCommandLine(virCommand *cmd, } +static char * +qemuBuildVHostUserFsChardevStr(const virDomainFSDef *fs, + const char *chardev_alias) +{ + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + + virBufferAddLit(&buf, "socket"); + virBufferAsprintf(&buf, ",id=%s", chardev_alias); + virBufferAddLit(&buf, ",path="); + virQEMUBuildBufferEscapeComma(&buf, QEMU_DOMAIN_FS_PRIVATE(fs)->vhostuser_fs_sock); + return virBufferContentAndReset(&buf); +} + + static int qemuBuildVHostUserFsCommandLine(virCommand *cmd, virDomainFSDef *fs, @@ -2040,16 +2054,14 @@ qemuBuildVHostUserFsCommandLine(virCommand *cmd, qemuDomainObjPrivate *priv) { g_autofree char *chardev_alias = NULL; + g_autofree char *chrdevstr = NULL; g_auto(virBuffer) opt = VIR_BUFFER_INITIALIZER; chardev_alias = qemuDomainGetVhostUserChrAlias(fs->info.alias); + chrdevstr = qemuBuildVHostUserFsChardevStr(fs, chardev_alias); virCommandAddArg(cmd, "-chardev"); - virBufferAddLit(&opt, "socket"); - virBufferAsprintf(&opt, ",id=%s", chardev_alias); - virBufferAddLit(&opt, ",path="); - virQEMUBuildBufferEscapeComma(&opt, QEMU_DOMAIN_FS_PRIVATE(fs)->vhostuser_fs_sock); - virCommandAddArgBuffer(cmd, &opt); + virCommandAddArg(cmd, chrdevstr); virCommandAddArg(cmd, "-device"); -- 2.31.1
On Wed, Oct 06, 2021 at 09:15:12 +0200, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 55 ++++++++++++++++++++++++++--------------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7e76e188c1..d0f961b8ea 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2047,6 +2047,36 @@ qemuBuildVHostUserFsChardevStr(const virDomainFSDef *fs, } +static char * +qemuBuildVHostUserFsDevStr(virDomainFSDef *fs, + const virDomainDef *def, + const char *chardev_alias, + qemuDomainObjPrivate *priv) +{ + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + + if (qemuBuildVirtioDevStr(&buf, "vhost-user-fs", priv->qemuCaps, + VIR_DOMAIN_DEVICE_FS, fs) < 0) + return NULL; + + virBufferAsprintf(&buf, ",id=%s", fs->info.alias); + virBufferAsprintf(&buf, ",chardev=%s", chardev_alias); + if (fs->queue_size) + virBufferAsprintf(&buf, ",queue-size=%llu", fs->queue_size); + virBufferAddLit(&buf, ",tag="); + virQEMUBuildBufferEscapeComma(&buf, fs->dst); + qemuBuildVirtioOptionsStr(&buf, fs->virtio); + + if (fs->info.bootIndex) + virBufferAsprintf(&buf, ",bootindex=%u", fs->info.bootIndex); + + if (qemuBuildDeviceAddressStr(&buf, def, &fs->info) < 0) + return NULL; + + return virBufferContentAndReset(&buf); +} + + static int qemuBuildVHostUserFsCommandLine(virCommand *cmd, virDomainFSDef *fs, @@ -2055,7 +2085,7 @@ qemuBuildVHostUserFsCommandLine(virCommand *cmd, { g_autofree char *chardev_alias = NULL; g_autofree char *chrdevstr = NULL; - g_auto(virBuffer) opt = VIR_BUFFER_INITIALIZER; + g_autofree char *devstr = NULL; chardev_alias = qemuDomainGetVhostUserChrAlias(fs->info.alias); chrdevstr = qemuBuildVHostUserFsChardevStr(fs, chardev_alias); @@ -2063,27 +2093,12 @@ qemuBuildVHostUserFsCommandLine(virCommand *cmd, virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, chrdevstr); + if (!(devstr = qemuBuildVHostUserFsDevStr(fs, def, chardev_alias, priv))) + return -1; + virCommandAddArg(cmd, "-device"); + virCommandAddArg(cmd, devstr); - if (qemuBuildVirtioDevStr(&opt, "vhost-user-fs", priv->qemuCaps, - VIR_DOMAIN_DEVICE_FS, fs) < 0) - return -1; - - virBufferAsprintf(&opt, ",id=%s", fs->info.alias); - virBufferAsprintf(&opt, ",chardev=%s", chardev_alias); - if (fs->queue_size) - virBufferAsprintf(&opt, ",queue-size=%llu", fs->queue_size); - virBufferAddLit(&opt, ",tag="); - virQEMUBuildBufferEscapeComma(&opt, fs->dst); - qemuBuildVirtioOptionsStr(&opt, fs->virtio); - - if (fs->info.bootIndex) - virBufferAsprintf(&opt, ",bootindex=%u", fs->info.bootIndex); - - if (qemuBuildDeviceAddressStr(&opt, def, &fs->info) < 0) - return -1; - - virCommandAddArgBuffer(cmd, &opt); return 0; } -- 2.31.1
On Wed, Oct 06, 2021 at 09:15:13 +0200, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 55 ++++++++++++++++++++++++++--------------- 1 file changed, 35 insertions(+), 20 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reconstruct the socket path from priv->libDir in every user. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 8 +++++--- src/qemu/qemu_extdevice.c | 6 ++---- src/qemu/qemu_virtiofs.c | 14 +++++++------- tests/qemuxml2argvtest.c | 12 ------------ 4 files changed, 14 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d0f961b8ea..dc4f91ce25 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2035,14 +2035,16 @@ qemuBuildDisksCommandLine(virCommand *cmd, static char * qemuBuildVHostUserFsChardevStr(const virDomainFSDef *fs, - const char *chardev_alias) + const char *chardev_alias, + qemuDomainObjPrivate *priv) { g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + g_autofree char *socket_path = qemuDomainGetVHostUserFSSocketPath(priv, fs); virBufferAddLit(&buf, "socket"); virBufferAsprintf(&buf, ",id=%s", chardev_alias); virBufferAddLit(&buf, ",path="); - virQEMUBuildBufferEscapeComma(&buf, QEMU_DOMAIN_FS_PRIVATE(fs)->vhostuser_fs_sock); + virQEMUBuildBufferEscapeComma(&buf, socket_path); return virBufferContentAndReset(&buf); } @@ -2088,7 +2090,7 @@ qemuBuildVHostUserFsCommandLine(virCommand *cmd, g_autofree char *devstr = NULL; chardev_alias = qemuDomainGetVhostUserChrAlias(fs->info.alias); - chrdevstr = qemuBuildVHostUserFsChardevStr(fs, chardev_alias); + chrdevstr = qemuBuildVHostUserFsChardevStr(fs, chardev_alias, priv); virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, chrdevstr); diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index 98b546fc73..ef0b3f1981 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -196,10 +196,8 @@ qemuExtDevicesStart(virQEMUDriver *driver, for (i = 0; i < def->nfss; i++) { virDomainFSDef *fs = def->fss[i]; - if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) { - if (fs->sock) - QEMU_DOMAIN_FS_PRIVATE(fs)->vhostuser_fs_sock = g_strdup(fs->sock); - else if (qemuVirtioFSStart(logManager, driver, vm, fs) < 0) + if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS && !fs->sock) { + if (qemuVirtioFSStart(logManager, driver, vm, fs) < 0) return -1; } } diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c index 15c05479c8..08a8b4ed42 100644 --- a/src/qemu/qemu_virtiofs.c +++ b/src/qemu/qemu_virtiofs.c @@ -183,8 +183,7 @@ qemuVirtioFSStart(virLogManager *logManager, if (!(pidfile = qemuVirtioFSCreatePidFilename(vm, fs->info.alias))) goto cleanup; - if (!(socket_path = qemuVirtioFSCreateSocketFilename(vm, fs->info.alias))) - goto cleanup; + socket_path = qemuDomainGetVHostUserFSSocketPath(vm->privateData, fs); if ((fd = qemuVirtioFSOpenChardev(driver, vm, socket_path)) < 0) goto cleanup; @@ -251,12 +250,9 @@ qemuVirtioFSStart(virLogManager *logManager, goto error; } - QEMU_DOMAIN_FS_PRIVATE(fs)->vhostuser_fs_sock = g_steal_pointer(&socket_path); ret = 0; cleanup: - if (socket_path) - unlink(socket_path); return ret; error: @@ -264,6 +260,8 @@ qemuVirtioFSStart(virLogManager *logManager, virProcessKillPainfully(pid, true); if (pidfile) unlink(pidfile); + if (socket_path) + unlink(socket_path); goto cleanup; } @@ -284,8 +282,10 @@ qemuVirtioFSStop(virQEMUDriver *driver G_GNUC_UNUSED, if (virPidFileForceCleanupPathFull(pidfile, true) < 0) { VIR_WARN("Unable to kill virtiofsd process"); } else { - if (QEMU_DOMAIN_FS_PRIVATE(fs)->vhostuser_fs_sock) - unlink(QEMU_DOMAIN_FS_PRIVATE(fs)->vhostuser_fs_sock); + g_autofree char *socket_path = NULL; + + socket_path = qemuDomainGetVHostUserFSSocketPath(vm->privateData, fs); + unlink(socket_path); } cleanup: diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index e35380293c..7df3946751 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -436,18 +436,6 @@ testCompareXMLToArgvCreateArgs(virQEMUDriver *drv, } } - for (i = 0; i < vm->def->nfss; i++) { - virDomainFSDef *fs = vm->def->fss[i]; - char *s; - - if (fs->fsdriver != VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS || - QEMU_DOMAIN_FS_PRIVATE(fs)->vhostuser_fs_sock) - continue; - - s = g_strdup_printf("/tmp/lib/domain--1-guest/fs%zu-fs.sock", i); - QEMU_DOMAIN_FS_PRIVATE(fs)->vhostuser_fs_sock = s; - } - if (vm->def->vsock) { virDomainVsockDef *vsock = vm->def->vsock; qemuDomainVsockPrivate *vsockPriv = -- 2.31.1
On Wed, Oct 06, 2021 at 09:15:14 +0200, Ján Tomko wrote:
Reconstruct the socket path from priv->libDir in every user.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 8 +++++--- src/qemu/qemu_extdevice.c | 6 ++---- src/qemu/qemu_virtiofs.c | 14 +++++++------- tests/qemuxml2argvtest.c | 12 ------------ 4 files changed, 14 insertions(+), 26 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This reverts commit 801e6da29c0202946d44b42136cc4ee229932a29 They are not needed anymore. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_domain.c | 41 ----------------------------------------- src/qemu/qemu_domain.h | 11 ----------- 2 files changed, 52 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 5f467760c3..c4c79d979a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -983,46 +983,6 @@ qemuDomainNetworkPrivateDispose(void *obj G_GNUC_UNUSED) } -static virClass *qemuDomainFSPrivateClass; -static void qemuDomainFSPrivateDispose(void *obj); - - -static int -qemuDomainFSPrivateOnceInit(void) -{ - if (!VIR_CLASS_NEW(qemuDomainFSPrivate, virClassForObject())) - return -1; - - return 0; -} - - -VIR_ONCE_GLOBAL_INIT(qemuDomainFSPrivate); - - -static virObject * -qemuDomainFSPrivateNew(void) -{ - qemuDomainFSPrivate *priv; - - if (qemuDomainFSPrivateInitialize() < 0) - return NULL; - - if (!(priv = virObjectNew(qemuDomainFSPrivateClass))) - return NULL; - - return (virObject *) priv; -} - - -static void -qemuDomainFSPrivateDispose(void *obj) -{ - qemuDomainFSPrivate *priv = obj; - - g_free(priv->vhostuser_fs_sock); -} - static virClass *qemuDomainVideoPrivateClass; static void qemuDomainVideoPrivateDispose(void *obj); @@ -3163,7 +3123,6 @@ virDomainXMLPrivateDataCallbacks virQEMUDriverPrivateDataCallbacks = { .graphicsNew = qemuDomainGraphicsPrivateNew, .networkNew = qemuDomainNetworkPrivateNew, .videoNew = qemuDomainVideoPrivateNew, - .fsNew = qemuDomainFSPrivateNew, .parse = qemuDomainObjPrivateXMLParse, .format = qemuDomainObjPrivateXMLFormat, .getParseOpaque = qemuDomainObjPrivateXMLGetParseOpaque, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 59cb703c16..7e717cf2ad 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -392,17 +392,6 @@ struct _qemuDomainNetworkPrivate { }; -#define QEMU_DOMAIN_FS_PRIVATE(dev) \ - ((qemuDomainFSPrivate *) (dev)->privateData) - -typedef struct _qemuDomainFSPrivate qemuDomainFSPrivate; -struct _qemuDomainFSPrivate { - virObject parent; - - char *vhostuser_fs_sock; -}; - - typedef enum { QEMU_PROCESS_EVENT_WATCHDOG = 0, QEMU_PROCESS_EVENT_GUESTPANIC, -- 2.31.1
On Wed, Oct 06, 2021 at 09:15:15 +0200, Ján Tomko wrote:
This reverts commit 801e6da29c0202946d44b42136cc4ee229932a29
They are not needed anymore.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_domain.c | 41 ----------------------------------------- src/qemu/qemu_domain.h | 11 ----------- 2 files changed, 52 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Iterate through the array to find the first free index. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_alias.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index 81a1e7eeed..4153050bec 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -336,13 +336,23 @@ qemuAssignDeviceNetAlias(virDomainDef *def, static int -qemuAssignDeviceFSAlias(virDomainFSDef *fss, - int idx) +qemuAssignDeviceFSAlias(virDomainDef *def, + virDomainFSDef *fss) { + size_t i; + int maxidx = 0; + if (fss->info.alias) return 0; - fss->info.alias = g_strdup_printf("fs%d", idx); + for (i = 0; i < def->nfss; i++) { + int idx; + + if ((idx = qemuDomainDeviceAliasIndex(&def->fss[i]->info, "fs")) >= maxidx) + maxidx = idx + 1; + } + + fss->info.alias = g_strdup_printf("fs%d", maxidx); return 0; } @@ -634,7 +644,7 @@ qemuAssignDeviceAliases(virDomainDef *def, virQEMUCaps *qemuCaps) } for (i = 0; i < def->nfss; i++) { - if (qemuAssignDeviceFSAlias(def->fss[i], i) < 0) + if (qemuAssignDeviceFSAlias(def, def->fss[i]) < 0) return -1; } for (i = 0; i < def->nsounds; i++) { -- 2.31.1
On Wed, Oct 06, 2021 at 09:15:16 +0200, Ján Tomko wrote:
Iterate through the array to find the first free index.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_alias.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index 81a1e7eeed..4153050bec 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -336,13 +336,23 @@ qemuAssignDeviceNetAlias(virDomainDef *def,
static int -qemuAssignDeviceFSAlias(virDomainFSDef *fss, - int idx) +qemuAssignDeviceFSAlias(virDomainDef *def, + virDomainFSDef *fss) { + size_t i; + int maxidx = 0; + if (fss->info.alias) return 0;
- fss->info.alias = g_strdup_printf("fs%d", idx); + for (i = 0; i < def->nfss; i++) { + int idx; + + if ((idx = qemuDomainDeviceAliasIndex(&def->fss[i]->info, "fs")) >= maxidx) + maxidx = idx + 1; + } + + fss->info.alias = g_strdup_printf("fs%d", maxidx); return 0; }
@@ -634,7 +644,7 @@ qemuAssignDeviceAliases(virDomainDef *def, virQEMUCaps *qemuCaps) }
for (i = 0; i < def->nfss; i++) { - if (qemuAssignDeviceFSAlias(def->fss[i], i) < 0) + if (qemuAssignDeviceFSAlias(def, def->fss[i]) < 0) return -1;
Are other devices also n^2 during startup of the VM?
On a Wednesday in 2021, Peter Krempa wrote:
On Wed, Oct 06, 2021 at 09:15:16 +0200, Ján Tomko wrote:
Iterate through the array to find the first free index.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_alias.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index 81a1e7eeed..4153050bec 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -336,13 +336,23 @@ qemuAssignDeviceNetAlias(virDomainDef *def,
static int -qemuAssignDeviceFSAlias(virDomainFSDef *fss, - int idx) +qemuAssignDeviceFSAlias(virDomainDef *def, + virDomainFSDef *fss) { + size_t i; + int maxidx = 0; + if (fss->info.alias) return 0;
- fss->info.alias = g_strdup_printf("fs%d", idx); + for (i = 0; i < def->nfss; i++) { + int idx; + + if ((idx = qemuDomainDeviceAliasIndex(&def->fss[i]->info, "fs")) >= maxidx) + maxidx = idx + 1; + } + + fss->info.alias = g_strdup_printf("fs%d", maxidx); return 0; }
@@ -634,7 +644,7 @@ qemuAssignDeviceAliases(virDomainDef *def, virQEMUCaps *qemuCaps) }
for (i = 0; i < def->nfss; i++) { - if (qemuAssignDeviceFSAlias(def->fss[i], i) < 0) + if (qemuAssignDeviceFSAlias(def, def->fss[i]) < 0) return -1;
Are other devices also n^2 during startup of the VM?
For the alias assingment, many of the hotpluggable ones are. Don't know about the rest of the startup code. Jano
On Wed, Oct 06, 2021 at 10:27:48 +0200, Ján Tomko wrote:
On a Wednesday in 2021, Peter Krempa wrote:
On Wed, Oct 06, 2021 at 09:15:16 +0200, Ján Tomko wrote:
Iterate through the array to find the first free index.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_alias.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
[...]
@@ -634,7 +644,7 @@ qemuAssignDeviceAliases(virDomainDef *def, virQEMUCaps *qemuCaps) }
for (i = 0; i < def->nfss; i++) { - if (qemuAssignDeviceFSAlias(def->fss[i], i) < 0) + if (qemuAssignDeviceFSAlias(def, def->fss[i]) < 0) return -1;
Are other devices also n^2 during startup of the VM?
For the alias assingment, many of the hotpluggable ones are.
Sigh. Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Other devices (includes 9p-based fsdev) call this wrapper before formatting the device. Add it here too. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index dc4f91ce25..8c8aafb13d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2095,6 +2095,9 @@ qemuBuildVHostUserFsCommandLine(virCommand *cmd, virCommandAddArg(cmd, "-chardev"); virCommandAddArg(cmd, chrdevstr); + if (qemuCommandAddExtDevice(cmd, &fs->info) < 0) + return -1; + if (!(devstr = qemuBuildVHostUserFsDevStr(fs, def, chardev_alias, priv))) return -1; -- 2.31.1
On Wed, Oct 06, 2021 at 09:15:17 +0200, Ján Tomko wrote:
Other devices (includes 9p-based fsdev) call this wrapper before formatting the device.
Add it here too.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 3 +++ 1 file changed, 3 insertions(+)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Prepare for hotplug support. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_alias.c | 2 +- src/qemu/qemu_alias.h | 4 ++++ src/qemu/qemu_command.c | 2 +- src/qemu/qemu_command.h | 6 ++++++ src/qemu/qemu_hotplug.h | 4 ++++ 5 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index 4153050bec..276a03cb56 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -335,7 +335,7 @@ qemuAssignDeviceNetAlias(virDomainDef *def, } -static int +int qemuAssignDeviceFSAlias(virDomainDef *def, virDomainFSDef *fss) { diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h index cfce05833d..604e667b9a 100644 --- a/src/qemu/qemu_alias.h +++ b/src/qemu/qemu_alias.h @@ -46,6 +46,10 @@ int qemuAssignDeviceNetAlias(virDomainDef *def, virDomainNetDef *net, int idx); +int +qemuAssignDeviceFSAlias(virDomainDef *def, + virDomainFSDef *fss); + int qemuAssignDeviceRedirdevAlias(virDomainDef *def, virDomainRedirdevDef *redirdev, int idx); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8c8aafb13d..28bca1519c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2049,7 +2049,7 @@ qemuBuildVHostUserFsChardevStr(const virDomainFSDef *fs, } -static char * +char * qemuBuildVHostUserFsDevStr(virDomainFSDef *fs, const virDomainDef *def, const char *chardev_alias, diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index a0e6af9d3f..6b8f90f737 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -144,6 +144,12 @@ char virDomainDiskDef *disk, virQEMUCaps *qemuCaps); +char * +qemuBuildVHostUserFsDevStr(virDomainFSDef *fs, + const virDomainDef *def, + const char *chardev_alias, + qemuDomainObjPrivate *priv); + /* Current, best practice */ int qemuBuildControllerDevStr(const virDomainDef *domainDef, virDomainControllerDef *def, diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 9f383f4602..244dd5278d 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -107,6 +107,10 @@ int qemuDomainAttachInputDevice(virQEMUDriver *driver, int qemuDomainAttachVsockDevice(virQEMUDriver *driver, virDomainObj *vm, virDomainVsockDef *vsock); +int +qemuDomainAttachFSDevice(virQEMUDriver *driver, + virDomainObj *vm, + virDomainFSDef *fs); int qemuDomainAttachLease(virQEMUDriver *driver, virDomainObj *vm, -- 2.31.1
On Wed, Oct 06, 2021 at 09:15:18 +0200, Ján Tomko wrote:
Prepare for hotplug support.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_alias.c | 2 +- src/qemu/qemu_alias.h | 4 ++++ src/qemu/qemu_command.c | 2 +- src/qemu/qemu_command.h | 6 ++++++ src/qemu/qemu_hotplug.h | 4 ++++ 5 files changed, 16 insertions(+), 2 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
We might need it later for device hotplug. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_domain.h | 11 +++++++---- src/qemu/qemu_process.c | 5 ++++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 7e717cf2ad..fef6a2f39f 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -101,6 +101,9 @@ struct _qemuDomainSecretInfo { char *ciphertext; /* encoded/encrypted secret */ }; +#define QEMU_TYPE_DOMAIN_LOG_CONTEXT qemu_domain_log_context_get_type() +G_DECLARE_FINAL_TYPE(qemuDomainLogContext, qemu_domain_log_context, QEMU, DOMAIN_LOG_CONTEXT, GObject); + typedef struct _qemuDomainObjPrivate qemuDomainObjPrivate; struct _qemuDomainObjPrivate { virQEMUDriver *driver; @@ -193,6 +196,10 @@ struct _qemuDomainObjPrivate { * provided by QEMU. */ virCPUDef *origCPU; + /* Log context. After startup, it is also used for hotplugging devices + * with a log file */ + qemuDomainLogContext *logCtxt; + /* If true virtlogd is used as stdio handler for character devices. */ bool chardevStdioLogd; @@ -418,10 +425,6 @@ struct qemuProcessEvent { }; void qemuProcessEventFree(struct qemuProcessEvent *event); - -#define QEMU_TYPE_DOMAIN_LOG_CONTEXT qemu_domain_log_context_get_type() -G_DECLARE_FINAL_TYPE(qemuDomainLogContext, qemu_domain_log_context, QEMU, DOMAIN_LOG_CONTEXT, GObject); - typedef struct _qemuDomainSaveCookie qemuDomainSaveCookie; struct _qemuDomainSaveCookie { virObject parent; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1d0165af6d..4a1fd753ee 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7179,7 +7179,7 @@ qemuProcessLaunch(virConnectPtr conn, int ret = -1; int rv; int logfile = -1; - g_autoptr(qemuDomainLogContext) logCtxt = NULL; + qemuDomainLogContext *logCtxt = NULL; qemuDomainObjPrivate *priv = vm->privateData; g_autoptr(virCommand) cmd = NULL; struct qemuProcessHookData hookData; @@ -7233,6 +7233,7 @@ qemuProcessLaunch(virConnectPtr conn, virLastErrorPrefixMessage("%s", _("can't connect to virtlogd")); goto cleanup; } + priv->logCtxt = logCtxt; logfile = qemuDomainLogContextGetWriteFD(logCtxt); if (qemuProcessGenID(vm, flags) < 0) @@ -7966,6 +7967,8 @@ void qemuProcessStop(virQEMUDriver *driver, qemuDBusStop(driver, vm); + g_clear_object(&priv->logCtxt); + vm->def->id = -1; /* Wake up anything waiting on domain condition */ -- 2.31.1
On Wed, Oct 06, 2021 at 09:15:19 +0200, Ján Tomko wrote:
We might need it later for device hotplug.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_domain.h | 11 +++++++---- src/qemu/qemu_process.c | 5 ++++- 2 files changed, 11 insertions(+), 5 deletions(-)
So from the code it looks like we will keep a connection to virtlogd open for the lifetime of the VM rather than we do now for the startup of the VM. The commit message doesn't justify or mention this and that is IMO a substantial change which could have consequences. Additionally there's no code whic would re-create the context on libvirtd restart, so even the 'might need it later for device hotplug' would not work properly.
Remove the local variable in favor of the one stored in priv. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_process.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 4a1fd753ee..17435c0ee9 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7179,7 +7179,6 @@ qemuProcessLaunch(virConnectPtr conn, int ret = -1; int rv; int logfile = -1; - qemuDomainLogContext *logCtxt = NULL; qemuDomainObjPrivate *priv = vm->privateData; g_autoptr(virCommand) cmd = NULL; struct qemuProcessHookData hookData; @@ -7228,25 +7227,24 @@ qemuProcessLaunch(virConnectPtr conn, hookData.cfg = cfg; VIR_DEBUG("Creating domain log file"); - if (!(logCtxt = qemuDomainLogContextNew(driver, vm, + if (!(priv->logCtxt = qemuDomainLogContextNew(driver, vm, QEMU_DOMAIN_LOG_CONTEXT_MODE_START))) { virLastErrorPrefixMessage("%s", _("can't connect to virtlogd")); goto cleanup; } - priv->logCtxt = logCtxt; - logfile = qemuDomainLogContextGetWriteFD(logCtxt); + logfile = qemuDomainLogContextGetWriteFD(priv->logCtxt); if (qemuProcessGenID(vm, flags) < 0) goto cleanup; if (qemuExtDevicesStart(driver, vm, - qemuDomainLogContextGetManager(logCtxt), + qemuDomainLogContextGetManager(priv->logCtxt), incoming != NULL) < 0) goto cleanup; VIR_DEBUG("Building emulator command line"); if (!(cmd = qemuBuildCommandLine(driver, - qemuDomainLogContextGetManager(logCtxt), + qemuDomainLogContextGetManager(priv->logCtxt), driver->securityManager, vm, incoming ? incoming->launchURI : NULL, @@ -7265,11 +7263,11 @@ qemuProcessLaunch(virConnectPtr conn, VIR_HOOK_SUBOP_BEGIN) < 0) goto cleanup; - qemuLogOperation(vm, "starting up", cmd, logCtxt); + qemuLogOperation(vm, "starting up", cmd, priv->logCtxt); - qemuDomainObjCheckTaint(driver, vm, logCtxt, incoming != NULL); + qemuDomainObjCheckTaint(driver, vm, priv->logCtxt, incoming != NULL); - qemuDomainLogContextMarkPosition(logCtxt); + qemuDomainLogContextMarkPosition(priv->logCtxt); VIR_DEBUG("Building mount namespace"); @@ -7343,7 +7341,7 @@ qemuProcessLaunch(virConnectPtr conn, VIR_DEBUG("Waiting for handshake from child"); if (virCommandHandshakeWait(cmd) < 0) { /* Read errors from child that occurred between fork and exec. */ - qemuProcessReportLogError(logCtxt, + qemuProcessReportLogError(priv->logCtxt, _("Process exited prior to exec")); goto cleanup; } @@ -7423,7 +7421,7 @@ qemuProcessLaunch(virConnectPtr conn, goto cleanup; VIR_DEBUG("Waiting for monitor to show up"); - if (qemuProcessWaitForMonitor(driver, vm, asyncJob, logCtxt) < 0) + if (qemuProcessWaitForMonitor(driver, vm, asyncJob, priv->logCtxt) < 0) goto cleanup; if (qemuConnectAgent(driver, vm) < 0) -- 2.31.1
On Wed, Oct 06, 2021 at 09:15:20 +0200, Ján Tomko wrote:
Remove the local variable in favor of the one stored in priv.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_process.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 4a1fd753ee..17435c0ee9 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c
[...]
if (qemuProcessGenID(vm, flags) < 0) goto cleanup;
if (qemuExtDevicesStart(driver, vm, - qemuDomainLogContextGetManager(logCtxt), + qemuDomainLogContextGetManager(priv->logCtxt),
I'm not a fan of making all the supporting processes log into the same log file ...
incoming != NULL) < 0) goto cleanup;
VIR_DEBUG("Building emulator command line"); if (!(cmd = qemuBuildCommandLine(driver, - qemuDomainLogContextGetManager(logCtxt), + qemuDomainLogContextGetManager(priv->logCtxt), driver->securityManager, vm,
.. as qemu itself, but this is prior art.
incoming ? incoming->launchURI : NULL,
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
On Wed, Oct 06, 2021 at 09:15:20 +0200, Ján Tomko wrote:
Remove the local variable in favor of the one stored in priv.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_process.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 4a1fd753ee..17435c0ee9 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c
[...]
@@ -7228,25 +7227,24 @@ qemuProcessLaunch(virConnectPtr conn, hookData.cfg = cfg;
VIR_DEBUG("Creating domain log file"); - if (!(logCtxt = qemuDomainLogContextNew(driver, vm, + if (!(priv->logCtxt = qemuDomainLogContextNew(driver, vm, QEMU_DOMAIN_LOG_CONTEXT_MODE_START))) {a
Broken indentation.
https://bugzilla.redhat.com/show_bug.cgi?id=1897708 Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_driver.c | 9 +++- src/qemu/qemu_hotplug.c | 96 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 760d30a867..65b2ef8e86 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6945,8 +6945,15 @@ qemuDomainAttachDeviceLive(virDomainObj *vm, } break; - case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_FS: + ret = qemuDomainAttachFSDevice(driver, vm, dev->data.fs); + if (ret == 0) { + alias = dev->data.fs->info.alias; + dev->data.fs = NULL; + } + break; + + case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_SOUND: case VIR_DOMAIN_DEVICE_VIDEO: case VIR_DOMAIN_DEVICE_GRAPHICS: diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 22239b2689..f5dad0b829 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -35,6 +35,7 @@ #include "qemu_security.h" #include "qemu_block.h" #include "qemu_snapshot.h" +#include "qemu_virtiofs.h" #include "domain_audit.h" #include "netdev_bandwidth_conf.h" #include "domain_nwfilter.h" @@ -3396,6 +3397,101 @@ qemuDomainAttachVsockDevice(virQEMUDriver *driver, } +int +qemuDomainAttachFSDevice(virQEMUDriver *driver, + virDomainObj *vm, + virDomainFSDef *fs) +{ + qemuDomainObjPrivate *priv = vm->privateData; + virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_FS, + { .fs = fs } }; + g_autoptr(virDomainChrSourceDef) chardev = NULL; + virErrorPtr origErr = NULL; + bool releaseaddr = false; + bool chardevAdded = false; + bool started = false; + g_autofree char *devstr = NULL; + g_autofree char *charAlias = NULL; + int ret = -1; + + if (fs->fsdriver != VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("only virtiofs filesystems can be hotplugged")); + return -1; + } + + if (qemuDomainEnsureVirtioAddress(&releaseaddr, vm, &dev) < 0) + return -1; + + if (qemuAssignDeviceFSAlias(vm->def, fs) < 0) + goto cleanup; + + chardev = qemuDomainGetVHostUserChrSourceDef(priv, fs); + charAlias = qemuDomainGetVhostUserChrAlias(fs->info.alias); + + if (!(devstr = qemuBuildVHostUserFsDevStr(fs, vm->def, charAlias, priv))) + goto cleanup; + + if (!fs->sock) { + if (qemuVirtioFSPrepareDomain(driver, fs) < 0) + goto cleanup; + + if (qemuVirtioFSStart(qemuDomainLogContextGetManager(priv->logCtxt), driver, vm, fs) < 0) + goto cleanup; + started = true; + + if (qemuVirtioFSSetupCgroup(vm, fs, priv->cgroup) < 0) + goto cleanup; + } + + qemuDomainObjEnterMonitor(driver, vm); + + if (qemuMonitorAttachCharDev(priv->mon, charAlias, chardev) < 0) + goto exit_monitor; + chardevAdded = true; + + if (qemuDomainAttachExtensionDevice(priv->mon, &fs->info) < 0) + goto exit_monitor; + + if (qemuMonitorAddDevice(priv->mon, devstr) < 0) { + ignore_value(qemuDomainDetachExtensionDevice(priv->mon, &fs->info)); + goto exit_monitor; + } + + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + releaseaddr = false; + goto cleanup; + } + + VIR_APPEND_ELEMENT(vm->def->fss, vm->def->nfss, fs); + + ret = 0; + + audit: + virDomainAuditFS(vm, NULL, fs, "attach", ret == 0); + cleanup: + if (ret < 0) { + virErrorPreserveLast(&origErr); + if (releaseaddr) + qemuDomainReleaseDeviceAddress(vm, &fs->info); + if (started) + qemuVirtioFSStop(driver, vm, fs); + virErrorRestore(&origErr); + } + + return ret; + + exit_monitor: + virErrorPreserveLast(&origErr); + if (chardevAdded) + ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias)); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + releaseaddr = false; + virErrorRestore(&origErr); + goto audit; +} + + int qemuDomainAttachLease(virQEMUDriver *driver, virDomainObj *vm, -- 2.31.1
On Wed, Oct 06, 2021 at 09:15:21 +0200, Ján Tomko wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1897708
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_driver.c | 9 +++- src/qemu/qemu_hotplug.c | 96 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 1 deletion(-)
Preliminary note. The tree fails to compile after this commit: ../../../libvirt/src/qemu/qemu_hotplug.c:3401:1: error: no previous prototype for ‘qemuDomainAttachFSDevice’ [-Werror=missing-prototypes] 3401 | qemuDomainAttachFSDevice(virQEMUDriver *driver, | ^~~~~~~~~~~~~~~~~~~~~~~~ ../../../libvirt/src/qemu/qemu_hotplug.c: In function ‘qemuDomainAttachFSDevice’: ../../../libvirt/src/qemu/qemu_hotplug.c:3426:9: error: implicit declaration of function ‘qemuAssignDeviceFSAlias’; did you mean ‘qemuAssignDeviceRNGAlias’? [-Werror=implicit-function-declaration] 3426 | if (qemuAssignDeviceFSAlias(vm->def, fs) < 0) | ^~~~~~~~~~~~~~~~~~~~~~~ | qemuAssignDeviceRNGAlias ../../../libvirt/src/qemu/qemu_hotplug.c:3426:9: error: nested extern declaration of ‘qemuAssignDeviceFSAlias’ [-Werror=nested-externs] ../../../libvirt/src/qemu/qemu_hotplug.c:3432:20: error: implicit declaration of function ‘qemuBuildVHostUserFsDevStr’; did you mean ‘qemuBuildUSBHostdevDevStr’? [-Werror=implicit-function-declaration] 3432 | if (!(devstr = qemuBuildVHostUserFsDevStr(fs, vm->def, charAlias, priv))) | ^~~~~~~~~~~~~~~~~~~~~~~~~~ | qemuBuildUSBHostdevDevStr ../../../libvirt/src/qemu/qemu_hotplug.c:3432:20: error: nested extern declaration of ‘qemuBuildVHostUserFsDevStr’ [-Werror=nested-externs] ../../../libvirt/src/qemu/qemu_hotplug.c:3432:18: error: assignment to ‘char *’ from ‘int’ makes pointer from integer without a cast [-Werror=int-conversion] 3432 | if (!(devstr = qemuBuildVHostUserFsDevStr(fs, vm->def, charAlias, priv))) | ^ cc1: all warnings being treated as errors
On Wed, Oct 06, 2021 at 09:36:46 +0200, Peter Krempa wrote:
On Wed, Oct 06, 2021 at 09:15:21 +0200, Ján Tomko wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1897708
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_driver.c | 9 +++- src/qemu/qemu_hotplug.c | 96 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 1 deletion(-)
Preliminary note. The tree fails to compile after this commit:
../../../libvirt/src/qemu/qemu_hotplug.c:3401:1: error: no previous prototype for ‘qemuDomainAttachFSDevice’ [-Werror=missing-prototypes] 3401 | qemuDomainAttachFSDevice(virQEMUDriver *driver, | ^~~~~~~~~~~~~~~~~~~~~~~~ ../../../libvirt/src/qemu/qemu_hotplug.c: In function ‘qemuDomainAttachFSDevice’:
Never mind. I was lacking one patch, everything is okay.
On Wed, Oct 06, 2021 at 09:15:21 +0200, Ján Tomko wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1897708
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_driver.c | 9 +++- src/qemu/qemu_hotplug.c | 96 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 1 deletion(-)
[...]
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 22239b2689..f5dad0b829 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c
[...]
@@ -3396,6 +3397,101 @@ qemuDomainAttachVsockDevice(virQEMUDriver *driver, }
+int +qemuDomainAttachFSDevice(virQEMUDriver *driver, + virDomainObj *vm, + virDomainFSDef *fs) +{ + qemuDomainObjPrivate *priv = vm->privateData; + virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_FS, + { .fs = fs } }; + g_autoptr(virDomainChrSourceDef) chardev = NULL; + virErrorPtr origErr = NULL; + bool releaseaddr = false; + bool chardevAdded = false; + bool started = false; + g_autofree char *devstr = NULL; + g_autofree char *charAlias = NULL; + int ret = -1; + + if (fs->fsdriver != VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("only virtiofs filesystems can be hotplugged")); + return -1; + } + + if (qemuDomainEnsureVirtioAddress(&releaseaddr, vm, &dev) < 0) + return -1; + + if (qemuAssignDeviceFSAlias(vm->def, fs) < 0) + goto cleanup; + + chardev = qemuDomainGetVHostUserChrSourceDef(priv, fs); + charAlias = qemuDomainGetVhostUserChrAlias(fs->info.alias); + + if (!(devstr = qemuBuildVHostUserFsDevStr(fs, vm->def, charAlias, priv))) + goto cleanup; + + if (!fs->sock) { + if (qemuVirtioFSPrepareDomain(driver, fs) < 0) + goto cleanup; + + if (qemuVirtioFSStart(qemuDomainLogContextGetManager(priv->logCtxt), driver, vm, fs) < 0) + goto cleanup; + started = true;
As noted, this won't work after restart of libvirtd.
+ + if (qemuVirtioFSSetupCgroup(vm, fs, priv->cgroup) < 0) + goto cleanup; + } + + qemuDomainObjEnterMonitor(driver, vm); + + if (qemuMonitorAttachCharDev(priv->mon, charAlias, chardev) < 0) + goto exit_monitor; + chardevAdded = true; + + if (qemuDomainAttachExtensionDevice(priv->mon, &fs->info) < 0) + goto exit_monitor; + + if (qemuMonitorAddDevice(priv->mon, devstr) < 0) { + ignore_value(qemuDomainDetachExtensionDevice(priv->mon, &fs->info)); + goto exit_monitor; + } + + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + releaseaddr = false; + goto cleanup; + } + + VIR_APPEND_ELEMENT(vm->def->fss, vm->def->nfss, fs);
This clears 'fs' ...
+ + ret = 0; + + audit: + virDomainAuditFS(vm, NULL, fs, "attach", ret == 0);
... but you reference it here.
+ cleanup: + if (ret < 0) { + virErrorPreserveLast(&origErr); + if (releaseaddr) + qemuDomainReleaseDeviceAddress(vm, &fs->info); + if (started) + qemuVirtioFSStop(driver, vm, fs); + virErrorRestore(&origErr); + } + + return ret; + + exit_monitor: + virErrorPreserveLast(&origErr); + if (chardevAdded) + ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias)); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + releaseaddr = false; + virErrorRestore(&origErr); + goto audit; +}
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.c | 24 ++++++++++++ src/conf/domain_conf.h | 2 + src/libvirt_private.syms | 1 + src/qemu/qemu_hotplug.c | 81 +++++++++++++++++++++++++++++++++++++++- 4 files changed, 106 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b8370f6950..057dbf91a5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -28829,6 +28829,30 @@ virDomainFSRemove(virDomainDef *def, size_t i) return fs; } +ssize_t +virDomainFSDefFind(virDomainDef *def, + virDomainFSDef *fs) +{ + size_t i = 0; + + for (i = 0; i < def->nfss; i++) { + virDomainFSDef *tmp = def->fss[i]; + + if (fs->dst && STRNEQ_NULLABLE(fs->dst, tmp->dst)) + continue; + + if (fs->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + !virDomainDeviceInfoAddressIsEqual(&fs->info, &tmp->info)) + continue; + + if (fs->info.alias && STRNEQ_NULLABLE(fs->info.alias, tmp->info.alias)) + continue; + + return i; + } + return -1; +} + virDomainFSDef * virDomainGetFilesystemForTarget(virDomainDef *def, const char *target) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c23c233184..1fcef7b0e1 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3757,6 +3757,8 @@ virDomainFSDef *virDomainGetFilesystemForTarget(virDomainDef *def, int virDomainFSInsert(virDomainDef *def, virDomainFSDef *fs); int virDomainFSIndexByName(virDomainDef *def, const char *name); virDomainFSDef *virDomainFSRemove(virDomainDef *def, size_t i); +ssize_t virDomainFSDefFind(virDomainDef *def, + virDomainFSDef *fs); unsigned int virDomainVideoDefaultRAM(const virDomainDef *def, const virDomainVideoType type); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fd0eea0777..687a3bbc4c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -410,6 +410,7 @@ virDomainFeatureTypeFromString; virDomainFeatureTypeToString; virDomainFSAccessModeTypeToString; virDomainFSCacheModeTypeToString; +virDomainFSDefFind; virDomainFSDefFree; virDomainFSDefNew; virDomainFSDriverTypeToString; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f5dad0b829..713da5ebfc 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -5180,6 +5180,45 @@ qemuDomainRemoveRedirdevDevice(virQEMUDriver *driver, } +static int +qemuDomainRemoveFSDevice(virQEMUDriver *driver, + virDomainObj *vm, + virDomainFSDef *fs) +{ + g_autofree char *charAlias = NULL; + qemuDomainObjPrivate *priv = vm->privateData; + ssize_t idx; + int rc = 0; + + VIR_DEBUG("Removing FS device %s from domain %p %s", + fs->info.alias, vm, vm->def->name); + + charAlias = qemuDomainGetVhostUserChrAlias(fs->info.alias); + + qemuDomainObjEnterMonitor(driver, vm); + + if (qemuMonitorDetachCharDev(priv->mon, charAlias) < 0) + rc = -1; + + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return -1; + + virDomainAuditFS(vm, fs, NULL, "detach", rc == 0); + + if (rc < 0) + return -1; + + if (!fs->sock) + qemuVirtioFSStop(driver, vm, fs); + + if ((idx = virDomainFSDefFind(vm->def, fs)) >= 0) + virDomainFSRemove(vm->def, idx); + qemuDomainReleaseDeviceAddress(vm, &fs->info); + virDomainFSDefFree(fs); + return 0; +} + + static void qemuDomainRemoveAuditDevice(virDomainObj *vm, virDomainDeviceDef *detach, @@ -5218,6 +5257,10 @@ qemuDomainRemoveAuditDevice(virDomainObj *vm, virDomainAuditRedirdev(vm, detach->data.redirdev, "detach", success); break; + case VIR_DOMAIN_DEVICE_FS: + virDomainAuditFS(vm, detach->data.fs, NULL, "detach", success); + break; + case VIR_DOMAIN_DEVICE_LEASE: case VIR_DOMAIN_DEVICE_CONTROLLER: case VIR_DOMAIN_DEVICE_WATCHDOG: @@ -5225,7 +5268,6 @@ qemuDomainRemoveAuditDevice(virDomainObj *vm, /* These devices don't have associated audit logs */ break; - case VIR_DOMAIN_DEVICE_FS: case VIR_DOMAIN_DEVICE_SOUND: case VIR_DOMAIN_DEVICE_VIDEO: case VIR_DOMAIN_DEVICE_GRAPHICS: @@ -5323,9 +5365,13 @@ qemuDomainRemoveDevice(virQEMUDriver *driver, return -1; break; + case VIR_DOMAIN_DEVICE_FS: + if (qemuDomainRemoveFSDevice(driver, vm, dev->data.fs) < 0) + return -1; + break; + case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_LEASE: - case VIR_DOMAIN_DEVICE_FS: case VIR_DOMAIN_DEVICE_SOUND: case VIR_DOMAIN_DEVICE_VIDEO: case VIR_DOMAIN_DEVICE_GRAPHICS: @@ -6020,6 +6066,31 @@ qemuDomainDetachPrepVsock(virDomainObj *vm, } +static int +qemuDomainDetachPrepFS(virDomainObj *vm, + virDomainFSDef *match, + virDomainFSDef **detach) +{ + ssize_t idx; + + if ((idx = virDomainFSDefFind(vm->def, match)) < 0) { + virReportError(VIR_ERR_DEVICE_MISSING, "%s", + _("matching filesystem not found")); + return -1; + } + + if (vm->def->fss[idx]->fsdriver != VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("only virtiofs filesystems can be hotplugged")); + return -1; + } + + *detach = vm->def->fss[idx]; + + return 0; +} + + static int qemuDomainDetachDeviceLease(virQEMUDriver *driver, virDomainObj *vm, @@ -6141,6 +6212,12 @@ qemuDomainDetachDeviceLive(virDomainObj *vm, break; case VIR_DOMAIN_DEVICE_FS: + if (qemuDomainDetachPrepFS(vm, match->data.fs, + &detach.data.fs) < 0) { + return -1; + } + break; + case VIR_DOMAIN_DEVICE_SOUND: case VIR_DOMAIN_DEVICE_VIDEO: case VIR_DOMAIN_DEVICE_GRAPHICS: -- 2.31.1
On Wed, Oct 06, 2021 at 09:15:22 +0200, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.c | 24 ++++++++++++ src/conf/domain_conf.h | 2 + src/libvirt_private.syms | 1 + src/qemu/qemu_hotplug.c | 81 +++++++++++++++++++++++++++++++++++++++- 4 files changed, 106 insertions(+), 2 deletions(-)
[...]
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f5dad0b829..713da5ebfc 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -5180,6 +5180,45 @@ qemuDomainRemoveRedirdevDevice(virQEMUDriver *driver, }
+static int +qemuDomainRemoveFSDevice(virQEMUDriver *driver, + virDomainObj *vm, + virDomainFSDef *fs) +{ + g_autofree char *charAlias = NULL; + qemuDomainObjPrivate *priv = vm->privateData; + ssize_t idx; + int rc = 0; + + VIR_DEBUG("Removing FS device %s from domain %p %s", + fs->info.alias, vm, vm->def->name); + + charAlias = qemuDomainGetVhostUserChrAlias(fs->info.alias); + + qemuDomainObjEnterMonitor(driver, vm); + + if (qemuMonitorDetachCharDev(priv->mon, charAlias) < 0) + rc = -1;
Note that you must not assume here that the check below [1] that allows only virtio-fs device unplug guards this code too. The *Remove* code is executed always when we get a 'DEVICE_DELETED' event from qemu thus also for guest-initiated unplug, which theoretically can happen also for p9fs device.
+ + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return -1; + + virDomainAuditFS(vm, fs, NULL, "detach", rc == 0); + + if (rc < 0) + return -1; + + if (!fs->sock) + qemuVirtioFSStop(driver, vm, fs); + + if ((idx = virDomainFSDefFind(vm->def, fs)) >= 0) + virDomainFSRemove(vm->def, idx); + qemuDomainReleaseDeviceAddress(vm, &fs->info); + virDomainFSDefFree(fs); + return 0; +} + + static void qemuDomainRemoveAuditDevice(virDomainObj *vm, virDomainDeviceDef *detach,
[...]
@@ -6020,6 +6066,31 @@ qemuDomainDetachPrepVsock(virDomainObj *vm, }
+static int +qemuDomainDetachPrepFS(virDomainObj *vm, + virDomainFSDef *match, + virDomainFSDef **detach) +{ + ssize_t idx; + + if ((idx = virDomainFSDefFind(vm->def, match)) < 0) { + virReportError(VIR_ERR_DEVICE_MISSING, "%s", + _("matching filesystem not found")); + return -1; + } + + if (vm->def->fss[idx]->fsdriver != VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("only virtiofs filesystems can be hotplugged")); + return -1; + }
[1]. This check only guards host-initiated delete.
+ + *detach = vm->def->fss[idx]; + + return 0; +} + + static int qemuDomainDetachDeviceLease(virQEMUDriver *driver, virDomainObj *vm,
participants (2)
-
Ján Tomko -
Peter Krempa