From a02a161bb8a6caf0db4dd446ed1cdf53d97b40b9 Mon Sep 17 00:00:00 2001
From: Peter Krempa <pkrempa@redhat.com>
Date: Mon, 25 May 2015 14:19:50 +0200
Subject: [PATCH] qemu: libxl: vcpupin: Don't reset pinning when pinning to all
 pcpus

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 69b8967804..a7be74572a 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2246,14 +2246,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;
@@ -2269,7 +2261,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 3e6633c867..c8cc95dd91 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5033,7 +5033,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;
@@ -5092,12 +5091,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) {
@@ -5146,19 +5139,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;
@@ -5178,26 +5165,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 d588e5ae53..4e3f82afb5 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.
-- 
GitLab