Re: [libvirt PATCH 2/3] qemu: wait more for virtiofsd to exit

[Masayoshi Mizuma <msys.mizuma@xxxxxxxxx>]

On Fri, Jun 18, 2021 at 01:45:18PM +0200, Ján Tomko wrote:
> In some cases, such as doing intense I/O on slow filesystems,
> it can take virtiofsd as long as 42 seconds to exit.
> 
> Add a delay of extra 45 seconds before we forcefully kill it.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1940276
> 
> Signed-off-by: Ján Tomko <jtomko@xxxxxxxxxx>
> ---
>  src/qemu/qemu_virtiofs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c
> index edaedf0304..3f99f0caa4 100644
> --- a/src/qemu/qemu_virtiofs.c
> +++ b/src/qemu/qemu_virtiofs.c
> @@ -281,7 +281,7 @@ qemuVirtioFSStop(virQEMUDriver *driver G_GNUC_UNUSED,
>      if (!(pidfile = qemuVirtioFSCreatePidFilename(vm, fs->info.alias)))
>          goto cleanup;
>  
> -    if (virPidFileForceCleanupPath(pidfile) < 0) {
> +    if (virPidFileForceCleanupPathDelay(pidfile, 30) < 0) {

The main process of virtiofsd is waiting for the child process on
waitpid(). And virPidFileForceCleanupPath()
(and virPidFileForceCleanupPathDelay()) will send SIGTERM to the main
process, so waitpid() returns as EINTR and the main process exits.
However, the child process and the sub threads may still remain for the
IO completion.

How about adding a timer to wait for the closing of the main process
instead of sending SIGTERM? Like as follows:

@@ -275,13 +278,35 @@ qemuVirtioFSStop(virQEMUDriver *driver G_GNUC_UNUSED,
 {
     g_autofree char *pidfile = NULL;
     virErrorPtr orig_err;
+    unsigned long long timeout = 60 * 1000;
+    virTimeBackOffVar timebackoff;
+    bool terminated = false;
+    pid_t pid = (pid_t) -1;
 
     virErrorPreserveLast(&orig_err);
 
     if (!(pidfile = qemuVirtioFSCreatePidFilename(vm, fs->info.alias)))
         goto cleanup;
 
-    if (virPidFileForceCleanupPath(pidfile) < 0) {
+    if (virPidFileReadPath(pidfile, &pid) < 0)
+        goto cleanup;
+
+    /* in case the IO threads are still running, wait for the completion. */
+    if (virProcessKill(pid, 0) == 0) {
+        if (virTimeBackOffStart(&timebackoff, 1, timeout) < 0)
+            goto cleanup;
+
+        while (virTimeBackOffWait(&timebackoff)) {
+            if (virProcessKill(pid, 0) != 0) {
+               terminated = true;
+               break;
+            }
+        }
+    } else {
+        terminated = true;
+    }
+
+    if ((virPidFileForceCleanupPath(pidfile) < 0) || !terminated) {
         VIR_WARN("Unable to kill virtiofsd process");
     } else {
         if (QEMU_DOMAIN_FS_PRIVATE(fs)->vhostuser_fs_sock)
-- 
2.27.0

Thanks,
Masa