ping
On 2020/7/20 10:09, Bihong Yu wrote:
>
>
> On 2020/7/18 5:14, Daniel Henrique Barboza wrote:
>>
>>
>> On 7/17/20 8:10 AM, Bihong Yu wrote:
>>>> From c328ff62b11d58553fd2032a85fd3295e009b3d3 Mon Sep 17 00:00:00 2001
>>> From: Bihong Yu <yubihong@xxxxxxxxxx>
>>> 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.
>>
>> Which cases are you referring to? Do you have a reproducer for this behavior? I
>> tried it up starting and stopping libvirtd, fetching capabilities, starting vms
>> and so on, and wasn't able to see any 'qmp.pid' file hanging around.
>
> Yes, I have a reproducer for this behavior. I refer case is that send kill signal
> to libvirtd when libvirtd is fetching capabilities before libvirtd calling
> qemuProcessQMPFree() and qemuProcessQMPStop().
>
> When libvirtd restart, it can not find the temporary qmp directory and has no way
> to clear the residual QMP caps processes.
>
> Thanks,
>
> Bihong Yu
>
>>
>> About the code, I looked it up and all calls to qemuProcessQMPNew() are being
>> cleaned up accordingly with a qemuProcessQMPFree() function, which calls
>> qemuProcessQMPStop(), and the Stop function cleans up both the pid file and
>> the uniqDir:
>>
>> ----- qemuProcessQMPStop(qemuProcessQMPPtr proc) -----
>>
>> if (proc->pidfile)
>> unlink(proc->pidfile);
>>
>> if (proc->uniqDir)
>> rmdir(proc->uniqDir);
>>
>> ------
>>
>> The proper fix here would be to understand in which conditions the 'qmp.pid' file
>> is being left behind and make the code go through the existing cleanup path,
>> fixing qemuProcessQMPFree and/or qemuProcessQMPStop if needed. What you're doing
>> here works, but it's fixing the symptom of a bug instead of the bug itself.
>>
>>
>> Thanks,
>>
>>
>> DHB
>>
>>
>>>
>>> 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
>>> + *
>>> + * 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);
>>>
>> .
