[PATCH v3 1/2] util: Add virGetCpuHaltPollTime

[Michal Prívozník]

On 7/20/21 4:55 AM, Yang Fei wrote:
> 
> 
> On 2021/7/19 17:05, Peter Krempa wrote:
>> On Fri, Jul 16, 2021 at 18:42:22 +0800, Yang Fei wrote:
>>> Add helper function virGetCpuHaltPollTime to obtain halt polling
>>> time. If the kernel support halt polling time statistic, and mount
>>> debugfs. This function will take effect on KVM VMs.
>>>
>>> Signed-off-by: Yang Fei <yangfei85 at huawei.com>
>>> ---
>>>  src/libvirt_private.syms |  1 +
>>>  src/util/virutil.c       | 43 ++++++++++++++++++++++++++++++++++++++++
>>>  src/util/virutil.h       |  4 ++++
>>>  3 files changed, 48 insertions(+)
>>>
>>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>>> index 68e4b6aab8..64aff4eca4 100644
>>> --- a/src/libvirt_private.syms
>>> +++ b/src/libvirt_private.syms
>>> @@ -3479,6 +3479,7 @@ virDoesUserExist;
>>>  virDoubleToStr;
>>>  virFormatIntDecimal;
>>>  virFormatIntPretty;
>>> +virGetCpuHaltPollTime;
>>>  virGetDeviceID;
>>>  virGetDeviceUnprivSGIO;
>>>  virGetGroupID;
>>> diff --git a/src/util/virutil.c b/src/util/virutil.c
>>> index 311cbbf93a..f5304644c0 100644
>>> --- a/src/util/virutil.c
>>> +++ b/src/util/virutil.c
>>> @@ -1936,3 +1936,46 @@ virPipeNonBlock(int fds[2])
>>>  {
>>>      return virPipeImpl(fds, true, true);
>>>  }
>>> +
>>> +int
>>> +virGetCpuHaltPollTime(pid_t pid,
>>> +                      unsigned long long *haltPollSuccess,
>>> +                      unsigned long long *haltPollFail)
>>> +{
>>> +    g_autofree char *pidToStr = NULL;
>>> +    g_autofree char *debugFsPath = NULL;
>>> +    g_autofree char *completePath = NULL;
>>> +    struct dirent *ent = NULL;
>>> +    g_autoptr(DIR) dir = NULL;
>>> +    int ret = -1;
>>> +    bool found = false;
>>> +
>>> +    if (!(debugFsPath = virFileFindMountPoint("debugfs")))
>>> +        return ret;
>>
>> Here '-1' is returned, but no libvirt error is reported ...
>>
>>> +
>>> +    completePath = g_strdup_printf("%s/%s", debugFsPath, "kvm");
>>> +    if (virDirOpenIfExists(&dir, completePath) != 1)
>>> +        return ret;
>>
>> While here -1 is reported and an error may be reported (e.g on EACCESS).
>> We generally avoid this pattern because the caller cant properly handle
>> that.
>>
>>> +
>>> +    pidToStr = g_strdup_printf("%d%c", pid, '-');
>>
>> I'm fairly certain this will fail to compile on mingw since %d isn't the
>> proper formatting modifier for pid_t.
>>
> 
> In the last version, I try to use %lld but failed in the ninja test.
> How about forcibly converts the variable pid to long long?
> Like this:
>          g_strdup_printf("%lld%c", (long long)pid, '-')
> I find that this is used elsewhere in the code.

Yes, that works.

> 
>>> +    while (virDirRead(dir, &ent, NULL) > 0) {
>>> +        if (STRPREFIX(ent->d_name, pidToStr)) {
>>
>> Any idea what the suffix after '-' in the debugfs entry is? If we can
>> figure it out we won't have to look up the correct file.
>>
> 
> The full directory name format is "pid-fd", The fd is assigned during VM creation.
> As I know, we can find it in /proc/(qemu-kvm pid)/fd, but also need to look up which
> one points to "anon_inode:kvm-vm". That may not be any more efficient than look up
> it in the kvm directory.

Right. I tried to find that in the kernel sources, but wasn't
successful. In that case I think you can do what you're doing.

Michal