[libvirt] [PATCH 33/34] qemu: driver: Refactor qemuDomainHelperGetVcpus

[John Ferlan]

On 11/20/2015 10:22 AM, Peter Krempa wrote:
> Change some of the control structures and switch to using the new vcpu
> structure.
> ---
>  src/qemu/qemu_driver.c | 77 ++++++++++++++++++++++++++++----------------------
>  1 file changed, 43 insertions(+), 34 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index c659328..b9f8e72 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -1422,11 +1422,17 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss,
> 
> 
>  static int
> -qemuDomainHelperGetVcpus(virDomainObjPtr vm, virVcpuInfoPtr info, int maxinfo,
> -                         unsigned char *cpumaps, int maplen)
> +qemuDomainHelperGetVcpus(virDomainObjPtr vm,
> +                         virVcpuInfoPtr info,
> +                         int maxinfo,
> +                         unsigned char *cpumaps,
> +                         int maplen)
>  {
> -    size_t i, v;
> -    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    size_t ncpuinfo = 0;
> +    size_t i;
> +
> +    if (maxinfo == 0)
> +        return 0;
> 
>      if (!qemuDomainHasVCpuPids(vm)) {
>          virReportError(VIR_ERR_OPERATION_INVALID,
> @@ -1434,43 +1440,46 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm, virVcpuInfoPtr info, int maxinfo,
>          return -1;
>      }
> 
> -    /* Clamp to actual number of vcpus */
> -    if (maxinfo > priv->nvcpupids)
> -        maxinfo = priv->nvcpupids;
> -
> -    if (maxinfo >= 1) {
> -        if (info != NULL) {
> -            memset(info, 0, sizeof(*info) * maxinfo);
> -            for (i = 0; i < maxinfo; i++) {
> -                info[i].number = i;
> -                info[i].state = VIR_VCPU_RUNNING;
> -
> -                if (qemuGetProcessInfo(&(info[i].cpuTime),
> -                                       &(info[i].cpu),
> -                                       NULL,
> -                                       vm->pid,
> -                                       qemuDomainGetVCpuPid(vm, i)) < 0) {
> -                    virReportSystemError(errno, "%s",
> -                                         _("cannot get vCPU placement & pCPU time"));
> -                    return -1;
> -                }
> +    if (info)
> +        memset(info, 0, sizeof(*info) * maxinfo);
> +
> +    if (cpumaps)
> +        memset(cpumaps, 0, sizeof(*cpumaps) * maxinfo);
> +
> +    for (i = 0; i < virDomainDefGetVCpusMax(vm->def) && ncpuinfo < maxinfo; i++) {

This line is longer than 80 cols.

> +        virDomainVCpuInfoPtr vcpu = virDomainDefGetVCpu(vm->def, i);
> +        pid_t vcpupid = qemuDomainGetVCpuPid(vm, i);
> +
> +        if (!vcpu->online)
> +            continue;

So if the goal is to eventually allow vcpu 0 & 2 of 4 vcpu's to be
online, then this algorithm will need a slight adjustment.

Of course there's also the what if 'vcpupid == 0' that hasn't been
checked here (comments from patch 32).

I "believe" what needs to be done is change the [i] below to [ncpuinfo]
- that way the info & cpumaps would be returned with only the
ONLINE/RUNNING vCPU's and 'info[]' won't have gaps which won't be
accessible if a "2" is returned...  I think the same holds true for the
VIR_GET_CPUMAP


ACK with some adjustments.

John

> +
> +        if (info) {
> +            info[i].number = i;
> +            info[i].state = VIR_VCPU_RUNNING;
> +
> +            if (qemuGetProcessInfo(&(info[i].cpuTime), &(info[i].cpu), NULL,
> +                                   vm->pid, vcpupid) < 0) {
> +                virReportSystemError(errno, "%s",
> +                                     _("cannot get vCPU placement & pCPU time"));
> +                return -1;
>              }
>          }
> 
> -        if (cpumaps != NULL) {
> -            for (v = 0; v < maxinfo; v++) {
> -                unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, v);
> -                virBitmapPtr map = NULL;
> +        if (cpumaps) {
> +            unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, i);
> +            virBitmapPtr map = NULL;
> 
> -                if (!(map = virProcessGetAffinity(qemuDomainGetVCpuPid(vm, v))))
> -                    return -1;
> +            if (!(map = virProcessGetAffinity(vcpupid)))
> +                return -1;
> 
> -                virBitmapToDataBuf(map, cpumap, maplen);
> -                virBitmapFree(map);
> -            }
> +            virBitmapToDataBuf(map, cpumap, maplen);
> +            virBitmapFree(map);
>          }
> +
> +        ncpuinfo++;
>      }
> -    return maxinfo;
> +
> +    return ncpuinfo;
>  }
> 
>