[PATCH v2] qemu: Move qemuExtDevicesStop() before removing the pidfiles

Masayoshi Mizuma posted 1 patch 3 years, 5 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20201111133524.31076-1-msys.mizuma@gmail.com
src/qemu/qemu_process.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH v2] qemu: Move qemuExtDevicesStop() before removing the pidfiles
Posted by Masayoshi Mizuma 3 years, 5 months ago
From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>

A qemu guest which has virtiofs config fails to start if the previous
starting failed because of invalid option or something.

That's because the virtiofsd isn't killed by virPidFileForceCleanupPath()
on the former failure because the pidfile was already removed by
virFileDeleteTree(priv->libDir) in qemuProcessStop(), so
virPidFileForceCleanupPath() just returned.

Move qemuExtDevicesStop() before virFileDeleteTree(priv->libDir) so that
virPidFileForceCleanupPath() can kill virtiofsd correctly.

For example of the reproduction:

  # virsh start guest
  error: Failed to start domain guest
  error: internal error: process exited while connecting to monitor: qemu-system-x86_64: -foo: invalid option

  ... fix the option ...

  # virsh start guest
  error: Failed to start domain guest
  error: Cannot open log file: '/var/log/libvirt/qemu/guest-fs0-virtiofsd.log': Device or resource busy
  #

Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
---
 src/qemu/qemu_process.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0a36b49c85..1963de9fb8 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7638,6 +7638,8 @@ void qemuProcessStop(virQEMUDriverPtr driver,
     /* Do this before we delete the tree and remove pidfile. */
     qemuProcessKillManagedPRDaemon(vm);
 
+    qemuExtDevicesStop(driver, vm);
+
     virFileDeleteTree(priv->libDir);
     virFileDeleteTree(priv->channelTargetDir);
 
@@ -7654,8 +7656,6 @@ void qemuProcessStop(virQEMUDriverPtr driver,
 
     qemuDomainCleanupRun(driver, vm);
 
-    qemuExtDevicesStop(driver, vm);
-
     qemuDBusStop(driver, vm);
 
     vm->def->id = -1;
-- 
2.27.0

Re: [PATCH v2] qemu: Move qemuExtDevicesStop() before removing the pidfiles
Posted by Michal Privoznik 3 years, 5 months ago
On 11/11/20 2:35 PM, Masayoshi Mizuma wrote:
> From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
> 
> A qemu guest which has virtiofs config fails to start if the previous
> starting failed because of invalid option or something.
> 
> That's because the virtiofsd isn't killed by virPidFileForceCleanupPath()
> on the former failure because the pidfile was already removed by
> virFileDeleteTree(priv->libDir) in qemuProcessStop(), so
> virPidFileForceCleanupPath() just returned.
> 
> Move qemuExtDevicesStop() before virFileDeleteTree(priv->libDir) so that
> virPidFileForceCleanupPath() can kill virtiofsd correctly.
> 
> For example of the reproduction:
> 
>    # virsh start guest
>    error: Failed to start domain guest
>    error: internal error: process exited while connecting to monitor: qemu-system-x86_64: -foo: invalid option
> 
>    ... fix the option ...
> 
>    # virsh start guest
>    error: Failed to start domain guest
>    error: Cannot open log file: '/var/log/libvirt/qemu/guest-fs0-virtiofsd.log': Device or resource busy
>    #
> 
> Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
> ---
>   src/qemu/qemu_process.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

and pushed.

Michal