On Fri, Feb 19, 2021 at 09:49:36AM -0500, Stefan Berger wrote:
This patch partially reverts commit 5cde9dee where the
qemuExtDevicesStop()
was moved to a location before the QEMU process is stopped. It may be
alright to tear down some devices before QEMU is stopped, but it doesn't work
for the external TPM (swtpm) which assumes that QEMU sends it a signal to stop
it before libvirt may try to clean it up. So this patch introduces a function
qemuExtDevicesStopEarly() to accommodate those devices that can/must
be torn down early, which seems to be the case with virtio and its virtiofsd,
and recreates the location of the call to qemuExtDevicesStop() from before the
regression for all other devices.
I don't think splitting the cleanup like this is a good idea.
Also I think 5cde9dee was reall wrong, because we shouldn't tear
down /any/ external processes while QEMU itself still exists.
The comit message for 5cde9dee talks aboutneeding to run before
virFileDeleteTree(priv->libDir) which makes sense. The solution
was wrong though IMHO.
We shouldn't be calling virFileDeleteTree() until all the processes
are stopped.
So we should revert 5cde9dee entirely, and then move the
virFileDeleteTree calls further to the end of qemuProcessStop
Fixes: 5cde9dee8c70b17c458d031ab6cf71dce476eea2
Cc: Masayoshi Mizuma <m.mizuma(a)jp.fujitsu.com>
Signed-off-by: Stefan Berger <stefanb(a)linux.ibm.com>
---
src/qemu/qemu_extdevice.c | 27 ++++++++++++++++++++-------
src/qemu/qemu_extdevice.h | 4 ++++
src/qemu/qemu_process.c | 5 ++++-
3 files changed, 28 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
index 8fe7ceaa10..96fdc9a589 100644
--- a/src/qemu/qemu_extdevice.c
+++ b/src/qemu/qemu_extdevice.c
@@ -205,7 +205,27 @@ qemuExtDevicesStart(virQEMUDriverPtr driver,
return 0;
}
+/* qemuExtDevicesStopEarly stops devices that may be stopped
+ * before QEMU terminates
+ */
+void
+qemuExtDevicesStopEarly(virQEMUDriverPtr driver,
+ virDomainObjPtr vm)
+{
+ virDomainDefPtr def = vm->def;
+ size_t i;
+ for (i = 0; i < def->nfss; i++) {
+ virDomainFSDefPtr fs = def->fss[i];
+
+ if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS)
+ qemuVirtioFSStop(driver, vm, fs);
+ }
+}
+
+/* qemuExtDevicesStop stops devices that may only be stopped
+ * after QEMU terminated
+ */
void
qemuExtDevicesStop(virQEMUDriverPtr driver,
virDomainObjPtr vm)
@@ -236,13 +256,6 @@ qemuExtDevicesStop(virQEMUDriverPtr driver,
if (actualType == VIR_DOMAIN_NET_TYPE_ETHERNET && net->downscript)
virNetDevRunEthernetScript(net->ifname, net->downscript);
}
-
- for (i = 0; i < def->nfss; i++) {
- virDomainFSDefPtr fs = def->fss[i];
-
- if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS)
- qemuVirtioFSStop(driver, vm, fs);
- }
}
diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h
index 49373a15a1..df0cbd6993 100644
--- a/src/qemu/qemu_extdevice.h
+++ b/src/qemu/qemu_extdevice.h
@@ -51,6 +51,10 @@ int qemuExtDevicesStart(virQEMUDriverPtr driver,
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
G_GNUC_WARN_UNUSED_RESULT;
+void qemuExtDevicesStopEarly(virQEMUDriverPtr driver,
+ virDomainObjPtr vm)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
void qemuExtDevicesStop(virQEMUDriverPtr driver,
virDomainObjPtr vm)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7feb35e609..b5b2ddef4c 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7659,7 +7659,7 @@ void qemuProcessStop(virQEMUDriverPtr driver,
/* Do this before we delete the tree and remove pidfile. */
qemuProcessKillManagedPRDaemon(vm);
- qemuExtDevicesStop(driver, vm);
+ qemuExtDevicesStopEarly(driver, vm);
virFileDeleteTree(priv->libDir);
virFileDeleteTree(priv->channelTargetDir);
@@ -7677,6 +7677,9 @@ void qemuProcessStop(virQEMUDriverPtr driver,
qemuDomainCleanupRun(driver, vm);
+ /* tear down external devices after QEMU is gone */
+ qemuExtDevicesStop(driver, vm);
+
qemuDBusStop(driver, vm);
vm->def->id = -1;
--
2.26.2
Regards,
Daniel
--
|:
https://berrange.com -o-
https://www.flickr.com/photos/dberrange :|
|:
https://libvirt.org -o-
https://fstop138.berrange.com :|
|:
https://entangle-photo.org -o-
https://www.instagram.com/dberrange :|