[PATCH V2] Xen: support maxvcpus in xm and xl config

[PATCH V2] Xen: support maxvcpus in xm and xl config

[Jim Fehlig]

From: Ian Campbell <ian.campbell@citrix.com>

xend prior to 4.0 understands vcpus as maxvcpus and vcpu_avail
as a bit map of which cpus are online (default is all).

xend from 4.0 onwards understands maxvcpus as maxvcpus and
vcpus as the number which are online (from 0..N-1). The
upstream commit (68a94cf528e6 "xm: Add maxvcpus support")
claims that if maxvcpus is omitted then the old behaviour
(i.e. obeying vcpu_avail) is retained, but AFAICT it was not,
in this case vcpu==maxcpus==online cpus. This is good for us
because handling anything else would be fiddly.

This patch changes parsing of the virDomainDef maxvcpus and vcpus
entries to use the corresponding 'maxvcpus' and 'vcpus' settings
from xm and xl config. It also drops use of the old Xen 3.x
'vcpu_avail' setting.

The change also removes the maxvcpus limit of MAX_VIRT_VCPUS (since
maxvcpus is simply a count, not a bit mask), which is particularly
crucial on ARM where MAX_VIRT_CPUS == 1 (since all guests are
expected to support vcpu placement, and therefore only the boot
vcpu's info lives in the shared info page).

Existing tests adjusted accordingly, and new tests added for the
'maxvcpus' setting.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---

This is a respin of Ian's patch [1] to support parsing/formatting of the
'maxvcpus' xm and xl config setting. It goes a bit further and removes
the old Xen 3.x 'vcpu_avail' setting and lifts the maxvcpus limit of
MAX_VIRT_CPUS entirely.

The patch is based on my recent series [2] removing xend config version from
the codebase.

[1] https://www.redhat.com/archives/libvir-list/2015-November/msg01155.html
[2] https://www.redhat.com/archives/libvir-list/2015-December/msg00616.html


 src/xenconfig/xen_common.c                    | 25 +++++++++++----------
 tests/xlconfigdata/test-paravirt-maxvcpus.cfg | 13 +++++++++++
 tests/xlconfigdata/test-paravirt-maxvcpus.xml | 28 ++++++++++++++++++++++++
 tests/xlconfigtest.c                          |  1 +
 tests/xmconfigdata/test-paravirt-maxvcpus.cfg | 13 +++++++++++
 tests/xmconfigdata/test-paravirt-maxvcpus.xml | 31 +++++++++++++++++++++++++++
 tests/xmconfigdata/test-paravirt-vcpu.cfg     |  4 ++--
 tests/xmconfigtest.c                          |  1 +
 8 files changed, 101 insertions(+), 15 deletions(-)

diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
index f3e7e18..54f5791 100644
--- a/src/xenconfig/xen_common.c
+++ b/src/xenconfig/xen_common.c
@@ -488,19 +488,22 @@ xenParseCPUFeatures(virConfPtr conf, virDomainDefPtr def)
     const char *str = NULL;
     int val = 0;
 
-    if (xenConfigGetULong(conf, "vcpus", &count, 1) < 0 ||
-        MAX_VIRT_CPUS < count)
+    if (xenConfigGetULong(conf, "vcpus", &count, 1) < 0)
         return -1;
 
     if (virDomainDefSetVcpusMax(def, count) < 0)
         return -1;
 
-    if (xenConfigGetULong(conf, "vcpu_avail", &count, -1) < 0)
+    if (virDomainDefSetVcpus(def, count) < 0)
         return -1;
 
-    if (virDomainDefSetVcpus(def, MIN(count_one_bits_l(count),
-                                      virDomainDefGetVcpusMax(def))) < 0)
-        return -1;
+    if (virConfGetValue(conf, "maxvcpus")) {
+        if (xenConfigGetULong(conf, "maxvcpus", &count, 0) < 0)
+            return -1;
+
+        if (virDomainDefSetVcpusMax(def, count) < 0)
+            return -1;
+    }
 
     if (xenConfigGetString(conf, "cpus", &str, NULL) < 0)
         return -1;
@@ -1494,14 +1497,10 @@ xenFormatCPUAllocation(virConfPtr conf, virDomainDefPtr def)
     int ret = -1;
     char *cpus = NULL;
 
-    if (xenConfigSetInt(conf, "vcpus", virDomainDefGetVcpusMax(def)) < 0)
+    if (virDomainDefGetVcpus(def) < virDomainDefGetVcpusMax(def) &&
+        xenConfigSetInt(conf, "maxvcpus", virDomainDefGetVcpusMax(def)) < 0)
         goto cleanup;
-
-    /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is
-       either 32, or 64 on a platform where long is big enough.  */
-    if (virDomainDefHasVcpusOffline(def) &&
-        xenConfigSetInt(conf, "vcpu_avail",
-                        (1UL << virDomainDefGetVcpus(def)) - 1) < 0)
+    if (xenConfigSetInt(conf, "vcpus", virDomainDefGetVcpus(def)) < 0)
         goto cleanup;
 
     if ((def->cpumask != NULL) &&
diff --git a/tests/xlconfigdata/test-paravirt-maxvcpus.cfg b/tests/xlconfigdata/test-paravirt-maxvcpus.cfg
new file mode 100644
index 0000000..d506b75
--- /dev/null
+++ b/tests/xlconfigdata/test-paravirt-maxvcpus.cfg
@@ -0,0 +1,13 @@
+name = "XenGuest1"
+uuid = "45b60f51-88a9-47a8-a3b3-5e66d71b2283"
+maxmem = 512
+memory = 512
+maxvcpus = 8
+vcpus = 4
+localtime = 0
+on_poweroff = "preserve"
+on_reboot = "restart"
+on_crash = "preserve"
+vif = [ "mac=5a:36:0e:be:00:09" ]
+bootloader = "/usr/bin/pygrub"
+disk = [ "/var/lib/xen/images/debian/disk.qcow2,qcow2,xvda,w,backendtype=qdisk" ]
diff --git a/tests/xlconfigdata/test-paravirt-maxvcpus.xml b/tests/xlconfigdata/test-paravirt-maxvcpus.xml
new file mode 100644
index 0000000..2e1f8f8
--- /dev/null
+++ b/tests/xlconfigdata/test-paravirt-maxvcpus.xml
@@ -0,0 +1,28 @@
+<domain type='xen'>
+  <name>XenGuest1</name>
+  <uuid>45b60f51-88a9-47a8-a3b3-5e66d71b2283</uuid>
+  <memory unit='KiB'>524288</memory>
+  <currentMemory unit='KiB'>524288</currentMemory>
+  <vcpu placement='static' current='4'>8</vcpu>
+  <bootloader>/usr/bin/pygrub</bootloader>
+  <os>
+    <type arch='x86_64' machine='xenpv'>linux</type>
+  </os>
+  <clock offset='utc' adjustment='reset'/>
+  <on_poweroff>preserve</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>preserve</on_crash>
+  <devices>
+    <disk type='file' device='disk'>
+      <driver name='qemu' type='qcow2'/>
+      <source file='/var/lib/xen/images/debian/disk.qcow2'/>
+      <target dev='xvda' bus='xen'/>
+    </disk>
+    <interface type='ethernet'>
+      <mac address='5a:36:0e:be:00:09'/>
+    </interface>
+    <console type='pty'>
+      <target type='xen' port='0'/>
+    </console>
+  </devices>
+</domain>
diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c
index de7f871..1db11e7 100644
--- a/tests/xlconfigtest.c
+++ b/tests/xlconfigtest.c
@@ -192,6 +192,7 @@ mymain(void)
             ret = -1;                                                   \
     } while (0)
 
+    DO_TEST("paravirt-maxvcpus");
     DO_TEST("new-disk");
     DO_TEST("spice");
     DO_TEST("spice-features");
diff --git a/tests/xmconfigdata/test-paravirt-maxvcpus.cfg b/tests/xmconfigdata/test-paravirt-maxvcpus.cfg
new file mode 100644
index 0000000..8d1ac4d
--- /dev/null
+++ b/tests/xmconfigdata/test-paravirt-maxvcpus.cfg
@@ -0,0 +1,13 @@
+name = "XenGuest1"
+uuid = "c7a5fdb0-cdaf-9455-926a-d65c16db1809"
+maxmem = 579
+memory = 394
+maxvcpus = 4
+vcpus = 2
+localtime = 0
+on_poweroff = "destroy"
+on_reboot = "restart"
+on_crash = "restart"
+vif = [ "mac=00:16:3e:66:94:9c,bridge=br0,script=vif-bridge" ]
+bootloader = "/usr/bin/pygrub"
+disk = [ "phy:/dev/HostVG/XenGuest1,xvda,w" ]
diff --git a/tests/xmconfigdata/test-paravirt-maxvcpus.xml b/tests/xmconfigdata/test-paravirt-maxvcpus.xml
new file mode 100644
index 0000000..3b0e0ce
--- /dev/null
+++ b/tests/xmconfigdata/test-paravirt-maxvcpus.xml
@@ -0,0 +1,31 @@
+<domain type='xen'>
+  <name>XenGuest1</name>
+  <uuid>c7a5fdb0-cdaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>592896</memory>
+  <currentMemory unit='KiB'>403456</currentMemory>
+  <vcpu placement='static' current='2'>4</vcpu>
+  <bootloader>/usr/bin/pygrub</bootloader>
+  <os>
+    <type arch='i686' machine='xenpv'>linux</type>
+  </os>
+  <clock offset='utc' adjustment='reset'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>restart</on_crash>
+  <devices>
+    <disk type='block' device='disk'>
+      <driver name='phy'/>
+      <source dev='/dev/HostVG/XenGuest1'/>
+      <target dev='xvda' bus='xen'/>
+    </disk>
+    <interface type='bridge'>
+      <mac address='00:16:3e:66:94:9c'/>
+      <source bridge='br0'/>
+      <script path='vif-bridge'/>
+    </interface>
+    <console type='pty'>
+      <target type='xen' port='0'/>
+    </console>
+    <memballoon model='xen'/>
+  </devices>
+</domain>
diff --git a/tests/xmconfigdata/test-paravirt-vcpu.cfg b/tests/xmconfigdata/test-paravirt-vcpu.cfg
index 1e7765d..8d1ac4d 100644
--- a/tests/xmconfigdata/test-paravirt-vcpu.cfg
+++ b/tests/xmconfigdata/test-paravirt-vcpu.cfg
@@ -2,8 +2,8 @@ name = "XenGuest1"
 uuid = "c7a5fdb0-cdaf-9455-926a-d65c16db1809"
 maxmem = 579
 memory = 394
-vcpus = 4
-vcpu_avail = 3
+maxvcpus = 4
+vcpus = 2
 localtime = 0
 on_poweroff = "destroy"
 on_reboot = "restart"
diff --git a/tests/xmconfigtest.c b/tests/xmconfigtest.c
index c2084c2..3d47693 100644
--- a/tests/xmconfigtest.c
+++ b/tests/xmconfigtest.c
@@ -215,6 +215,7 @@ mymain(void)
     DO_TEST("paravirt-net-e1000");
     DO_TEST("paravirt-net-vifname");
     DO_TEST("paravirt-vcpu");
+    DO_TEST("paravirt-maxvcpus");
     DO_TEST("fullvirt-new-cdrom");
     DO_TEST("fullvirt-utc");
     DO_TEST("fullvirt-localtime");
-- 
2.1.4

Re: [PATCH V2] Xen: support maxvcpus in xm and xl config

[Ian Campbell]

On Tue, 2015-12-15 at 15:20 -0700, Jim Fehlig wrote:
> From: Ian Campbell <ian.campbell@citrix.com>
> 
> xend prior to 4.0 understands vcpus as maxvcpus and vcpu_avail
> as a bit map of which cpus are online (default is all).
> 
> xend from 4.0 onwards understands maxvcpus as maxvcpus and
> vcpus as the number which are online (from 0..N-1). The
> upstream commit (68a94cf528e6 "xm: Add maxvcpus support")
> claims that if maxvcpus is omitted then the old behaviour
> (i.e. obeying vcpu_avail) is retained, but AFAICT it was not,
> in this case vcpu==maxcpus==online cpus. This is good for us
> because handling anything else would be fiddly.
> 
> This patch changes parsing of the virDomainDef maxvcpus and vcpus
> entries to use the corresponding 'maxvcpus' and 'vcpus' settings
> from xm and xl config. It also drops use of the old Xen 3.x
> 'vcpu_avail' setting.
> 
> The change also removes the maxvcpus limit of MAX_VIRT_VCPUS (since
> maxvcpus is simply a count, not a bit mask), which is particularly
> crucial on ARM where MAX_VIRT_CPUS == 1 (since all guests are
> expected to support vcpu placement, and therefore only the boot
> vcpu's info lives in the shared info page).
> 
> Existing tests adjusted accordingly, and new tests added for the
> 'maxvcpus' setting.
> 
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>

Acked-by: Ian Campbell <Ian.campbell@citrix.com>
Tested-by: Ian Campbell <Ian.campbell@citrix.com>
(as far as "domxml-from-native xen-xl" goes, I seem to have another issue
actually starting a domain on ARM, which I'll investigate...)

Re: [PATCH V2] Xen: support maxvcpus in xm and xl config

[Ian Campbell]

On Wed, 2015-12-16 at 10:11 +0000, Ian Campbell wrote:
> On Tue, 2015-12-15 at 15:20 -0700, Jim Fehlig wrote:
> > From: Ian Campbell <ian.campbell@citrix.com>
> > 
> > xend prior to 4.0 understands vcpus as maxvcpus and vcpu_avail
> > as a bit map of which cpus are online (default is all).
> > 
> > xend from 4.0 onwards understands maxvcpus as maxvcpus and
> > vcpus as the number which are online (from 0..N-1). The
> > upstream commit (68a94cf528e6 "xm: Add maxvcpus support")
> > claims that if maxvcpus is omitted then the old behaviour
> > (i.e. obeying vcpu_avail) is retained, but AFAICT it was not,
> > in this case vcpu==maxcpus==online cpus. This is good for us
> > because handling anything else would be fiddly.
> > 
> > This patch changes parsing of the virDomainDef maxvcpus and vcpus
> > entries to use the corresponding 'maxvcpus' and 'vcpus' settings
> > from xm and xl config. It also drops use of the old Xen 3.x
> > 'vcpu_avail' setting.
> > 
> > The change also removes the maxvcpus limit of MAX_VIRT_VCPUS (since
> > maxvcpus is simply a count, not a bit mask), which is particularly
> > crucial on ARM where MAX_VIRT_CPUS == 1 (since all guests are
> > expected to support vcpu placement, and therefore only the boot
> > vcpu's info lives in the shared info page).
> > 
> > Existing tests adjusted accordingly, and new tests added for the
> > 'maxvcpus' setting.
> > 
> > Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> 
> Acked-by: Ian Campbell <Ian.campbell@citrix.com>
> Tested-by: Ian Campbell <Ian.campbell@citrix.com>
> (as far as "domxml-from-native xen-xl" goes, I seem to have another issue
> actually starting a domain on ARM, which I'll investigate...)

Turned out to be a mismatch between my build-time and run-time libxl
versions, fixed by a clean rebuild. So that's a full Tested-by.

I noticed that the newer cmdline= (inplace of root=+extra= etc) wasn't
supported. I'll knock something up.

Ian.

Re: [libvirt] [PATCH V2] Xen: support maxvcpus in xm and xl config

[Michal Privoznik]

On 15.12.2015 23:20, Jim Fehlig wrote:
> From: Ian Campbell <ian.campbell@citrix.com>
> 
> xend prior to 4.0 understands vcpus as maxvcpus and vcpu_avail
> as a bit map of which cpus are online (default is all).
> 
> xend from 4.0 onwards understands maxvcpus as maxvcpus and
> vcpus as the number which are online (from 0..N-1). The
> upstream commit (68a94cf528e6 "xm: Add maxvcpus support")
> claims that if maxvcpus is omitted then the old behaviour
> (i.e. obeying vcpu_avail) is retained, but AFAICT it was not,
> in this case vcpu==maxcpus==online cpus. This is good for us
> because handling anything else would be fiddly.
> 
> This patch changes parsing of the virDomainDef maxvcpus and vcpus
> entries to use the corresponding 'maxvcpus' and 'vcpus' settings
> from xm and xl config. It also drops use of the old Xen 3.x
> 'vcpu_avail' setting.
> 
> The change also removes the maxvcpus limit of MAX_VIRT_VCPUS (since
> maxvcpus is simply a count, not a bit mask), which is particularly
> crucial on ARM where MAX_VIRT_CPUS == 1 (since all guests are
> expected to support vcpu placement, and therefore only the boot
> vcpu's info lives in the shared info page).
> 
> Existing tests adjusted accordingly, and new tests added for the
> 'maxvcpus' setting.
> 
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> ---
> 
> This is a respin of Ian's patch [1] to support parsing/formatting of the
> 'maxvcpus' xm and xl config setting. It goes a bit further and removes
> the old Xen 3.x 'vcpu_avail' setting and lifts the maxvcpus limit of
> MAX_VIRT_CPUS entirely.
> 
> The patch is based on my recent series [2] removing xend config version from
> the codebase.
> 
> [1] https://www.redhat.com/archives/libvir-list/2015-November/msg01155.html
> [2] https://www.redhat.com/archives/libvir-list/2015-December/msg00616.html
> 
> 
>  src/xenconfig/xen_common.c                    | 25 +++++++++++----------
>  tests/xlconfigdata/test-paravirt-maxvcpus.cfg | 13 +++++++++++
>  tests/xlconfigdata/test-paravirt-maxvcpus.xml | 28 ++++++++++++++++++++++++
>  tests/xlconfigtest.c                          |  1 +
>  tests/xmconfigdata/test-paravirt-maxvcpus.cfg | 13 +++++++++++
>  tests/xmconfigdata/test-paravirt-maxvcpus.xml | 31 +++++++++++++++++++++++++++
>  tests/xmconfigdata/test-paravirt-vcpu.cfg     |  4 ++--
>  tests/xmconfigtest.c                          |  1 +
>  8 files changed, 101 insertions(+), 15 deletions(-)

ACK

Michal

Re: [libvirt] [PATCH V2] Xen: support maxvcpus in xm and xl config

[Jim Fehlig]

On 12/18/2015 04:47 AM, Michal Privoznik wrote:
> On 15.12.2015 23:20, Jim Fehlig wrote:
>> From: Ian Campbell <ian.campbell@citrix.com>
>>
>> xend prior to 4.0 understands vcpus as maxvcpus and vcpu_avail
>> as a bit map of which cpus are online (default is all).
>>
>> xend from 4.0 onwards understands maxvcpus as maxvcpus and
>> vcpus as the number which are online (from 0..N-1). The
>> upstream commit (68a94cf528e6 "xm: Add maxvcpus support")
>> claims that if maxvcpus is omitted then the old behaviour
>> (i.e. obeying vcpu_avail) is retained, but AFAICT it was not,
>> in this case vcpu==maxcpus==online cpus. This is good for us
>> because handling anything else would be fiddly.
>>
>> This patch changes parsing of the virDomainDef maxvcpus and vcpus
>> entries to use the corresponding 'maxvcpus' and 'vcpus' settings
>> from xm and xl config. It also drops use of the old Xen 3.x
>> 'vcpu_avail' setting.
>>
>> The change also removes the maxvcpus limit of MAX_VIRT_VCPUS (since
>> maxvcpus is simply a count, not a bit mask), which is particularly
>> crucial on ARM where MAX_VIRT_CPUS == 1 (since all guests are
>> expected to support vcpu placement, and therefore only the boot
>> vcpu's info lives in the shared info page).
>>
>> Existing tests adjusted accordingly, and new tests added for the
>> 'maxvcpus' setting.
>>
>> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>> ---
>>
>> This is a respin of Ian's patch [1] to support parsing/formatting of the
>> 'maxvcpus' xm and xl config setting. It goes a bit further and removes
>> the old Xen 3.x 'vcpu_avail' setting and lifts the maxvcpus limit of
>> MAX_VIRT_CPUS entirely.
>>
>> The patch is based on my recent series [2] removing xend config version from
>> the codebase.
>>
>> [1] https://www.redhat.com/archives/libvir-list/2015-November/msg01155.html
>> [2] https://www.redhat.com/archives/libvir-list/2015-December/msg00616.html
>>
>>
>>  src/xenconfig/xen_common.c                    | 25 +++++++++++----------
>>  tests/xlconfigdata/test-paravirt-maxvcpus.cfg | 13 +++++++++++
>>  tests/xlconfigdata/test-paravirt-maxvcpus.xml | 28 ++++++++++++++++++++++++
>>  tests/xlconfigtest.c                          |  1 +
>>  tests/xmconfigdata/test-paravirt-maxvcpus.cfg | 13 +++++++++++
>>  tests/xmconfigdata/test-paravirt-maxvcpus.xml | 31 +++++++++++++++++++++++++++
>>  tests/xmconfigdata/test-paravirt-vcpu.cfg     |  4 ++--
>>  tests/xmconfigtest.c                          |  1 +
>>  8 files changed, 101 insertions(+), 15 deletions(-)
> ACK

Thanks, pushed now.

Regards,
Jim