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.

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 69b896780488d16645323e7ad22e904d28990571..a7be74572a3997a0fcae0d0a9f94f18b59ef4e59 100644 (file)
--- 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 3e6633c867133142012e457c36da2f6112b61551..c8cc95dd915271a1229bc8b8b825e490be356d4c 100644 (file)
--- 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 d588e5ae5370372e04a200c5e7a583ab94cbd47b..4e3f82afb539a26bb325b31189dab7de3b0be236 100644 (file)
--- 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.