>From c328ff62b11d58553fd2032a85fd3295e009b3d3 Mon Sep 17 00:00:00 2001
From: Bihong Yu <yubihong@huawei.com>
Date: Fri, 17 Jul 2020 16:55:12 +0800
Subject: [PATCH] qemu: clear residual QMP caps processes during QEMU driver
 initialization

In some cases, the QMP capabilities processes possible residue. So we need to
clear the residual QMP caps processes during starting libvirt.

Signed-off-by:Bihong Yu <yubihong@huawei.com>
---
 src/qemu/qemu_driver.c  |  2 ++
 src/qemu/qemu_process.c | 40 ++++++++++++++++++++++++++++++++++++++++
 src/qemu/qemu_process.h |  2 ++
 3 files changed, 44 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d185666..d627fd7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -889,6 +889,8 @@ qemuStateInitialize(bool privileged,
         run_gid = cfg->group;
     }

+    qemuProcessQMPClear(cfg->libDir);
+
     qemu_driver->qemuCapsCache = virQEMUCapsCacheNew(cfg->libDir,
                                                      cfg->cacheDir,
                                                      run_uid,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 70fc24b..d545e3e 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8312,6 +8312,46 @@ static qemuMonitorCallbacks callbacks = {
 };


+/**
+ * qemuProcessQMPClear
+ *
+ * Try to kill residual QMP caps processes
+ */
+void
+qemuProcessQMPClear(const char *libDir)
+{
+    virErrorPtr orig_err;
+    DIR *dirp = NULL;
+    struct dirent *dp;
+
+    if (!virFileExists(libDir))
+        return;
+
+    if (virDirOpen(&dirp, libDir) < 0)
+        return;
+
+    virErrorPreserveLast(&orig_err);
+    while (virDirRead(dirp, &dp, libDir) > 0) {
+        g_autofree char *qmp_uniqDir = NULL;
+        g_autofree char *qmp_pidfile = NULL;
+
+        if (!STRPREFIX(dp->d_name, "qmp-"))
+            continue;
+
+        qmp_uniqDir = g_strdup_printf("%s/%s", libDir, dp->d_name);
+        qmp_pidfile = g_strdup_printf("%s/%s", qmp_uniqDir, "qmp.pid");
+
+        ignore_value(virPidFileForceCleanupPath(qmp_pidfile));
+
+        if (qmp_uniqDir)
+            rmdir(qmp_uniqDir);
+    }
+    virErrorRestore(&orig_err);
+
+    VIR_DIR_CLOSE(dirp);
+}
+
+
 static void
 qemuProcessQMPStop(qemuProcessQMPPtr proc)
 {
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 15e67b9..b039e6c 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -233,6 +233,8 @@ qemuProcessQMPPtr qemuProcessQMPNew(const char *binary,
                                     gid_t runGid,
                                     bool forceTCG);

+void qemuProcessQMPClear(const char *libDir);
+
 void qemuProcessQMPFree(qemuProcessQMPPtr proc);

 int qemuProcessQMPStart(qemuProcessQMPPtr proc);
-- 
1.8.3.1

> In some cases, the QMP capabilities processes possible residue. So we need to
> clear the residual QMP caps processes during starting libvirt.

I agree with you that leaving processes behind is more serious than
leaving temporary files and thus we should try to kill such processes
when libvirtd starts.

> Signed-off-by:Bihong Yu <yubihong@huawei.com>
> ---
>  src/qemu/qemu_driver.c  |  2 ++
>  src/qemu/qemu_process.c | 40 ++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_process.h |  2 ++
>  3 files changed, 44 insertions(+)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index d185666..d627fd7 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -889,6 +889,8 @@ qemuStateInitialize(bool privileged,
>          run_gid = cfg->group;
>      }
> 
> +    qemuProcessQMPClear(cfg->libDir);
> +
>      qemu_driver->qemuCapsCache = virQEMUCapsCacheNew(cfg->libDir,
>                                                       cfg->cacheDir,
>                                                       run_uid,
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 70fc24b..d545e3e 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -8312,6 +8312,46 @@ static qemuMonitorCallbacks callbacks = {
>  };
> 
> 
> +/**
> + * qemuProcessQMPClear

How about qemuProcessQMPClearAll or something like that to make it a bit
more obvious it's not working with a single process only?

> + *
> + * Try to kill residual QMP caps processes
> + */
> +void
> +qemuProcessQMPClear(const char *libDir)
> +{
> +    virErrorPtr orig_err;
> +    DIR *dirp = NULL;
> +    struct dirent *dp;
> +
> +    if (!virFileExists(libDir))

I think virFileIsDir would be more appropriate here.

> +        return;
> +

You should save the error here by calling

    virErrorPreserveLast(&orig_err);

> +    if (virDirOpen(&dirp, libDir) < 0)
> +        return;

And here you would need to jump to the end of the function to restore
the original error.

> +
> +    virErrorPreserveLast(&orig_err);
> +    while (virDirRead(dirp, &dp, libDir) > 0) {
> +        g_autofree char *qmp_uniqDir = NULL;
> +        g_autofree char *qmp_pidfile = NULL;
> +
> +        if (!STRPREFIX(dp->d_name, "qmp-"))
> +            continue;
> +
> +        qmp_uniqDir = g_strdup_printf("%s/%s", libDir, dp->d_name);
> +        qmp_pidfile = g_strdup_printf("%s/%s", qmp_uniqDir, "qmp.pid");
> +
> +        ignore_value(virPidFileForceCleanupPath(qmp_pidfile));
> +
> +        if (qmp_uniqDir)

There's no way qmp_uniqDir could be NULL here.

> +            rmdir(qmp_uniqDir);
> +    }

Add a cleanup label and jump here when virDirOpen fails.

> +    virErrorRestore(&orig_err);
> +
> +    VIR_DIR_CLOSE(dirp);
> +}
> +
> +

I would've sworn we had some auto magic that would save/restore the
error when entering/leaving a function, but I can't find it. So I guess
it does not really exist or I don't know what to look for.

>  static void
>  qemuProcessQMPStop(qemuProcessQMPPtr proc)
>  {
> diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
> index 15e67b9..b039e6c 100644
> --- a/src/qemu/qemu_process.h
> +++ b/src/qemu/qemu_process.h
> @@ -233,6 +233,8 @@ qemuProcessQMPPtr qemuProcessQMPNew(const char *binary,
>                                      gid_t runGid,
>                                      bool forceTCG);
> 
> +void qemuProcessQMPClear(const char *libDir);
> +
>  void qemuProcessQMPFree(qemuProcessQMPPtr proc);
> 
>  int qemuProcessQMPStart(qemuProcessQMPPtr proc);

Jirka