Re: [libvirt] [PATCH v2.1 05/21] refactor virDomainVcpuPinAdd()

22 Aug 2012

On Tue, Aug 21, 2012 at 05:18:28PM +0800, Hu Tao wrote:
...
---
 src/conf/domain_conf.c   |   81 ++++++++++++++++++----------------------------
 src/conf/domain_conf.h   |    3 +-
 src/libxl/libxl_driver.c |   13 +++++++-
 src/qemu/qemu_driver.c   |   26 +++++++++++++--
 src/xen/xend_internal.c  |   13 +++++++-
 5 files changed, 82 insertions(+), 54 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4e52177..56ee4c9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10921,69 +10921,52 @@ cleanup:
     return bitmap;
 }
-int
-virDomainVcpuPinAdd(virDomainDefPtr def,
-                    unsigned char *cpumap,
-                    int maplen,
-                    int vcpu)
+int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr *vcpupin_list,
+                        int *nvcpupin,
+                        unsigned char *cpumap,
+                        int maplen,
+                        int vcpu)
 {
-    virDomainVcpuPinDefPtr *vcpupin_list = NULL;
     virDomainVcpuPinDefPtr vcpupin = NULL;
     char *cpumask = NULL;
-    if ((cpumask = bitmapFromBytemap(cpumap, maplen)) == NULL)
-        goto cleanup;
-
-    /* No vcpupin exists yet. */
-    if (!def->cputune.nvcpupin) {
-        if (VIR_ALLOC(vcpupin) < 0) {
-            virReportOOMError();
-            goto cleanup;
-        }
+    if (!vcpupin_list)
+        return -1;
-        if (VIR_ALLOC(vcpupin_list) < 0) {
-            virReportOOMError();
-            VIR_FREE(vcpupin);
-            goto cleanup;
-        }
+    if ((cpumask = bitmapFromBytemap(cpumap, maplen)) == NULL)
+        return -1;
+    vcpupin = virDomainVcpuPinFindByVcpu(vcpupin_list,
+                                         *nvcpupin,
+                                         vcpu);
+    if (vcpupin) {
         vcpupin->vcpuid = vcpu;
         vcpupin->cpumask = cpumask;
-        vcpupin_list[def->cputune.nvcpupin++] = vcpupin;
-        def->cputune.vcpupin = vcpupin_list;
-    } else {
-        if (virDomainVcpuPinIsDuplicate(def->cputune.vcpupin,
-                                        def->cputune.nvcpupin,
-                                        vcpu)) {
-            vcpupin = virDomainVcpuPinFindByVcpu(def->cputune.vcpupin,
-                                                 def->cputune.nvcpupin,
-                                                 vcpu);
-            vcpupin->vcpuid = vcpu;
-            vcpupin->cpumask = cpumask;
-        } else {
-            if (VIR_ALLOC(vcpupin) < 0) {
-                virReportOOMError();
-                goto cleanup;
-            }
+        return 0;
+    }
+
+    /* No existing vcpupin matches vcpu, adding a new one */
+
+    if (VIR_ALLOC(vcpupin) < 0) {
+        virReportOOMError();
+        VIR_FREE(cpumask);
+        return -1;
+    }
+    vcpupin->vcpuid = vcpu;
+    vcpupin->cpumask = cpumask;
-            if (VIR_REALLOC_N(def->cputune.vcpupin, def->cputune.nvcpupin + 1) < 0) {
-                virReportOOMError();
-                VIR_FREE(vcpupin);
-                goto cleanup;
-            }
-            vcpupin->vcpuid = vcpu;
-            vcpupin->cpumask = cpumask;
-            def->cputune.vcpupin[def->cputune.nvcpupin++] = vcpupin;
-       }
+    if (VIR_REALLOC_N(vcpupin_list, *nvcpupin + 1) < 0) {
+        virReportOOMError();
+        VIR_FREE(cpumask);
+        VIR_FREE(vcpupin);
+        return -1;
     }
-    return 0;
+    vcpupin_list[(*nvcpupin)++] = vcpupin;
-cleanup:
-    VIR_FREE(cpumask);
-    return -1;
+    return 0;
 }
int
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 74abe6c..30aef6b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1968,7 +1968,8 @@ int virDomainCpuSetParse(const char *str,
 char *virDomainCpuSetFormat(char *cpuset,
                             int maxcpu);
-int virDomainVcpuPinAdd(virDomainDefPtr def,
+int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr *vcpupin_list,
+                        int *nvcpupin,
                         unsigned char *cpumap,
                         int maplen,
                         int vcpu);
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 398a9a2..7881cd1 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2454,7 +2454,18 @@ libxlDomainPinVcpu(virDomainPtr dom, unsigned int vcpu, unsigned char *cpumap,
         goto cleanup;
     }
-    if (virDomainVcpuPinAdd(vm->def, cpumap, maplen, vcpu) < 0) {
+    if (!vm->def->cputune->vcpupin) {
+        if (VIR_ALLOC(vm->def->cputune->vcpupin) < 0) {
+            virReportOOMError();
+            goto cleanup;
+        }
+        vm->def->cputune->nvcpupin = 0;
+    }
+    if (virDomainVcpuPinAdd(vm->def->cputune->vcpupin
+                            &vm->def->cputune->nvcpupin,
+                            cpumap,
+                            maplen,
+                            vcpu) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        "%s", _("failed to update or add vcpupin xml"));
         goto cleanup;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 109d18d..17c66d7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3785,7 +3785,18 @@ qemudDomainPinVcpuFlags(virDomainPtr dom,
                 goto cleanup;
             }
         } else {
-            if (virDomainVcpuPinAdd(vm->def, cpumap, maplen, vcpu) < 0) {
+            if (!vm->def->cputune.vcpupin) {
+                if (VIR_ALLOC(vm->def->cputune.vcpupin) < 0) {
+                    virReportOOMError();
+                    goto cleanup;
+                }
+                vm->def->cputune.nvcpupin = 0;
+            }
+            if (virDomainVcpuPinAdd(vm->def->cputune.vcpupin,
+                                    &vm->def->cputune.nvcpupin,
+                                    cpumap,
+                                    maplen,
+                                    vcpu) < 0) {
                 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                                _("failed to update or add vcpupin xml of "
                                  "a running domain"));
@@ -3807,7 +3818,18 @@ qemudDomainPinVcpuFlags(virDomainPtr dom,
                 goto cleanup;
             }
         } else {
-            if (virDomainVcpuPinAdd(persistentDef, cpumap, maplen, vcpu) < 0) {
+            if (!persistentDef->cputune.vcpupin) {
+                if (VIR_ALLOC(persistentDef->cputune.vcpupin) < 0) {
+                    virReportOOMError();
+                    goto cleanup;
+                }
+                persistentDef->cputune.nvcpupin = 0;
+            }
+            if (virDomainVcpuPinAdd(persistentDef->cputune.vcpupin,
+                                    &persistentDef->cputune.nvcpupin,
+                                    cpumap,
+                                    maplen,
+                                    vcpu) < 0) {
                 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                                _("failed to update or add vcpupin xml of "
                                  "a persistent domain"));
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index f93b249..f29f533 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -2296,7 +2296,18 @@ xenDaemonDomainPinVcpu(virDomainPtr domain, unsigned int vcpu,
         goto cleanup;
if (ret == 0) {
-        if (virDomainVcpuPinAdd(def, cpumap, maplen, vcpu) < 0) {
+        if (!def->cputune.vcpupin) {
+            if (VIR_ALLOC(def->cputune.vcpupin) < 0) {
+                virReportOOMError();
+                goto cleanup;
+            }
+            def->cputune.nvcpupin = 0;
+        }
+        if (virDomainVcpuPinAdd(def->cputune->vcpupin,
+                                &def->cputune->nvcpupin,
+                                cpumap,
+                                maplen,
+                                vcpu) < 0) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            "%s", _("failed to add vcpupin xml entry"));
             return -1;
Clearly you didn't tried to compile the xen drivers !
Please get a recent fedora and install xen on it to be able to catch
those. i had to squash in the following. With that ACK,

Daniel

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 7881cd1..d8ecf13 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2454,15 +2454,15 @@ libxlDomainPinVcpu(virDomainPtr dom, unsigned int vcpu, unsigned char *cpumap,
         goto cleanup;
     }
 
-    if (!vm->def->cputune->vcpupin) {
-        if (VIR_ALLOC(vm->def->cputune->vcpupin) < 0) {
+    if (!vm->def->cputune.vcpupin) {
+        if (VIR_ALLOC(vm->def->cputune.vcpupin) < 0) {
             virReportOOMError();
             goto cleanup;
         }
-        vm->def->cputune->nvcpupin = 0;
+        vm->def->cputune.nvcpupin = 0;
     }
-    if (virDomainVcpuPinAdd(vm->def->cputune->vcpupin
-                            &vm->def->cputune->nvcpupin,
+    if (virDomainVcpuPinAdd(vm->def->cputune.vcpupin,
+                            &vm->def->cputune.nvcpupin,
                             cpumap,
                             maplen,
                             vcpu) < 0) {
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index f29f533..99def42 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -2303,8 +2303,8 @@ xenDaemonDomainPinVcpu(virDomainPtr domain, unsigned int vcpu,
             }
             def->cputune.nvcpupin = 0;
         }
-        if (virDomainVcpuPinAdd(def->cputune->vcpupin,
-                                &def->cputune->nvcpupin,
+        if (virDomainVcpuPinAdd(def->cputune.vcpupin,
+                                &def->cputune.nvcpupin,
                                 cpumap,
                                 maplen,
                                 vcpu) < 0) {

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

    