Re: [libvirt] [PATCH LIBVIRT v1 1/2] libxl: Correct value for xendConfigVersion to xen{Parse, Format}ConfigCommon
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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH LIBVIRT v1 1/2] libxl: Correct value for xendConfigVersion to xen{Parse, Format}ConfigCommon
On Fri, 2015-12-04 at 10:01 +, 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH LIBVIRT v1 1/2] libxl: Correct value for xendConfigVersion to xen{Parse, Format}ConfigCommon
On Fri, Dec 04, 2015 at 02:19:33PM +, Ian Campbell wrote: > On Fri, 2015-12-04 at 10:01 +, 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH LIBVIRT v1 1/2] libxl: Correct value for xendConfigVersion to xen{Parse, Format}ConfigCommon
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
> ---
> 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
