[PATCH 0/2] qemu_hotplug: Fix two issues with chardev hotplug

And while at it, make the code 5 lines shorter. Michal Prívozník (2): qemu_hotplug: Close FDs in QEMU on failed chardev hotplug qemu_hotplug: Create chardev files before attempting to relabel them src/qemu/qemu_hotplug.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) -- 2.35.1

When hotplugging a chardev, Libvirt opens corresponding file/binds to a socket/does whatever necessary to obtain an FD that is later passed to QEMU. However, if something fails after the FDs were transferred to QEMU and before chardev is actually added via monitor, these FDs are never closed in QEMU. This is rather suboptimal. Fixes: 15bdced9b3d0b86a48506bfb1c27d6b2d5377dc2 Fixes: ad81aa8ad07e52c9bd4840de84d2ed59998b4d2a Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_hotplug.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 27e68370cf..fac893c80e 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2129,17 +2129,6 @@ qemuDomainAttachChrDevice(virQEMUDriver *driver, if (qemuProcessPrepareHostBackendChardevHotplug(vm, dev) < 0) goto cleanup; - if (charpriv->sourcefd || charpriv->logfd || charpriv->directfd) { - qemuDomainObjEnterMonitor(driver, vm); - - if (qemuFDPassTransferMonitor(charpriv->sourcefd, priv->mon) < 0 || - qemuFDPassTransferMonitor(charpriv->logfd, priv->mon) < 0 || - qemuFDPassDirectTransferMonitor(charpriv->directfd, priv->mon) < 0) - goto exit_monitor; - - qemuDomainObjExitMonitor(vm); - } - if (guestfwd) { if (!(netdevprops = qemuBuildChannelGuestfwdNetdevProps(chr))) goto cleanup; @@ -2161,6 +2150,11 @@ qemuDomainAttachChrDevice(virQEMUDriver *driver, qemuDomainObjEnterMonitor(driver, vm); + if (qemuFDPassTransferMonitor(charpriv->sourcefd, priv->mon) < 0 || + qemuFDPassTransferMonitor(charpriv->logfd, priv->mon) < 0 || + qemuFDPassDirectTransferMonitor(charpriv->directfd, priv->mon) < 0) + goto exit_monitor; + if (qemuHotplugChardevAttach(priv->mon, charAlias, chr->source) < 0) goto exit_monitor; chardevAttached = true; @@ -2206,6 +2200,7 @@ qemuDomainAttachChrDevice(virQEMUDriver *driver, qemuMonitorDetachCharDev(priv->mon, charAlias); qemuFDPassTransferMonitorRollback(charpriv->sourcefd, priv->mon); qemuFDPassTransferMonitorRollback(charpriv->logfd, priv->mon); + qemuFDPassDirectTransferMonitorRollback(charpriv->directfd, priv->mon); qemuDomainObjExitMonitor(vm); virErrorRestore(&orig_err); -- 2.35.1

On Mon, Jul 18, 2022 at 4:50 PM Michal Privoznik <mprivozn@redhat.com> wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_hotplug.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-)
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>

When hotplugging a chardev, Libvirt opens corresponding file/binds to a socket/does whatever necessary to obtain an FD that is later passed to QEMU. However, due to wrong placement of the function that does all of this (qemuProcessPrepareHostBackendChardevHotplug()) it may happen that a file is set seclabel on, only to be unlink()-ed and created again (the former is done by qemuSecuritySetChardevLabel(), the latter by aforementioned function). The unlink()-ing is done for UNIX sockets with mode='bind' and happens inside qemuOpenChrChardevUNIXSocket(). However, these steps can be swapped simply. Fixes: ad81aa8ad07e52c9bd4840de84d2ed59998b4d2a Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_hotplug.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index fac893c80e..38a010423f 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2115,6 +2115,9 @@ qemuDomainAttachChrDevice(virQEMUDriver *driver, if (qemuDomainAttachChrDeviceAssignAddr(vm, chr, &need_release) < 0) goto cleanup; + if (qemuProcessPrepareHostBackendChardevHotplug(vm, dev) < 0) + goto cleanup; + if (qemuDomainNamespaceSetupChardev(vm, chr, &teardowndevice) < 0) goto cleanup; @@ -2126,9 +2129,6 @@ qemuDomainAttachChrDevice(virQEMUDriver *driver, goto cleanup; teardowncgroup = true; - if (qemuProcessPrepareHostBackendChardevHotplug(vm, dev) < 0) - goto cleanup; - if (guestfwd) { if (!(netdevprops = qemuBuildChannelGuestfwdNetdevProps(chr))) goto cleanup; -- 2.35.1

On Mon, Jul 18, 2022 at 4:50 PM Michal Privoznik <mprivozn@redhat.com> wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_hotplug.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
participants (2)
-
Kristina Hanicova
-
Michal Privoznik