From mboxrd@z Thu Jan 1 00:00:00 1970 From: John Ferlan Subject: Re: [libvirt] [PATCH 10/12] Introduce support for parsing/formatting Xen xl config format Date: Mon, 12 Jan 2015 10:27:54 -0500 Message-ID: <54B3E7FA.4070003@redhat.com> References: <1420866224-29746-1-git-send-email-jfehlig@suse.com> <1420866224-29746-11-git-send-email-jfehlig@suse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1420866224-29746-11-git-send-email-jfehlig@suse.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jim Fehlig , libvir-list@redhat.com Cc: Kiarie Kahurani , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org 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 || (xendConfigVersion < XEND_CONFIG_MIN_VERS_PVFB_NEWCONF)) { > if (def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) { > if (xenConfigSetInt(conf, "sdl", 1) < 0) > diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c > new file mode 100644 > index 0000000..10027c1 > --- /dev/null > +++ b/src/xenconfig/xen_xl.c > @@ -0,0 +1,492 @@ > +/* > + * xen_xl.c: Xen XL parsing functions Should there be copyright date here? > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library. If not, see > + * . > + * > + * Author: Kiarie Kahurani > + */ > + > +#include > + > +#include > + > +#include "virconf.h" > +#include "virerror.h" > +#include "domain_conf.h" > +#include "viralloc.h" > +#include "virstring.h" > +#include "virstoragefile.h" > +#include "xen_xl.h" > + > +#define VIR_FROM_THIS VIR_FROM_NONE > + > +/* > + * Xen provides a libxl utility library, with several useful functions, > + * specifically xlu_disk_parse for parsing xl disk config strings. > + * Although the libxlutil library is installed, until recently the > + * corresponding header file wasn't. Use the header file if detected during > + * configure, otherwise provide extern declarations for any functions used. > + */ > +#ifdef HAVE_LIBXLUTIL_H > +# include > +#else > +typedef struct XLU_Config XLU_Config; > + > +extern XLU_Config *xlu_cfg_init(FILE *report, > + const char *report_filename); > + > +extern int xlu_disk_parse(XLU_Config *cfg, > + int nspecs, > + const char *const *specs, > + libxl_device_disk *disk); > +#endif > + > +static int > +xenParseXLSpice(virConfPtr conf, virDomainDefPtr def) > +{ > + virDomainGraphicsDefPtr graphics = NULL; > + unsigned long port; > + char *listenAddr = NULL; > + int val; > + > + if (STREQ(def->os.type, "hvm")) { > + if (xenConfigGetBool(conf, "spice", &val, 0) < 0) > + return -1; > + > + if (val) { > + if (VIR_ALLOC(graphics) < 0) > + return -1; > + > + graphics->type = VIR_DOMAIN_GRAPHICS_TYPE_SPICE; > + if (xenConfigCopyStringOpt(conf, "spicehost", &listenAddr) < 0) > + goto cleanup; > + if (listenAddr && > + virDomainGraphicsListenSetAddress(graphics, 0, listenAddr, > + -1, true) < 0) { > + goto cleanup; > + } > + VIR_FREE(listenAddr); > + > + if (xenConfigGetULong(conf, "spicetls_port", &port, 0) < 0) > + goto cleanup; > + graphics->data.spice.tlsPort = (int)port; > + > + if (xenConfigGetULong(conf, "spiceport", &port, 0) < 0) > + goto cleanup; > + > + graphics->data.spice.port = (int)port; > + > + if (!graphics->data.spice.tlsPort && > + !graphics->data.spice.port) > + graphics->data.spice.autoport = 1; > + > + if (xenConfigGetBool(conf, "spicedisable_ticketing", &val, 0) < 0) > + goto cleanup; > + if (val) { > + if (xenConfigCopyStringOpt(conf, "spicepasswd", > + &graphics->data.spice.auth.passwd) < 0) > + goto cleanup; > + } > + > + if (xenConfigGetBool(conf, "spiceagent_mouse", > + &graphics->data.spice.mousemode, 0) < 0) > + goto cleanup; > + if (xenConfigGetBool(conf, "spicedvagent", &val, 0) < 0) > + goto cleanup; > + if (val) { > + if (xenConfigGetBool(conf, "spice_clipboard_sharing", > + &graphics->data.spice.copypaste, > + 0) < 0) > + goto cleanup; > + } > + > + if (VIR_ALLOC_N(def->graphics, 1) < 0) > + goto cleanup; > + def->graphics[0] = graphics; > + def->ngraphics = 1; > + } > + } > + > + return 0; > + > + cleanup: > + virDomainGraphicsDefFree(graphics); > + return -1; > +} > + > +/* > + * positional parameters > + * (If the strings are not separated by "=" > + * the string is split following ',' and assigned to > + * the following options in the following order) > + * target,format,vdev,access > + * ================================================================ > + * > + * The parameters below cannot be specified as positional parameters: > + * > + * other parameters > + * devtype = > + * backendtype = > + * parameters not taken care of > + * backend = > + * script =