[PATCH] qemu: clear residual QMP caps processes during QEMU driver initialization

[Jiri Denemark]

> 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 at 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