[libvirt PATCH v3 01/13] util: Helper functions to get process info

[Michal Prívozník]

On 12/10/21 21:34, Praveen K Paladugu wrote:
> Move qemuGetProcessInfo and qemuGetSchedInfo methods to util and share them
> with ch driver.
> 
> Signed-off-by: Praveen K Paladugu <prapal at linux.microsoft.com>
> ---
>  src/libvirt_private.syms |   2 +
>  src/qemu/qemu_driver.c   | 116 ++-------------------------------------
>  src/util/virprocess.c    | 108 ++++++++++++++++++++++++++++++++++++
>  src/util/virprocess.h    |   5 ++
>  4 files changed, 120 insertions(+), 111 deletions(-)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index da27ee7b53..56adc192cd 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -3101,8 +3101,10 @@ virProcessGetAffinity;
>  virProcessGetMaxMemLock;
>  virProcessGetNamespaces;
>  virProcessGetPids;
> +virProcessGetSchedInfo;
>  virProcessGetStartTime;
>  virProcessGetStat;
> +virProcessGetStatInfo;
>  virProcessGroupGet;
>  virProcessGroupKill;
>  virProcessKill;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index e444ad2d45..ba3efef42b 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c


> @@ -1463,7 +1356,7 @@ qemuDomainHelperGetVcpus(virDomainObj *vm,
>              vcpuinfo->number = i;
>              vcpuinfo->state = VIR_VCPU_RUNNING;
>  
> -            if (qemuGetProcessInfo(&vcpuinfo->cpuTime,
> +            if (virProcessGetStatInfo(&vcpuinfo->cpuTime,
>                                     &vcpuinfo->cpu, NULL,
>                                     vm->pid, vcpupid) < 0) {

Indentation.

>                  virReportSystemError(errno, "%s",
> @@ -1483,7 +1376,7 @@ qemuDomainHelperGetVcpus(virDomainObj *vm,
>          }
>  
>          if (cpuwait) {
> -            if (qemuGetSchedInfo(&(cpuwait[ncpuinfo]), vm->pid, vcpupid) < 0)
> +            if (virProcessGetSchedInfo(&(cpuwait[ncpuinfo]), vm->pid, vcpupid) < 0)
>                  return -1;
>          }
>  
> @@ -2626,7 +2519,8 @@ qemuDomainGetInfo(virDomainPtr dom,
>      }
>  
>      if (virDomainObjIsActive(vm)) {
> -        if (qemuGetProcessInfo(&(info->cpuTime), NULL, NULL, vm->pid, 0) < 0) {
> +        if (virProcessGetStatInfo(&(info->cpuTime), NULL, NULL,
> +                                vm->pid, 0) < 0) {

And here too.

>              virReportError(VIR_ERR_OPERATION_FAILED, "%s",
>                             _("cannot read cputime for domain"));
>              goto cleanup;
> @@ -10623,7 +10517,7 @@ qemuDomainMemoryStatsInternal(virQEMUDriver *driver,
>          ret = 0;
>      }
>  
> -    if (qemuGetProcessInfo(NULL, NULL, &rss, vm->pid, 0) < 0) {
> +    if (virProcessGetStatInfo(NULL, NULL, &rss, vm->pid, 0) < 0) {
>          virReportError(VIR_ERR_OPERATION_FAILED, "%s",
>                         _("cannot get RSS for domain"));
>      } else {
> diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> index 7b0ad9c97b..3be9080b67 100644
> --- a/src/util/virprocess.c
> +++ b/src/util/virprocess.c
> @@ -1764,3 +1764,111 @@ virProcessGetStat(pid_t pid,
>  
>      return ret;
>  }
> +
> +
> +int
> +virProcessGetStatInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss,
> +                      pid_t pid, pid_t tid)

Since you are touching this, it would be nice if these arguments are on
separate lines. This applies here and to the rest of the patches. The
idea behind is that when a new argument is introduced a smaller diff can
be produced, which in turn opens a possibility of less conflicts during
backports.

I'm not going to raise this in the rest of my review.

Reviewed-by: Michal Privoznik <mprivozn at redhat.com>

Michal