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@xxxxxxxxxx>
> ---
> 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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list