[libvirt] [PATCH 02/11] qemu: extract helper to gather vcpu data

[Eric Blake]

On 09/02/2014 06:31 AM, Francesco Romani wrote:
> Extracts an helper to gether the VCpu

s/an/a/
s/gether/gather/

> information.
> 
> Signed-off-by: Francesco Romani <fromani at redhat.com>
> ---
>  src/qemu/qemu_driver.c | 29 +++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index bbd16ed..1842e60 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -172,6 +172,12 @@ static int qemuDomainGetBalloonMemory(virQEMUDriverPtr driver,
>                                        virDomainObjPtr vm,
>                                        unsigned long *memory);
>  
> +static int qemuDomainHelperGetVcpus(virDomainObjPtr vm,
> +                                    virVcpuInfoPtr info,
> +                                    int maxinfo,
> +                                    unsigned char *cpumaps,
> +                                    int maplen);
> +

As I commented in 1, a non-recursive static function generally should
not need forward declarations.  Just hoist the implementation here.

>  virQEMUDriverPtr qemu_driver = NULL;
>  
>  
> @@ -4974,10 +4980,7 @@ qemuDomainGetVcpus(virDomainPtr dom,
>                     int maplen)
>  {
>      virDomainObjPtr vm;
> -    size_t i;
> -    int v, maxcpu, hostcpus;
>      int ret = -1;
> -    qemuDomainObjPrivatePtr priv;
>  
>      if (!(vm = qemuDomObjFromDomain(dom)))
>          goto cleanup;
> @@ -4992,7 +4995,25 @@ qemuDomainGetVcpus(virDomainPtr dom,
>          goto cleanup;
>      }
>  
> -    priv = vm->privateData;
> +    ret = qemuDomainHelperGetVcpus(vm, info, maxinfo, cpumaps, maplen);
> +
> + cleanup:
> +    if (vm)
> +        virObjectUnlock(vm);

Ouch.  You have a double free.  This frees vm, even though it was calling...

> +    return ret;
> +}
> +
> +static int
> +qemuDomainHelperGetVcpus(virDomainObjPtr vm,
> +                         virVcpuInfoPtr info,
> +                         int maxinfo,
> +                         unsigned char *cpumaps,
> +                         int maplen)
> +{
> +    int ret = -1;
> +    int v, maxcpu, hostcpus;
> +    size_t i;
> +    qemuDomainObjPrivatePtr priv = vm->privateData;

...a function that now has transfer semantics.  But unlike patch 1,
where transfer semantics were necessary because of the way you drop lock
in order to do a monitor call, this patch appears to not need them; and
the solution is to just sanitize the cleanup label (at which point it
becomes a mere 'return ret', so you could then replace all 'goto
cleanup' with a direct return).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 539 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140902/4fc80144/attachment-0001.sig>