Re: [PATCH v2 3/3] qemu: Introduce qemuDomainGetStatsCpuHaltPollTime

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(a)huawei.com&gt; > --- > 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 .