[libvirt] [PATCH LIBVIRT v1 0/2] Support maxvcpus (AKA >1 vcpu on Xen/ARM)
libvirt currently clamps the maximum number of vcpus to MAX_VIRT_CPUS == XEN_LEGACY_MAX_VCPUS, which on ARM is 1 (because all guests are expected to support vcpu info placement). Even on x86 this limitation is a hold over from an older xm interface where the maximum number of vcpus was expressed as a bitmap and so limited to the number of bits in an unsigned long (for which XEN_LEGACY_MAX_VCPUS is a convenient proxy), which doesn't apply to xl on x86 which uses a bitmap data type to express larger bitmaps. To do this it was necessary to add a new value to xenConfigVersionEnum corresponding to this new version. I've tested with tests/x?configtest (including new test case) on x86 and on ARM with xlconfigtest and via domxml-from-native. As you might infer from some of the changlogs I feel a bit like I am abusing xendConfigVersion somewhat here (given that xl != xend, and that real xend never went passed xend_config_version) but its use is quite widespread in the common xl/xm support code in libvirt so a great deal of refactoring/renaming would otherwise seem to be required. I'm open to other ideas or suggestions though. Ian.
libxlConnectDomainXMLFromNative calls both xenParseXM and xenParseXL with cfg->verInfo->xen_version_major, however AFAICT they both (either inherently, or through there use of xenParseConfigCommon expect a value from xenConfigVersionEnum (which does not correspond to xen_version_major). The actual xend backend passes priv->xendConfigVersion, which seems to have been gotten from xend and I assume conforms to that enum, via the node/xend_config_format value in the xend sxp. Add a new value to that enum, XEND_CONFIG_VERSION_4_0_0, on the basis that the xl syntax was originally based on (and intended to be compatible with) xm circa that point in time and that I will shortly want to add handling of a cfg file difference which occured in xend in 4.0.0 (maxvcpus instead of vpu_avail bitmask). Pass this from every caller of xen{Parse,Format}X? in the libxl driver. Update xlconfigtest to pass this new value, and regenerate the output files with that (all of the changes are expected AFAICT). The enum and the parameters are already slightly misnamed now (since they kind-of apply to xl too), but I didn't go through and rename them. In the future we may want to add new values to the enum to reflect evolution of the xl cfg syntax (xend was essentially static from 4.0 until it was removed), at that point renaming should probably be considered. Note also that xend's xend_config_format remained at 4 until it's removal in Xen 4.5, so there will be no actual change in behaviour for xm/xend here. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> --- src/libxl/libxl_driver.c | 8 ++++---- src/xenconfig/xen_sxpr.h | 1 + tests/xlconfigdata/test-fullvirt-direct-kernel-boot.cfg | 3 ++- tests/xlconfigdata/test-fullvirt-direct-kernel-boot.xml | 2 +- tests/xlconfigdata/test-fullvirt-multiusb.cfg | 3 ++- tests/xlconfigdata/test-fullvirt-multiusb.xml | 2 +- tests/xlconfigdata/test-new-disk.cfg | 3 ++- tests/xlconfigdata/test-new-disk.xml | 2 +- tests/xlconfigdata/test-spice-features.cfg | 3 ++- tests/xlconfigdata/test-spice-features.xml | 2 +- tests/xlconfigdata/test-spice.cfg | 3 ++- tests/xlconfigdata/test-spice.xml | 2 +- tests/xlconfigtest.c | 10 +++++----- 13 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 35d7fae..892ae44 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2587,14 +2587,14 @@ libxlConnectDomainXMLFromNative(virConnectPtr conn, goto cleanup; if (!(def = xenParseXL(conf, cfg->caps, - cfg->verInfo->xen_version_major))) + XEND_CONFIG_VERSION_4_0_0))) goto cleanup; } else if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) { if (!(conf = virConfReadMem(nativeConfig, strlen(nativeConfig), 0))) goto cleanup; if (!(def = xenParseXM(conf, - cfg->verInfo->xen_version_major, + XEND_CONFIG_VERSION_4_0_0, cfg->caps))) goto cleanup; } else if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_SEXPR)) { @@ -2647,10 +2647,10 @@ libxlConnectDomainXMLToNative(virConnectPtr conn, const char * nativeFormat, goto cleanup; if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XL)) { - if (!(conf = xenFormatXL(def, conn, cfg->verInfo->xen_version_major))) + if (!(conf = xenFormatXL(def, conn, XEND_CONFIG_VERSION_4_0_0))) goto cleanup; } else if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) { - if (!(conf = xenFormatXM(conn, def, cfg->verInfo->xen_version_major))) + if (!(conf = xenFormatXM(conn, def, XEND_CONFIG_VERSION_4_0_0))) goto cleanup; } else { diff --git a/src/xenconfig/xen_sxpr.h b/src/xenconfig/xen_sxpr.h index f354a50..bb9ed3c 100644 --- a/src/xenconfig/xen_sxpr.h +++ b/src/xenconfig/xen_sxpr.h @@ -37,6 +37,7 @@ typedef enum { XEND_CONFIG_VERSION_3_0_3 = 2, XEND_CONFIG_VERSION_3_0_4 = 3, XEND_CONFIG_VERSION_3_1_0 = 4, + XEND_CONFIG_VERSION_4_0_0 = 5, } xenConfigVersionEnum; /* helper functions to get the dom id from a sexpr */ diff --git a/tests/xlconfigdata/test-fullvirt-direct-kernel-boot.cfg b/tests/xlconfigdata/test-fullvirt-direct-kernel-boot.cfg index 1fac3a5..f452af6 100644 --- a/tests/xlconfigdata/test-fullvirt-direct-kernel-boot.cfg +++ b/tests/xlconfigdata/test-fullvirt-direct-kernel-boot.cfg @@ -8,6 +8,7 @@ acpi = 1 apic = 1 hap = 0 viridian = 0 +rtc_timeoffset = 0 localtime = 0 on_poweroff = "destroy" on_reboot = "restart" @@ -18,7 +19,7 @@ vnc = 1 vncunused = 1 vnclisten = "127.0.0.1" vncpasswd = "123poi" -vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu" ] +vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000" ] parallel = "none" serial = "none" builder = "hvm" diff --git a/tests/xlconfigdata/test-fullvirt-direct-kernel-boot.xml b/tests/xlconfigdata/test-fullvirt-direct-kernel-boot.xml index 414f645..5298d19 100644 --- a/tests/xlconfigdata/test-fullvirt-direct-kernel-boot.xml +++ b/tests/xlconfigdata/test-fullvirt-direct-kernel-boot.xml @@ -17,7 +17,7 @@ <apic/> <pae/> </features> - <clock offset='utc' adjustment='reset'/> + <clock offset='variable' adjustment='0' basis='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>restart</on_crash> diff --git a/tests/xlconfigdata/test-fullvirt-multiusb.cfg b/tests/xlconfigdata/test-fullvirt-multiusb.cfg index 68a2614..d0482a8 100755 --- a/tests/xlconfigdata/test-fullvirt-multiusb.cfg +++ b/tests/xlconfigdata/test-fullvirt-multiusb.cfg @@ -8,6 +8,7 @@ acpi = 1 apic = 1 hap = 0 viridian = 0 +rtc_timeoffset = 0 localtime = 0 on_poweroff = "destroy" on_reboot = "restart" @@ -18,7 +19,7 @@ vnc = 1 vncunused = 1 vnclisten = "127.0.0.1" vncpasswd = "123poi" -vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu" ] +vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000" ] parallel = "none" serial = "none" builder = "hvm" diff --git a/tests/xlconfigdata/test-fullvirt-multiusb.xml b/tests/xlconfigdata/test-fullvirt-multiusb.xml index 642c242..7331613 100644 --- a/tests/xlconfigdata/test-fullvirt-multiusb.xml +++ b/tests/xlconfigdata/test-fullvirt-multiusb.xml @@ -14,7 +14,7 @@ <apic/> <pae/> </features> - <clock offset='utc' adjustment='reset'/> + <clock offset='variable' adjustment='0' basis='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>restart</on_crash> diff --git a/tests/xlconfigdata/test-new-disk.cfg b/tests/xlconfigdata/test-new-disk.cfg index 9e9f106..9b9fb36 100644 --- a/tests/xlconfigdata/test-new-disk.cfg +++ b/tests/xlconfigdata/test-new-disk.cfg @@ -8,6 +8,7 @@ acpi = 1 apic = 1 hap = 0 viridian = 0 +rtc_timeoffset = 0 localtime = 0 on_poweroff = "destroy" on_reboot = "restart" @@ -17,7 +18,7 @@ sdl = 0 vnc = 1 vncunused = 1 vnclisten = "127.0.0.1" -vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu" ] +vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000" ] parallel = "none" serial = "none" builder = "hvm" diff --git a/tests/xlconfigdata/test-new-disk.xml b/tests/xlconfigdata/test-new-disk.xml index 1c96a62..7a74926 100644 --- a/tests/xlconfigdata/test-new-disk.xml +++ b/tests/xlconfigdata/test-new-disk.xml @@ -14,7 +14,7 @@ <apic/> <pae/> </features> - <clock offset='utc' adjustment='reset'/> + <clock offset='variable' adjustment='0' basis='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>restart</on_crash> diff --git a/tests/xlconfigdata/test-spice-features.cfg b/tests/xlconfigdata/test-spice-features.cfg index c3e7111..152cb27 100644 --- a/tests/xlconfigdata/test-spice-features.cfg +++ b/tests/xlconfigdata/test-spice-features.cfg @@ -8,12 +8,13 @@ acpi = 1 apic = 1 hap = 0 viridian = 0 +rtc_timeoffset = 0 localtime = 0 on_poweroff = "destroy" on_reboot = "restart" on_crash = "restart" device_model = "/usr/lib/xen/bin/qemu-dm" -vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu" ] +vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000" ] parallel = "none" serial = "none" builder = "hvm" diff --git a/tests/xlconfigdata/test-spice-features.xml b/tests/xlconfigdata/test-spice-features.xml index 8f3fcf5..10e74e0 100644 --- a/tests/xlconfigdata/test-spice-features.xml +++ b/tests/xlconfigdata/test-spice-features.xml @@ -14,7 +14,7 @@ <apic/> <pae/> </features> - <clock offset='utc' adjustment='reset'/> + <clock offset='variable' adjustment='0' basis='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>restart</on_crash> diff --git a/tests/xlconfigdata/test-spice.cfg b/tests/xlconfigdata/test-spice.cfg index d89f2ba..1a96114 100644 --- a/tests/xlconfigdata/test-spice.cfg +++ b/tests/xlconfigdata/test-spice.cfg @@ -8,12 +8,13 @@ acpi = 1 apic = 1 hap = 0 viridian = 0 +rtc_timeoffset = 0 localtime = 0 on_poweroff = "destroy" on_reboot = "restart" on_crash = "restart" device_model = "/usr/lib/xen/bin/qemu-dm" -vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu" ] +vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000" ] parallel = "none" serial = "none" builder = "hvm" diff --git a/tests/xlconfigdata/test-spice.xml b/tests/xlconfigdata/test-spice.xml index e5b43d9..0884d76 100644 --- a/tests/xlconfigdata/test-spice.xml +++ b/tests/xlconfigdata/test-spice.xml @@ -14,7 +14,7 @@ <apic/> <pae/> </features> - <clock offset='utc' adjustment='reset'/> + <clock offset='variable' adjustment='0' basis='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>restart</on_crash> diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c index 952b504..08c43fb 100644 --- a/tests/xlconfigtest.c +++ b/tests/xlconfigtest.c @@ -193,15 +193,15 @@ mymain(void) ret = -1; \ } while (0) - DO_TEST("new-disk", 3); - DO_TEST("spice", 3); - DO_TEST("spice-features", 3); + DO_TEST("new-disk", 5); + DO_TEST("spice", 5); + DO_TEST("spice-features", 5); #ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST - DO_TEST("fullvirt-multiusb", 3); + DO_TEST("fullvirt-multiusb", 5); #endif #ifdef LIBXL_HAVE_BUILDINFO_KERNEL - DO_TEST("fullvirt-direct-kernel-boot", 3); + DO_TEST("fullvirt-direct-kernel-boot", 5); #endif virObjectUnref(caps); -- 2.1.4
On 11/26/2015 09:59 AM, Ian Campbell wrote:
libxlConnectDomainXMLFromNative calls both xenParseXM and xenParseXL with cfg->verInfo->xen_version_major, however AFAICT they both (either inherently, or through there use of xenParseConfigCommon expect a value from xenConfigVersionEnum (which does not correspond to xen_version_major).
I recall being a little apprehensive about using xen_version_major when writing the code, and apparently that was justified. But now that we are revisiting this, I wonder if we care about these old xend config versions at all. Is anyone even running Xen 3.0.x, or 3.1.x, particularly with a newish libvirt? IMO we could drop all of this xend config nonsense and go with the code associated with the last xend config version, i.e. XEND_CONFIG_VERSION_3_1_0. And while mentioning dropping support for these old xend config versions, do I dare mention dropping the xend driver altogether? :-) It will certainly need to be done some day. Question is whether that's a bit premature now. Regards, Jim
The actual xend backend passes priv->xendConfigVersion, which seems to have been gotten from xend and I assume conforms to that enum, via the node/xend_config_format value in the xend sxp.
Add a new value to that enum, XEND_CONFIG_VERSION_4_0_0, on the basis that the xl syntax was originally based on (and intended to be compatible with) xm circa that point in time and that I will shortly want to add handling of a cfg file difference which occured in xend in 4.0.0 (maxvcpus instead of vpu_avail bitmask).
Pass this from every caller of xen{Parse,Format}X? in the libxl driver.
Update xlconfigtest to pass this new value, and regenerate the output files with that (all of the changes are expected AFAICT).
The enum and the parameters are already slightly misnamed now (since they kind-of apply to xl too), but I didn't go through and rename them. In the future we may want to add new values to the enum to reflect evolution of the xl cfg syntax (xend was essentially static from 4.0 until it was removed), at that point renaming should probably be considered.
Note also that xend's xend_config_format remained at 4 until it's removal in Xen 4.5, so there will be no actual change in behaviour for xm/xend here.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com> --- src/libxl/libxl_driver.c | 8 ++++---- src/xenconfig/xen_sxpr.h | 1 + tests/xlconfigdata/test-fullvirt-direct-kernel-boot.cfg | 3 ++- tests/xlconfigdata/test-fullvirt-direct-kernel-boot.xml | 2 +- tests/xlconfigdata/test-fullvirt-multiusb.cfg | 3 ++- tests/xlconfigdata/test-fullvirt-multiusb.xml | 2 +- tests/xlconfigdata/test-new-disk.cfg | 3 ++- tests/xlconfigdata/test-new-disk.xml | 2 +- tests/xlconfigdata/test-spice-features.cfg | 3 ++- tests/xlconfigdata/test-spice-features.xml | 2 +- tests/xlconfigdata/test-spice.cfg | 3 ++- tests/xlconfigdata/test-spice.xml | 2 +- tests/xlconfigtest.c | 10 +++++----- 13 files changed, 25 insertions(+), 19 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 35d7fae..892ae44 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2587,14 +2587,14 @@ libxlConnectDomainXMLFromNative(virConnectPtr conn, goto cleanup; if (!(def = xenParseXL(conf, cfg->caps, - cfg->verInfo->xen_version_major))) + XEND_CONFIG_VERSION_4_0_0))) goto cleanup; } else if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) { if (!(conf = virConfReadMem(nativeConfig, strlen(nativeConfig), 0))) goto cleanup;
if (!(def = xenParseXM(conf, - cfg->verInfo->xen_version_major, + XEND_CONFIG_VERSION_4_0_0, cfg->caps))) goto cleanup; } else if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_SEXPR)) { @@ -2647,10 +2647,10 @@ libxlConnectDomainXMLToNative(virConnectPtr conn, const char * nativeFormat, goto cleanup;
if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XL)) { - if (!(conf = xenFormatXL(def, conn, cfg->verInfo->xen_version_major))) + if (!(conf = xenFormatXL(def, conn, XEND_CONFIG_VERSION_4_0_0))) goto cleanup; } else if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) { - if (!(conf = xenFormatXM(conn, def, cfg->verInfo->xen_version_major))) + if (!(conf = xenFormatXM(conn, def, XEND_CONFIG_VERSION_4_0_0))) goto cleanup; } else {
diff --git a/src/xenconfig/xen_sxpr.h b/src/xenconfig/xen_sxpr.h index f354a50..bb9ed3c 100644 --- a/src/xenconfig/xen_sxpr.h +++ b/src/xenconfig/xen_sxpr.h @@ -37,6 +37,7 @@ typedef enum { XEND_CONFIG_VERSION_3_0_3 = 2, XEND_CONFIG_VERSION_3_0_4 = 3, XEND_CONFIG_VERSION_3_1_0 = 4, + XEND_CONFIG_VERSION_4_0_0 = 5, } xenConfigVersionEnum;
/* helper functions to get the dom id from a sexpr */ diff --git a/tests/xlconfigdata/test-fullvirt-direct-kernel-boot.cfg b/tests/xlconfigdata/test-fullvirt-direct-kernel-boot.cfg index 1fac3a5..f452af6 100644 --- a/tests/xlconfigdata/test-fullvirt-direct-kernel-boot.cfg +++ b/tests/xlconfigdata/test-fullvirt-direct-kernel-boot.cfg @@ -8,6 +8,7 @@ acpi = 1 apic = 1 hap = 0 viridian = 0 +rtc_timeoffset = 0 localtime = 0 on_poweroff = "destroy" on_reboot = "restart" @@ -18,7 +19,7 @@ vnc = 1 vncunused = 1 vnclisten = "127.0.0.1" vncpasswd = "123poi" -vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu" ] +vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000" ] parallel = "none" serial = "none" builder = "hvm" diff --git a/tests/xlconfigdata/test-fullvirt-direct-kernel-boot.xml b/tests/xlconfigdata/test-fullvirt-direct-kernel-boot.xml index 414f645..5298d19 100644 --- a/tests/xlconfigdata/test-fullvirt-direct-kernel-boot.xml +++ b/tests/xlconfigdata/test-fullvirt-direct-kernel-boot.xml @@ -17,7 +17,7 @@ <apic/> <pae/> </features> - <clock offset='utc' adjustment='reset'/> + <clock offset='variable' adjustment='0' basis='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>restart</on_crash> diff --git a/tests/xlconfigdata/test-fullvirt-multiusb.cfg b/tests/xlconfigdata/test-fullvirt-multiusb.cfg index 68a2614..d0482a8 100755 --- a/tests/xlconfigdata/test-fullvirt-multiusb.cfg +++ b/tests/xlconfigdata/test-fullvirt-multiusb.cfg @@ -8,6 +8,7 @@ acpi = 1 apic = 1 hap = 0 viridian = 0 +rtc_timeoffset = 0 localtime = 0 on_poweroff = "destroy" on_reboot = "restart" @@ -18,7 +19,7 @@ vnc = 1 vncunused = 1 vnclisten = "127.0.0.1" vncpasswd = "123poi" -vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu" ] +vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000" ] parallel = "none" serial = "none" builder = "hvm" diff --git a/tests/xlconfigdata/test-fullvirt-multiusb.xml b/tests/xlconfigdata/test-fullvirt-multiusb.xml index 642c242..7331613 100644 --- a/tests/xlconfigdata/test-fullvirt-multiusb.xml +++ b/tests/xlconfigdata/test-fullvirt-multiusb.xml @@ -14,7 +14,7 @@ <apic/> <pae/> </features> - <clock offset='utc' adjustment='reset'/> + <clock offset='variable' adjustment='0' basis='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>restart</on_crash> diff --git a/tests/xlconfigdata/test-new-disk.cfg b/tests/xlconfigdata/test-new-disk.cfg index 9e9f106..9b9fb36 100644 --- a/tests/xlconfigdata/test-new-disk.cfg +++ b/tests/xlconfigdata/test-new-disk.cfg @@ -8,6 +8,7 @@ acpi = 1 apic = 1 hap = 0 viridian = 0 +rtc_timeoffset = 0 localtime = 0 on_poweroff = "destroy" on_reboot = "restart" @@ -17,7 +18,7 @@ sdl = 0 vnc = 1 vncunused = 1 vnclisten = "127.0.0.1" -vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu" ] +vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000" ] parallel = "none" serial = "none" builder = "hvm" diff --git a/tests/xlconfigdata/test-new-disk.xml b/tests/xlconfigdata/test-new-disk.xml index 1c96a62..7a74926 100644 --- a/tests/xlconfigdata/test-new-disk.xml +++ b/tests/xlconfigdata/test-new-disk.xml @@ -14,7 +14,7 @@ <apic/> <pae/> </features> - <clock offset='utc' adjustment='reset'/> + <clock offset='variable' adjustment='0' basis='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>restart</on_crash> diff --git a/tests/xlconfigdata/test-spice-features.cfg b/tests/xlconfigdata/test-spice-features.cfg index c3e7111..152cb27 100644 --- a/tests/xlconfigdata/test-spice-features.cfg +++ b/tests/xlconfigdata/test-spice-features.cfg @@ -8,12 +8,13 @@ acpi = 1 apic = 1 hap = 0 viridian = 0 +rtc_timeoffset = 0 localtime = 0 on_poweroff = "destroy" on_reboot = "restart" on_crash = "restart" device_model = "/usr/lib/xen/bin/qemu-dm" -vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu" ] +vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000" ] parallel = "none" serial = "none" builder = "hvm" diff --git a/tests/xlconfigdata/test-spice-features.xml b/tests/xlconfigdata/test-spice-features.xml index 8f3fcf5..10e74e0 100644 --- a/tests/xlconfigdata/test-spice-features.xml +++ b/tests/xlconfigdata/test-spice-features.xml @@ -14,7 +14,7 @@ <apic/> <pae/> </features> - <clock offset='utc' adjustment='reset'/> + <clock offset='variable' adjustment='0' basis='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>restart</on_crash> diff --git a/tests/xlconfigdata/test-spice.cfg b/tests/xlconfigdata/test-spice.cfg index d89f2ba..1a96114 100644 --- a/tests/xlconfigdata/test-spice.cfg +++ b/tests/xlconfigdata/test-spice.cfg @@ -8,12 +8,13 @@ acpi = 1 apic = 1 hap = 0 viridian = 0 +rtc_timeoffset = 0 localtime = 0 on_poweroff = "destroy" on_reboot = "restart" on_crash = "restart" device_model = "/usr/lib/xen/bin/qemu-dm" -vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu" ] +vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000" ] parallel = "none" serial = "none" builder = "hvm" diff --git a/tests/xlconfigdata/test-spice.xml b/tests/xlconfigdata/test-spice.xml index e5b43d9..0884d76 100644 --- a/tests/xlconfigdata/test-spice.xml +++ b/tests/xlconfigdata/test-spice.xml @@ -14,7 +14,7 @@ <apic/> <pae/> </features> - <clock offset='utc' adjustment='reset'/> + <clock offset='variable' adjustment='0' basis='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>restart</on_crash> diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c index 952b504..08c43fb 100644 --- a/tests/xlconfigtest.c +++ b/tests/xlconfigtest.c @@ -193,15 +193,15 @@ mymain(void) ret = -1; \ } while (0)
- DO_TEST("new-disk", 3); - DO_TEST("spice", 3); - DO_TEST("spice-features", 3); + DO_TEST("new-disk", 5); + DO_TEST("spice", 5); + DO_TEST("spice-features", 5);
#ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST - DO_TEST("fullvirt-multiusb", 3); + DO_TEST("fullvirt-multiusb", 5); #endif #ifdef LIBXL_HAVE_BUILDINFO_KERNEL - DO_TEST("fullvirt-direct-kernel-boot", 3); + DO_TEST("fullvirt-direct-kernel-boot", 5); #endif
virObjectUnref(caps);
On Thu, Dec 03, 2015 at 11:13:06PM -0700, Jim Fehlig wrote:
On 11/26/2015 09:59 AM, Ian Campbell wrote:
libxlConnectDomainXMLFromNative calls both xenParseXM and xenParseXL with cfg->verInfo->xen_version_major, however AFAICT they both (either inherently, or through there use of xenParseConfigCommon expect a value from xenConfigVersionEnum (which does not correspond to xen_version_major).
I recall being a little apprehensive about using xen_version_major when writing the code, and apparently that was justified. But now that we are revisiting this, I wonder if we care about these old xend config versions at all. Is anyone even running Xen 3.0.x, or 3.1.x, particularly with a newish libvirt? IMO we could drop all of this xend config nonsense and go with the code associated with the last xend config version, i.e. XEND_CONFIG_VERSION_3_1_0.
And while mentioning dropping support for these old xend config versions, do I dare mention dropping the xend driver altogether? :-) It will certainly need to be done some day. Question is whether that's a bit premature now.
We just decided to drop QEMU driver code supporting for RHEL-5 vintage distros, requiring RHEL-6 as minimum. So I think it is entirely reasonable to drop Xen driver code supporting similar vintage XenD. That would certainly simplify or even eliminate the current crazy xend_config_version code we have I think we need to continue suppoorting XenD driver for a while, but at least you can simplify the code shared with libxl. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Fri, 2015-12-04 at 10:01 +0000, Daniel P. Berrange wrote:
On Thu, Dec 03, 2015 at 11:13:06PM -0700, Jim Fehlig wrote:
On 11/26/2015 09:59 AM, Ian Campbell wrote:
libxlConnectDomainXMLFromNative calls both xenParseXM and xenParseXL with cfg->verInfo->xen_version_major, however AFAICT they both (either inherently, or through there use of xenParseConfigCommon expect a value from xenConfigVersionEnum (which does not correspond to xen_version_major).
I recall being a little apprehensive about using xen_version_major when writing the code, and apparently that was justified. But now that we are revisiting this, I wonder if we care about these old xend config versions at all. Is anyone even running Xen 3.0.x, or 3.1.x, particularly with a newish libvirt? IMO we could drop all of this xend config nonsense and go with the code associated with the last xend config version, i.e. XEND_CONFIG_VERSION_3_1_0.
And while mentioning dropping support for these old xend config versions, do I dare mention dropping the xend driver altogether? :-) It will certainly need to be done some day. Question is whether that's a bit premature now.
We just decided to drop QEMU driver code supporting for RHEL-5 vintage distros, requiring RHEL-6 as minimum. So I think it is entirely reasonable to drop Xen driver code supporting similar vintage XenD. That would certainly simplify or even eliminate the current crazy xend_config_version code we have
RHEL 6.0 looks[0] to have been release on 2010-11-09, which was in the middle of Xen 4.0 and 4.1[1]. 4.0 seems like a decent enough cut off point (and is what is in Debian oldstable AKA Wheezy, FWIW) plus it is after the currently latest XEND_CONFIG_VERSION, so all that code could be removed. Shall I respin this series with that as a precursor? Ian. [0] https://access.redhat.com/articles/3078 [1] http://wiki.xen.org/wiki/Xen_Release_Features
I think we need to continue suppoorting XenD driver for a while, but at least you can simplify the code shared with libxl.
Regards, Daniel
On Fri, Dec 04, 2015 at 02:19:33PM +0000, Ian Campbell wrote:
On Fri, 2015-12-04 at 10:01 +0000, Daniel P. Berrange wrote:
On Thu, Dec 03, 2015 at 11:13:06PM -0700, Jim Fehlig wrote:
On 11/26/2015 09:59 AM, Ian Campbell wrote:
libxlConnectDomainXMLFromNative calls both xenParseXM and xenParseXL with cfg->verInfo->xen_version_major, however AFAICT they both (either inherently, or through there use of xenParseConfigCommon expect a value from xenConfigVersionEnum (which does not correspond to xen_version_major).
I recall being a little apprehensive about using xen_version_major when writing the code, and apparently that was justified. But now that we are revisiting this, I wonder if we care about these old xend config versions at all. Is anyone even running Xen 3.0.x, or 3.1.x, particularly with a newish libvirt? IMO we could drop all of this xend config nonsense and go with the code associated with the last xend config version, i.e. XEND_CONFIG_VERSION_3_1_0.
And while mentioning dropping support for these old xend config versions, do I dare mention dropping the xend driver altogether? :-) It will certainly need to be done some day. Question is whether that's a bit premature now.
We just decided to drop QEMU driver code supporting for RHEL-5 vintage distros, requiring RHEL-6 as minimum. So I think it is entirely reasonable to drop Xen driver code supporting similar vintage XenD. That would certainly simplify or even eliminate the current crazy xend_config_version code we have
RHEL 6.0 looks[0] to have been release on 2010-11-09, which was in the middle of Xen 4.0 and 4.1[1]. 4.0 seems like a decent enough cut off point (and is what is in Debian oldstable AKA Wheezy, FWIW) plus it is after the currently latest XEND_CONFIG_VERSION, so all that code could be removed.
Shall I respin this series with that as a precursor?
Yeah, that sounds reasonable. Would let us drop all Xen 3.x support essentially. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
This new cfg field was addded in 4.0 by 68a94cf528e6 "xm: Add maxvcpus support" and, more crucially these day, it is what xl supports. This removes the MAX_VIRT_CPUS limitation for such versions of Xen (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). Add a new test case to both XM and XL config tests covering this case. Note that although xm gained support for maxvcpus in Xen 4.0 the xend_config_format was never bumped beyond 4 and the internal handling remained in terms of vcpu_avail. Therefore the support for xend >= XEND_CONFIG_VERSION_4_0_0 is somewhat theoretical. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> --- src/xenconfig/xen_common.c | 72 ++++++++++++++++++++------- src/xenconfig/xen_xm.c | 7 +++ 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 | 30 +++++++++++ tests/xmconfigtest.c | 1 + 8 files changed, 146 insertions(+), 19 deletions(-) create mode 100644 tests/xlconfigdata/test-paravirt-maxvcpus.cfg create mode 100644 tests/xlconfigdata/test-paravirt-maxvcpus.xml create mode 100644 tests/xmconfigdata/test-paravirt-maxvcpus.cfg create mode 100644 tests/xmconfigdata/test-paravirt-maxvcpus.xml diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 0890c73..6f2afcc 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -492,24 +492,49 @@ xenParsePCI(virConfPtr conf, virDomainDefPtr def) static int -xenParseCPUFeatures(virConfPtr conf, virDomainDefPtr def) +xenParseCPUFeatures(virConfPtr conf, virDomainDefPtr def, + int xendConfigVersion) { unsigned long count = 0; const char *str = NULL; int val = 0; - if (xenConfigGetULong(conf, "vcpus", &count, 1) < 0 || - MAX_VIRT_CPUS < count) - return -1; + /* + * 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. + * + * xl understands vcpus + maxvcpus, but not vcpu_avail + * (ever). + */ + if (xendConfigVersion < XEND_CONFIG_VERSION_4_0_0) { + if (xenConfigGetULong(conf, "vcpus", &count, 1) < 0 || + MAX_VIRT_CPUS < count) + return -1; + def->maxvcpus = count; - def->maxvcpus = count; - if (xenConfigGetULong(conf, "vcpu_avail", &count, -1) < 0) - return -1; + if (xenConfigGetULong(conf, "vcpu_avail", &count, -1) < 0) + return -1; + def->vcpus = MIN(count_one_bits_l(count), def->maxvcpus); + } else { + if (xenConfigGetULong(conf, "vcpus", &count, 1) < 0) + return -1; + def->vcpus = count; + + if (xenConfigGetULong(conf, "maxvcpus", &count, def->vcpus) < 0) + return -1; + def->maxvcpus = count; + } - def->vcpus = MIN(count_one_bits_l(count), def->maxvcpus); if (xenConfigGetString(conf, "cpus", &str, NULL) < 0) return -1; - if (str && (virBitmapParse(str, 0, &def->cpumask, 4096) < 0)) return -1; @@ -1032,7 +1057,7 @@ xenParseConfigCommon(virConfPtr conf, if (xenParseEventsActions(conf, def) < 0) return -1; - if (xenParseCPUFeatures(conf, def) < 0) + if (xenParseCPUFeatures(conf, def, xendConfigVersion) < 0) return -1; if (xenParseTimeOffset(conf, def, xendConfigVersion) < 0) @@ -1519,19 +1544,28 @@ xenFormatCharDev(virConfPtr conf, virDomainDefPtr def) static int -xenFormatCPUAllocation(virConfPtr conf, virDomainDefPtr def) +xenFormatCPUAllocation(virConfPtr conf, virDomainDefPtr def, + int xendConfigVersion) { int ret = -1; char *cpus = NULL; - if (xenConfigSetInt(conf, "vcpus", def->maxvcpus) < 0) - goto cleanup; + if (xendConfigVersion >= XEND_CONFIG_VERSION_4_0_0) { + if (def->vcpus < def->maxvcpus && + xenConfigSetInt(conf, "maxvcpus", def->maxvcpus) < 0) + goto cleanup; + if (xenConfigSetInt(conf, "vcpus", def->vcpus) < 0) + goto cleanup; + } else { + if (xenConfigSetInt(conf, "vcpus", def->maxvcpus) < 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 (def->vcpus < def->maxvcpus && - xenConfigSetInt(conf, "vcpu_avail", (1UL << def->vcpus) - 1) < 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 (def->vcpus < def->maxvcpus && + xenConfigSetInt(conf, "vcpu_avail", (1UL << def->vcpus) - 1) < 0) + goto cleanup; + } if ((def->cpumask != NULL) && ((cpus = virBitmapFormat(def->cpumask)) == NULL)) { @@ -1826,7 +1860,7 @@ xenFormatConfigCommon(virConfPtr conf, if (xenFormatMem(conf, def) < 0) return -1; - if (xenFormatCPUAllocation(conf, def) < 0) + if (xenFormatCPUAllocation(conf, def, xendConfigVersion) < 0) return -1; if (xenFormatCPUFeatures(conf, def, xendConfigVersion) < 0) diff --git a/src/xenconfig/xen_xm.c b/src/xenconfig/xen_xm.c index a4d1203..a2981f7 100644 --- a/src/xenconfig/xen_xm.c +++ b/src/xenconfig/xen_xm.c @@ -482,6 +482,13 @@ xenParseXM(virConfPtr conf, if (xenParseConfigCommon(conf, def, caps, xendConfigVersion) < 0) goto cleanup; + /* + * xend vcpu_avail field is an integer bitmap, so cannot handle more than + * MAX_VIRT_CPUS. + */ + if (MAX_VIRT_CPUS < def->maxvcpus || MAX_VIRT_CPUS < def->vcpus) + goto cleanup; + if (xenParseXMOS(conf, def) < 0) goto cleanup; 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 08c43fb..b6f9b84 100644 --- a/tests/xlconfigtest.c +++ b/tests/xlconfigtest.c @@ -193,6 +193,7 @@ mymain(void) ret = -1; \ } while (0) + DO_TEST("paravirt-maxvcpus", 5); DO_TEST("new-disk", 5); DO_TEST("spice", 5); DO_TEST("spice-features", 5); 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..52463d8 --- /dev/null +++ b/tests/xmconfigdata/test-paravirt-maxvcpus.xml @@ -0,0 +1,30 @@ +<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> + </devices> +</domain> diff --git a/tests/xmconfigtest.c b/tests/xmconfigtest.c index 79b09ca..ff1f52b 100644 --- a/tests/xmconfigtest.c +++ b/tests/xmconfigtest.c @@ -220,6 +220,7 @@ mymain(void) DO_TEST("paravirt-net-e1000", 3); DO_TEST("paravirt-net-vifname", 3); DO_TEST("paravirt-vcpu", 2); + DO_TEST("paravirt-maxvcpus", 5); DO_TEST("fullvirt-old-cdrom", 1); DO_TEST("fullvirt-new-cdrom", 2); DO_TEST("fullvirt-utc", 2); -- 2.1.4
participants (3)
-
Daniel P. Berrange -
Ian Campbell -
Jim Fehlig