[libvirt] [PATCH 20/35] qemu: libxl: vcpupin: Don't reset pinning when pinning to all pcpus

[Peter Krempa]

In the pre-NUMA ages pinning a vCPU to all pCPUs was eaqual to deleting
the pinning info. Now it does not entirely work that way. Pinning a vCPU
to all pCPUs might be a desired operation. Additionally removal of the
pinning will result into using the default pinning information at the
next boot which might be different from all vcpus.

This patch removes the false assumption that we should remove the
pinning after pinning to all vCPUs and tweaks the documentation for
virsh.

A later patch will implement a new flag for the virDomainPinVcpuFlags
API that will allow to remove the pinning in a sane way.
---
 src/libxl/libxl_driver.c |  9 --------
 src/qemu/qemu_driver.c   | 57 ++++++++++++++++--------------------------------
 tools/virsh.pod          |  3 +--
 3 files changed, 20 insertions(+), 49 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 9be89f2..2ff9496 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2244,14 +2244,6 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu,
         }
     }

-    /* full bitmap means reset the settings (if any). */
-    if (virBitmapIsAllSet(pcpumap)) {
-        virDomainPinDel(&targetDef->cputune.vcpupin,
-                        &targetDef->cputune.nvcpupin,
-                        vcpu);
-        goto done;
-    }
-
     if (!targetDef->cputune.vcpupin) {
         if (VIR_ALLOC(targetDef->cputune.vcpupin) < 0)
             goto endjob;
@@ -2267,7 +2259,6 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu,
         goto endjob;
     }

- done:
     ret = 0;

     if (flags & VIR_DOMAIN_AFFECT_LIVE) {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2be26c6..96fc0d4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5029,7 +5029,6 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
     virCgroupPtr cgroup_vcpu = NULL;
     int ret = -1;
     qemuDomainObjPrivatePtr priv;
-    bool doReset = false;
     size_t newVcpuPinNum = 0;
     virDomainPinDefPtr *newVcpuPin = NULL;
     virBitmapPtr pcpumap = NULL;
@@ -5088,12 +5087,6 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
         goto endjob;
     }

-    /* pinning to all physical cpus means resetting,
-     * so check if we can reset setting.
-     */
-    if (virBitmapIsAllSet(pcpumap))
-        doReset = true;
-
     if (flags & VIR_DOMAIN_AFFECT_LIVE) {

         if (priv->vcpupids == NULL) {
@@ -5142,19 +5135,13 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
             }
         }

-        if (doReset) {
-            virDomainPinDel(&vm->def->cputune.vcpupin,
-                            &vm->def->cputune.nvcpupin,
-                            vcpu);
-        } else {
-            if (vm->def->cputune.vcpupin)
-                virDomainPinDefArrayFree(vm->def->cputune.vcpupin,
-                                         vm->def->cputune.nvcpupin);
+        if (vm->def->cputune.vcpupin)
+            virDomainPinDefArrayFree(vm->def->cputune.vcpupin,
+                                     vm->def->cputune.nvcpupin);

-            vm->def->cputune.vcpupin = newVcpuPin;
-            vm->def->cputune.nvcpupin = newVcpuPinNum;
-            newVcpuPin = NULL;
-        }
+        vm->def->cputune.vcpupin = newVcpuPin;
+        vm->def->cputune.nvcpupin = newVcpuPinNum;
+        newVcpuPin = NULL;

         if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
             goto endjob;
@@ -5174,26 +5161,20 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,

     if (flags & VIR_DOMAIN_AFFECT_CONFIG) {

-        if (doReset) {
-            virDomainPinDel(&persistentDef->cputune.vcpupin,
-                            &persistentDef->cputune.nvcpupin,
-                            vcpu);
-        } else {
-            if (!persistentDef->cputune.vcpupin) {
-                if (VIR_ALLOC(persistentDef->cputune.vcpupin) < 0)
-                    goto endjob;
-                persistentDef->cputune.nvcpupin = 0;
-            }
-            if (virDomainPinAdd(&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"));
+        if (!persistentDef->cputune.vcpupin) {
+            if (VIR_ALLOC(persistentDef->cputune.vcpupin) < 0)
                 goto endjob;
-            }
+            persistentDef->cputune.nvcpupin = 0;
+        }
+        if (virDomainPinAdd(&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"));
+            goto endjob;
         }

         ret = virDomainSaveConfig(cfg->configDir, persistentDef);
diff --git a/tools/virsh.pod b/tools/virsh.pod
index d588e5a..4e3f82a 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2365,8 +2365,7 @@ I<vcpu> or omit I<vcpu> to list all at once.
 I<cpulist> is a list of physical CPU numbers. Its syntax is a comma
 separated list and a special markup using '-' and '^' (ex. '0-4', '0-3,^2') can
 also be allowed. The '-' denotes the range and the '^' denotes exclusive.
-If you want to reset vcpupin setting, that is, to pin the I<vcpu> to all
-physical cpus, simply specify 'r' as a I<cpulist>.
+For pinning the I<vcpu> to all physical cpus specify 'r' as a I<cpulist>.
 If I<--live> is specified, affect a running guest.
 If I<--config> is specified, affect the next boot of a persistent guest.
 If I<--current> is specified, affect the current guest state.
-- 
2.4.1