[libvirt] [PATCHv3 4/8] qemu: bulk stats: implement VCPU group

[Wang Rui]

On 2014/9/8 21:05, Francesco Romani wrote:
> This patch implements the VIR_DOMAIN_STATS_VCPU
> group of statistics.
> To do so, this patch also extracts a helper to gather the
> VCpu information.
> 
> Signed-off-by: Francesco Romani <fromani at redhat.com>
> ---
>  include/libvirt/libvirt.h.in |   1 +
>  src/libvirt.c                |  13 +++
>  src/qemu/qemu_driver.c       | 210 ++++++++++++++++++++++++++++++-------------
>  3 files changed, 160 insertions(+), 64 deletions(-)
[...]
> +static int
> +qemuDomainHelperGetVcpus(virDomainObjPtr vm, virVcpuInfoPtr info, int maxinfo,
> +                         unsigned char *cpumaps, int maplen)
> +{
> +    int v, maxcpu, hostcpus;
> +    size_t i;

'v' can be size_t type ,too.(Although the code is moved here from qemuDomainGetVcpus. :-) )

> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +
> +    if ((hostcpus = nodeGetCPUCount()) < 0)
> +        return -1;
> +
> +    maxcpu = maplen * 8;
> +    if (maxcpu > hostcpus)
> +        maxcpu = hostcpus;
> +
> +    /* 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 (priv->vcpupids != NULL &&
> +                    qemuGetProcessInfo(&(info[i].cpuTime),
> +                                       &(info[i].cpu),
> +                                       NULL,
> +                                       vm->pid,
> +                                       priv->vcpupids[i]) < 0) {
> +                    virReportSystemError(errno, "%s",
> +                                         _("cannot get vCPU placement & pCPU time"));
> +                    return -1;
> +                }
> +            }
> +        }
> +
> +        if (cpumaps != NULL) {
> +            memset(cpumaps, 0, maplen * maxinfo);
> +            if (priv->vcpupids != NULL) {
> +                for (v = 0; v < maxinfo; v++) {
> +                    unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, v);
> +                    virBitmapPtr map = NULL;
> +                    unsigned char *tmpmap = NULL;
> +                    int tmpmapLen = 0;
> +
> +                    if (virProcessGetAffinity(priv->vcpupids[v],
> +                                              &map, maxcpu) < 0)
> +                        return -1;
> +                    virBitmapToData(map, &tmpmap, &tmpmapLen);
> +                    if (tmpmapLen > maplen)
> +                        tmpmapLen = maplen;
> +                    memcpy(cpumap, tmpmap, tmpmapLen);
> +
> +                    VIR_FREE(tmpmap);
> +                    virBitmapFree(map);
> +                }
> +            } else {
> +                virReportError(VIR_ERR_OPERATION_INVALID,
> +                               "%s", _("cpu affinity is not available"));
> +                return -1;
> +            }
> +        }
> +    }
> +    return maxinfo;
> +}
[...]
> +
> +static int
> +qemuDomainGetStatsVcpu(virConnectPtr conn ATTRIBUTE_UNUSED,
> +                       virDomainObjPtr dom,
> +                       virDomainStatsRecordPtr record,
> +                       int *maxparams,
> +                       unsigned int privflags ATTRIBUTE_UNUSED)
> +{
> +    size_t i;
> +    int ret = -1;
> +    char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
> +    virVcpuInfoPtr cpuinfo = NULL;
> +
> +    if (virTypedParamsAddUInt(&record->params,
> +                              &record->nparams,
> +                              maxparams,
> +                              "vcpu.current",
> +                              (unsigned) dom->def->vcpus) < 0)
> +        return -1;
> +
> +    if (virTypedParamsAddUInt(&record->params,
> +                              &record->nparams,
> +                              maxparams,
> +                              "vcpu.maximum",
> +                              (unsigned) dom->def->maxvcpus) < 0)
> +        return -1;
> +
> +    if (VIR_ALLOC_N(cpuinfo, dom->def->vcpus) < 0)
> +        return -1;
> +
> +    if ((ret = qemuDomainHelperGetVcpus(dom,
> +                                        cpuinfo,
> +                                        dom->def->vcpus,
> +                                        NULL,
> +                                        0)) < 0)
> +        return 0;

Memory of 'cpuinfo' will be leaked. Should we go to cleanup?