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

3 Sep 2014


      ----- Original Message -----
...
From: "Eric Blake" <eblake@redhat.com>
To: "Francesco Romani" <fromani@redhat.com>, libvir-list@redhat.com
Sent: Tuesday, September 2, 2014 11:42:09 PM
Subject: Re: [libvirt] [PATCH 02/11] qemu: extract helper to gather vcpu data
On 09/02/2014 06:31 AM, Francesco Romani wrote:
...
Extracts an helper to gether the VCpu
s/an/a/
s/gether/gather/
Will fix,
...
...
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).
Thanks, will fix.

-- 
Francesco Romani
RedHat Engineering Virtualization R & D
Phone: 8261328
IRC: fromani

    