> 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@xxxxxxxxxx>
> ---
> 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
