[PATCH v2 3/3] qemu: Introduce qemuDomainGetStatsCpuHaltPollTime

[Michal Prívozník]

On 7/14/21 2:28 PM, Yang Fei wrote:
> This function is used to obtain the halt polling time. The kernel
> provides statistics about haltpollsuccess.time and
> haltpollfail.time. We add it in domstats, so that we can use
> command 'virsh domstats VM' to get it if system support.
> 
> Signed-off-by: Yang Fei <yangfei85 at huawei.com>
> ---
>  src/qemu/qemu_driver.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 235f575901..6163037ec3 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -17839,6 +17839,31 @@ qemuDomainGetStatsCpuCgroup(virDomainObj *dom,
>      return 0;
>  }
>  
> +#ifdef __linux__
> +static int
> +qemuDomainGetStatsCpuHaltPollTime(virDomainObj *dom,
> +                                 virTypedParamList *params)
> +{
> +    unsigned long long haltPollSuccess = 0;
> +    unsigned long long haltPollFail = 0;
> +    pid_t pid = dom->pid;
> +    int err = 0;
> +
> +    err = virGetCpuHaltPollTime(pid, &haltPollSuccess, &haltPollFail);
> +    if (!err && virTypedParamListAddULLong(params, haltPollSuccess, "haltpollsuccess.time") < 0)
> +        return -1;
> +    if (!err && virTypedParamListAddULLong(params, haltPollFail, "haltpollfail.time") < 0)
> +        return -1;

I know you copied this pattern from qemuDomainGetStatsCpuCgroup() but
it's not as appealing is it could be. How about:

if (virGetCpuHaltPollTime() < 0)
  return 0;

if (virTypedParamListAddULLong() < 0 ||
    virTypedParamListAddULLong() < 0)
    return -1;

return 0;


Also, please document these new parameters in src/libvirt-domain.c just
like every other parameter is (look around line 11626).

> +    return 0;
> +}
> +#else
> +static int
> +qemuDomainGetStatsCpuHaltPollTime(virDomainObj *dom,
> +                                 virTypedParamList *params)
> +{
> +    return 0;
> +}
> +#endif

Once my suggestions from previous patches are worked in, this non-Linux
variant won't be necessary.

>  
>  static int
>  qemuDomainGetStatsCpu(virQEMUDriver *driver,
> @@ -17852,6 +17877,9 @@ qemuDomainGetStatsCpu(virQEMUDriver *driver,
>      if (qemuDomainGetStatsCpuCache(driver, dom, params) < 0)
>          return -1;
>  
> +    if (qemuDomainGetStatsCpuHaltPollTime(dom, params) < 0)
> +        return -1;
> +
>      return 0;
>  }
>  
> 

Michal