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@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;
+    return 0;
+}
+#else
+static int
+qemuDomainGetStatsCpuHaltPollTime(virDomainObj *dom,
+                                 virTypedParamList *params)
+{
+    return 0;
+}
+#endif
 
 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;
 }
 
-- 
2.23.0

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

On 2021/7/14 22:17, Michal Prívozník wrote:
> 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@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).
> 

OK, I'll add it in next version.

>> +    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
> 
> .
>