Re: [Xen-devel] [libvirt] [PATCH 10/12] Introduce support for parsing/formatting Xen xl config format
Eric Blake wrote: > On 01/09/2015 10:03 PM, Jim Fehlig wrote: > >> Introduce a parser/formatter for the xl config format. Since the >> deprecation of xm/xend, the VM config file format has diverged as >> new features are added to libxl. This patch adds support for parsing >> and formating the xl config format. It supports the existing xm config >> format, plus adds support for spice graphics and xl disk config syntax. >> >> > > >> xl disk config is parsed with the help of xlu_disk_parse() from >> libxlutil, libxl's utility library. Although the library exists >> in all Xen versions supported by the libxl virt driver, only >> recently has the corresponding header file been included. A check >> for the header is done in configure.ac. If not found, xlu_disk_parse() >> is declared externally. >> >> Signed-off-by: Kiarie Kahurani >> Signed-off-by: Jim Fehlig >> --- >> configure.ac | 3 + >> po/POTFILES.in | 1 + >> src/Makefile.am| 4 +- >> src/libvirt_xenconfig.syms | 4 + >> src/xenconfig/xen_common.c | 3 +- >> src/xenconfig/xen_xl.c | 492 >> + >> src/xenconfig/xen_xl.h | 33 +++ >> 7 files changed, 538 insertions(+), 2 deletions(-) >> > > This patch fails to build on RHEL 5: > http://fpaste.org/168738/84783142/ > I'm able to reproduce the failures and have a fix for the next version. Regards, Jim ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [libvirt] [PATCH 10/12] Introduce support for parsing/formatting Xen xl config format
John Ferlan wrote:
> On 01/10/2015 12:03 AM, Jim Fehlig wrote:
>
>> Introduce a parser/formatter for the xl config format. Since the
>> deprecation of xm/xend, the VM config file format has diverged as
>> new features are added to libxl. This patch adds support for parsing
>> and formating the xl config format. It supports the existing xm config
>> format, plus adds support for spice graphics and xl disk config syntax.
>>
>> Disk config is specified a bit differently in xl as compared to xm. In
>> xl, disk config consists of comma-separated positional parameters and
>> keyword/value pairs separated by commas. Positional parameters are
>> specified as follows
>>
>>target, format, vdev, access
>>
>> Supported keys for key=value options are
>>
>> devtype, backend, backendtype, script, direct-io-safe,
>>
>> The positional paramters can also be specified in key/value form. For
>> example the following xl disk config are equivalent
>>
>> /dev/vg/guest-volume,,hda
>> /dev/vg/guest-volume,raw,hda,rw
>> format=raw, vdev=hda, access=rw, target=/dev/vg/guest-volume
>>
>> See $xen_sources/docs/misc/xl-disk-configuration.txt for more details.
>>
>> xl disk config is parsed with the help of xlu_disk_parse() from
>> libxlutil, libxl's utility library. Although the library exists
>> in all Xen versions supported by the libxl virt driver, only
>> recently has the corresponding header file been included. A check
>> for the header is done in configure.ac. If not found, xlu_disk_parse()
>> is declared externally.
>>
>> Signed-off-by: Kiarie Kahurani
>> Signed-off-by: Jim Fehlig
>> ---
>> configure.ac | 3 +
>> po/POTFILES.in | 1 +
>> src/Makefile.am| 4 +-
>> src/libvirt_xenconfig.syms | 4 +
>> src/xenconfig/xen_common.c | 3 +-
>> src/xenconfig/xen_xl.c | 492
>> +
>> src/xenconfig/xen_xl.h | 33 +++
>> 7 files changed, 538 insertions(+), 2 deletions(-)
>>
>>
>
> The following is just from the Coverity check... I don't have all the
> build environments that have proved to be problematic over the last week
> or so...
>
> I assume all you've done is take the generated code and use that rather
> than going through the problems as a result of attempting to generate
> the code.
>
>
>
>> diff --git a/configure.ac b/configure.ac
>> index 9d12079..f370475 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -891,6 +891,9 @@ if test $fail = 1; then
>> fi
>>
>> if test "$with_libxl" = "yes"; then
>> +dnl If building with libxl, use the libxl utility header and lib too
>> +AC_CHECK_HEADERS([libxlutil.h])
>> +LIBXL_LIBS="$LIBXL_LIBS -lxlutil"
>> AC_DEFINE_UNQUOTED([WITH_LIBXL], 1, [whether libxenlight driver is
>> enabled])
>> fi
>> AM_CONDITIONAL([WITH_LIBXL], [test "$with_libxl" = "yes"])
>> diff --git a/po/POTFILES.in b/po/POTFILES.in
>> index e7cb2cc..094c8e3 100644
>> --- a/po/POTFILES.in
>> +++ b/po/POTFILES.in
>> @@ -247,6 +247,7 @@ src/xenapi/xenapi_driver.c
>> src/xenapi/xenapi_utils.c
>> src/xenconfig/xen_common.c
>> src/xenconfig/xen_sxpr.c
>> +src/xenconfig/xen_xl.c
>> src/xenconfig/xen_xm.c
>> tests/virpolkittest.c
>> tools/libvirt-guests.sh.in
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index e0e47d0..875fb5e 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -1004,7 +1004,8 @@ XENCONFIG_SOURCES =
>> \
>> xenconfig/xenxs_private.h \
>> xenconfig/xen_common.c xenconfig/xen_common.h \
>> xenconfig/xen_sxpr.c xenconfig/xen_sxpr.h \
>> -xenconfig/xen_xm.c xenconfig/xen_xm.h
>> +xenconfig/xen_xm.c xenconfig/xen_xm.h \
>> +xenconfig/xen_xl.c xenconfig/xen_xl.h
>>
>> pkgdata_DATA = cpu/cpu_map.xml
>>
>> @@ -1061,6 +1062,7 @@ endif WITH_VMX
>> if WITH_XENCONFIG
>> noinst_LTLIBRARIES += libvirt_xenconfig.la
>> libvirt_la_BUILT_LIBADD += libvirt_xenconfig.la
>> +libvirt_la_LIBADD += $(LIBXL_LIBS)
>> libvirt_xenconfig_la_CFLAGS = \
>> -I$(srcdir)/conf $(AM_CFLAGS)
>> libvirt_xenconfig_la_SOURCES = $(XENCONFIG_SOURCES)
>> diff --git a/src/libvirt_xenconfig.syms b/src/libvirt_xenconfig.syms
>> index 6541685..3e2e5d6 100644
>> --- a/src/libvirt_xenconfig.syms
>> +++ b/src/libvirt_xenconfig.syms
>> @@ -16,6 +16,10 @@ xenParseSxprChar;
>> xenParseSxprSound;
>> xenParseSxprString;
>>
>> +#xenconfig/xen_xl.h
>> +xenFormatXL;
>> +xenParseXL;
>> +
>> # xenconfig/xen_xm.h
>> xenFormatXM;
>> xenParseXM;
>> diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
>> index b40a722..a2a1474 100644
>> --- a/src/xenconfig/xen_common.c
>> +++ b/src/xenconfig/xen_common.c
>> @@ -1812,7 +1812,8 @@ xenFormatVfb(virConfPtr conf, virDomainDefPtr def, int
>> xendConfigVersion)
>> {
>> int hvm = STREQ(def->os.type, "hvm") ? 1 : 0;
>>
>> -if (def-
Re: [Xen-devel] [libvirt] [PATCH 10/12] Introduce support for parsing/formatting Xen xl config format
On 01/09/2015 10:03 PM, Jim Fehlig wrote: > Introduce a parser/formatter for the xl config format. Since the > deprecation of xm/xend, the VM config file format has diverged as > new features are added to libxl. This patch adds support for parsing > and formating the xl config format. It supports the existing xm config > format, plus adds support for spice graphics and xl disk config syntax. > > xl disk config is parsed with the help of xlu_disk_parse() from > libxlutil, libxl's utility library. Although the library exists > in all Xen versions supported by the libxl virt driver, only > recently has the corresponding header file been included. A check > for the header is done in configure.ac. If not found, xlu_disk_parse() > is declared externally. > > Signed-off-by: Kiarie Kahurani > Signed-off-by: Jim Fehlig > --- > configure.ac | 3 + > po/POTFILES.in | 1 + > src/Makefile.am| 4 +- > src/libvirt_xenconfig.syms | 4 + > src/xenconfig/xen_common.c | 3 +- > src/xenconfig/xen_xl.c | 492 > + > src/xenconfig/xen_xl.h | 33 +++ > 7 files changed, 538 insertions(+), 2 deletions(-) This patch fails to build on RHEL 5: http://fpaste.org/168738/84783142/ There, the version of xen is so old that there is no support for newer xenlight interaction; we still want the old xen support code to compile, but you should probably consider making things conditional so that xen_xl.c is attempted only when new-enough xenlight support exists (the same way that src/libxl is avoided when only older xen is present). It may be as simple as making the file conditional on HAVE_LIBXL, although I haven't yet tried that. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [libvirt] [PATCH 10/12] Introduce support for parsing/formatting Xen xl config format
John Ferlan wrote: > On 01/10/2015 12:03 AM, Jim Fehlig wrote: > >> Introduce a parser/formatter for the xl config format. Since the >> deprecation of xm/xend, the VM config file format has diverged as >> new features are added to libxl. This patch adds support for parsing >> and formating the xl config format. It supports the existing xm config >> format, plus adds support for spice graphics and xl disk config syntax. >> >> Disk config is specified a bit differently in xl as compared to xm. In >> xl, disk config consists of comma-separated positional parameters and >> keyword/value pairs separated by commas. Positional parameters are >> specified as follows >> >>target, format, vdev, access >> >> Supported keys for key=value options are >> >> devtype, backend, backendtype, script, direct-io-safe, >> >> The positional paramters can also be specified in key/value form. For >> example the following xl disk config are equivalent >> >> /dev/vg/guest-volume,,hda >> /dev/vg/guest-volume,raw,hda,rw >> format=raw, vdev=hda, access=rw, target=/dev/vg/guest-volume >> >> See $xen_sources/docs/misc/xl-disk-configuration.txt for more details. >> >> xl disk config is parsed with the help of xlu_disk_parse() from >> libxlutil, libxl's utility library. Although the library exists >> in all Xen versions supported by the libxl virt driver, only >> recently has the corresponding header file been included. A check >> for the header is done in configure.ac. If not found, xlu_disk_parse() >> is declared externally. >> >> Signed-off-by: Kiarie Kahurani >> Signed-off-by: Jim Fehlig >> --- >> configure.ac | 3 + >> po/POTFILES.in | 1 + >> src/Makefile.am| 4 +- >> src/libvirt_xenconfig.syms | 4 + >> src/xenconfig/xen_common.c | 3 +- >> src/xenconfig/xen_xl.c | 492 >> + >> src/xenconfig/xen_xl.h | 33 +++ >> 7 files changed, 538 insertions(+), 2 deletions(-) >> >> > > The following is just from the Coverity check... I don't have all the > build environments that have proved to be problematic over the last week > or so... > > I assume all you've done is take the generated code and use that rather > than going through the problems as a result of attempting to generate > the code. > In place of the generated code I'm using libxlutil from Xen. The initial series copied Xen's flex parser for parsing disk config strings, but thankfully Ian Campbell noticed this and suggested using Xen's libxlutil instead, which is provided specifically for this purpose. Thanks for the review. I'll address the issues you've noted in the next version. Regards, Jim ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [libvirt] [PATCH 10/12] Introduce support for parsing/formatting Xen xl config format
On 01/10/2015 12:03 AM, Jim Fehlig wrote:
> Introduce a parser/formatter for the xl config format. Since the
> deprecation of xm/xend, the VM config file format has diverged as
> new features are added to libxl. This patch adds support for parsing
> and formating the xl config format. It supports the existing xm config
> format, plus adds support for spice graphics and xl disk config syntax.
>
> Disk config is specified a bit differently in xl as compared to xm. In
> xl, disk config consists of comma-separated positional parameters and
> keyword/value pairs separated by commas. Positional parameters are
> specified as follows
>
>target, format, vdev, access
>
> Supported keys for key=value options are
>
> devtype, backend, backendtype, script, direct-io-safe,
>
> The positional paramters can also be specified in key/value form. For
> example the following xl disk config are equivalent
>
> /dev/vg/guest-volume,,hda
> /dev/vg/guest-volume,raw,hda,rw
> format=raw, vdev=hda, access=rw, target=/dev/vg/guest-volume
>
> See $xen_sources/docs/misc/xl-disk-configuration.txt for more details.
>
> xl disk config is parsed with the help of xlu_disk_parse() from
> libxlutil, libxl's utility library. Although the library exists
> in all Xen versions supported by the libxl virt driver, only
> recently has the corresponding header file been included. A check
> for the header is done in configure.ac. If not found, xlu_disk_parse()
> is declared externally.
>
> Signed-off-by: Kiarie Kahurani
> Signed-off-by: Jim Fehlig
> ---
> configure.ac | 3 +
> po/POTFILES.in | 1 +
> src/Makefile.am| 4 +-
> src/libvirt_xenconfig.syms | 4 +
> src/xenconfig/xen_common.c | 3 +-
> src/xenconfig/xen_xl.c | 492
> +
> src/xenconfig/xen_xl.h | 33 +++
> 7 files changed, 538 insertions(+), 2 deletions(-)
>
The following is just from the Coverity check... I don't have all the
build environments that have proved to be problematic over the last week
or so...
I assume all you've done is take the generated code and use that rather
than going through the problems as a result of attempting to generate
the code.
> diff --git a/configure.ac b/configure.ac
> index 9d12079..f370475 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -891,6 +891,9 @@ if test $fail = 1; then
> fi
>
> if test "$with_libxl" = "yes"; then
> +dnl If building with libxl, use the libxl utility header and lib too
> +AC_CHECK_HEADERS([libxlutil.h])
> +LIBXL_LIBS="$LIBXL_LIBS -lxlutil"
> AC_DEFINE_UNQUOTED([WITH_LIBXL], 1, [whether libxenlight driver is
> enabled])
> fi
> AM_CONDITIONAL([WITH_LIBXL], [test "$with_libxl" = "yes"])
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index e7cb2cc..094c8e3 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -247,6 +247,7 @@ src/xenapi/xenapi_driver.c
> src/xenapi/xenapi_utils.c
> src/xenconfig/xen_common.c
> src/xenconfig/xen_sxpr.c
> +src/xenconfig/xen_xl.c
> src/xenconfig/xen_xm.c
> tests/virpolkittest.c
> tools/libvirt-guests.sh.in
> diff --git a/src/Makefile.am b/src/Makefile.am
> index e0e47d0..875fb5e 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -1004,7 +1004,8 @@ XENCONFIG_SOURCES =
> \
> xenconfig/xenxs_private.h \
> xenconfig/xen_common.c xenconfig/xen_common.h \
> xenconfig/xen_sxpr.c xenconfig/xen_sxpr.h \
> - xenconfig/xen_xm.c xenconfig/xen_xm.h
> + xenconfig/xen_xm.c xenconfig/xen_xm.h \
> + xenconfig/xen_xl.c xenconfig/xen_xl.h
>
> pkgdata_DATA = cpu/cpu_map.xml
>
> @@ -1061,6 +1062,7 @@ endif WITH_VMX
> if WITH_XENCONFIG
> noinst_LTLIBRARIES += libvirt_xenconfig.la
> libvirt_la_BUILT_LIBADD += libvirt_xenconfig.la
> +libvirt_la_LIBADD += $(LIBXL_LIBS)
> libvirt_xenconfig_la_CFLAGS = \
> -I$(srcdir)/conf $(AM_CFLAGS)
> libvirt_xenconfig_la_SOURCES = $(XENCONFIG_SOURCES)
> diff --git a/src/libvirt_xenconfig.syms b/src/libvirt_xenconfig.syms
> index 6541685..3e2e5d6 100644
> --- a/src/libvirt_xenconfig.syms
> +++ b/src/libvirt_xenconfig.syms
> @@ -16,6 +16,10 @@ xenParseSxprChar;
> xenParseSxprSound;
> xenParseSxprString;
>
> +#xenconfig/xen_xl.h
> +xenFormatXL;
> +xenParseXL;
> +
> # xenconfig/xen_xm.h
> xenFormatXM;
> xenParseXM;
> diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
> index b40a722..a2a1474 100644
> --- a/src/xenconfig/xen_common.c
> +++ b/src/xenconfig/xen_common.c
> @@ -1812,7 +1812,8 @@ xenFormatVfb(virConfPtr conf, virDomainDefPtr def, int
> xendConfigVersion)
> {
> int hvm = STREQ(def->os.type, "hvm") ? 1 : 0;
>
> -if (def->ngraphics == 1) {
> +if (def->ngraphics == 1 &&
> +def->graphics[0]->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
> if (hvm || (xendConf
