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@xxxxxxxxxx>
> ---
> 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);
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list