All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/12] Replace Xen xl parsing/formatting impl
@ 2015-01-10  5:03 Jim Fehlig
  2015-01-10  5:03 ` [PATCH 01/12] Revert "bootstrap.conf: add check for flex" Jim Fehlig
                   ` (13 more replies)
  0 siblings, 14 replies; 25+ messages in thread
From: Jim Fehlig @ 2015-01-10  5:03 UTC (permalink / raw)
  To: libvir-list; +Cc: xen-devel

The first attempt to implement support for parsing/formatting Xen's
xl disk config format copied Xen's flex-based parser into libvirt, which
has proved to be challenging in the context of autotools.  But as it turns
out, Xen provides an interface to the parser via libxlutil.

This series reverts the first attempt, along with subsequent attempts to
fix it, and replaces it with an implementation based on libxlutil.  The
first nine patches revert the original implementation and subsequent fixes.
Patch 10 provides an implemenation based on libxlutil.  Patches 11 and
12 are basically unchanged from patches 3 and 4 in the first attempt.

One upshot of using libxlutil instead of copying the flex source is
removing the potential for source divergence.

Jim Fehlig (10):
  Revert "bootstrap.conf: add check for flex"
  Revert "src/Makefile: Fix parallel build after xen_xl_disk parser
    introduction"
  Revert "src/Makefile: move the new xen_xl_disk parser code at the
    correct place"
  Revert "Revert "src/Makefile.am: fix build breaker for xenconfig""
  Revert "build: fix xenconfig VPATH builds"
  Revert "src/Makefile.am: fix build breaker for xenconfig"
  Revert "libxl: Add support for parsing/formating Xen XL config"
  Revert "tests: Tests for the xen-xl parser"
  Revert "src/xenconfig: Xen-xl parser"
  Introduce support for parsing/formatting Xen xl config format

Kiarie Kahurani (2):
  tests: Tests for the xen-xl parser
  libxl: Add support for parsing/formating Xen XL config

 .gitignore                           |   1 -
 bootstrap.conf                       |   3 +-
 cfg.mk                               |   3 +-
 configure.ac                         |   4 +-
 src/Makefile.am                      |  49 ++-----
 src/xenconfig/xen_xl.c               | 205 ++++++++++++++--------------
 src/xenconfig/xen_xl_disk.l          | 256 -----------------------------------
 src/xenconfig/xen_xl_disk_i.h        |  39 ------
 tests/xlconfigdata/test-new-disk.cfg |   2 +-
 9 files changed, 114 insertions(+), 448 deletions(-)
 delete mode 100644 src/xenconfig/xen_xl_disk.l
 delete mode 100644 src/xenconfig/xen_xl_disk_i.h

-- 
1.8.4.5

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 01/12] Revert "bootstrap.conf: add check for flex"
  2015-01-10  5:03 [PATCH 00/12] Replace Xen xl parsing/formatting impl Jim Fehlig
@ 2015-01-10  5:03 ` Jim Fehlig
  2015-01-10  5:03 ` [PATCH 02/12] Revert "src/Makefile: Fix parallel build after xen_xl_disk parser introduction" Jim Fehlig
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 25+ messages in thread
From: Jim Fehlig @ 2015-01-10  5:03 UTC (permalink / raw)
  To: libvir-list; +Cc: Jim Fehlig, xen-devel

This reverts commit cab767831f74ec72809dacd07cb782a88a097f21.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 bootstrap.conf | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/bootstrap.conf b/bootstrap.conf
index 22c1c06..c06ee4c 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -1,6 +1,6 @@
 # Bootstrap configuration.
 
-# Copyright (C) 2010-2015 Red Hat, Inc.
+# Copyright (C) 2010-2014 Red Hat, Inc.
 
 # This library is free software; you can redistribute it and/or
 # modify it under the terms of the GNU Lesser General Public
@@ -204,7 +204,6 @@ buildreq="\
 autoconf   2.59
 automake   1.9.6
 autopoint  -
-flex       -
 gettext    0.17
 git        1.5.5
 gzip       -
-- 
1.8.4.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 02/12] Revert "src/Makefile: Fix parallel build after xen_xl_disk parser introduction"
  2015-01-10  5:03 [PATCH 00/12] Replace Xen xl parsing/formatting impl Jim Fehlig
  2015-01-10  5:03 ` [PATCH 01/12] Revert "bootstrap.conf: add check for flex" Jim Fehlig
@ 2015-01-10  5:03 ` Jim Fehlig
  2015-01-10  5:03 ` [PATCH 03/12] Revert "src/Makefile: move the new xen_xl_disk parser code at the correct place" Jim Fehlig
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 25+ messages in thread
From: Jim Fehlig @ 2015-01-10  5:03 UTC (permalink / raw)
  To: libvir-list; +Cc: Jim Fehlig, xen-devel

This reverts commit 533349ff43ddf091026fbcb0d9a714d9cc570dc7.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 src/Makefile.am | 21 +++++----------------
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 2eaaf11..a6ea8e0 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1053,21 +1053,12 @@ libvirt_vmx_la_SOURCES = $(VMX_SOURCES)
 endif WITH_VMX
 
 if WITH_XENCONFIG
-# Disable the default rule for lex files because we need to generate the
-# xen_xl_disk files into srcdir instead of builddir.
-.l.c:
-
-$(XENXLDISKPARSER_GENERATED): $(XENXLDISKPARSER_SOURCES)
-	$(AM_V_LEX) $(LEXCOMPILE) $<
-
-AM_LFLAGS = -Pxl_disk_ --header-file=$(abs_srcdir)/xenconfig/xen_xl_disk.h \
-		--outfile=$(abs_srcdir)/xenconfig/xen_xl_disk.c
-XENXLDISKPARSER_GENERATED = xenconfig/xen_xl_disk.c xenconfig/xen_xl_disk.h
-
-BUILT_SOURCES += $(XENXLDISKPARSER_GENERATED)
+AM_LFLAGS = -Pxl_disk_ --header-file=../$*.h
+LEX_OUTPUT_ROOT = lex.xl_disk_
+BUILT_SOURCES += xenconfig/xen_xl_disk.c xenconfig/xen_xl_disk.h
 # Generated header file is not implicitly added to dist
-EXTRA_DIST += $(XENXLDISKPARSER_GENERATED)
-MAINTAINERCLEANFILES += $(XENXLDISKPARSER_GENERATED)
+EXTRA_DIST += xenconfig/xen_xl_disk.h
+CLEANFILES += xenconfig/xen_xl_disk.h xenconfig/xen_xl_disk.c
 
 XENXLDISKPARSER_SOURCES = xenconfig/xen_xl_disk.l
 
@@ -1087,8 +1078,6 @@ libvirt_xenxldiskparser_la_CFLAGS = \
 		-I$(srcdir)/conf $(AM_CFLAGS) -Wno-unused-parameter
 libvirt_xenxldiskparser_la_SOURCES = \
 	$(XENXLDISKPARSER_SOURCES)
-libvirt_xenxldiskparser_la_DEPENDENCIES = \
-	$(XENXLDISKPARSER_GENERATED)
 
 noinst_LTLIBRARIES += libvirt_xenconfig.la
 libvirt_la_BUILT_LIBADD += libvirt_xenconfig.la
-- 
1.8.4.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 03/12] Revert "src/Makefile: move the new xen_xl_disk parser code at the correct place"
  2015-01-10  5:03 [PATCH 00/12] Replace Xen xl parsing/formatting impl Jim Fehlig
  2015-01-10  5:03 ` [PATCH 01/12] Revert "bootstrap.conf: add check for flex" Jim Fehlig
  2015-01-10  5:03 ` [PATCH 02/12] Revert "src/Makefile: Fix parallel build after xen_xl_disk parser introduction" Jim Fehlig
@ 2015-01-10  5:03 ` Jim Fehlig
  2015-01-10  5:03 ` [PATCH 04/12] Revert "Revert "src/Makefile.am: fix build breaker for xenconfig"" Jim Fehlig
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 25+ messages in thread
From: Jim Fehlig @ 2015-01-10  5:03 UTC (permalink / raw)
  To: libvir-list; +Cc: Jim Fehlig, xen-devel

This reverts commit edacdb3d12256af4f6e31ec65c9dd4797fb3aa0d.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 src/Makefile.am | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index a6ea8e0..850122a 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1000,6 +1000,23 @@ CPU_SOURCES =							\
 VMX_SOURCES =							\
 		vmx/vmx.c vmx/vmx.h
 
+AM_LFLAGS = -Pxl_disk_ --header-file=../$*.h
+LEX_OUTPUT_ROOT = lex.xl_disk_
+BUILT_SOURCES += xenconfig/xen_xl_disk.c xenconfig/xen_xl_disk.h
+# Generated header file is not implicitly added to dist
+EXTRA_DIST += xenconfig/xen_xl_disk.h
+CLEANFILES += xenconfig/xen_xl_disk.h xenconfig/xen_xl_disk.c
+
+XENXLDISKPARSER_SOURCES = xenconfig/xen_xl_disk.l
+
+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_xl.c xenconfig/xen_xl.h		\
+		xenconfig/xen_xl_disk_i.h
+
 pkgdata_DATA =	cpu/cpu_map.xml
 
 EXTRA_DIST +=	$(pkgdata_DATA)
@@ -1053,23 +1070,6 @@ libvirt_vmx_la_SOURCES = $(VMX_SOURCES)
 endif WITH_VMX
 
 if WITH_XENCONFIG
-AM_LFLAGS = -Pxl_disk_ --header-file=../$*.h
-LEX_OUTPUT_ROOT = lex.xl_disk_
-BUILT_SOURCES += xenconfig/xen_xl_disk.c xenconfig/xen_xl_disk.h
-# Generated header file is not implicitly added to dist
-EXTRA_DIST += xenconfig/xen_xl_disk.h
-CLEANFILES += xenconfig/xen_xl_disk.h xenconfig/xen_xl_disk.c
-
-XENXLDISKPARSER_SOURCES = xenconfig/xen_xl_disk.l
-
-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_xl.c xenconfig/xen_xl.h		\
-		xenconfig/xen_xl_disk_i.h
-
 # Flex generated XL disk parser needs to be compiled without WARN_FLAGS
 # Add the generated object to its own library to control CFLAGS
 noinst_LTLIBRARIES += libvirt_xenxldiskparser.la
-- 
1.8.4.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 04/12] Revert "Revert "src/Makefile.am: fix build breaker for xenconfig""
  2015-01-10  5:03 [PATCH 00/12] Replace Xen xl parsing/formatting impl Jim Fehlig
                   ` (2 preceding siblings ...)
  2015-01-10  5:03 ` [PATCH 03/12] Revert "src/Makefile: move the new xen_xl_disk parser code at the correct place" Jim Fehlig
@ 2015-01-10  5:03 ` Jim Fehlig
  2015-01-10  5:03 ` [PATCH 05/12] Revert "build: fix xenconfig VPATH builds" Jim Fehlig
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 25+ messages in thread
From: Jim Fehlig @ 2015-01-10  5:03 UTC (permalink / raw)
  To: libvir-list; +Cc: Jim Fehlig, xen-devel

This reverts commit e662968fd980158e8f8d8990bb43378dbc3d036a.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 src/Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 850122a..aaa5af2 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1015,7 +1015,7 @@ XENCONFIG_SOURCES =						\
 		xenconfig/xen_sxpr.c xenconfig/xen_sxpr.h	\
 		xenconfig/xen_xm.c xenconfig/xen_xm.h		\
 		xenconfig/xen_xl.c xenconfig/xen_xl.h		\
-		xenconfig/xen_xl_disk_i.h
+		xenconfig/xen_xl_disk.h
 
 pkgdata_DATA =	cpu/cpu_map.xml
 
-- 
1.8.4.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 05/12] Revert "build: fix xenconfig VPATH builds"
  2015-01-10  5:03 [PATCH 00/12] Replace Xen xl parsing/formatting impl Jim Fehlig
                   ` (3 preceding siblings ...)
  2015-01-10  5:03 ` [PATCH 04/12] Revert "Revert "src/Makefile.am: fix build breaker for xenconfig"" Jim Fehlig
@ 2015-01-10  5:03 ` Jim Fehlig
  2015-01-10  5:03 ` [PATCH 06/12] Revert "src/Makefile.am: fix build breaker for xenconfig" Jim Fehlig
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 25+ messages in thread
From: Jim Fehlig @ 2015-01-10  5:03 UTC (permalink / raw)
  To: libvir-list; +Cc: Jim Fehlig, xen-devel

This reverts commit 1b21d300691a78f73d94446616a6d1f9fd88991e.

Conflicts:
	src/Makefile.am

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 src/Makefile.am | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index aaa5af2..95ba12c 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1,6 +1,6 @@
 ## Process this file with automake to produce Makefile.in
 
-## Copyright (C) 2005-2015 Red Hat, Inc.
+## Copyright (C) 2005-2014 Red Hat, Inc.
 ##
 ## This library is free software; you can redistribute it and/or
 ## modify it under the terms of the GNU Lesser General Public
@@ -1074,7 +1074,6 @@ if WITH_XENCONFIG
 # Add the generated object to its own library to control CFLAGS
 noinst_LTLIBRARIES += libvirt_xenxldiskparser.la
 libvirt_xenxldiskparser_la_CFLAGS = \
-		-I$(srcdir)/xenconfig \
 		-I$(srcdir)/conf $(AM_CFLAGS) -Wno-unused-parameter
 libvirt_xenxldiskparser_la_SOURCES = \
 	$(XENXLDISKPARSER_SOURCES)
-- 
1.8.4.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 06/12] Revert "src/Makefile.am: fix build breaker for xenconfig"
  2015-01-10  5:03 [PATCH 00/12] Replace Xen xl parsing/formatting impl Jim Fehlig
                   ` (4 preceding siblings ...)
  2015-01-10  5:03 ` [PATCH 05/12] Revert "build: fix xenconfig VPATH builds" Jim Fehlig
@ 2015-01-10  5:03 ` Jim Fehlig
  2015-01-10  5:03 ` [PATCH 07/12] Revert "libxl: Add support for parsing/formating Xen XL config" Jim Fehlig
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 25+ messages in thread
From: Jim Fehlig @ 2015-01-10  5:03 UTC (permalink / raw)
  To: libvir-list; +Cc: Jim Fehlig, xen-devel

This reverts commit 703ef9667abf016ef1040eac296a81792b366932.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 src/Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 95ba12c..c7975e5 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1015,7 +1015,7 @@ XENCONFIG_SOURCES =						\
 		xenconfig/xen_sxpr.c xenconfig/xen_sxpr.h	\
 		xenconfig/xen_xm.c xenconfig/xen_xm.h		\
 		xenconfig/xen_xl.c xenconfig/xen_xl.h		\
-		xenconfig/xen_xl_disk.h
+		xenconfig/xen_xl_disk_i.h
 
 pkgdata_DATA =	cpu/cpu_map.xml
 
-- 
1.8.4.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 07/12] Revert "libxl: Add support for parsing/formating Xen XL config"
  2015-01-10  5:03 [PATCH 00/12] Replace Xen xl parsing/formatting impl Jim Fehlig
                   ` (5 preceding siblings ...)
  2015-01-10  5:03 ` [PATCH 06/12] Revert "src/Makefile.am: fix build breaker for xenconfig" Jim Fehlig
@ 2015-01-10  5:03 ` Jim Fehlig
  2015-01-10  5:03 ` [PATCH 08/12] Revert "tests: Tests for the xen-xl parser" Jim Fehlig
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 25+ messages in thread
From: Jim Fehlig @ 2015-01-10  5:03 UTC (permalink / raw)
  To: libvir-list; +Cc: Jim Fehlig, xen-devel

This reverts commit 4f524212ce614e1ca84b34dd8330a48957c8f823.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 src/libxl/libxl_driver.c | 32 ++++++++------------------------
 1 file changed, 8 insertions(+), 24 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 4135670..53c87ce 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -48,7 +48,6 @@
 #include "libxl_migration.h"
 #include "xen_xm.h"
 #include "xen_sxpr.h"
-#include "xen_xl.h"
 #include "virtypedparam.h"
 #include "viruri.h"
 #include "virstring.h"
@@ -68,7 +67,6 @@ VIR_LOG_INIT("libxl.libxl_driver");
 #define LIBXL_DOM_REQ_CRASH    3
 #define LIBXL_DOM_REQ_HALT     4
 
-#define LIBXL_CONFIG_FORMAT_XL "xen-xl"
 #define LIBXL_CONFIG_FORMAT_XM "xen-xm"
 #define LIBXL_CONFIG_FORMAT_SEXPR "xen-sxpr"
 
@@ -2216,17 +2214,7 @@ libxlConnectDomainXMLFromNative(virConnectPtr conn,
     if (virConnectDomainXMLFromNativeEnsureACL(conn) < 0)
         goto cleanup;
 
-    if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XL)) {
-        if (!(conf = virConfReadMem(nativeConfig, strlen(nativeConfig), 0)))
-            goto cleanup;
-        if (!(def = xenParseXL(conf,
-                               cfg->caps,
-                               cfg->verInfo->xen_version_major))) {
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("parsing xl config failed"));
-            goto cleanup;
-        }
-    } else if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) {
+    if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) {
         if (!(conf = virConfReadMem(nativeConfig, strlen(nativeConfig), 0)))
             goto cleanup;
 
@@ -2281,24 +2269,20 @@ libxlConnectDomainXMLToNative(virConnectPtr conn, const char * nativeFormat,
     if (virConnectDomainXMLToNativeEnsureACL(conn) < 0)
         goto cleanup;
 
+    if (STRNEQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("unsupported config type %s"), nativeFormat);
+        goto cleanup;
+    }
+
     if (!(def = virDomainDefParseString(domainXml,
                                         cfg->caps, driver->xmlopt,
                                         1 << VIR_DOMAIN_VIRT_XEN,
                                         VIR_DOMAIN_XML_INACTIVE)))
         goto cleanup;
 
-    if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XL)) {
-        if (!(conf = xenFormatXL(def, conn, cfg->verInfo->xen_version_major)))
-            goto cleanup;
-    } else if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) {
-        if (!(conf = xenFormatXM(conn, def, cfg->verInfo->xen_version_major)))
-            goto cleanup;
-    } else {
-
-        virReportError(VIR_ERR_INVALID_ARG,
-                       _("unsupported config type %s"), nativeFormat);
+    if (!(conf = xenFormatXM(conn, def, cfg->verInfo->xen_version_major)))
         goto cleanup;
-    }
 
     if (VIR_ALLOC_N(ret, len) < 0)
         goto cleanup;
-- 
1.8.4.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 08/12] Revert "tests: Tests for the xen-xl parser"
  2015-01-10  5:03 [PATCH 00/12] Replace Xen xl parsing/formatting impl Jim Fehlig
                   ` (6 preceding siblings ...)
  2015-01-10  5:03 ` [PATCH 07/12] Revert "libxl: Add support for parsing/formating Xen XL config" Jim Fehlig
@ 2015-01-10  5:03 ` Jim Fehlig
  2015-01-10  5:03 ` [PATCH 09/12] Revert "src/xenconfig: Xen-xl parser" Jim Fehlig
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 25+ messages in thread
From: Jim Fehlig @ 2015-01-10  5:03 UTC (permalink / raw)
  To: libvir-list; +Cc: Jim Fehlig, xen-devel

This reverts commit 6b818d3b09f4e74ac2ea1d4020896be1e6871867.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 tests/Makefile.am                    |   9 +-
 tests/testutilsxen.c                 |  50 --------
 tests/testutilsxen.h                 |   9 +-
 tests/xlconfigdata/test-new-disk.cfg |  26 ----
 tests/xlconfigdata/test-new-disk.xml |  51 --------
 tests/xlconfigdata/test-spice.cfg    |  32 -----
 tests/xlconfigdata/test-spice.xml    |  45 -------
 tests/xlconfigtest.c                 | 224 -----------------------------------
 8 files changed, 2 insertions(+), 444 deletions(-)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index af85335..e9418ea 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -138,7 +138,6 @@ EXTRA_DIST =		\
 	vmx2xmldata \
 	xencapsdata \
 	xmconfigdata \
-	xlconfigdata \
 	xml2sexprdata \
 	xml2vmxdata \
 	vmwareverdata \
@@ -226,8 +225,7 @@ ssh_LDADD = $(COVERAGE_LDFLAGS)
 
 if WITH_XEN
 test_programs += xml2sexprtest sexpr2xmltest \
-	xmconfigtest xencapstest statstest reconnect \
-	xlconfigtest
+	xmconfigtest xencapstest statstest reconnect
 endif WITH_XEN
 if WITH_QEMU
 test_programs += qemuxml2argvtest qemuxml2xmltest qemuxmlnstest \
@@ -479,11 +477,6 @@ sexpr2xmltest_SOURCES = \
 	testutils.c testutils.h
 sexpr2xmltest_LDADD = $(xen_LDADDS)
 
-xlconfigtest_SOURCES = \
-	xlconfigtest.c testutilsxen.c testutilsxen.h \
-	testutils.c testutils.h
-xlconfigtest_LDADD =$(xen_LDADDS)
-
 xmconfigtest_SOURCES = \
 	xmconfigtest.c testutilsxen.c testutilsxen.h \
 	testutils.c testutils.h
diff --git a/tests/testutilsxen.c b/tests/testutilsxen.c
index df1d124..a50a8a2 100644
--- a/tests/testutilsxen.c
+++ b/tests/testutilsxen.c
@@ -69,53 +69,3 @@ virCapsPtr testXenCapsInit(void)
     virObjectUnref(caps);
     return NULL;
 }
-
-
-virCapsPtr
-testXLInitCaps(void)
-{
-    virCapsPtr caps;
-    virCapsGuestPtr guest;
-    virCapsGuestMachinePtr *machines;
-    int nmachines;
-    static const char *const x86_machines[] = {
-        "xenfv"
-    };
-    static const char *const xen_machines[] = {
-        "xenpv"
-    };
-
-    if ((caps = virCapabilitiesNew(virArchFromHost(),
-                                   false, false)) == NULL)
-        return NULL;
-    nmachines = ARRAY_CARDINALITY(x86_machines);
-    if ((machines = virCapabilitiesAllocMachines(x86_machines, nmachines)) == NULL)
-        goto cleanup;
-    if ((guest = virCapabilitiesAddGuest(caps, "hvm", VIR_ARCH_X86_64,
-                                         "/usr/lib/xen/bin/qemu-dm", NULL,
-                                         nmachines, machines)) == NULL)
-        goto cleanup;
-    machines = NULL;
-    if (virCapabilitiesAddGuestDomain(guest, "xen", NULL,
-                                      NULL, 0, NULL) == NULL)
-        goto cleanup;
-    nmachines = ARRAY_CARDINALITY(xen_machines);
-    if ((machines = virCapabilitiesAllocMachines(xen_machines, nmachines)) == NULL)
-        goto cleanup;
-
-    if ((guest = virCapabilitiesAddGuest(caps, "xen", VIR_ARCH_X86_64,
-                                        "/usr/lib/xen/bin/qemu-dm", NULL,
-                                        nmachines, machines)) == NULL)
-        goto cleanup;
-    machines = NULL;
-
-    if (virCapabilitiesAddGuestDomain(guest, "xen", NULL,
-                                      NULL, 0, NULL) == NULL)
-        goto cleanup;
-    return caps;
-
- cleanup:
-    virCapabilitiesFreeMachines(machines, nmachines);
-    virObjectUnref(caps);
-    return NULL;
-}
diff --git a/tests/testutilsxen.h b/tests/testutilsxen.h
index c78350d..54155e5 100644
--- a/tests/testutilsxen.h
+++ b/tests/testutilsxen.h
@@ -1,10 +1,3 @@
-#ifndef _TESTUTILSXEN_H_
-# define _TESTUTILSXEN_H_
-
-# include "capabilities.h"
+#include "capabilities.h"
 
 virCapsPtr testXenCapsInit(void);
-
-virCapsPtr testXLInitCaps(void);
-
-#endif /* _TESTUTILSXEN_H_ */
diff --git a/tests/xlconfigdata/test-new-disk.cfg b/tests/xlconfigdata/test-new-disk.cfg
deleted file mode 100644
index afc0a46..0000000
--- a/tests/xlconfigdata/test-new-disk.cfg
+++ /dev/null
@@ -1,26 +0,0 @@
-name = "XenGuest2"
-uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809"
-maxmem = 579
-memory = 394
-vcpus = 1
-builder = "hvm"
-kernel = "/usr/lib/xen/boot/hvmloader"
-boot = "d"
-pae = 1
-acpi = 1
-apic = 1
-hap = 0
-viridian = 0
-localtime = 0
-on_poweroff = "destroy"
-on_reboot = "restart"
-on_crash = "restart"
-device_model = "/usr/lib/xen/bin/qemu-dm"
-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" ]
-parallel = "none"
-serial = "none"
-disk = [ "/dev/HostVG/XenGuest2,raw,hda,w,", "/var/lib/libvirt/images/XenGuest2-home,qcow2,hdb,w,", "/root/boot.iso,raw,hdc,r,devtype=cdrom" ]
diff --git a/tests/xlconfigdata/test-new-disk.xml b/tests/xlconfigdata/test-new-disk.xml
deleted file mode 100644
index 1c96a62..0000000
--- a/tests/xlconfigdata/test-new-disk.xml
+++ /dev/null
@@ -1,51 +0,0 @@
-<domain type='xen'>
-  <name>XenGuest2</name>
-  <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid>
-  <memory unit='KiB'>592896</memory>
-  <currentMemory unit='KiB'>403456</currentMemory>
-  <vcpu placement='static'>1</vcpu>
-  <os>
-    <type arch='x86_64' machine='xenfv'>hvm</type>
-    <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader>
-    <boot dev='cdrom'/>
-  </os>
-  <features>
-    <acpi/>
-    <apic/>
-    <pae/>
-  </features>
-  <clock offset='utc' adjustment='reset'/>
-  <on_poweroff>destroy</on_poweroff>
-  <on_reboot>restart</on_reboot>
-  <on_crash>restart</on_crash>
-  <devices>
-    <emulator>/usr/lib/xen/bin/qemu-dm</emulator>
-    <disk type='block' device='disk'>
-      <driver name='phy' type='raw'/>
-      <source dev='/dev/HostVG/XenGuest2'/>
-      <target dev='hda' bus='ide'/>
-    </disk>
-    <disk type='file' device='disk'>
-      <driver name='qemu' type='qcow2'/>
-      <source file='/var/lib/libvirt/images/XenGuest2-home'/>
-      <target dev='hdb' bus='ide'/>
-    </disk>
-    <disk type='file' device='cdrom'>
-      <driver name='qemu' type='raw'/>
-      <source file='/root/boot.iso'/>
-      <target dev='hdc' bus='ide'/>
-      <readonly/>
-    </disk>
-    <interface type='bridge'>
-      <mac address='00:16:3e:66:92:9c'/>
-      <source bridge='xenbr1'/>
-      <script path='vif-bridge'/>
-      <model type='e1000'/>
-    </interface>
-    <input type='mouse' bus='ps2'/>
-    <input type='keyboard' bus='ps2'/>
-    <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1'>
-      <listen type='address' address='127.0.0.1'/>
-    </graphics>
-  </devices>
-</domain>
diff --git a/tests/xlconfigdata/test-spice.cfg b/tests/xlconfigdata/test-spice.cfg
deleted file mode 100644
index f7aa55c..0000000
--- a/tests/xlconfigdata/test-spice.cfg
+++ /dev/null
@@ -1,32 +0,0 @@
-name = "XenGuest2"
-uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809"
-maxmem = 579
-memory = 394
-vcpus = 1
-builder = "hvm"
-kernel = "/usr/lib/xen/boot/hvmloader"
-boot = "d"
-pae = 1
-acpi = 1
-apic = 1
-hap = 0
-viridian = 0
-rtc_timeoffset = 0
-localtime = 1
-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" ]
-parallel = "none"
-serial = "none"
-disk = [ "/dev/HostVG/XenGuest2,raw,hda,w,", "/root/boot.iso,raw,hdc,w," ]
-sdl = 0
-vnc = 0
-spice = 1
-spicehost = "127.0.0.1"
-spiceport = 590
-spicetls_port = 500
-spicedisable_ticketing = 1
-spicepasswd = "thebeast"
-spiceagent_mouse = 0
diff --git a/tests/xlconfigdata/test-spice.xml b/tests/xlconfigdata/test-spice.xml
deleted file mode 100644
index 1e3f78d..0000000
--- a/tests/xlconfigdata/test-spice.xml
+++ /dev/null
@@ -1,45 +0,0 @@
-<domain type='xen'>
-  <name>XenGuest2</name>
-  <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid>
-  <memory unit='KiB'>592896</memory>
-  <currentMemory unit='KiB'>403456</currentMemory>
-  <vcpu placement='static'>1</vcpu>
-  <os>
-    <type arch='x86_64' machine='xenfv'>hvm</type>
-    <loader>/usr/lib/xen/boot/hvmloader</loader>
-    <boot dev='cdrom'/>
-  </os>
-  <features>
-    <acpi/>
-    <apic/>
-    <pae/>
-  </features>
-  <clock offset='variable' adjustment='0' basis='localtime'/>
-  <on_poweroff>destroy</on_poweroff>
-  <on_reboot>restart</on_reboot>
-  <on_crash>restart</on_crash>
-  <devices>
-    <emulator>/usr/lib/xen/bin/qemu-dm</emulator>
-    <disk type='block' device='disk'>
-      <driver name='phy' type='raw'/>
-      <source dev='/dev/HostVG/XenGuest2'/>
-      <target dev='hda' bus='ide'/>
-    </disk>
-    <disk type='block' device='disk'>
-      <driver name='phy' type='raw'/>
-      <source dev='/root/boot.iso'/>
-      <target dev='hdc' bus='ide'/>
-    </disk>
-    <interface type='bridge'>
-      <mac address='00:16:3e:66:92:9c'/>
-      <source bridge='xenbr1'/>
-      <script path='vif-bridge'/>
-      <model type='e1000'/>
-    </interface>
-    <input type='mouse' bus='ps2'/>
-    <input type='keyboard' bus='ps2'/>
-    <graphics type='spice' port='590' tlsPort='500' autoport='no' listen='127.0.0.1' passwd='thebeast'>
-      <listen type='address' address='127.0.0.1'/>
-    </graphics>
-  </devices>
-</domain>
diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c
deleted file mode 100644
index b966c15..0000000
--- a/tests/xlconfigtest.c
+++ /dev/null
@@ -1,224 +0,0 @@
-/*
- * xlconfigtest.c: Test backend for xl_internal config file handling
- *
- * Copyright (C) 2007, 2010-2011, 2014 Red Hat, Inc.
- *
- * 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
- * <http://www.gnu.org/licenses/>.
- *
- * Author: Daniel P. Berrange <berrange@redhat.com>
- * Author: Kiarie Kahurani <davidkiarie4@gmail.com>
- *
- */
-
-#include <config.h>
-
-#include <stdio.h>
-#include <string.h>
-#include <unistd.h>
-
-#include "internal.h"
-#include "datatypes.h"
-#include "xenconfig/xen_xl.h"
-#include "viralloc.h"
-#include "virstring.h"
-#include "testutils.h"
-#include "testutilsxen.h"
-#include "xen/xen_driver.h"
-
-#define VIR_FROM_THIS VIR_FROM_NONE
-
-static virCapsPtr caps;
-static virDomainXMLOptionPtr xmlopt;
-/*
- * parses the xml, creates a domain def and compare with equivalent xm config
- */
-static int
-testCompareParseXML(const char *xmcfg, const char *xml, int xendConfigVersion)
-{
-    char *xmlData = NULL;
-    char *xmcfgData = NULL;
-    char *gotxmcfgData = NULL;
-    virConfPtr conf = NULL;
-    virConnectPtr conn = NULL;
-    int wrote = 4096;
-    int ret = -1;
-    virDomainDefPtr def = NULL;
-
-    if (VIR_ALLOC_N(gotxmcfgData, wrote) < 0)
-        goto fail;
-
-    conn = virGetConnect();
-    if (!conn) goto fail;
-
-    if (virtTestLoadFile(xml, &xmlData) < 0)
-        goto fail;
-
-    if (virtTestLoadFile(xmcfg, &xmcfgData) < 0)
-        goto fail;
-
-    if (!(def = virDomainDefParseString(xmlData, caps, xmlopt,
-                                        1 << VIR_DOMAIN_VIRT_XEN,
-                                        VIR_DOMAIN_XML_INACTIVE)))
-        goto fail;
-
-    if (!virDomainDefCheckABIStability(def, def)) {
-        fprintf(stderr, "ABI stability check failed on %s", xml);
-        goto fail;
-    }
-
-    if (!(conf = xenFormatXL(def, conn,  xendConfigVersion)))
-        goto fail;
-
-    if (virConfWriteMem(gotxmcfgData, &wrote, conf) < 0)
-        goto fail;
-    gotxmcfgData[wrote] = '\0';
-
-    if (STRNEQ(xmcfgData, gotxmcfgData)) {
-        virtTestDifference(stderr, xmcfgData, gotxmcfgData);
-        goto fail;
-    }
-
-    ret = 0;
-
- fail:
-    VIR_FREE(xmlData);
-    VIR_FREE(xmcfgData);
-    VIR_FREE(gotxmcfgData);
-    if (conf)
-        virConfFree(conf);
-    virDomainDefFree(def);
-    virObjectUnref(conn);
-
-    return ret;
-}
-/*
- * parses the xl config, develops domain def and compares with equivalent xm config
- */
-static int
-testCompareFormatXML(const char *xmcfg, const char *xml, int xendConfigVersion)
-{
-    char *xmlData = NULL;
-    char *xmcfgData = NULL;
-    char *gotxml = NULL;
-    virConfPtr conf = NULL;
-    int ret = -1;
-    virConnectPtr conn;
-    virDomainDefPtr def = NULL;
-
-    conn = virGetConnect();
-    if (!conn) goto fail;
-
-    if (virtTestLoadFile(xml, &xmlData) < 0)
-        goto fail;
-
-    if (virtTestLoadFile(xmcfg, &xmcfgData) < 0)
-        goto fail;
-
-    if (!(conf = virConfReadMem(xmcfgData, strlen(xmcfgData), 0)))
-        goto fail;
-
-    if (!(def = xenParseXL(conf, caps, xendConfigVersion)))
-        goto fail;
-
-    if (!(gotxml = virDomainDefFormat(def, VIR_DOMAIN_XML_INACTIVE |
-                                      VIR_DOMAIN_XML_SECURE)))
-        goto fail;
-
-    if (STRNEQ(xmlData, gotxml)) {
-        virtTestDifference(stderr, xmlData, gotxml);
-        goto fail;
-    }
-
-    ret = 0;
-
- fail:
-    if (conf)
-        virConfFree(conf);
-    VIR_FREE(xmlData);
-    VIR_FREE(xmcfgData);
-    VIR_FREE(gotxml);
-    virDomainDefFree(def);
-    virObjectUnref(conn);
-
-    return ret;
-}
-
-
-struct testInfo {
-    const char *name;
-    int version;
-    int mode;
-};
-
-static int
-testCompareHelper(const void *data)
-{
-    int result = -1;
-    const struct testInfo *info = data;
-    char *xml = NULL;
-    char *cfg = NULL;
-
-    if (virAsprintf(&xml, "%s/xlconfigdata/test-%s.xml",
-                    abs_srcdir, info->name) < 0 ||
-        virAsprintf(&cfg, "%s/xlconfigdata/test-%s.cfg",
-                    abs_srcdir, info->name) < 0)
-        goto cleanup;
-
-    if (info->mode == 0)
-        result = testCompareParseXML(cfg, xml, info->version);
-    else
-        result = testCompareFormatXML(cfg, xml, info->version);
-
- cleanup:
-    VIR_FREE(xml);
-    VIR_FREE(cfg);
-
-    return result;
-}
-
-
-static int
-mymain(void)
-{
-    int ret = 0;
-
-    if (!(caps = testXLInitCaps()))
-        return EXIT_FAILURE;
-
-    if (!(xmlopt = xenDomainXMLConfInit()))
-        return EXIT_FAILURE;
-
-#define DO_TEST(name, version)                                          \
-    do {                                                                \
-        struct testInfo info0 = { name, version, 0 };                   \
-        struct testInfo info1 = { name, version, 1 };                   \
-        if (virtTestRun("Xen XM-2-XML Parse  " name,                    \
-                        testCompareHelper, &info0) < 0)                 \
-            ret = -1;                                                   \
-        if (virtTestRun("Xen XM-2-XML Format " name,                    \
-                        testCompareHelper, &info1) < 0)                 \
-            ret = -1;                                                   \
-    } while (0)
-
-    DO_TEST("new-disk", 3);
-//    DO_TEST("spice", 3);
-
-    virObjectUnref(caps);
-    virObjectUnref(xmlopt);
-
-    return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
-}
-
-VIRT_TEST_MAIN(mymain)
-- 
1.8.4.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 09/12] Revert "src/xenconfig: Xen-xl parser"
  2015-01-10  5:03 [PATCH 00/12] Replace Xen xl parsing/formatting impl Jim Fehlig
                   ` (7 preceding siblings ...)
  2015-01-10  5:03 ` [PATCH 08/12] Revert "tests: Tests for the xen-xl parser" Jim Fehlig
@ 2015-01-10  5:03 ` Jim Fehlig
  2015-01-12 14:51   ` [libvirt] " John Ferlan
       [not found]   ` <54B3DF75.2000406@redhat.com>
  2015-01-10  5:03 ` [PATCH 10/12] Introduce support for parsing/formatting Xen xl config format Jim Fehlig
                   ` (4 subsequent siblings)
  13 siblings, 2 replies; 25+ messages in thread
From: Jim Fehlig @ 2015-01-10  5:03 UTC (permalink / raw)
  To: libvir-list; +Cc: Jim Fehlig, xen-devel

This reverts commit 2c78051a14acfb7aba078d569b1632dfe0ca0853.

Conflicts:
	src/Makefile.am

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 .gitignore                    |   1 -
 cfg.mk                        |   3 +-
 configure.ac                  |   1 -
 po/POTFILES.in                |   1 -
 src/Makefile.am               |  25 +--
 src/libvirt_xenconfig.syms    |   4 -
 src/xenconfig/xen_common.c    |   3 +-
 src/xenconfig/xen_xl.c        | 499 ------------------------------------------
 src/xenconfig/xen_xl.h        |  33 ---
 src/xenconfig/xen_xl_disk.l   | 256 ----------------------
 src/xenconfig/xen_xl_disk_i.h |  39 ----
 11 files changed, 4 insertions(+), 861 deletions(-)

diff --git a/.gitignore b/.gitignore
index eac2203..9d09709 100644
--- a/.gitignore
+++ b/.gitignore
@@ -140,7 +140,6 @@
 /src/remote/*_protocol.[ch]
 /src/rpc/virkeepaliveprotocol.[ch]
 /src/rpc/virnetprotocol.[ch]
-/src/xenconfig/xen_xl_disk.[ch]
 /src/test_libvirt*.aug
 /src/test_virtlockd.aug
 /src/util/virkeymaps.h
diff --git a/cfg.mk b/cfg.mk
index 3df3dcb..21f83c3 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -89,9 +89,8 @@ distdir: sc_vulnerable_makefile_CVE-2012-3386.z
 endif
 
 # Files that should never cause syntax check failures.
-#  (^(HACKING|docs/(news\.html\.in|.*\.patch))|\.(po|fig|gif|ico|png))$$
 VC_LIST_ALWAYS_EXCLUDE_REGEX = \
-  (^(HACKING|docs/(news\.html\.in|.*\.patch)|src/xenconfig/xen_xl_disk.[chl])|\.(po|fig|gif|ico|png))$$
+  (^(HACKING|docs/(news\.html\.in|.*\.patch))|\.(po|fig|gif|ico|png))$$
 
 # Functions like free() that are no-ops on NULL arguments.
 useless_free_options =				\
diff --git a/configure.ac b/configure.ac
index 167b875..9d12079 100644
--- a/configure.ac
+++ b/configure.ac
@@ -146,7 +146,6 @@ m4_ifndef([LT_INIT], [
 ])
 AM_PROG_CC_C_O
 AM_PROG_LD
-AM_PROG_LEX
 
 AC_MSG_CHECKING([for how to mark DSO non-deletable at runtime])
 LIBVIRT_NODELETE=
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 094c8e3..e7cb2cc 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -247,7 +247,6 @@ 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 c7975e5..e0e47d0 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1000,22 +1000,11 @@ CPU_SOURCES =							\
 VMX_SOURCES =							\
 		vmx/vmx.c vmx/vmx.h
 
-AM_LFLAGS = -Pxl_disk_ --header-file=../$*.h
-LEX_OUTPUT_ROOT = lex.xl_disk_
-BUILT_SOURCES += xenconfig/xen_xl_disk.c xenconfig/xen_xl_disk.h
-# Generated header file is not implicitly added to dist
-EXTRA_DIST += xenconfig/xen_xl_disk.h
-CLEANFILES += xenconfig/xen_xl_disk.h xenconfig/xen_xl_disk.c
-
-XENXLDISKPARSER_SOURCES = xenconfig/xen_xl_disk.l
-
 XENCONFIG_SOURCES =						\
 		xenconfig/xenxs_private.h			\
-		xenconfig/xen_common.c xenconfig/xen_common.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_xl.c xenconfig/xen_xl.h		\
-		xenconfig/xen_xl_disk_i.h
+		xenconfig/xen_xm.c xenconfig/xen_xm.h
 
 pkgdata_DATA =	cpu/cpu_map.xml
 
@@ -1070,19 +1059,10 @@ libvirt_vmx_la_SOURCES = $(VMX_SOURCES)
 endif WITH_VMX
 
 if WITH_XENCONFIG
-# Flex generated XL disk parser needs to be compiled without WARN_FLAGS
-# Add the generated object to its own library to control CFLAGS
-noinst_LTLIBRARIES += libvirt_xenxldiskparser.la
-libvirt_xenxldiskparser_la_CFLAGS = \
-		-I$(srcdir)/conf $(AM_CFLAGS) -Wno-unused-parameter
-libvirt_xenxldiskparser_la_SOURCES = \
-	$(XENXLDISKPARSER_SOURCES)
-
 noinst_LTLIBRARIES += libvirt_xenconfig.la
 libvirt_la_BUILT_LIBADD += libvirt_xenconfig.la
 libvirt_xenconfig_la_CFLAGS = \
 		-I$(srcdir)/conf $(AM_CFLAGS)
-libvirt_xenconfig_la_LIBADD = libvirt_xenxldiskparser.la
 libvirt_xenconfig_la_SOURCES = $(XENCONFIG_SOURCES)
 endif WITH_XENCONFIG
 
@@ -1844,7 +1824,6 @@ EXTRA_DIST +=							\
 		$(VBOX_DRIVER_EXTRA_DIST)			\
 		$(VMWARE_DRIVER_SOURCES)			\
 		$(XENCONFIG_SOURCES)				\
-		$(XENXLDISKPARSER_SOURCES)			\
 		$(ACCESS_DRIVER_POLKIT_POLICY)
 
 check-local: check-augeas
diff --git a/src/libvirt_xenconfig.syms b/src/libvirt_xenconfig.syms
index 3e2e5d6..6541685 100644
--- a/src/libvirt_xenconfig.syms
+++ b/src/libvirt_xenconfig.syms
@@ -16,10 +16,6 @@ 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 a2a1474..b40a722 100644
--- a/src/xenconfig/xen_common.c
+++ b/src/xenconfig/xen_common.c
@@ -1812,8 +1812,7 @@ xenFormatVfb(virConfPtr conf, virDomainDefPtr def, int xendConfigVersion)
 {
     int hvm = STREQ(def->os.type, "hvm") ? 1 : 0;
 
-    if (def->ngraphics == 1 &&
-        def->graphics[0]->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
+    if (def->ngraphics == 1) {
         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
deleted file mode 100644
index 8d1d2a7..0000000
--- a/src/xenconfig/xen_xl.c
+++ /dev/null
@@ -1,499 +0,0 @@
-/*
- * xen_xl.c: Xen XL parsing functions
- *
- * 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
- * <http://www.gnu.org/licenses/>.
- *
- * Author: Kiarie Kahurani <davidkiarie4@gmail.com>
- */
-
-#include <config.h>
-
-#include "virconf.h"
-#include "virerror.h"
-#include "domain_conf.h"
-#include "viralloc.h"
-#include "virstring.h"
-#include "xen_xl.h"
-#include "xen_xl_disk.h"
-#include "xen_xl_disk_i.h"
-
-#define VIR_FROM_THIS VIR_FROM_NONE
-
-
-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;
-}
-
-
-void
-xenXLDiskParserError(xenXLDiskParserContext *dpc,
-                     const char *erroneous,
-                     const char *message)
-{
-    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                   _("disk config %s not supported: %s"),
-                   erroneous, message);
-
-    if (!dpc->err)
-        dpc->err = EINVAL;
-}
-
-
-static int
-xenXLDiskParserPrep(xenXLDiskParserContext *dpc,
-                    const char *spec,
-                    virDomainDiskDefPtr disk)
-{
-    int err;
-
-    dpc->spec = spec;
-    dpc->disk = disk;
-    dpc->access_set = 0;
-
-    err = xl_disk_lex_init_extra(dpc, &dpc->scanner);
-    if (err)
-        goto fail;
-
-    dpc->buf = xl_disk__scan_bytes(spec, strlen(spec), dpc->scanner);
-    if (!dpc->buf) {
-        err = ENOMEM;
-        goto fail;
-    }
-
-    return 0;
-
- fail:
-    virReportSystemError(errno, "%s",
-                         _("failed to initialize disk configuration parser"));
-    return err;
-}
-
-
-static void
-xenXLDiskParserCleanup(xenXLDiskParserContext *dpc)
-{
-    if (dpc->buf) {
-        xl_disk__delete_buffer(dpc->buf, dpc->scanner);
-        dpc->buf = NULL;
-    }
-
-    if (dpc->scanner) {
-        xl_disk_lex_destroy(dpc->scanner);
-        dpc->scanner = NULL;
-    }
-}
-
-
-/*
- * positional parameters
- *     (If the <diskspec> 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 = <devtype>
- *    backendtype = <backend-type>
- * parameters not taken care of
- *    backend = <domain-name>
- *    script = <script>
- *    direct-io-safe
- *
- * ================================================================
- * The parser does not take any deprecated parameters
- *
- * For more information refer to /xen/docs/misc/xl-disk-configuration.txt
- */
-static int
-xenParseXLDisk(virConfPtr conf, virDomainDefPtr def)
-{
-    virConfValuePtr list = virConfGetValue(conf, "disk");
-    xenXLDiskParserContext dpc;
-    virDomainDiskDefPtr disk;
-
-    memset(&dpc, 0, sizeof(dpc));
-
-    if (list && list->type == VIR_CONF_LIST) {
-        list = list->list;
-        while (list) {
-            char *disk_spec = list->str;
-            const char *driver;
-
-            if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
-                goto skipdisk;
-
-            if (!(disk = virDomainDiskDefNew()))
-                    return -1;
-
-            disk->src->readonly = 0;
-            disk->src->format = VIR_STORAGE_FILE_LAST;
-
-            if (xenXLDiskParserPrep(&dpc, disk_spec, disk))
-                goto fail;
-
-            xl_disk_lex(dpc.scanner);
-
-            if (dpc.err)
-                goto fail;
-
-            if (disk->src->format == VIR_STORAGE_FILE_LAST)
-                disk->src->format = VIR_STORAGE_FILE_RAW;
-
-            if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
-                disk->removable = true;
-                disk->src->readonly = true;
-                if (virDomainDiskSetDriver(disk, "qemu") < 0)
-                    goto fail;
-
-                virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE);
-                if (!disk->src->path || STREQ(disk->src->path, ""))
-                    disk->src->format = VIR_STORAGE_FILE_NONE;
-            }
-
-            if (STRPREFIX(disk->dst, "xvd") || !STREQ(def->os.type, "hvm"))
-                disk->bus = VIR_DOMAIN_DISK_BUS_XEN;
-            else if (STRPREFIX(disk->dst, "sd"))
-                disk->bus = VIR_DOMAIN_DISK_BUS_SCSI;
-            else
-                disk->bus = VIR_DOMAIN_DISK_BUS_IDE;
-
-            driver = virDomainDiskGetDriver(disk);
-            if (!driver) {
-                switch (disk->src->format) {
-                case VIR_STORAGE_FILE_QCOW:
-                case VIR_STORAGE_FILE_QCOW2:
-                case VIR_STORAGE_FILE_VHD:
-                    driver = "qemu";
-                    if (virDomainDiskSetDriver(disk, "qemu") < 0)
-                        goto fail;
-                    break;
-                default:
-                    driver = "phy";
-                    if (virDomainDiskSetDriver(disk, "phy") < 0)
-                        goto fail;
-                }
-            }
-
-            if (STREQ(driver, "phy"))
-                virDomainDiskSetType(disk, VIR_STORAGE_TYPE_BLOCK);
-            else
-                virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE);
-
-            if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0)
-                goto fail;
-
-        skipdisk:
-            list = list->next;
-            xenXLDiskParserCleanup(&dpc);
-        }
-    }
-    return 0;
-
- fail:
-    xenXLDiskParserCleanup(&dpc);
-    virDomainDiskDefFree(disk);
-    return -1;
-}
-
-
-virDomainDefPtr
-xenParseXL(virConfPtr conf, virCapsPtr caps, int xendConfigVersion)
-{
-    virDomainDefPtr def = NULL;
-
-    if (VIR_ALLOC(def) < 0)
-        return NULL;
-
-    def->virtType = VIR_DOMAIN_VIRT_XEN;
-    def->id = -1;
-
-    if (xenParseConfigCommon(conf, def, caps, xendConfigVersion) < 0)
-        goto cleanup;
-
-    if (xenParseXLDisk(conf, def) < 0)
-        goto cleanup;
-
-    if (xenParseXLSpice(conf, def) < 0)
-        goto cleanup;
-
-    return def;
-
- cleanup:
-    virDomainDefFree(def);
-    return NULL;
-}
-
-
-static int
-xenFormatXLDisk(virConfValuePtr list, virDomainDiskDefPtr disk)
-{
-    virBuffer buf = VIR_BUFFER_INITIALIZER;
-    virConfValuePtr val, tmp;
-    const char *src = virDomainDiskGetSource(disk);
-    int format = virDomainDiskGetFormat(disk);
-
-    /* target */
-    virBufferAsprintf(&buf, "%s,", src);
-    /* format */
-    switch (format) {
-        case VIR_STORAGE_FILE_RAW:
-            virBufferAddLit(&buf, "raw,");
-            break;
-        case VIR_STORAGE_FILE_VHD:
-            virBufferAddLit(&buf, "xvhd,");
-            break;
-        case VIR_STORAGE_FILE_QCOW:
-            virBufferAddLit(&buf, "qcow,");
-            break;
-        case VIR_STORAGE_FILE_QCOW2:
-            virBufferAddLit(&buf, "qcow2,");
-            break;
-      /* set default */
-        default:
-            virBufferAddLit(&buf, "raw,");
-    }
-
-    /* device */
-    virBufferAdd(&buf, disk->dst, -1);
-
-    virBufferAddLit(&buf, ",");
-
-    if (disk->src->readonly)
-        virBufferAddLit(&buf, "r,");
-    else if (disk->src->shared)
-        virBufferAddLit(&buf, "!,");
-    else
-        virBufferAddLit(&buf, "w,");
-    if (disk->transient) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("transient disks not supported yet"));
-        goto cleanup;
-    }
-
-    if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM)
-        virBufferAddLit(&buf, "devtype=cdrom");
-
-    if (virBufferCheckError(&buf) < 0)
-        goto cleanup;
-
-    if (VIR_ALLOC(val) < 0)
-        goto cleanup;
-
-    val->type = VIR_CONF_STRING;
-    val->str = virBufferContentAndReset(&buf);
-    tmp = list->list;
-    while (tmp && tmp->next)
-        tmp = tmp->next;
-    if (tmp)
-        tmp->next = val;
-    else
-        list->list = val;
-    return 0;
-
- cleanup:
-    virBufferFreeAndReset(&buf);
-    return -1;
-}
-
-
-static int
-xenFormatXLDomainDisks(virConfPtr conf, virDomainDefPtr def)
-{
-    virConfValuePtr diskVal = NULL;
-    size_t i = 0;
-
-    if (VIR_ALLOC(diskVal) < 0)
-        return -1;
-
-    diskVal->type = VIR_CONF_LIST;
-    diskVal->list = NULL;
-
-    for (i = 0; i < def->ndisks; i++) {
-        if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
-            continue;
-        if (xenFormatXLDisk(diskVal, def->disks[i]) < 0)
-
-                goto cleanup;
-    }
-
-    if (diskVal->list != NULL) {
-        int ret = virConfSetValue(conf, "disk", diskVal);
-        diskVal = NULL;
-        if (ret < 0)
-            goto cleanup;
-    }
-
-    return 0;
-
- cleanup:
-    virConfFreeValue(diskVal);
-    return 0;
-}
-
-
-static int
-xenFormatXLSpice(virConfPtr conf, virDomainDefPtr def)
-{
-    const char *listenAddr = NULL;
-
-    if (STREQ(def->os.type, "hvm")) {
-        if (def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
-            /* set others to false but may not be necessary */
-            if (xenConfigSetInt(conf, "sdl", 0) < 0)
-                return -1;
-
-            if (xenConfigSetInt(conf, "vnc", 0) < 0)
-                return -1;
-
-            if (xenConfigSetInt(conf, "spice", 1) < 0)
-                return -1;
-
-            if (xenConfigSetInt(conf, "spiceport",
-                                def->graphics[0]->data.spice.port) < 0)
-                return -1;
-
-            if (xenConfigSetInt(conf, "spicetls_port",
-                                def->graphics[0]->data.spice.tlsPort) < 0)
-                return -1;
-
-            if (def->graphics[0]->data.spice.auth.passwd) {
-                if (xenConfigSetInt(conf, "spicedisable_ticketing", 1) < 0)
-                    return -1;
-
-                if (def->graphics[0]->data.spice.auth.passwd &&
-                    xenConfigSetString(conf, "spicepasswd",
-                                def->graphics[0]->data.spice.auth.passwd) < 0)
-                    return -1;
-            }
-
-            listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0);
-            if (listenAddr &&
-                xenConfigSetString(conf, "spicehost", listenAddr) < 0)
-                return -1;
-
-            if (xenConfigSetInt(conf, "spicemouse_mouse",
-                                def->graphics[0]->data.spice.mousemode) < 0)
-                return -1;
-
-            if (def->graphics[0]->data.spice.copypaste) {
-                if (xenConfigSetInt(conf, "spicedvagent", 1) < 0)
-                    return -1;
-                if (xenConfigSetInt(conf, "spice_clipboard_sharing",
-                                def->graphics[0]->data.spice.copypaste) < 0)
-                return -1;
-            }
-        }
-    }
-
-    return 0;
-}
-
-
-virConfPtr
-xenFormatXL(virDomainDefPtr def, virConnectPtr conn, int xendConfigVersion)
-{
-    virConfPtr conf = NULL;
-
-    if (!(conf = virConfNew()))
-        goto cleanup;
-
-    if (xenFormatConfigCommon(conf, def, conn, xendConfigVersion) < 0)
-        goto cleanup;
-
-    if (xenFormatXLDomainDisks(conf, def) < 0)
-        goto cleanup;
-
-    if (xenFormatXLSpice(conf, def) < 0)
-        goto cleanup;
-
-    return conf;
-
- cleanup:
-    if (conf)
-        virConfFree(conf);
-    return NULL;
-}
diff --git a/src/xenconfig/xen_xl.h b/src/xenconfig/xen_xl.h
deleted file mode 100644
index 536e9b7..0000000
--- a/src/xenconfig/xen_xl.h
+++ /dev/null
@@ -1,33 +0,0 @@
-/*
- * xen_xl.h: Xen XL parsing functions
- *
- * 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
- * <http://www.gnu.org/licenses/>.
- *
- * Author: Kiarie Kahurani<davidkiarie4@gmail.com>
- */
-
-#ifndef __VIR_XEN_XL_H__
-# define __VIR_XEN_XL_H__
-
-# include "virconf.h"
-# include "domain_conf.h"
-# include "xen_common.h"
-
-virDomainDefPtr xenParseXL(virConfPtr conn, virCapsPtr caps,
-                           int xendConfigVersion);
-virConfPtr xenFormatXL(virDomainDefPtr def,
-                       virConnectPtr, int xendConfigVersion);
-
-#endif /* __VIR_XEN_XL_H__ */
diff --git a/src/xenconfig/xen_xl_disk.l b/src/xenconfig/xen_xl_disk.l
deleted file mode 100644
index 164aa32..0000000
--- a/src/xenconfig/xen_xl_disk.l
+++ /dev/null
@@ -1,256 +0,0 @@
-/*
- * xen_xl_disk.l - parser for disk specification strings
- *
- * Copyright (C) 2011      Citrix Ltd.
- * Author Ian Jackson <ian.jackson@eu.citrix.com>
- *
- * This program 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; version 2.1 only. with the special
- * exception on linking described in file LICENSE.
- *
- * This program 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.
- */
-
-/*
- * Parsing the old xm/xend/xl-4.1 disk specs is a tricky problem,
- * because the target string might in theory contain "," which is the
- * delimiter we use for stripping off things on the RHS, and ":",
- * which is the delimiter we use for stripping off things on the LHS.
- *
- * In this parser we do not support such target strings in the old
- * syntax; if the target string has to contain "," or ":" the new
- * syntax's "target=" should be used.
- */
-%{
-# include <config.h>
-
-# include <stdio.h>
-
-# include "viralloc.h"
-# include "virstoragefile.h"
-# include "virstring.h"
-# include "domain_conf.h"
-# include "xen_xl.h"
-# include "xen_xl_disk_i.h"
-
-#define YY_NO_INPUT
-#define VIR_FROM_THIS VIR_FROM_NONE
-
-/* Some versions of flex have a bug (Fedora bugzilla 612465) which causes
- * it to fail to declare these functions, which it defines.  So declare
- * them ourselves.  Hopefully we won't have to simultaneously support
- * a flex version which declares these differently somehow. */
-int xl_disk_lexget_column(yyscan_t yyscanner);
-void xl_disk_lexset_column(int  column_no, yyscan_t yyscanner);
-
-
-/*----- useful macros and functions used in actions -----
- * we use macros in the actual rules to keep the actions short
- * and particularly to avoid repeating boilerplate values such as
- * DPC->disk, yytext, etc. */
-
-/* For actions whose patterns contain '=', finds the start of the value */
-#define FROMEQUALS (strchr(yytext,'=')+1)
-
-/* Chops the delimiter off, modifying yytext and yyleng. */
-#define STRIP(delim) do{                                                \
-	if (yyleng>0 && yytext[yyleng-1]==(delim))                      \
-	    yytext[--yyleng] = 0;                                       \
-    }while(0)
-
-/* Sets a string value, checking it hasn't been set already. */
-#define SAVESTRING(what,loc,val) do{					\
-	savestring(DPC, what " respecified", &DPC->disk->loc, (val));	\
-    }while(0)
-
-
-static void
-savestring(xenXLDiskParserContext *dpc,
-           const char *what_respecified,
-           char **update,
-           const char *value)
-{
-    if (*update) {
-        if (**update) {
-            xenXLDiskParserError(dpc, value, what_respecified);
-            return;
-        }
-
-        VIR_FREE(*update); /* do not complain about overwriting empty strings */
-    }
-
-    ignore_value(VIR_STRDUP(*update, value));
-}
-
-#define DPC dpc /* our convention in lexer helper functions */
-
-/* Sets ->readwrite from the string. */
-static void
-setaccess(xenXLDiskParserContext *dpc, const char *str)
-{
-    if (STREQ(str, "rw") || STREQ(str, "w")) {
-        dpc->disk->src->readonly = 0;
-    } else if (STREQ(str, "r") || STREQ(str, "ro")) {
-        dpc->disk->src->readonly = 1;
-    } else if (STREQ(str, "w!") || STREQ(str, "!")) {
-        dpc->disk->src->readonly = 0;
-	dpc->disk->src->shared = 1;
-    } else {
-        xenXLDiskParserError(dpc, str, "unknown value for access");
-    }
-    dpc->access_set = 1;
-}
-
-/* Sets ->format from the string.  IDL should provide something for this. */
-static void
-setformat(xenXLDiskParserContext *dpc, const char *str)
-{
-    if (STREQ(str, "") || STREQ(str, "raw"))
-        virDomainDiskSetFormat(dpc->disk, VIR_STORAGE_FILE_RAW);
-    else if (STREQ(str, "qcow"))
-        virDomainDiskSetFormat(dpc->disk, VIR_STORAGE_FILE_QCOW);
-    else if (STREQ(str, "qcow2"))
-        virDomainDiskSetFormat(dpc->disk, VIR_STORAGE_FILE_QCOW2);
-    else if (STREQ(str, "vhd"))
-        virDomainDiskSetFormat(dpc->disk, VIR_STORAGE_FILE_VHD);
-    else
-        xenXLDiskParserError(dpc, str, "unknown value for format");
-}
-
-
-/* Sets ->backend from the string.  IDL should provide something for this. */
-static void
-setdrivertype(xenXLDiskParserContext *dpc, const char *str)
-{
-    if (STREQ(str, "phy"))
-        ignore_value(virDomainDiskSetDriver(dpc->disk, "phy"));
-    else if (STREQ(str, "tap"))
-        ignore_value(virDomainDiskSetDriver(dpc->disk, "tap"));
-    else if (STREQ(str, "file") || STREQ(str, ""))
-        ignore_value(virDomainDiskSetDriver(dpc->disk, "qemu"));
-    else
-        xenXLDiskParserError(dpc, str, "unknown value for backendtype");
-}
-
-
-/* Handles a vdev positional parameter which includes a devtype. */
-static int
-vdev_and_devtype(xenXLDiskParserContext *dpc, char *str)
-{
-    /* returns 1 if it was <vdev>:<devtype>, 0 (doing nothing) otherwise */
-    char *colon = strrchr(str, ':');
-    if (!colon)
-        return 0;
-
-    *colon++ = 0;
-    SAVESTRING("vdev", dst, str);
-
-    if (STREQ(colon,"cdrom")) {
-        DPC->disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM;
-    } else if (STREQ(colon, "disk")) {
-        DPC->disk->device = VIR_DOMAIN_DISK_DEVICE_DISK;
-    } else {
-        xenXLDiskParserError(DPC, colon, "unknown deprecated type");
-    }
-    return 1;
-}
-
-#undef DPC /* needs to be defined differently the actual lexer */
-#define DPC ((xenXLDiskParserContext*)yyextra)
-
-%}
-
-%option warn
-%option nodefault
-%option batch
-%option 8bit
-%option noyywrap
-%option reentrant
-%option nounput
-
-%x LEXERR
-
-%%
-
- /*----- the scanner rules which do the parsing -----*/
-
-[ \t\n]+/([^ \t\n].*)? { /* ignore whitespace before parameters */ }
-
- /* ordinary parameters setting enums or strings */
-
-format=[^,]*,?	{ STRIP(','); setformat(DPC, FROMEQUALS); }
-
-cdrom,?		{ DPC->disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM; }
-devtype=cdrom,?	{ DPC->disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM; }
-devtype=disk,?	{ DPC->disk->device = VIR_DOMAIN_DISK_DEVICE_DISK; }
-devtype=[^,]*,?	{ xenXLDiskParserError(DPC, yytext,"unknown value for type"); }
-
-access=[^,]*,?	{ STRIP(','); setaccess(DPC, FROMEQUALS); }
-backendtype=[^,]*,? { STRIP(','); setdrivertype(DPC, FROMEQUALS); }
-
-vdev=[^,]*,?	{ STRIP(','); SAVESTRING("vdev", dst, FROMEQUALS); }
-
- /* the target magic parameter, eats the rest of the string */
-
-target=.*	{ STRIP(','); SAVESTRING("target", src->path, FROMEQUALS); }
-
- /* unknown parameters */
-
-[a-z][-a-z0-9]*=[^,],? { xenXLDiskParserError(DPC, yytext, "unknown parameter"); }
-
-  /* the "/.*" in these patterns ensures that they count as if they
-   * matched the whole string, so these patterns take precedence */
-
-(raw|qcow2?|vhd):/.* {
-                    STRIP(':');
-                    DPC->had_depr_prefix=1;
-                    setformat(DPC, yytext);
-                 }
-
-tapdisk:/.*	{ DPC->had_depr_prefix=1; }
-tap2?:/.*	{ DPC->had_depr_prefix=1; }
-aio:/.*		{ DPC->had_depr_prefix=1; }
-ioemu:/.*	{ DPC->had_depr_prefix=1; }
-file:/.*	{ DPC->had_depr_prefix=1; }
-phy:/.*		{ DPC->had_depr_prefix=1; }
-[a-z][a-z0-9]*:/([^a-z0-9].*)? {
-		  xenXLDiskParserError(DPC, yytext, "unknown deprecated disk prefix");
-		  return 0;
-		}
-
- /* positional parameters */
-
-[^=,]*,|[^=,]+,?  {
-    STRIP(',');
-
-    if (DPC->err) {
-        /* previous errors may just lead to subsequent ones */
-    } else if (!DPC->disk->src->path) {
-        SAVESTRING("target", src->path, yytext);
-    } else if (DPC->disk->src->format == VIR_STORAGE_FILE_LAST){
-        setformat(DPC, yytext);
-    }
-     else if (!DPC->disk->dst) {
-        if (!vdev_and_devtype(DPC, yytext))
-            SAVESTRING("vdev", dst, yytext);
-    } else if (!DPC->access_set) {
-        DPC->access_set = 1;
-        setaccess(DPC, yytext);
-    } else {
-        xenXLDiskParserError(DPC, yytext, "too many positional parameters");
-        return 0; /* don't print any more errors */
-    }
-}
-
-. {
-    BEGIN(LEXERR);
-    yymore();
-}
-<LEXERR>.* {
-    xenXLDiskParserError(DPC, yytext, "bad disk syntax");
-    return 0;
-}
diff --git a/src/xenconfig/xen_xl_disk_i.h b/src/xenconfig/xen_xl_disk_i.h
deleted file mode 100644
index 063dedf..0000000
--- a/src/xenconfig/xen_xl_disk_i.h
+++ /dev/null
@@ -1,39 +0,0 @@
-/*
- * xen_xl_disk_i.h - common header for disk spec parser
- *
- * Copyright (C) 2011      Citrix Ltd.
- * Author Ian Jackson <ian.jackson@eu.citrix.com>
- *
- * This program 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; version 2.1 only. with the special
- * exception on linking described in file LICENSE.
- *
- * This program 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.
- */
-
-#ifndef __VIR_XEN_XL_DISK_I_H__
-# define __VIR_XEN_XL_DISK_I_H__
-
-# include "virconf.h"
-# include "domain_conf.h"
-
-
-typedef struct {
-    int err;
-    void *scanner;
-    YY_BUFFER_STATE buf;
-    virDomainDiskDefPtr disk;
-    int access_set;
-    int had_depr_prefix;
-    const char *spec;
-} xenXLDiskParserContext;
-
-void xenXLDiskParserError(xenXLDiskParserContext *dpc,
-                          const char *erroneous,
-                          const char *message);
-
-#endif /* __VIR_XEN_XL_DISK_I_H__ */
-- 
1.8.4.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 10/12] Introduce support for parsing/formatting Xen xl config format
  2015-01-10  5:03 [PATCH 00/12] Replace Xen xl parsing/formatting impl Jim Fehlig
                   ` (8 preceding siblings ...)
  2015-01-10  5:03 ` [PATCH 09/12] Revert "src/xenconfig: Xen-xl parser" Jim Fehlig
@ 2015-01-10  5:03 ` Jim Fehlig
  2015-01-12 15:27   ` [libvirt] " John Ferlan
  2015-01-12 17:49   ` Eric Blake
  2015-01-10  5:03 ` [PATCH 11/12] tests: Tests for the xen-xl parser Jim Fehlig
                   ` (3 subsequent siblings)
  13 siblings, 2 replies; 25+ messages in thread
From: Jim Fehlig @ 2015-01-10  5:03 UTC (permalink / raw)
  To: libvir-list; +Cc: Kiarie Kahurani, Jim Fehlig, xen-devel

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 <davidkiarie4@gmail.com>
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 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(-)

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
+ *
+ * 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
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Author: Kiarie Kahurani <davidkiarie4@gmail.com>
+ */
+
+#include <config.h>
+
+#include <libxl.h>
+
+#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 <libxlutil.h>
+#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 <diskspec> 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 = <devtype>
+ *    backendtype = <backend-type>
+ * parameters not taken care of
+ *    backend = <domain-name>
+ *    script = <script>
+ *    direct-io-safe
+ *
+ * ================================================================
+ * The parser does not take any deprecated parameters
+ *
+ * For more information refer to /xen/docs/misc/xl-disk-configuration.txt
+ */
+static int
+xenParseXLDisk(virConfPtr conf, virDomainDefPtr def)
+{
+    virConfValuePtr list = virConfGetValue(conf, "disk");
+    XLU_Config *xluconf;
+    virDomainDiskDefPtr disk;
+    libxl_device_disk *libxldisk;
+
+    if (VIR_ALLOC(libxldisk) < 0)
+        return -1;
+
+    if (!(xluconf = xlu_cfg_init(stderr, "command line")))
+        return -1;
+
+    if (list && list->type == VIR_CONF_LIST) {
+        list = list->list;
+        while (list) {
+            const char *disk_spec = list->str;
+
+            if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
+                goto skipdisk;
+
+            libxl_device_disk_init(libxldisk);
+
+            if (xlu_disk_parse(xluconf, 1, &disk_spec, libxldisk))
+                goto skipdisk;
+
+            if (!(disk = virDomainDiskDefNew()))
+                return -1;
+
+            if (VIR_STRDUP(disk->dst, libxldisk->vdev) < 0)
+                goto fail;
+
+            if (virDomainDiskSetSource(disk, libxldisk->pdev_path) < 0)
+                goto fail;
+
+            disk->src->readonly = libxldisk->readwrite ? 0 : 1;
+            disk->removable = libxldisk->removable;
+
+            if (libxldisk->is_cdrom) {
+                if (virDomainDiskSetDriver(disk, "qemu") < 0)
+                    goto fail;
+
+                virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE);
+                disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM;
+                if (!disk->src->path || STREQ(disk->src->path, ""))
+                    disk->src->format = VIR_STORAGE_FILE_NONE;
+                else
+                    disk->src->format = VIR_STORAGE_FILE_RAW;
+            } else {
+                switch (libxldisk->format) {
+                case LIBXL_DISK_FORMAT_QCOW:
+                    disk->src->format = VIR_STORAGE_FILE_QCOW;
+                    break;
+
+                case LIBXL_DISK_FORMAT_QCOW2:
+                    disk->src->format = VIR_STORAGE_FILE_QCOW2;
+                    break;
+
+                case LIBXL_DISK_FORMAT_VHD:
+                    disk->src->format = VIR_STORAGE_FILE_VHD;
+                    break;
+
+                case LIBXL_DISK_FORMAT_RAW:
+                case LIBXL_DISK_FORMAT_UNKNOWN:
+                    disk->src->format = VIR_STORAGE_FILE_RAW;
+                    break;
+
+                case LIBXL_DISK_FORMAT_EMPTY:
+                    break;
+                }
+
+                switch (libxldisk->backend) {
+                case LIBXL_DISK_BACKEND_QDISK:
+                case LIBXL_DISK_BACKEND_UNKNOWN:
+                    if (virDomainDiskSetDriver(disk, "qemu") < 0)
+                        goto fail;
+                    virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE);
+                    break;
+
+                case LIBXL_DISK_BACKEND_TAP:
+                    if (virDomainDiskSetDriver(disk, "tap") < 0)
+                        goto fail;
+                    virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE);
+                    break;
+
+                case LIBXL_DISK_BACKEND_PHY:
+                    if (virDomainDiskSetDriver(disk, "phy") < 0)
+                        goto fail;
+                    virDomainDiskSetType(disk, VIR_STORAGE_TYPE_BLOCK);
+                    break;
+                }
+            }
+
+            if (STRPREFIX(libxldisk->vdev, "xvd") || !STREQ(def->os.type, "hvm"))
+                disk->bus = VIR_DOMAIN_DISK_BUS_XEN;
+            else if (STRPREFIX(libxldisk->vdev, "sd"))
+                disk->bus = VIR_DOMAIN_DISK_BUS_SCSI;
+            else
+                disk->bus = VIR_DOMAIN_DISK_BUS_IDE;
+
+            if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0)
+                goto fail;
+
+        skipdisk:
+            list = list->next;
+        }
+    }
+    return 0;
+
+ fail:
+    virDomainDiskDefFree(disk);
+    return -1;
+}
+
+
+virDomainDefPtr
+xenParseXL(virConfPtr conf, virCapsPtr caps, int xendConfigVersion)
+{
+    virDomainDefPtr def = NULL;
+
+    if (VIR_ALLOC(def) < 0)
+        return NULL;
+
+    def->virtType = VIR_DOMAIN_VIRT_XEN;
+    def->id = -1;
+
+    if (xenParseConfigCommon(conf, def, caps, xendConfigVersion) < 0)
+        goto cleanup;
+
+    if (xenParseXLDisk(conf, def) < 0)
+        goto cleanup;
+
+    if (xenParseXLSpice(conf, def) < 0)
+        goto cleanup;
+
+    return def;
+
+ cleanup:
+    virDomainDefFree(def);
+    return NULL;
+}
+
+
+static int
+xenFormatXLDisk(virConfValuePtr list, virDomainDiskDefPtr disk)
+{
+    virBuffer buf = VIR_BUFFER_INITIALIZER;
+    virConfValuePtr val, tmp;
+    const char *src = virDomainDiskGetSource(disk);
+    int format = virDomainDiskGetFormat(disk);
+    const char *driver = virDomainDiskGetDriver(disk);
+
+    /* target */
+    virBufferAsprintf(&buf, "%s,", src);
+    /* format */
+    switch (format) {
+        case VIR_STORAGE_FILE_RAW:
+            virBufferAddLit(&buf, "raw,");
+            break;
+        case VIR_STORAGE_FILE_VHD:
+            virBufferAddLit(&buf, "xvhd,");
+            break;
+        case VIR_STORAGE_FILE_QCOW:
+            virBufferAddLit(&buf, "qcow,");
+            break;
+        case VIR_STORAGE_FILE_QCOW2:
+            virBufferAddLit(&buf, "qcow2,");
+            break;
+      /* set default */
+        default:
+            virBufferAddLit(&buf, "raw,");
+    }
+
+    /* device */
+    virBufferAdd(&buf, disk->dst, -1);
+
+    virBufferAddLit(&buf, ",");
+
+    if (disk->src->readonly)
+        virBufferAddLit(&buf, "r,");
+    else if (disk->src->shared)
+        virBufferAddLit(&buf, "!,");
+    else
+        virBufferAddLit(&buf, "w,");
+    if (disk->transient) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("transient disks not supported yet"));
+        goto cleanup;
+    }
+
+    if (STREQ_NULLABLE(driver, "qemu"))
+        virBufferAddLit(&buf, "backendtype=qdisk");
+    else if (STREQ_NULLABLE(driver, "tap"))
+        virBufferAddLit(&buf, "backendtype=tap");
+    else if (STREQ_NULLABLE(driver, "phy"))
+        virBufferAddLit(&buf, "backendtype=phy");
+
+    if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM)
+        virBufferAddLit(&buf, ",devtype=cdrom");
+
+    if (virBufferCheckError(&buf) < 0)
+        goto cleanup;
+
+    if (VIR_ALLOC(val) < 0)
+        goto cleanup;
+
+    val->type = VIR_CONF_STRING;
+    val->str = virBufferContentAndReset(&buf);
+    tmp = list->list;
+    while (tmp && tmp->next)
+        tmp = tmp->next;
+    if (tmp)
+        tmp->next = val;
+    else
+        list->list = val;
+    return 0;
+
+ cleanup:
+    virBufferFreeAndReset(&buf);
+    return -1;
+}
+
+
+static int
+xenFormatXLDomainDisks(virConfPtr conf, virDomainDefPtr def)
+{
+    virConfValuePtr diskVal = NULL;
+    size_t i = 0;
+
+    if (VIR_ALLOC(diskVal) < 0)
+        return -1;
+
+    diskVal->type = VIR_CONF_LIST;
+    diskVal->list = NULL;
+
+    for (i = 0; i < def->ndisks; i++) {
+        if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
+            continue;
+        if (xenFormatXLDisk(diskVal, def->disks[i]) < 0)
+
+                goto cleanup;
+    }
+
+    if (diskVal->list != NULL) {
+        int ret = virConfSetValue(conf, "disk", diskVal);
+        diskVal = NULL;
+        if (ret < 0)
+            goto cleanup;
+    }
+
+    return 0;
+
+ cleanup:
+    virConfFreeValue(diskVal);
+    return 0;
+}
+
+
+static int
+xenFormatXLSpice(virConfPtr conf, virDomainDefPtr def)
+{
+    const char *listenAddr = NULL;
+
+    if (STREQ(def->os.type, "hvm")) {
+        if (def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
+            /* set others to false but may not be necessary */
+            if (xenConfigSetInt(conf, "sdl", 0) < 0)
+                return -1;
+
+            if (xenConfigSetInt(conf, "vnc", 0) < 0)
+                return -1;
+
+            if (xenConfigSetInt(conf, "spice", 1) < 0)
+                return -1;
+
+            if (xenConfigSetInt(conf, "spiceport",
+                                def->graphics[0]->data.spice.port) < 0)
+                return -1;
+
+            if (xenConfigSetInt(conf, "spicetls_port",
+                                def->graphics[0]->data.spice.tlsPort) < 0)
+                return -1;
+
+            if (def->graphics[0]->data.spice.auth.passwd) {
+                if (xenConfigSetInt(conf, "spicedisable_ticketing", 1) < 0)
+                    return -1;
+
+                if (def->graphics[0]->data.spice.auth.passwd &&
+                    xenConfigSetString(conf, "spicepasswd",
+                                def->graphics[0]->data.spice.auth.passwd) < 0)
+                    return -1;
+            }
+
+            listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0);
+            if (listenAddr &&
+                xenConfigSetString(conf, "spicehost", listenAddr) < 0)
+                return -1;
+
+            if (xenConfigSetInt(conf, "spicemouse_mouse",
+                                def->graphics[0]->data.spice.mousemode) < 0)
+                return -1;
+
+            if (def->graphics[0]->data.spice.copypaste) {
+                if (xenConfigSetInt(conf, "spicedvagent", 1) < 0)
+                    return -1;
+                if (xenConfigSetInt(conf, "spice_clipboard_sharing",
+                                def->graphics[0]->data.spice.copypaste) < 0)
+                return -1;
+            }
+        }
+    }
+
+    return 0;
+}
+
+
+virConfPtr
+xenFormatXL(virDomainDefPtr def, virConnectPtr conn, int xendConfigVersion)
+{
+    virConfPtr conf = NULL;
+
+    if (!(conf = virConfNew()))
+        goto cleanup;
+
+    if (xenFormatConfigCommon(conf, def, conn, xendConfigVersion) < 0)
+        goto cleanup;
+
+    if (xenFormatXLDomainDisks(conf, def) < 0)
+        goto cleanup;
+
+    if (xenFormatXLSpice(conf, def) < 0)
+        goto cleanup;
+
+    return conf;
+
+ cleanup:
+    if (conf)
+        virConfFree(conf);
+    return NULL;
+}
diff --git a/src/xenconfig/xen_xl.h b/src/xenconfig/xen_xl.h
new file mode 100644
index 0000000..536e9b7
--- /dev/null
+++ b/src/xenconfig/xen_xl.h
@@ -0,0 +1,33 @@
+/*
+ * xen_xl.h: Xen XL parsing functions
+ *
+ * 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
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Author: Kiarie Kahurani<davidkiarie4@gmail.com>
+ */
+
+#ifndef __VIR_XEN_XL_H__
+# define __VIR_XEN_XL_H__
+
+# include "virconf.h"
+# include "domain_conf.h"
+# include "xen_common.h"
+
+virDomainDefPtr xenParseXL(virConfPtr conn, virCapsPtr caps,
+                           int xendConfigVersion);
+virConfPtr xenFormatXL(virDomainDefPtr def,
+                       virConnectPtr, int xendConfigVersion);
+
+#endif /* __VIR_XEN_XL_H__ */
-- 
1.8.4.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 11/12] tests: Tests for the xen-xl parser
  2015-01-10  5:03 [PATCH 00/12] Replace Xen xl parsing/formatting impl Jim Fehlig
                   ` (9 preceding siblings ...)
  2015-01-10  5:03 ` [PATCH 10/12] Introduce support for parsing/formatting Xen xl config format Jim Fehlig
@ 2015-01-10  5:03 ` Jim Fehlig
  2015-01-10  5:03 ` [PATCH 12/12] libxl: Add support for parsing/formating Xen XL config Jim Fehlig
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 25+ messages in thread
From: Jim Fehlig @ 2015-01-10  5:03 UTC (permalink / raw)
  To: libvir-list; +Cc: Kiarie Kahurani, Jim Fehlig, xen-devel

From: Kiarie Kahurani <davidkiarie4@gmail.com>

add tests for the xen_xl config parser

Signed-off-by: Kiarie Kahurani <davidkiarie4@gmail.com>
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 tests/Makefile.am                    |   9 +-
 tests/testutilsxen.c                 |  50 ++++++++
 tests/testutilsxen.h                 |   9 +-
 tests/xlconfigdata/test-new-disk.cfg |  26 ++++
 tests/xlconfigdata/test-new-disk.xml |  51 ++++++++
 tests/xlconfigdata/test-spice.cfg    |  32 +++++
 tests/xlconfigdata/test-spice.xml    |  45 +++++++
 tests/xlconfigtest.c                 | 224 +++++++++++++++++++++++++++++++++++
 8 files changed, 444 insertions(+), 2 deletions(-)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index e9418ea..af85335 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -138,6 +138,7 @@ EXTRA_DIST =		\
 	vmx2xmldata \
 	xencapsdata \
 	xmconfigdata \
+	xlconfigdata \
 	xml2sexprdata \
 	xml2vmxdata \
 	vmwareverdata \
@@ -225,7 +226,8 @@ ssh_LDADD = $(COVERAGE_LDFLAGS)
 
 if WITH_XEN
 test_programs += xml2sexprtest sexpr2xmltest \
-	xmconfigtest xencapstest statstest reconnect
+	xmconfigtest xencapstest statstest reconnect \
+	xlconfigtest
 endif WITH_XEN
 if WITH_QEMU
 test_programs += qemuxml2argvtest qemuxml2xmltest qemuxmlnstest \
@@ -477,6 +479,11 @@ sexpr2xmltest_SOURCES = \
 	testutils.c testutils.h
 sexpr2xmltest_LDADD = $(xen_LDADDS)
 
+xlconfigtest_SOURCES = \
+	xlconfigtest.c testutilsxen.c testutilsxen.h \
+	testutils.c testutils.h
+xlconfigtest_LDADD =$(xen_LDADDS)
+
 xmconfigtest_SOURCES = \
 	xmconfigtest.c testutilsxen.c testutilsxen.h \
 	testutils.c testutils.h
diff --git a/tests/testutilsxen.c b/tests/testutilsxen.c
index a50a8a2..df1d124 100644
--- a/tests/testutilsxen.c
+++ b/tests/testutilsxen.c
@@ -69,3 +69,53 @@ virCapsPtr testXenCapsInit(void)
     virObjectUnref(caps);
     return NULL;
 }
+
+
+virCapsPtr
+testXLInitCaps(void)
+{
+    virCapsPtr caps;
+    virCapsGuestPtr guest;
+    virCapsGuestMachinePtr *machines;
+    int nmachines;
+    static const char *const x86_machines[] = {
+        "xenfv"
+    };
+    static const char *const xen_machines[] = {
+        "xenpv"
+    };
+
+    if ((caps = virCapabilitiesNew(virArchFromHost(),
+                                   false, false)) == NULL)
+        return NULL;
+    nmachines = ARRAY_CARDINALITY(x86_machines);
+    if ((machines = virCapabilitiesAllocMachines(x86_machines, nmachines)) == NULL)
+        goto cleanup;
+    if ((guest = virCapabilitiesAddGuest(caps, "hvm", VIR_ARCH_X86_64,
+                                         "/usr/lib/xen/bin/qemu-dm", NULL,
+                                         nmachines, machines)) == NULL)
+        goto cleanup;
+    machines = NULL;
+    if (virCapabilitiesAddGuestDomain(guest, "xen", NULL,
+                                      NULL, 0, NULL) == NULL)
+        goto cleanup;
+    nmachines = ARRAY_CARDINALITY(xen_machines);
+    if ((machines = virCapabilitiesAllocMachines(xen_machines, nmachines)) == NULL)
+        goto cleanup;
+
+    if ((guest = virCapabilitiesAddGuest(caps, "xen", VIR_ARCH_X86_64,
+                                        "/usr/lib/xen/bin/qemu-dm", NULL,
+                                        nmachines, machines)) == NULL)
+        goto cleanup;
+    machines = NULL;
+
+    if (virCapabilitiesAddGuestDomain(guest, "xen", NULL,
+                                      NULL, 0, NULL) == NULL)
+        goto cleanup;
+    return caps;
+
+ cleanup:
+    virCapabilitiesFreeMachines(machines, nmachines);
+    virObjectUnref(caps);
+    return NULL;
+}
diff --git a/tests/testutilsxen.h b/tests/testutilsxen.h
index 54155e5..c78350d 100644
--- a/tests/testutilsxen.h
+++ b/tests/testutilsxen.h
@@ -1,3 +1,10 @@
-#include "capabilities.h"
+#ifndef _TESTUTILSXEN_H_
+# define _TESTUTILSXEN_H_
+
+# include "capabilities.h"
 
 virCapsPtr testXenCapsInit(void);
+
+virCapsPtr testXLInitCaps(void);
+
+#endif /* _TESTUTILSXEN_H_ */
diff --git a/tests/xlconfigdata/test-new-disk.cfg b/tests/xlconfigdata/test-new-disk.cfg
new file mode 100644
index 0000000..b672b4a
--- /dev/null
+++ b/tests/xlconfigdata/test-new-disk.cfg
@@ -0,0 +1,26 @@
+name = "XenGuest2"
+uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809"
+maxmem = 579
+memory = 394
+vcpus = 1
+builder = "hvm"
+kernel = "/usr/lib/xen/boot/hvmloader"
+boot = "d"
+pae = 1
+acpi = 1
+apic = 1
+hap = 0
+viridian = 0
+localtime = 0
+on_poweroff = "destroy"
+on_reboot = "restart"
+on_crash = "restart"
+device_model = "/usr/lib/xen/bin/qemu-dm"
+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" ]
+parallel = "none"
+serial = "none"
+disk = [ "/dev/HostVG/XenGuest2,raw,hda,w,backendtype=phy", "/var/lib/libvirt/images/XenGuest2-home,qcow2,hdb,w,backendtype=qdisk", "/root/boot.iso,raw,hdc,r,backendtype=qdisk,devtype=cdrom" ]
diff --git a/tests/xlconfigdata/test-new-disk.xml b/tests/xlconfigdata/test-new-disk.xml
new file mode 100644
index 0000000..1c96a62
--- /dev/null
+++ b/tests/xlconfigdata/test-new-disk.xml
@@ -0,0 +1,51 @@
+<domain type='xen'>
+  <name>XenGuest2</name>
+  <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>592896</memory>
+  <currentMemory unit='KiB'>403456</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='x86_64' machine='xenfv'>hvm</type>
+    <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader>
+    <boot dev='cdrom'/>
+  </os>
+  <features>
+    <acpi/>
+    <apic/>
+    <pae/>
+  </features>
+  <clock offset='utc' adjustment='reset'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>restart</on_crash>
+  <devices>
+    <emulator>/usr/lib/xen/bin/qemu-dm</emulator>
+    <disk type='block' device='disk'>
+      <driver name='phy' type='raw'/>
+      <source dev='/dev/HostVG/XenGuest2'/>
+      <target dev='hda' bus='ide'/>
+    </disk>
+    <disk type='file' device='disk'>
+      <driver name='qemu' type='qcow2'/>
+      <source file='/var/lib/libvirt/images/XenGuest2-home'/>
+      <target dev='hdb' bus='ide'/>
+    </disk>
+    <disk type='file' device='cdrom'>
+      <driver name='qemu' type='raw'/>
+      <source file='/root/boot.iso'/>
+      <target dev='hdc' bus='ide'/>
+      <readonly/>
+    </disk>
+    <interface type='bridge'>
+      <mac address='00:16:3e:66:92:9c'/>
+      <source bridge='xenbr1'/>
+      <script path='vif-bridge'/>
+      <model type='e1000'/>
+    </interface>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1'>
+      <listen type='address' address='127.0.0.1'/>
+    </graphics>
+  </devices>
+</domain>
diff --git a/tests/xlconfigdata/test-spice.cfg b/tests/xlconfigdata/test-spice.cfg
new file mode 100644
index 0000000..f7aa55c
--- /dev/null
+++ b/tests/xlconfigdata/test-spice.cfg
@@ -0,0 +1,32 @@
+name = "XenGuest2"
+uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809"
+maxmem = 579
+memory = 394
+vcpus = 1
+builder = "hvm"
+kernel = "/usr/lib/xen/boot/hvmloader"
+boot = "d"
+pae = 1
+acpi = 1
+apic = 1
+hap = 0
+viridian = 0
+rtc_timeoffset = 0
+localtime = 1
+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" ]
+parallel = "none"
+serial = "none"
+disk = [ "/dev/HostVG/XenGuest2,raw,hda,w,", "/root/boot.iso,raw,hdc,w," ]
+sdl = 0
+vnc = 0
+spice = 1
+spicehost = "127.0.0.1"
+spiceport = 590
+spicetls_port = 500
+spicedisable_ticketing = 1
+spicepasswd = "thebeast"
+spiceagent_mouse = 0
diff --git a/tests/xlconfigdata/test-spice.xml b/tests/xlconfigdata/test-spice.xml
new file mode 100644
index 0000000..1e3f78d
--- /dev/null
+++ b/tests/xlconfigdata/test-spice.xml
@@ -0,0 +1,45 @@
+<domain type='xen'>
+  <name>XenGuest2</name>
+  <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>592896</memory>
+  <currentMemory unit='KiB'>403456</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='x86_64' machine='xenfv'>hvm</type>
+    <loader>/usr/lib/xen/boot/hvmloader</loader>
+    <boot dev='cdrom'/>
+  </os>
+  <features>
+    <acpi/>
+    <apic/>
+    <pae/>
+  </features>
+  <clock offset='variable' adjustment='0' basis='localtime'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>restart</on_crash>
+  <devices>
+    <emulator>/usr/lib/xen/bin/qemu-dm</emulator>
+    <disk type='block' device='disk'>
+      <driver name='phy' type='raw'/>
+      <source dev='/dev/HostVG/XenGuest2'/>
+      <target dev='hda' bus='ide'/>
+    </disk>
+    <disk type='block' device='disk'>
+      <driver name='phy' type='raw'/>
+      <source dev='/root/boot.iso'/>
+      <target dev='hdc' bus='ide'/>
+    </disk>
+    <interface type='bridge'>
+      <mac address='00:16:3e:66:92:9c'/>
+      <source bridge='xenbr1'/>
+      <script path='vif-bridge'/>
+      <model type='e1000'/>
+    </interface>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <graphics type='spice' port='590' tlsPort='500' autoport='no' listen='127.0.0.1' passwd='thebeast'>
+      <listen type='address' address='127.0.0.1'/>
+    </graphics>
+  </devices>
+</domain>
diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c
new file mode 100644
index 0000000..b966c15
--- /dev/null
+++ b/tests/xlconfigtest.c
@@ -0,0 +1,224 @@
+/*
+ * xlconfigtest.c: Test backend for xl_internal config file handling
+ *
+ * Copyright (C) 2007, 2010-2011, 2014 Red Hat, Inc.
+ *
+ * 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
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Author: Daniel P. Berrange <berrange@redhat.com>
+ * Author: Kiarie Kahurani <davidkiarie4@gmail.com>
+ *
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+
+#include "internal.h"
+#include "datatypes.h"
+#include "xenconfig/xen_xl.h"
+#include "viralloc.h"
+#include "virstring.h"
+#include "testutils.h"
+#include "testutilsxen.h"
+#include "xen/xen_driver.h"
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+static virCapsPtr caps;
+static virDomainXMLOptionPtr xmlopt;
+/*
+ * parses the xml, creates a domain def and compare with equivalent xm config
+ */
+static int
+testCompareParseXML(const char *xmcfg, const char *xml, int xendConfigVersion)
+{
+    char *xmlData = NULL;
+    char *xmcfgData = NULL;
+    char *gotxmcfgData = NULL;
+    virConfPtr conf = NULL;
+    virConnectPtr conn = NULL;
+    int wrote = 4096;
+    int ret = -1;
+    virDomainDefPtr def = NULL;
+
+    if (VIR_ALLOC_N(gotxmcfgData, wrote) < 0)
+        goto fail;
+
+    conn = virGetConnect();
+    if (!conn) goto fail;
+
+    if (virtTestLoadFile(xml, &xmlData) < 0)
+        goto fail;
+
+    if (virtTestLoadFile(xmcfg, &xmcfgData) < 0)
+        goto fail;
+
+    if (!(def = virDomainDefParseString(xmlData, caps, xmlopt,
+                                        1 << VIR_DOMAIN_VIRT_XEN,
+                                        VIR_DOMAIN_XML_INACTIVE)))
+        goto fail;
+
+    if (!virDomainDefCheckABIStability(def, def)) {
+        fprintf(stderr, "ABI stability check failed on %s", xml);
+        goto fail;
+    }
+
+    if (!(conf = xenFormatXL(def, conn,  xendConfigVersion)))
+        goto fail;
+
+    if (virConfWriteMem(gotxmcfgData, &wrote, conf) < 0)
+        goto fail;
+    gotxmcfgData[wrote] = '\0';
+
+    if (STRNEQ(xmcfgData, gotxmcfgData)) {
+        virtTestDifference(stderr, xmcfgData, gotxmcfgData);
+        goto fail;
+    }
+
+    ret = 0;
+
+ fail:
+    VIR_FREE(xmlData);
+    VIR_FREE(xmcfgData);
+    VIR_FREE(gotxmcfgData);
+    if (conf)
+        virConfFree(conf);
+    virDomainDefFree(def);
+    virObjectUnref(conn);
+
+    return ret;
+}
+/*
+ * parses the xl config, develops domain def and compares with equivalent xm config
+ */
+static int
+testCompareFormatXML(const char *xmcfg, const char *xml, int xendConfigVersion)
+{
+    char *xmlData = NULL;
+    char *xmcfgData = NULL;
+    char *gotxml = NULL;
+    virConfPtr conf = NULL;
+    int ret = -1;
+    virConnectPtr conn;
+    virDomainDefPtr def = NULL;
+
+    conn = virGetConnect();
+    if (!conn) goto fail;
+
+    if (virtTestLoadFile(xml, &xmlData) < 0)
+        goto fail;
+
+    if (virtTestLoadFile(xmcfg, &xmcfgData) < 0)
+        goto fail;
+
+    if (!(conf = virConfReadMem(xmcfgData, strlen(xmcfgData), 0)))
+        goto fail;
+
+    if (!(def = xenParseXL(conf, caps, xendConfigVersion)))
+        goto fail;
+
+    if (!(gotxml = virDomainDefFormat(def, VIR_DOMAIN_XML_INACTIVE |
+                                      VIR_DOMAIN_XML_SECURE)))
+        goto fail;
+
+    if (STRNEQ(xmlData, gotxml)) {
+        virtTestDifference(stderr, xmlData, gotxml);
+        goto fail;
+    }
+
+    ret = 0;
+
+ fail:
+    if (conf)
+        virConfFree(conf);
+    VIR_FREE(xmlData);
+    VIR_FREE(xmcfgData);
+    VIR_FREE(gotxml);
+    virDomainDefFree(def);
+    virObjectUnref(conn);
+
+    return ret;
+}
+
+
+struct testInfo {
+    const char *name;
+    int version;
+    int mode;
+};
+
+static int
+testCompareHelper(const void *data)
+{
+    int result = -1;
+    const struct testInfo *info = data;
+    char *xml = NULL;
+    char *cfg = NULL;
+
+    if (virAsprintf(&xml, "%s/xlconfigdata/test-%s.xml",
+                    abs_srcdir, info->name) < 0 ||
+        virAsprintf(&cfg, "%s/xlconfigdata/test-%s.cfg",
+                    abs_srcdir, info->name) < 0)
+        goto cleanup;
+
+    if (info->mode == 0)
+        result = testCompareParseXML(cfg, xml, info->version);
+    else
+        result = testCompareFormatXML(cfg, xml, info->version);
+
+ cleanup:
+    VIR_FREE(xml);
+    VIR_FREE(cfg);
+
+    return result;
+}
+
+
+static int
+mymain(void)
+{
+    int ret = 0;
+
+    if (!(caps = testXLInitCaps()))
+        return EXIT_FAILURE;
+
+    if (!(xmlopt = xenDomainXMLConfInit()))
+        return EXIT_FAILURE;
+
+#define DO_TEST(name, version)                                          \
+    do {                                                                \
+        struct testInfo info0 = { name, version, 0 };                   \
+        struct testInfo info1 = { name, version, 1 };                   \
+        if (virtTestRun("Xen XM-2-XML Parse  " name,                    \
+                        testCompareHelper, &info0) < 0)                 \
+            ret = -1;                                                   \
+        if (virtTestRun("Xen XM-2-XML Format " name,                    \
+                        testCompareHelper, &info1) < 0)                 \
+            ret = -1;                                                   \
+    } while (0)
+
+    DO_TEST("new-disk", 3);
+//    DO_TEST("spice", 3);
+
+    virObjectUnref(caps);
+    virObjectUnref(xmlopt);
+
+    return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
+}
+
+VIRT_TEST_MAIN(mymain)
-- 
1.8.4.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 12/12] libxl: Add support for parsing/formating Xen XL config
  2015-01-10  5:03 [PATCH 00/12] Replace Xen xl parsing/formatting impl Jim Fehlig
                   ` (10 preceding siblings ...)
  2015-01-10  5:03 ` [PATCH 11/12] tests: Tests for the xen-xl parser Jim Fehlig
@ 2015-01-10  5:03 ` Jim Fehlig
  2015-01-12 15:06 ` [PATCH 00/12] Replace Xen xl parsing/formatting impl Ian Campbell
       [not found] ` <1421075193.26317.95.camel@citrix.com>
  13 siblings, 0 replies; 25+ messages in thread
From: Jim Fehlig @ 2015-01-10  5:03 UTC (permalink / raw)
  To: libvir-list; +Cc: Kiarie Kahurani, Jim Fehlig, xen-devel

From: Kiarie Kahurani <davidkiarie4@gmail.com>

Now that xenconfig supports parsing and formatting Xen's
XL config format, integrate it into the libxl driver's
connectDomainXML{From,To}Native functions.

Signed-off-by: Kiarie Kahurani <davidkiarie4@gmail.com>
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 src/libxl/libxl_driver.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 53c87ce..4135670 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -48,6 +48,7 @@
 #include "libxl_migration.h"
 #include "xen_xm.h"
 #include "xen_sxpr.h"
+#include "xen_xl.h"
 #include "virtypedparam.h"
 #include "viruri.h"
 #include "virstring.h"
@@ -67,6 +68,7 @@ VIR_LOG_INIT("libxl.libxl_driver");
 #define LIBXL_DOM_REQ_CRASH    3
 #define LIBXL_DOM_REQ_HALT     4
 
+#define LIBXL_CONFIG_FORMAT_XL "xen-xl"
 #define LIBXL_CONFIG_FORMAT_XM "xen-xm"
 #define LIBXL_CONFIG_FORMAT_SEXPR "xen-sxpr"
 
@@ -2214,7 +2216,17 @@ libxlConnectDomainXMLFromNative(virConnectPtr conn,
     if (virConnectDomainXMLFromNativeEnsureACL(conn) < 0)
         goto cleanup;
 
-    if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) {
+    if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XL)) {
+        if (!(conf = virConfReadMem(nativeConfig, strlen(nativeConfig), 0)))
+            goto cleanup;
+        if (!(def = xenParseXL(conf,
+                               cfg->caps,
+                               cfg->verInfo->xen_version_major))) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("parsing xl config failed"));
+            goto cleanup;
+        }
+    } else if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) {
         if (!(conf = virConfReadMem(nativeConfig, strlen(nativeConfig), 0)))
             goto cleanup;
 
@@ -2269,20 +2281,24 @@ libxlConnectDomainXMLToNative(virConnectPtr conn, const char * nativeFormat,
     if (virConnectDomainXMLToNativeEnsureACL(conn) < 0)
         goto cleanup;
 
-    if (STRNEQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) {
-        virReportError(VIR_ERR_INVALID_ARG,
-                       _("unsupported config type %s"), nativeFormat);
-        goto cleanup;
-    }
-
     if (!(def = virDomainDefParseString(domainXml,
                                         cfg->caps, driver->xmlopt,
                                         1 << VIR_DOMAIN_VIRT_XEN,
                                         VIR_DOMAIN_XML_INACTIVE)))
         goto cleanup;
 
-    if (!(conf = xenFormatXM(conn, def, cfg->verInfo->xen_version_major)))
+    if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XL)) {
+        if (!(conf = xenFormatXL(def, conn, cfg->verInfo->xen_version_major)))
+            goto cleanup;
+    } else if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) {
+        if (!(conf = xenFormatXM(conn, def, cfg->verInfo->xen_version_major)))
+            goto cleanup;
+    } else {
+
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("unsupported config type %s"), nativeFormat);
         goto cleanup;
+    }
 
     if (VIR_ALLOC_N(ret, len) < 0)
         goto cleanup;
-- 
1.8.4.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [libvirt] [PATCH 09/12] Revert "src/xenconfig: Xen-xl parser"
  2015-01-10  5:03 ` [PATCH 09/12] Revert "src/xenconfig: Xen-xl parser" Jim Fehlig
@ 2015-01-12 14:51   ` John Ferlan
       [not found]   ` <54B3DF75.2000406@redhat.com>
  1 sibling, 0 replies; 25+ messages in thread
From: John Ferlan @ 2015-01-12 14:51 UTC (permalink / raw)
  To: Jim Fehlig, libvir-list; +Cc: xen-devel



On 01/10/2015 12:03 AM, Jim Fehlig wrote:
> This reverts commit 2c78051a14acfb7aba078d569b1632dfe0ca0853.
> 
> Conflicts:
> 	src/Makefile.am
> 
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> ---
>  .gitignore                    |   1 -
>  cfg.mk                        |   3 +-
>  configure.ac                  |   1 -
>  po/POTFILES.in                |   1 -
>  src/Makefile.am               |  25 +--
>  src/libvirt_xenconfig.syms    |   4 -
>  src/xenconfig/xen_common.c    |   3 +-
>  src/xenconfig/xen_xl.c        | 499 ------------------------------------------
>  src/xenconfig/xen_xl.h        |  33 ---
>  src/xenconfig/xen_xl_disk.l   | 256 ----------------------
>  src/xenconfig/xen_xl_disk_i.h |  39 ----
>  11 files changed, 4 insertions(+), 861 deletions(-)
> 

OK - so reverting is fine; however, xen_xl_disk.{c,h} still exist...
Simple enough solution, but they will show up in someone's git status
output since they are also removed from .gitignore.

There are a couple Coverity issues from the next 3 patches - I'll note
them for those directly.

ACK

John


> diff --git a/.gitignore b/.gitignore
> index eac2203..9d09709 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -140,7 +140,6 @@
>  /src/remote/*_protocol.[ch]
>  /src/rpc/virkeepaliveprotocol.[ch]
>  /src/rpc/virnetprotocol.[ch]
> -/src/xenconfig/xen_xl_disk.[ch]
>  /src/test_libvirt*.aug
>  /src/test_virtlockd.aug
>  /src/util/virkeymaps.h
> diff --git a/cfg.mk b/cfg.mk
> index 3df3dcb..21f83c3 100644
> --- a/cfg.mk
> +++ b/cfg.mk
> @@ -89,9 +89,8 @@ distdir: sc_vulnerable_makefile_CVE-2012-3386.z
>  endif
>  
>  # Files that should never cause syntax check failures.
> -#  (^(HACKING|docs/(news\.html\.in|.*\.patch))|\.(po|fig|gif|ico|png))$$
>  VC_LIST_ALWAYS_EXCLUDE_REGEX = \
> -  (^(HACKING|docs/(news\.html\.in|.*\.patch)|src/xenconfig/xen_xl_disk.[chl])|\.(po|fig|gif|ico|png))$$
> +  (^(HACKING|docs/(news\.html\.in|.*\.patch))|\.(po|fig|gif|ico|png))$$
>  
>  # Functions like free() that are no-ops on NULL arguments.
>  useless_free_options =				\
> diff --git a/configure.ac b/configure.ac
> index 167b875..9d12079 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -146,7 +146,6 @@ m4_ifndef([LT_INIT], [
>  ])
>  AM_PROG_CC_C_O
>  AM_PROG_LD
> -AM_PROG_LEX
>  
>  AC_MSG_CHECKING([for how to mark DSO non-deletable at runtime])
>  LIBVIRT_NODELETE=
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index 094c8e3..e7cb2cc 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -247,7 +247,6 @@ 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 c7975e5..e0e47d0 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -1000,22 +1000,11 @@ CPU_SOURCES =							\
>  VMX_SOURCES =							\
>  		vmx/vmx.c vmx/vmx.h
>  
> -AM_LFLAGS = -Pxl_disk_ --header-file=../$*.h
> -LEX_OUTPUT_ROOT = lex.xl_disk_
> -BUILT_SOURCES += xenconfig/xen_xl_disk.c xenconfig/xen_xl_disk.h
> -# Generated header file is not implicitly added to dist
> -EXTRA_DIST += xenconfig/xen_xl_disk.h
> -CLEANFILES += xenconfig/xen_xl_disk.h xenconfig/xen_xl_disk.c
> -
> -XENXLDISKPARSER_SOURCES = xenconfig/xen_xl_disk.l
> -
>  XENCONFIG_SOURCES =						\
>  		xenconfig/xenxs_private.h			\
> -		xenconfig/xen_common.c xenconfig/xen_common.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_xl.c xenconfig/xen_xl.h		\
> -		xenconfig/xen_xl_disk_i.h
> +		xenconfig/xen_xm.c xenconfig/xen_xm.h
>  
>  pkgdata_DATA =	cpu/cpu_map.xml
>  
> @@ -1070,19 +1059,10 @@ libvirt_vmx_la_SOURCES = $(VMX_SOURCES)
>  endif WITH_VMX
>  
>  if WITH_XENCONFIG
> -# Flex generated XL disk parser needs to be compiled without WARN_FLAGS
> -# Add the generated object to its own library to control CFLAGS
> -noinst_LTLIBRARIES += libvirt_xenxldiskparser.la
> -libvirt_xenxldiskparser_la_CFLAGS = \
> -		-I$(srcdir)/conf $(AM_CFLAGS) -Wno-unused-parameter
> -libvirt_xenxldiskparser_la_SOURCES = \
> -	$(XENXLDISKPARSER_SOURCES)
> -
>  noinst_LTLIBRARIES += libvirt_xenconfig.la
>  libvirt_la_BUILT_LIBADD += libvirt_xenconfig.la
>  libvirt_xenconfig_la_CFLAGS = \
>  		-I$(srcdir)/conf $(AM_CFLAGS)
> -libvirt_xenconfig_la_LIBADD = libvirt_xenxldiskparser.la
>  libvirt_xenconfig_la_SOURCES = $(XENCONFIG_SOURCES)
>  endif WITH_XENCONFIG
>  
> @@ -1844,7 +1824,6 @@ EXTRA_DIST +=							\
>  		$(VBOX_DRIVER_EXTRA_DIST)			\
>  		$(VMWARE_DRIVER_SOURCES)			\
>  		$(XENCONFIG_SOURCES)				\
> -		$(XENXLDISKPARSER_SOURCES)			\
>  		$(ACCESS_DRIVER_POLKIT_POLICY)
>  
>  check-local: check-augeas
> diff --git a/src/libvirt_xenconfig.syms b/src/libvirt_xenconfig.syms
> index 3e2e5d6..6541685 100644
> --- a/src/libvirt_xenconfig.syms
> +++ b/src/libvirt_xenconfig.syms
> @@ -16,10 +16,6 @@ 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 a2a1474..b40a722 100644
> --- a/src/xenconfig/xen_common.c
> +++ b/src/xenconfig/xen_common.c
> @@ -1812,8 +1812,7 @@ xenFormatVfb(virConfPtr conf, virDomainDefPtr def, int xendConfigVersion)
>  {
>      int hvm = STREQ(def->os.type, "hvm") ? 1 : 0;
>  
> -    if (def->ngraphics == 1 &&
> -        def->graphics[0]->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
> +    if (def->ngraphics == 1) {
>          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
> deleted file mode 100644
> index 8d1d2a7..0000000
> --- a/src/xenconfig/xen_xl.c
> +++ /dev/null
> @@ -1,499 +0,0 @@
> -/*
> - * xen_xl.c: Xen XL parsing functions
> - *
> - * 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
> - * <http://www.gnu.org/licenses/>.
> - *
> - * Author: Kiarie Kahurani <davidkiarie4@gmail.com>
> - */
> -
> -#include <config.h>
> -
> -#include "virconf.h"
> -#include "virerror.h"
> -#include "domain_conf.h"
> -#include "viralloc.h"
> -#include "virstring.h"
> -#include "xen_xl.h"
> -#include "xen_xl_disk.h"
> -#include "xen_xl_disk_i.h"
> -
> -#define VIR_FROM_THIS VIR_FROM_NONE
> -
> -
> -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;
> -}
> -
> -
> -void
> -xenXLDiskParserError(xenXLDiskParserContext *dpc,
> -                     const char *erroneous,
> -                     const char *message)
> -{
> -    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -                   _("disk config %s not supported: %s"),
> -                   erroneous, message);
> -
> -    if (!dpc->err)
> -        dpc->err = EINVAL;
> -}
> -
> -
> -static int
> -xenXLDiskParserPrep(xenXLDiskParserContext *dpc,
> -                    const char *spec,
> -                    virDomainDiskDefPtr disk)
> -{
> -    int err;
> -
> -    dpc->spec = spec;
> -    dpc->disk = disk;
> -    dpc->access_set = 0;
> -
> -    err = xl_disk_lex_init_extra(dpc, &dpc->scanner);
> -    if (err)
> -        goto fail;
> -
> -    dpc->buf = xl_disk__scan_bytes(spec, strlen(spec), dpc->scanner);
> -    if (!dpc->buf) {
> -        err = ENOMEM;
> -        goto fail;
> -    }
> -
> -    return 0;
> -
> - fail:
> -    virReportSystemError(errno, "%s",
> -                         _("failed to initialize disk configuration parser"));
> -    return err;
> -}
> -
> -
> -static void
> -xenXLDiskParserCleanup(xenXLDiskParserContext *dpc)
> -{
> -    if (dpc->buf) {
> -        xl_disk__delete_buffer(dpc->buf, dpc->scanner);
> -        dpc->buf = NULL;
> -    }
> -
> -    if (dpc->scanner) {
> -        xl_disk_lex_destroy(dpc->scanner);
> -        dpc->scanner = NULL;
> -    }
> -}
> -
> -
> -/*
> - * positional parameters
> - *     (If the <diskspec> 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 = <devtype>
> - *    backendtype = <backend-type>
> - * parameters not taken care of
> - *    backend = <domain-name>
> - *    script = <script>
> - *    direct-io-safe
> - *
> - * ================================================================
> - * The parser does not take any deprecated parameters
> - *
> - * For more information refer to /xen/docs/misc/xl-disk-configuration.txt
> - */
> -static int
> -xenParseXLDisk(virConfPtr conf, virDomainDefPtr def)
> -{
> -    virConfValuePtr list = virConfGetValue(conf, "disk");
> -    xenXLDiskParserContext dpc;
> -    virDomainDiskDefPtr disk;
> -
> -    memset(&dpc, 0, sizeof(dpc));
> -
> -    if (list && list->type == VIR_CONF_LIST) {
> -        list = list->list;
> -        while (list) {
> -            char *disk_spec = list->str;
> -            const char *driver;
> -
> -            if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
> -                goto skipdisk;
> -
> -            if (!(disk = virDomainDiskDefNew()))
> -                    return -1;
> -
> -            disk->src->readonly = 0;
> -            disk->src->format = VIR_STORAGE_FILE_LAST;
> -
> -            if (xenXLDiskParserPrep(&dpc, disk_spec, disk))
> -                goto fail;
> -
> -            xl_disk_lex(dpc.scanner);
> -
> -            if (dpc.err)
> -                goto fail;
> -
> -            if (disk->src->format == VIR_STORAGE_FILE_LAST)
> -                disk->src->format = VIR_STORAGE_FILE_RAW;
> -
> -            if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
> -                disk->removable = true;
> -                disk->src->readonly = true;
> -                if (virDomainDiskSetDriver(disk, "qemu") < 0)
> -                    goto fail;
> -
> -                virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE);
> -                if (!disk->src->path || STREQ(disk->src->path, ""))
> -                    disk->src->format = VIR_STORAGE_FILE_NONE;
> -            }
> -
> -            if (STRPREFIX(disk->dst, "xvd") || !STREQ(def->os.type, "hvm"))
> -                disk->bus = VIR_DOMAIN_DISK_BUS_XEN;
> -            else if (STRPREFIX(disk->dst, "sd"))
> -                disk->bus = VIR_DOMAIN_DISK_BUS_SCSI;
> -            else
> -                disk->bus = VIR_DOMAIN_DISK_BUS_IDE;
> -
> -            driver = virDomainDiskGetDriver(disk);
> -            if (!driver) {
> -                switch (disk->src->format) {
> -                case VIR_STORAGE_FILE_QCOW:
> -                case VIR_STORAGE_FILE_QCOW2:
> -                case VIR_STORAGE_FILE_VHD:
> -                    driver = "qemu";
> -                    if (virDomainDiskSetDriver(disk, "qemu") < 0)
> -                        goto fail;
> -                    break;
> -                default:
> -                    driver = "phy";
> -                    if (virDomainDiskSetDriver(disk, "phy") < 0)
> -                        goto fail;
> -                }
> -            }
> -
> -            if (STREQ(driver, "phy"))
> -                virDomainDiskSetType(disk, VIR_STORAGE_TYPE_BLOCK);
> -            else
> -                virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE);
> -
> -            if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0)
> -                goto fail;
> -
> -        skipdisk:
> -            list = list->next;
> -            xenXLDiskParserCleanup(&dpc);
> -        }
> -    }
> -    return 0;
> -
> - fail:
> -    xenXLDiskParserCleanup(&dpc);
> -    virDomainDiskDefFree(disk);
> -    return -1;
> -}
> -
> -
> -virDomainDefPtr
> -xenParseXL(virConfPtr conf, virCapsPtr caps, int xendConfigVersion)
> -{
> -    virDomainDefPtr def = NULL;
> -
> -    if (VIR_ALLOC(def) < 0)
> -        return NULL;
> -
> -    def->virtType = VIR_DOMAIN_VIRT_XEN;
> -    def->id = -1;
> -
> -    if (xenParseConfigCommon(conf, def, caps, xendConfigVersion) < 0)
> -        goto cleanup;
> -
> -    if (xenParseXLDisk(conf, def) < 0)
> -        goto cleanup;
> -
> -    if (xenParseXLSpice(conf, def) < 0)
> -        goto cleanup;
> -
> -    return def;
> -
> - cleanup:
> -    virDomainDefFree(def);
> -    return NULL;
> -}
> -
> -
> -static int
> -xenFormatXLDisk(virConfValuePtr list, virDomainDiskDefPtr disk)
> -{
> -    virBuffer buf = VIR_BUFFER_INITIALIZER;
> -    virConfValuePtr val, tmp;
> -    const char *src = virDomainDiskGetSource(disk);
> -    int format = virDomainDiskGetFormat(disk);
> -
> -    /* target */
> -    virBufferAsprintf(&buf, "%s,", src);
> -    /* format */
> -    switch (format) {
> -        case VIR_STORAGE_FILE_RAW:
> -            virBufferAddLit(&buf, "raw,");
> -            break;
> -        case VIR_STORAGE_FILE_VHD:
> -            virBufferAddLit(&buf, "xvhd,");
> -            break;
> -        case VIR_STORAGE_FILE_QCOW:
> -            virBufferAddLit(&buf, "qcow,");
> -            break;
> -        case VIR_STORAGE_FILE_QCOW2:
> -            virBufferAddLit(&buf, "qcow2,");
> -            break;
> -      /* set default */
> -        default:
> -            virBufferAddLit(&buf, "raw,");
> -    }
> -
> -    /* device */
> -    virBufferAdd(&buf, disk->dst, -1);
> -
> -    virBufferAddLit(&buf, ",");
> -
> -    if (disk->src->readonly)
> -        virBufferAddLit(&buf, "r,");
> -    else if (disk->src->shared)
> -        virBufferAddLit(&buf, "!,");
> -    else
> -        virBufferAddLit(&buf, "w,");
> -    if (disk->transient) {
> -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                       _("transient disks not supported yet"));
> -        goto cleanup;
> -    }
> -
> -    if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM)
> -        virBufferAddLit(&buf, "devtype=cdrom");
> -
> -    if (virBufferCheckError(&buf) < 0)
> -        goto cleanup;
> -
> -    if (VIR_ALLOC(val) < 0)
> -        goto cleanup;
> -
> -    val->type = VIR_CONF_STRING;
> -    val->str = virBufferContentAndReset(&buf);
> -    tmp = list->list;
> -    while (tmp && tmp->next)
> -        tmp = tmp->next;
> -    if (tmp)
> -        tmp->next = val;
> -    else
> -        list->list = val;
> -    return 0;
> -
> - cleanup:
> -    virBufferFreeAndReset(&buf);
> -    return -1;
> -}
> -
> -
> -static int
> -xenFormatXLDomainDisks(virConfPtr conf, virDomainDefPtr def)
> -{
> -    virConfValuePtr diskVal = NULL;
> -    size_t i = 0;
> -
> -    if (VIR_ALLOC(diskVal) < 0)
> -        return -1;
> -
> -    diskVal->type = VIR_CONF_LIST;
> -    diskVal->list = NULL;
> -
> -    for (i = 0; i < def->ndisks; i++) {
> -        if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
> -            continue;
> -        if (xenFormatXLDisk(diskVal, def->disks[i]) < 0)
> -
> -                goto cleanup;
> -    }
> -
> -    if (diskVal->list != NULL) {
> -        int ret = virConfSetValue(conf, "disk", diskVal);
> -        diskVal = NULL;
> -        if (ret < 0)
> -            goto cleanup;
> -    }
> -
> -    return 0;
> -
> - cleanup:
> -    virConfFreeValue(diskVal);
> -    return 0;
> -}
> -
> -
> -static int
> -xenFormatXLSpice(virConfPtr conf, virDomainDefPtr def)
> -{
> -    const char *listenAddr = NULL;
> -
> -    if (STREQ(def->os.type, "hvm")) {
> -        if (def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
> -            /* set others to false but may not be necessary */
> -            if (xenConfigSetInt(conf, "sdl", 0) < 0)
> -                return -1;
> -
> -            if (xenConfigSetInt(conf, "vnc", 0) < 0)
> -                return -1;
> -
> -            if (xenConfigSetInt(conf, "spice", 1) < 0)
> -                return -1;
> -
> -            if (xenConfigSetInt(conf, "spiceport",
> -                                def->graphics[0]->data.spice.port) < 0)
> -                return -1;
> -
> -            if (xenConfigSetInt(conf, "spicetls_port",
> -                                def->graphics[0]->data.spice.tlsPort) < 0)
> -                return -1;
> -
> -            if (def->graphics[0]->data.spice.auth.passwd) {
> -                if (xenConfigSetInt(conf, "spicedisable_ticketing", 1) < 0)
> -                    return -1;
> -
> -                if (def->graphics[0]->data.spice.auth.passwd &&
> -                    xenConfigSetString(conf, "spicepasswd",
> -                                def->graphics[0]->data.spice.auth.passwd) < 0)
> -                    return -1;
> -            }
> -
> -            listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0);
> -            if (listenAddr &&
> -                xenConfigSetString(conf, "spicehost", listenAddr) < 0)
> -                return -1;
> -
> -            if (xenConfigSetInt(conf, "spicemouse_mouse",
> -                                def->graphics[0]->data.spice.mousemode) < 0)
> -                return -1;
> -
> -            if (def->graphics[0]->data.spice.copypaste) {
> -                if (xenConfigSetInt(conf, "spicedvagent", 1) < 0)
> -                    return -1;
> -                if (xenConfigSetInt(conf, "spice_clipboard_sharing",
> -                                def->graphics[0]->data.spice.copypaste) < 0)
> -                return -1;
> -            }
> -        }
> -    }
> -
> -    return 0;
> -}
> -
> -
> -virConfPtr
> -xenFormatXL(virDomainDefPtr def, virConnectPtr conn, int xendConfigVersion)
> -{
> -    virConfPtr conf = NULL;
> -
> -    if (!(conf = virConfNew()))
> -        goto cleanup;
> -
> -    if (xenFormatConfigCommon(conf, def, conn, xendConfigVersion) < 0)
> -        goto cleanup;
> -
> -    if (xenFormatXLDomainDisks(conf, def) < 0)
> -        goto cleanup;
> -
> -    if (xenFormatXLSpice(conf, def) < 0)
> -        goto cleanup;
> -
> -    return conf;
> -
> - cleanup:
> -    if (conf)
> -        virConfFree(conf);
> -    return NULL;
> -}
> diff --git a/src/xenconfig/xen_xl.h b/src/xenconfig/xen_xl.h
> deleted file mode 100644
> index 536e9b7..0000000
> --- a/src/xenconfig/xen_xl.h
> +++ /dev/null
> @@ -1,33 +0,0 @@
> -/*
> - * xen_xl.h: Xen XL parsing functions
> - *
> - * 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
> - * <http://www.gnu.org/licenses/>.
> - *
> - * Author: Kiarie Kahurani<davidkiarie4@gmail.com>
> - */
> -
> -#ifndef __VIR_XEN_XL_H__
> -# define __VIR_XEN_XL_H__
> -
> -# include "virconf.h"
> -# include "domain_conf.h"
> -# include "xen_common.h"
> -
> -virDomainDefPtr xenParseXL(virConfPtr conn, virCapsPtr caps,
> -                           int xendConfigVersion);
> -virConfPtr xenFormatXL(virDomainDefPtr def,
> -                       virConnectPtr, int xendConfigVersion);
> -
> -#endif /* __VIR_XEN_XL_H__ */
> diff --git a/src/xenconfig/xen_xl_disk.l b/src/xenconfig/xen_xl_disk.l
> deleted file mode 100644
> index 164aa32..0000000
> --- a/src/xenconfig/xen_xl_disk.l
> +++ /dev/null
> @@ -1,256 +0,0 @@
> -/*
> - * xen_xl_disk.l - parser for disk specification strings
> - *
> - * Copyright (C) 2011      Citrix Ltd.
> - * Author Ian Jackson <ian.jackson@eu.citrix.com>
> - *
> - * This program 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; version 2.1 only. with the special
> - * exception on linking described in file LICENSE.
> - *
> - * This program 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.
> - */
> -
> -/*
> - * Parsing the old xm/xend/xl-4.1 disk specs is a tricky problem,
> - * because the target string might in theory contain "," which is the
> - * delimiter we use for stripping off things on the RHS, and ":",
> - * which is the delimiter we use for stripping off things on the LHS.
> - *
> - * In this parser we do not support such target strings in the old
> - * syntax; if the target string has to contain "," or ":" the new
> - * syntax's "target=" should be used.
> - */
> -%{
> -# include <config.h>
> -
> -# include <stdio.h>
> -
> -# include "viralloc.h"
> -# include "virstoragefile.h"
> -# include "virstring.h"
> -# include "domain_conf.h"
> -# include "xen_xl.h"
> -# include "xen_xl_disk_i.h"
> -
> -#define YY_NO_INPUT
> -#define VIR_FROM_THIS VIR_FROM_NONE
> -
> -/* Some versions of flex have a bug (Fedora bugzilla 612465) which causes
> - * it to fail to declare these functions, which it defines.  So declare
> - * them ourselves.  Hopefully we won't have to simultaneously support
> - * a flex version which declares these differently somehow. */
> -int xl_disk_lexget_column(yyscan_t yyscanner);
> -void xl_disk_lexset_column(int  column_no, yyscan_t yyscanner);
> -
> -
> -/*----- useful macros and functions used in actions -----
> - * we use macros in the actual rules to keep the actions short
> - * and particularly to avoid repeating boilerplate values such as
> - * DPC->disk, yytext, etc. */
> -
> -/* For actions whose patterns contain '=', finds the start of the value */
> -#define FROMEQUALS (strchr(yytext,'=')+1)
> -
> -/* Chops the delimiter off, modifying yytext and yyleng. */
> -#define STRIP(delim) do{                                                \
> -	if (yyleng>0 && yytext[yyleng-1]==(delim))                      \
> -	    yytext[--yyleng] = 0;                                       \
> -    }while(0)
> -
> -/* Sets a string value, checking it hasn't been set already. */
> -#define SAVESTRING(what,loc,val) do{					\
> -	savestring(DPC, what " respecified", &DPC->disk->loc, (val));	\
> -    }while(0)
> -
> -
> -static void
> -savestring(xenXLDiskParserContext *dpc,
> -           const char *what_respecified,
> -           char **update,
> -           const char *value)
> -{
> -    if (*update) {
> -        if (**update) {
> -            xenXLDiskParserError(dpc, value, what_respecified);
> -            return;
> -        }
> -
> -        VIR_FREE(*update); /* do not complain about overwriting empty strings */
> -    }
> -
> -    ignore_value(VIR_STRDUP(*update, value));
> -}
> -
> -#define DPC dpc /* our convention in lexer helper functions */
> -
> -/* Sets ->readwrite from the string. */
> -static void
> -setaccess(xenXLDiskParserContext *dpc, const char *str)
> -{
> -    if (STREQ(str, "rw") || STREQ(str, "w")) {
> -        dpc->disk->src->readonly = 0;
> -    } else if (STREQ(str, "r") || STREQ(str, "ro")) {
> -        dpc->disk->src->readonly = 1;
> -    } else if (STREQ(str, "w!") || STREQ(str, "!")) {
> -        dpc->disk->src->readonly = 0;
> -	dpc->disk->src->shared = 1;
> -    } else {
> -        xenXLDiskParserError(dpc, str, "unknown value for access");
> -    }
> -    dpc->access_set = 1;
> -}
> -
> -/* Sets ->format from the string.  IDL should provide something for this. */
> -static void
> -setformat(xenXLDiskParserContext *dpc, const char *str)
> -{
> -    if (STREQ(str, "") || STREQ(str, "raw"))
> -        virDomainDiskSetFormat(dpc->disk, VIR_STORAGE_FILE_RAW);
> -    else if (STREQ(str, "qcow"))
> -        virDomainDiskSetFormat(dpc->disk, VIR_STORAGE_FILE_QCOW);
> -    else if (STREQ(str, "qcow2"))
> -        virDomainDiskSetFormat(dpc->disk, VIR_STORAGE_FILE_QCOW2);
> -    else if (STREQ(str, "vhd"))
> -        virDomainDiskSetFormat(dpc->disk, VIR_STORAGE_FILE_VHD);
> -    else
> -        xenXLDiskParserError(dpc, str, "unknown value for format");
> -}
> -
> -
> -/* Sets ->backend from the string.  IDL should provide something for this. */
> -static void
> -setdrivertype(xenXLDiskParserContext *dpc, const char *str)
> -{
> -    if (STREQ(str, "phy"))
> -        ignore_value(virDomainDiskSetDriver(dpc->disk, "phy"));
> -    else if (STREQ(str, "tap"))
> -        ignore_value(virDomainDiskSetDriver(dpc->disk, "tap"));
> -    else if (STREQ(str, "file") || STREQ(str, ""))
> -        ignore_value(virDomainDiskSetDriver(dpc->disk, "qemu"));
> -    else
> -        xenXLDiskParserError(dpc, str, "unknown value for backendtype");
> -}
> -
> -
> -/* Handles a vdev positional parameter which includes a devtype. */
> -static int
> -vdev_and_devtype(xenXLDiskParserContext *dpc, char *str)
> -{
> -    /* returns 1 if it was <vdev>:<devtype>, 0 (doing nothing) otherwise */
> -    char *colon = strrchr(str, ':');
> -    if (!colon)
> -        return 0;
> -
> -    *colon++ = 0;
> -    SAVESTRING("vdev", dst, str);
> -
> -    if (STREQ(colon,"cdrom")) {
> -        DPC->disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM;
> -    } else if (STREQ(colon, "disk")) {
> -        DPC->disk->device = VIR_DOMAIN_DISK_DEVICE_DISK;
> -    } else {
> -        xenXLDiskParserError(DPC, colon, "unknown deprecated type");
> -    }
> -    return 1;
> -}
> -
> -#undef DPC /* needs to be defined differently the actual lexer */
> -#define DPC ((xenXLDiskParserContext*)yyextra)
> -
> -%}
> -
> -%option warn
> -%option nodefault
> -%option batch
> -%option 8bit
> -%option noyywrap
> -%option reentrant
> -%option nounput
> -
> -%x LEXERR
> -
> -%%
> -
> - /*----- the scanner rules which do the parsing -----*/
> -
> -[ \t\n]+/([^ \t\n].*)? { /* ignore whitespace before parameters */ }
> -
> - /* ordinary parameters setting enums or strings */
> -
> -format=[^,]*,?	{ STRIP(','); setformat(DPC, FROMEQUALS); }
> -
> -cdrom,?		{ DPC->disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM; }
> -devtype=cdrom,?	{ DPC->disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM; }
> -devtype=disk,?	{ DPC->disk->device = VIR_DOMAIN_DISK_DEVICE_DISK; }
> -devtype=[^,]*,?	{ xenXLDiskParserError(DPC, yytext,"unknown value for type"); }
> -
> -access=[^,]*,?	{ STRIP(','); setaccess(DPC, FROMEQUALS); }
> -backendtype=[^,]*,? { STRIP(','); setdrivertype(DPC, FROMEQUALS); }
> -
> -vdev=[^,]*,?	{ STRIP(','); SAVESTRING("vdev", dst, FROMEQUALS); }
> -
> - /* the target magic parameter, eats the rest of the string */
> -
> -target=.*	{ STRIP(','); SAVESTRING("target", src->path, FROMEQUALS); }
> -
> - /* unknown parameters */
> -
> -[a-z][-a-z0-9]*=[^,],? { xenXLDiskParserError(DPC, yytext, "unknown parameter"); }
> -
> -  /* the "/.*" in these patterns ensures that they count as if they
> -   * matched the whole string, so these patterns take precedence */
> -
> -(raw|qcow2?|vhd):/.* {
> -                    STRIP(':');
> -                    DPC->had_depr_prefix=1;
> -                    setformat(DPC, yytext);
> -                 }
> -
> -tapdisk:/.*	{ DPC->had_depr_prefix=1; }
> -tap2?:/.*	{ DPC->had_depr_prefix=1; }
> -aio:/.*		{ DPC->had_depr_prefix=1; }
> -ioemu:/.*	{ DPC->had_depr_prefix=1; }
> -file:/.*	{ DPC->had_depr_prefix=1; }
> -phy:/.*		{ DPC->had_depr_prefix=1; }
> -[a-z][a-z0-9]*:/([^a-z0-9].*)? {
> -		  xenXLDiskParserError(DPC, yytext, "unknown deprecated disk prefix");
> -		  return 0;
> -		}
> -
> - /* positional parameters */
> -
> -[^=,]*,|[^=,]+,?  {
> -    STRIP(',');
> -
> -    if (DPC->err) {
> -        /* previous errors may just lead to subsequent ones */
> -    } else if (!DPC->disk->src->path) {
> -        SAVESTRING("target", src->path, yytext);
> -    } else if (DPC->disk->src->format == VIR_STORAGE_FILE_LAST){
> -        setformat(DPC, yytext);
> -    }
> -     else if (!DPC->disk->dst) {
> -        if (!vdev_and_devtype(DPC, yytext))
> -            SAVESTRING("vdev", dst, yytext);
> -    } else if (!DPC->access_set) {
> -        DPC->access_set = 1;
> -        setaccess(DPC, yytext);
> -    } else {
> -        xenXLDiskParserError(DPC, yytext, "too many positional parameters");
> -        return 0; /* don't print any more errors */
> -    }
> -}
> -
> -. {
> -    BEGIN(LEXERR);
> -    yymore();
> -}
> -<LEXERR>.* {
> -    xenXLDiskParserError(DPC, yytext, "bad disk syntax");
> -    return 0;
> -}
> diff --git a/src/xenconfig/xen_xl_disk_i.h b/src/xenconfig/xen_xl_disk_i.h
> deleted file mode 100644
> index 063dedf..0000000
> --- a/src/xenconfig/xen_xl_disk_i.h
> +++ /dev/null
> @@ -1,39 +0,0 @@
> -/*
> - * xen_xl_disk_i.h - common header for disk spec parser
> - *
> - * Copyright (C) 2011      Citrix Ltd.
> - * Author Ian Jackson <ian.jackson@eu.citrix.com>
> - *
> - * This program 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; version 2.1 only. with the special
> - * exception on linking described in file LICENSE.
> - *
> - * This program 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.
> - */
> -
> -#ifndef __VIR_XEN_XL_DISK_I_H__
> -# define __VIR_XEN_XL_DISK_I_H__
> -
> -# include "virconf.h"
> -# include "domain_conf.h"
> -
> -
> -typedef struct {
> -    int err;
> -    void *scanner;
> -    YY_BUFFER_STATE buf;
> -    virDomainDiskDefPtr disk;
> -    int access_set;
> -    int had_depr_prefix;
> -    const char *spec;
> -} xenXLDiskParserContext;
> -
> -void xenXLDiskParserError(xenXLDiskParserContext *dpc,
> -                          const char *erroneous,
> -                          const char *message);
> -
> -#endif /* __VIR_XEN_XL_DISK_I_H__ */
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/12] Replace Xen xl parsing/formatting impl
  2015-01-10  5:03 [PATCH 00/12] Replace Xen xl parsing/formatting impl Jim Fehlig
                   ` (11 preceding siblings ...)
  2015-01-10  5:03 ` [PATCH 12/12] libxl: Add support for parsing/formating Xen XL config Jim Fehlig
@ 2015-01-12 15:06 ` Ian Campbell
       [not found] ` <1421075193.26317.95.camel@citrix.com>
  13 siblings, 0 replies; 25+ messages in thread
From: Ian Campbell @ 2015-01-12 15:06 UTC (permalink / raw)
  To: Jim Fehlig; +Cc: libvir-list, xen-devel

On Fri, 2015-01-09 at 22:03 -0700, Jim Fehlig wrote:
> The first attempt to implement support for parsing/formatting Xen's
> xl disk config format copied Xen's flex-based parser into libvirt, which
> has proved to be challenging in the context of autotools.  But as it turns
> out, Xen provides an interface to the parser via libxlutil.
> 
> This series reverts the first attempt, along with subsequent attempts to
> fix it, and replaces it with an implementation based on libxlutil.  The
> first nine patches revert the original implementation and subsequent fixes.
> Patch 10 provides an implemenation based on libxlutil.  Patches 11 and
> 12 are basically unchanged from patches 3 and 4 in the first attempt.
> 
> One upshot of using libxlutil instead of copying the flex source is
> removing the potential for source divergence.

Thanks for doing this, looks good to me, FWIW.

Is the presence/absence of xen-xl support exposed via virsh anywhere? If
so then I can arrange for my Xen osstest patches for libvirt testing to
use xen-xl when available but still fallback to xen-xm. I've had a look
in "virsh capabilities" and "virsh help domxml-from-native" but not
seeing xen-xm, so assuming xen-xl won't magically appear in any of those
places either.

(TBH, this may become moot since I suspect your patches will be well
established by the time my osstest patches hit osstest...)

Ian.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [libvirt] [PATCH 10/12] Introduce support for parsing/formatting Xen xl config format
  2015-01-10  5:03 ` [PATCH 10/12] Introduce support for parsing/formatting Xen xl config format Jim Fehlig
@ 2015-01-12 15:27   ` John Ferlan
  2015-01-12 16:41     ` Jim Fehlig
  2015-01-13  1:32     ` Jim Fehlig
  2015-01-12 17:49   ` Eric Blake
  1 sibling, 2 replies; 25+ messages in thread
From: John Ferlan @ 2015-01-12 15:27 UTC (permalink / raw)
  To: Jim Fehlig, libvir-list; +Cc: Kiarie Kahurani, xen-devel



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 <davidkiarie4@gmail.com>
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> ---
>  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
> + * <http://www.gnu.org/licenses/>.
> + *
> + * Author: Kiarie Kahurani <davidkiarie4@gmail.com>
> + */
> +
> +#include <config.h>
> +
> +#include <libxl.h>
> +
> +#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 <libxlutil.h>
> +#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 <diskspec> 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 = <devtype>
> + *    backendtype = <backend-type>
> + * parameters not taken care of
> + *    backend = <domain-name>
> + *    script = <script>
> + *    direct-io-safe
> + *
> + * ================================================================
> + * The parser does not take any deprecated parameters
> + *
> + * For more information refer to /xen/docs/misc/xl-disk-configuration.txt
> + */
> +static int
> +xenParseXLDisk(virConfPtr conf, virDomainDefPtr def)
> +{
> +    virConfValuePtr list = virConfGetValue(conf, "disk");
> +    XLU_Config *xluconf;
> +    virDomainDiskDefPtr disk;
> +    libxl_device_disk *libxldisk;
> +
> +    if (VIR_ALLOC(libxldisk) < 0)
> +        return -1;
> +
> +    if (!(xluconf = xlu_cfg_init(stderr, "command line")))
> +        return -1;

This leaks libxldisk

> +
> +    if (list && list->type == VIR_CONF_LIST) {
> +        list = list->list;
> +        while (list) {
> +            const char *disk_spec = list->str;
> +
> +            if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
> +                goto skipdisk;
> +
> +            libxl_device_disk_init(libxldisk);
> +
> +            if (xlu_disk_parse(xluconf, 1, &disk_spec, libxldisk))

Will this allocate anything into libxldisk? That will eventually be
cleared out by the libxl_device_disk_init() on the next pass? I note
numberous strdup's of libxkdisk->* fields, so I am assuming that there's
been an allocation, although I suppose it could use pointers to

> +                goto skipdisk;
> +
> +            if (!(disk = virDomainDiskDefNew()))
> +                return -1;
> +
> +            if (VIR_STRDUP(disk->dst, libxldisk->vdev) < 0)
> +                goto fail;
> +
> +            if (virDomainDiskSetSource(disk, libxldisk->pdev_path) < 0)
> +                goto fail;
> +
> +            disk->src->readonly = libxldisk->readwrite ? 0 : 1;
> +            disk->removable = libxldisk->removable;
> +
> +            if (libxldisk->is_cdrom) {
> +                if (virDomainDiskSetDriver(disk, "qemu") < 0)
> +                    goto fail;
> +
> +                virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE);
> +                disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM;
> +                if (!disk->src->path || STREQ(disk->src->path, ""))
> +                    disk->src->format = VIR_STORAGE_FILE_NONE;
> +                else
> +                    disk->src->format = VIR_STORAGE_FILE_RAW;
> +            } else {
> +                switch (libxldisk->format) {
> +                case LIBXL_DISK_FORMAT_QCOW:
> +                    disk->src->format = VIR_STORAGE_FILE_QCOW;
> +                    break;
> +
> +                case LIBXL_DISK_FORMAT_QCOW2:
> +                    disk->src->format = VIR_STORAGE_FILE_QCOW2;
> +                    break;
> +
> +                case LIBXL_DISK_FORMAT_VHD:
> +                    disk->src->format = VIR_STORAGE_FILE_VHD;
> +                    break;
> +
> +                case LIBXL_DISK_FORMAT_RAW:
> +                case LIBXL_DISK_FORMAT_UNKNOWN:
> +                    disk->src->format = VIR_STORAGE_FILE_RAW;
> +                    break;
> +
> +                case LIBXL_DISK_FORMAT_EMPTY:
> +                    break;
> +                }
> +
> +                switch (libxldisk->backend) {
> +                case LIBXL_DISK_BACKEND_QDISK:
> +                case LIBXL_DISK_BACKEND_UNKNOWN:
> +                    if (virDomainDiskSetDriver(disk, "qemu") < 0)
> +                        goto fail;
> +                    virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE);
> +                    break;
> +
> +                case LIBXL_DISK_BACKEND_TAP:
> +                    if (virDomainDiskSetDriver(disk, "tap") < 0)
> +                        goto fail;
> +                    virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE);
> +                    break;
> +
> +                case LIBXL_DISK_BACKEND_PHY:
> +                    if (virDomainDiskSetDriver(disk, "phy") < 0)
> +                        goto fail;
> +                    virDomainDiskSetType(disk, VIR_STORAGE_TYPE_BLOCK);
> +                    break;
> +                }
> +            }
> +
> +            if (STRPREFIX(libxldisk->vdev, "xvd") || !STREQ(def->os.type, "hvm"))
> +                disk->bus = VIR_DOMAIN_DISK_BUS_XEN;
> +            else if (STRPREFIX(libxldisk->vdev, "sd"))
> +                disk->bus = VIR_DOMAIN_DISK_BUS_SCSI;
> +            else
> +                disk->bus = VIR_DOMAIN_DISK_BUS_IDE;
> +
> +            if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0)
> +                goto fail;
> +
> +        skipdisk:
> +            list = list->next;
> +        }
> +    }

Coverity notes that libxldisk is leaked here especially if list == NULL
(or list->type != VIR_CONF_LIST).  Although I assume by inspection that
it'll be leaked whether the list finds something or not.

> +    return 0;
> +
> + fail:
> +    virDomainDiskDefFree(disk);

I would think the libxldisk needs to be addressed here as well although
Coverity didn't

> +    return -1;
> +}
> +
> +
> +virDomainDefPtr
> +xenParseXL(virConfPtr conf, virCapsPtr caps, int xendConfigVersion)
> +{
> +    virDomainDefPtr def = NULL;
> +
> +    if (VIR_ALLOC(def) < 0)
> +        return NULL;
> +
> +    def->virtType = VIR_DOMAIN_VIRT_XEN;
> +    def->id = -1;
> +
> +    if (xenParseConfigCommon(conf, def, caps, xendConfigVersion) < 0)
> +        goto cleanup;
> +
> +    if (xenParseXLDisk(conf, def) < 0)
> +        goto cleanup;
> +
> +    if (xenParseXLSpice(conf, def) < 0)
> +        goto cleanup;
> +
> +    return def;
> +
> + cleanup:
> +    virDomainDefFree(def);
> +    return NULL;
> +}
> +
> +
> +static int
> +xenFormatXLDisk(virConfValuePtr list, virDomainDiskDefPtr disk)
> +{
> +    virBuffer buf = VIR_BUFFER_INITIALIZER;
> +    virConfValuePtr val, tmp;
> +    const char *src = virDomainDiskGetSource(disk);
> +    int format = virDomainDiskGetFormat(disk);
> +    const char *driver = virDomainDiskGetDriver(disk);
> +
> +    /* target */
> +    virBufferAsprintf(&buf, "%s,", src);
> +    /* format */
> +    switch (format) {
> +        case VIR_STORAGE_FILE_RAW:
> +            virBufferAddLit(&buf, "raw,");
> +            break;
> +        case VIR_STORAGE_FILE_VHD:
> +            virBufferAddLit(&buf, "xvhd,");
> +            break;
> +        case VIR_STORAGE_FILE_QCOW:
> +            virBufferAddLit(&buf, "qcow,");
> +            break;
> +        case VIR_STORAGE_FILE_QCOW2:
> +            virBufferAddLit(&buf, "qcow2,");
> +            break;
> +      /* set default */
> +        default:
> +            virBufferAddLit(&buf, "raw,");
> +    }
> +
> +    /* device */
> +    virBufferAdd(&buf, disk->dst, -1);
> +
> +    virBufferAddLit(&buf, ",");
> +
> +    if (disk->src->readonly)
> +        virBufferAddLit(&buf, "r,");
> +    else if (disk->src->shared)
> +        virBufferAddLit(&buf, "!,");
> +    else
> +        virBufferAddLit(&buf, "w,");
> +    if (disk->transient) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("transient disks not supported yet"));
> +        goto cleanup;
> +    }
> +
> +    if (STREQ_NULLABLE(driver, "qemu"))
> +        virBufferAddLit(&buf, "backendtype=qdisk");
> +    else if (STREQ_NULLABLE(driver, "tap"))
> +        virBufferAddLit(&buf, "backendtype=tap");
> +    else if (STREQ_NULLABLE(driver, "phy"))
> +        virBufferAddLit(&buf, "backendtype=phy");
> +
> +    if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM)
> +        virBufferAddLit(&buf, ",devtype=cdrom");
> +
> +    if (virBufferCheckError(&buf) < 0)
> +        goto cleanup;
> +
> +    if (VIR_ALLOC(val) < 0)
> +        goto cleanup;
> +
> +    val->type = VIR_CONF_STRING;
> +    val->str = virBufferContentAndReset(&buf);
> +    tmp = list->list;
> +    while (tmp && tmp->next)
> +        tmp = tmp->next;
> +    if (tmp)
> +        tmp->next = val;
> +    else
> +        list->list = val;
> +    return 0;
> +
> + cleanup:
> +    virBufferFreeAndReset(&buf);
> +    return -1;
> +}
> +
> +
> +static int
> +xenFormatXLDomainDisks(virConfPtr conf, virDomainDefPtr def)
> +{
> +    virConfValuePtr diskVal = NULL;
> +    size_t i = 0;
> +
> +    if (VIR_ALLOC(diskVal) < 0)
> +        return -1;
> +
> +    diskVal->type = VIR_CONF_LIST;
> +    diskVal->list = NULL;
> +
> +    for (i = 0; i < def->ndisks; i++) {
> +        if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
> +            continue;
> +        if (xenFormatXLDisk(diskVal, def->disks[i]) < 0)
> +
> +                goto cleanup;
> +    }
> +
> +    if (diskVal->list != NULL) {
> +        int ret = virConfSetValue(conf, "disk", diskVal);
> +        diskVal = NULL;
> +        if (ret < 0)
> +            goto cleanup;
> +    }
> +

Coverity complains that 'diskVal' going out of scope leaks what it
points to - especially if "diskVal->list != NULL" takes the false branch
above.

This is something I pointed out in patch 5 of 6 in my Coverity series:

http://www.redhat.com/archives/libvir-list/2015-January/msg00251.html

Seems there should be a virConfFreeValue(diskVal) or VIR_FREE(diskVal)
here and...

> +    return 0;
> +
> + cleanup:
> +    virConfFreeValue(diskVal);
> +    return 0;

shouldn't this be return -1?


> +}
> +
> +
> +static int
> +xenFormatXLSpice(virConfPtr conf, virDomainDefPtr def)
> +{
> +    const char *listenAddr = NULL;
> +
> +    if (STREQ(def->os.type, "hvm")) {
> +        if (def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
> +            /* set others to false but may not be necessary */
> +            if (xenConfigSetInt(conf, "sdl", 0) < 0)
> +                return -1;
> +
> +            if (xenConfigSetInt(conf, "vnc", 0) < 0)
> +                return -1;
> +
> +            if (xenConfigSetInt(conf, "spice", 1) < 0)
> +                return -1;
> +
> +            if (xenConfigSetInt(conf, "spiceport",
> +                                def->graphics[0]->data.spice.port) < 0)
> +                return -1;
> +
> +            if (xenConfigSetInt(conf, "spicetls_port",
> +                                def->graphics[0]->data.spice.tlsPort) < 0)
> +                return -1;
> +
> +            if (def->graphics[0]->data.spice.auth.passwd) {
> +                if (xenConfigSetInt(conf, "spicedisable_ticketing", 1) < 0)
> +                    return -1;
> +
> +                if (def->graphics[0]->data.spice.auth.passwd &&
> +                    xenConfigSetString(conf, "spicepasswd",
> +                                def->graphics[0]->data.spice.auth.passwd) < 0)
> +                    return -1;
> +            }
> +
> +            listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0);
> +            if (listenAddr &&
> +                xenConfigSetString(conf, "spicehost", listenAddr) < 0)
> +                return -1;
> +
> +            if (xenConfigSetInt(conf, "spicemouse_mouse",
> +                                def->graphics[0]->data.spice.mousemode) < 0)
> +                return -1;
> +
> +            if (def->graphics[0]->data.spice.copypaste) {
> +                if (xenConfigSetInt(conf, "spicedvagent", 1) < 0)
> +                    return -1;
> +                if (xenConfigSetInt(conf, "spice_clipboard_sharing",
> +                                def->graphics[0]->data.spice.copypaste) < 0)
> +                return -1;
> +            }
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +
> +virConfPtr
> +xenFormatXL(virDomainDefPtr def, virConnectPtr conn, int xendConfigVersion)
> +{
> +    virConfPtr conf = NULL;
> +
> +    if (!(conf = virConfNew()))
> +        goto cleanup;
> +
> +    if (xenFormatConfigCommon(conf, def, conn, xendConfigVersion) < 0)
> +        goto cleanup;
> +
> +    if (xenFormatXLDomainDisks(conf, def) < 0)
> +        goto cleanup;
> +
> +    if (xenFormatXLSpice(conf, def) < 0)
> +        goto cleanup;
> +
> +    return conf;
> +
> + cleanup:
> +    if (conf)
> +        virConfFree(conf);
> +    return NULL;
> +}
> diff --git a/src/xenconfig/xen_xl.h b/src/xenconfig/xen_xl.h
> new file mode 100644
> index 0000000..536e9b7
> --- /dev/null
> +++ b/src/xenconfig/xen_xl.h
> @@ -0,0 +1,33 @@
> +/*
> + * xen_xl.h: Xen XL parsing functions
> + *

Copyright date stuff here too...

John

> + * 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
> + * <http://www.gnu.org/licenses/>.
> + *
> + * Author: Kiarie Kahurani<davidkiarie4@gmail.com>
> + */
> +
> +#ifndef __VIR_XEN_XL_H__
> +# define __VIR_XEN_XL_H__
> +
> +# include "virconf.h"
> +# include "domain_conf.h"
> +# include "xen_common.h"
> +
> +virDomainDefPtr xenParseXL(virConfPtr conn, virCapsPtr caps,
> +                           int xendConfigVersion);
> +virConfPtr xenFormatXL(virDomainDefPtr def,
> +                       virConnectPtr, int xendConfigVersion);
> +
> +#endif /* __VIR_XEN_XL_H__ */
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [libvirt] [PATCH 00/12] Replace Xen xl parsing/formatting impl
       [not found] ` <1421075193.26317.95.camel@citrix.com>
@ 2015-01-12 16:05   ` Eric Blake
  2015-01-12 16:23   ` Jim Fehlig
  1 sibling, 0 replies; 25+ messages in thread
From: Eric Blake @ 2015-01-12 16:05 UTC (permalink / raw)
  To: Ian Campbell, Jim Fehlig; +Cc: libvir-list, xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 1816 bytes --]

On 01/12/2015 08:06 AM, Ian Campbell wrote:
> On Fri, 2015-01-09 at 22:03 -0700, Jim Fehlig wrote:
>> The first attempt to implement support for parsing/formatting Xen's
>> xl disk config format copied Xen's flex-based parser into libvirt, which
>> has proved to be challenging in the context of autotools.  But as it turns
>> out, Xen provides an interface to the parser via libxlutil.
>>
>> This series reverts the first attempt, along with subsequent attempts to
>> fix it, and replaces it with an implementation based on libxlutil.  The
>> first nine patches revert the original implementation and subsequent fixes.
>> Patch 10 provides an implemenation based on libxlutil.  Patches 11 and
>> 12 are basically unchanged from patches 3 and 4 in the first attempt.
>>
>> One upshot of using libxlutil instead of copying the flex source is
>> removing the potential for source divergence.
> 
> Thanks for doing this, looks good to me, FWIW.
> 
> Is the presence/absence of xen-xl support exposed via virsh anywhere? If
> so then I can arrange for my Xen osstest patches for libvirt testing to
> use xen-xl when available but still fallback to xen-xm. I've had a look
> in "virsh capabilities" and "virsh help domxml-from-native" but not
> seeing xen-xm, so assuming xen-xl won't magically appear in any of those
> places either.

I'm not sure if 'virsh capabilities' can show it, but it does sound like
a nice place to enhance if possible.

Also, if 'virsh --version=long' doesn't state whether libxl support was
compiled in, it should be patched to do so; although that only shows
what the client side supports (and not necessarily what the remote
server side supports).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [libvirt] [PATCH 09/12] Revert "src/xenconfig: Xen-xl parser"
       [not found]   ` <54B3DF75.2000406@redhat.com>
@ 2015-01-12 16:07     ` Eric Blake
       [not found]     ` <54B3F12C.1050406@redhat.com>
  1 sibling, 0 replies; 25+ messages in thread
From: Eric Blake @ 2015-01-12 16:07 UTC (permalink / raw)
  To: John Ferlan, Jim Fehlig, libvir-list; +Cc: xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 1599 bytes --]

On 01/12/2015 07:51 AM, John Ferlan wrote:
> 
> 
> On 01/10/2015 12:03 AM, Jim Fehlig wrote:
>> This reverts commit 2c78051a14acfb7aba078d569b1632dfe0ca0853.
>>
>> Conflicts:
>> 	src/Makefile.am
>>
>> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>> ---
>>  .gitignore                    |   1 -
>>  cfg.mk                        |   3 +-
>>  configure.ac                  |   1 -
>>  po/POTFILES.in                |   1 -
>>  src/Makefile.am               |  25 +--
>>  src/libvirt_xenconfig.syms    |   4 -
>>  src/xenconfig/xen_common.c    |   3 +-
>>  src/xenconfig/xen_xl.c        | 499 ------------------------------------------
>>  src/xenconfig/xen_xl.h        |  33 ---
>>  src/xenconfig/xen_xl_disk.l   | 256 ----------------------
>>  src/xenconfig/xen_xl_disk_i.h |  39 ----
>>  11 files changed, 4 insertions(+), 861 deletions(-)
>>
> 
> OK - so reverting is fine; however, xen_xl_disk.{c,h} still exist...
> Simple enough solution, but they will show up in someone's git status
> output since they are also removed from .gitignore.

It's a transient issue - someone that only checked out git at v1.2.11
won't see the generated files; the few of us that do incremental work
can modify .git/info/exclude manually to ignore leftover generated
files.  But if you also want to explicitly ignore the generated files in
.gitignore, go for it.

ACK 1-9, and I'm liking the initial work of 10-12 other than the
Coverity issues that it introduces.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/12] Replace Xen xl parsing/formatting impl
       [not found] ` <1421075193.26317.95.camel@citrix.com>
  2015-01-12 16:05   ` [libvirt] " Eric Blake
@ 2015-01-12 16:23   ` Jim Fehlig
  2015-01-12 16:27     ` Ian Campbell
  1 sibling, 1 reply; 25+ messages in thread
From: Jim Fehlig @ 2015-01-12 16:23 UTC (permalink / raw)
  To: Ian Campbell; +Cc: libvir-list, xen-devel

Ian Campbell wrote:
> On Fri, 2015-01-09 at 22:03 -0700, Jim Fehlig wrote:
>   
>> The first attempt to implement support for parsing/formatting Xen's
>> xl disk config format copied Xen's flex-based parser into libvirt, which
>> has proved to be challenging in the context of autotools.  But as it turns
>> out, Xen provides an interface to the parser via libxlutil.
>>
>> This series reverts the first attempt, along with subsequent attempts to
>> fix it, and replaces it with an implementation based on libxlutil.  The
>> first nine patches revert the original implementation and subsequent fixes.
>> Patch 10 provides an implemenation based on libxlutil.  Patches 11 and
>> 12 are basically unchanged from patches 3 and 4 in the first attempt.
>>
>> One upshot of using libxlutil instead of copying the flex source is
>> removing the potential for source divergence.
>>     
>
> Thanks for doing this, looks good to me, FWIW.
>
> Is the presence/absence of xen-xl support exposed via virsh anywhere? If
> so then I can arrange for my Xen osstest patches for libvirt testing to
> use xen-xl when available but still fallback to xen-xm. I've had a look
> in "virsh capabilities" and "virsh help domxml-from-native" but not
> seeing xen-xm, so assuming xen-xl won't magically appear in any of those
> places either.
>   

AFAIK, the only place the supported native formats are listed is in the
virsh man page.  But thanks for the question, else I would have missed
adding xen-xl to the man page in 12/12.

Regards,
Jim

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/12] Replace Xen xl parsing/formatting impl
  2015-01-12 16:23   ` Jim Fehlig
@ 2015-01-12 16:27     ` Ian Campbell
  0 siblings, 0 replies; 25+ messages in thread
From: Ian Campbell @ 2015-01-12 16:27 UTC (permalink / raw)
  To: Jim Fehlig; +Cc: libvir-list, xen-devel

On Mon, 2015-01-12 at 09:23 -0700, Jim Fehlig wrote:
> Ian Campbell wrote:
> > On Fri, 2015-01-09 at 22:03 -0700, Jim Fehlig wrote:
> >   
> >> The first attempt to implement support for parsing/formatting Xen's
> >> xl disk config format copied Xen's flex-based parser into libvirt, which
> >> has proved to be challenging in the context of autotools.  But as it turns
> >> out, Xen provides an interface to the parser via libxlutil.
> >>
> >> This series reverts the first attempt, along with subsequent attempts to
> >> fix it, and replaces it with an implementation based on libxlutil.  The
> >> first nine patches revert the original implementation and subsequent fixes.
> >> Patch 10 provides an implemenation based on libxlutil.  Patches 11 and
> >> 12 are basically unchanged from patches 3 and 4 in the first attempt.
> >>
> >> One upshot of using libxlutil instead of copying the flex source is
> >> removing the potential for source divergence.
> >>     
> >
> > Thanks for doing this, looks good to me, FWIW.
> >
> > Is the presence/absence of xen-xl support exposed via virsh anywhere? If
> > so then I can arrange for my Xen osstest patches for libvirt testing to
> > use xen-xl when available but still fallback to xen-xm. I've had a look
> > in "virsh capabilities" and "virsh help domxml-from-native" but not
> > seeing xen-xm, so assuming xen-xl won't magically appear in any of those
> > places either.
> >   
> 
> AFAIK, the only place the supported native formats are listed is in the
> virsh man page.

Not to worry, I think I'll just use xen-xl everywhere then, osstest's
handling of test failures and regression detection will do the right
thing with versions of libvirt which don't have this applied.

>   But thanks for the question, else I would have missed
> adding xen-xl to the man page in 12/12.

No problem ;-)

Ian.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [libvirt] [PATCH 10/12] Introduce support for parsing/formatting Xen xl config format
  2015-01-12 15:27   ` [libvirt] " John Ferlan
@ 2015-01-12 16:41     ` Jim Fehlig
  2015-01-13  1:32     ` Jim Fehlig
  1 sibling, 0 replies; 25+ messages in thread
From: Jim Fehlig @ 2015-01-12 16:41 UTC (permalink / raw)
  To: John Ferlan; +Cc: libvir-list, Kiarie Kahurani, xen-devel

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 <davidkiarie4@gmail.com>
>> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>> ---
>>  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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [libvirt] [PATCH 09/12] Revert "src/xenconfig: Xen-xl parser"
       [not found]     ` <54B3F12C.1050406@redhat.com>
@ 2015-01-12 17:46       ` Jim Fehlig
  0 siblings, 0 replies; 25+ messages in thread
From: Jim Fehlig @ 2015-01-12 17:46 UTC (permalink / raw)
  To: Eric Blake; +Cc: libvir-list, John Ferlan, xen-devel

Eric Blake wrote:
> On 01/12/2015 07:51 AM, John Ferlan wrote:
>   
>> On 01/10/2015 12:03 AM, Jim Fehlig wrote:
>>     
>>> This reverts commit 2c78051a14acfb7aba078d569b1632dfe0ca0853.
>>>
>>> Conflicts:
>>> 	src/Makefile.am
>>>
>>> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>>> ---
>>>  .gitignore                    |   1 -
>>>  cfg.mk                        |   3 +-
>>>  configure.ac                  |   1 -
>>>  po/POTFILES.in                |   1 -
>>>  src/Makefile.am               |  25 +--
>>>  src/libvirt_xenconfig.syms    |   4 -
>>>  src/xenconfig/xen_common.c    |   3 +-
>>>  src/xenconfig/xen_xl.c        | 499 ------------------------------------------
>>>  src/xenconfig/xen_xl.h        |  33 ---
>>>  src/xenconfig/xen_xl_disk.l   | 256 ----------------------
>>>  src/xenconfig/xen_xl_disk_i.h |  39 ----
>>>  11 files changed, 4 insertions(+), 861 deletions(-)
>>>
>>>       
>> OK - so reverting is fine; however, xen_xl_disk.{c,h} still exist...
>> Simple enough solution, but they will show up in someone's git status
>> output since they are also removed from .gitignore.
>>     
>
> It's a transient issue - someone that only checked out git at v1.2.11
> won't see the generated files; the few of us that do incremental work
> can modify .git/info/exclude manually to ignore leftover generated
> files.  But if you also want to explicitly ignore the generated files in
> .gitignore, go for it.
>
> ACK 1-9, and I'm liking the initial work of 10-12 other than the
> Coverity issues that it introduces.
>   

I've pushed 1-9 and will resend 10-12 after fixing the Coverity issues
and addressing John's comments.

Regards,
Jim

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [libvirt] [PATCH 10/12] Introduce support for parsing/formatting Xen xl config format
  2015-01-10  5:03 ` [PATCH 10/12] Introduce support for parsing/formatting Xen xl config format Jim Fehlig
  2015-01-12 15:27   ` [libvirt] " John Ferlan
@ 2015-01-12 17:49   ` Eric Blake
  2015-01-13  1:33     ` Jim Fehlig
  1 sibling, 1 reply; 25+ messages in thread
From: Eric Blake @ 2015-01-12 17:49 UTC (permalink / raw)
  To: Jim Fehlig, libvir-list; +Cc: Kiarie Kahurani, xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 1924 bytes --]

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 <davidkiarie4@gmail.com>
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> ---
>  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


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [libvirt] [PATCH 10/12] Introduce support for parsing/formatting Xen xl config format
  2015-01-12 15:27   ` [libvirt] " John Ferlan
  2015-01-12 16:41     ` Jim Fehlig
@ 2015-01-13  1:32     ` Jim Fehlig
  1 sibling, 0 replies; 25+ messages in thread
From: Jim Fehlig @ 2015-01-13  1:32 UTC (permalink / raw)
  To: John Ferlan; +Cc: libvir-list, Kiarie Kahurani, xen-devel

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 <davidkiarie4@gmail.com>
>> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>> ---
>>  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?
>   

Don't know, but I'll add one.

>   
>> + *
>> + * 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
>> + * <http://www.gnu.org/licenses/>.
>> + *
>> + * Author: Kiarie Kahurani <davidkiarie4@gmail.com>
>> + */
>> +
>> +#include <config.h>
>> +
>> +#include <libxl.h>
>> +
>> +#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 <libxlutil.h>
>> +#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 <diskspec> 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 = <devtype>
>> + *    backendtype = <backend-type>
>> + * parameters not taken care of
>> + *    backend = <domain-name>
>> + *    script = <script>
>> + *    direct-io-safe
>> + *
>> + * ================================================================
>> + * The parser does not take any deprecated parameters
>> + *
>> + * For more information refer to /xen/docs/misc/xl-disk-configuration.txt
>> + */
>> +static int
>> +xenParseXLDisk(virConfPtr conf, virDomainDefPtr def)
>> +{
>> +    virConfValuePtr list = virConfGetValue(conf, "disk");
>> +    XLU_Config *xluconf;
>> +    virDomainDiskDefPtr disk;
>> +    libxl_device_disk *libxldisk;
>> +
>> +    if (VIR_ALLOC(libxldisk) < 0)
>> +        return -1;
>> +
>> +    if (!(xluconf = xlu_cfg_init(stderr, "command line")))
>> +        return -1;
>>     
>
> This leaks libxldisk
>
>   
>> +
>> +    if (list && list->type == VIR_CONF_LIST) {
>> +        list = list->list;
>> +        while (list) {
>> +            const char *disk_spec = list->str;
>> +
>> +            if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
>> +                goto skipdisk;
>> +
>> +            libxl_device_disk_init(libxldisk);
>> +
>> +            if (xlu_disk_parse(xluconf, 1, &disk_spec, libxldisk))
>>     
>
> Will this allocate anything into libxldisk? That will eventually be
> cleared out by the libxl_device_disk_init() on the next pass? I note
> numberous strdup's of libxkdisk->* fields, so I am assuming that there's
> been an allocation, although I suppose it could use pointers to
>   

Thanks.  I need to use libxl_device_disk_dispose.  I also need to
dispose of the XLU_Config object.  I've reworked this function a bit to
address this and other issues you've pointed out.

Regards,
Jim

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [libvirt] [PATCH 10/12] Introduce support for parsing/formatting Xen xl config format
  2015-01-12 17:49   ` Eric Blake
@ 2015-01-13  1:33     ` Jim Fehlig
  0 siblings, 0 replies; 25+ messages in thread
From: Jim Fehlig @ 2015-01-13  1:33 UTC (permalink / raw)
  To: Eric Blake; +Cc: libvir-list, Kiarie Kahurani, xen-devel

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 <davidkiarie4@gmail.com>
>> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>> ---
>>  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

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2015-01-13  1:33 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-10  5:03 [PATCH 00/12] Replace Xen xl parsing/formatting impl Jim Fehlig
2015-01-10  5:03 ` [PATCH 01/12] Revert "bootstrap.conf: add check for flex" Jim Fehlig
2015-01-10  5:03 ` [PATCH 02/12] Revert "src/Makefile: Fix parallel build after xen_xl_disk parser introduction" Jim Fehlig
2015-01-10  5:03 ` [PATCH 03/12] Revert "src/Makefile: move the new xen_xl_disk parser code at the correct place" Jim Fehlig
2015-01-10  5:03 ` [PATCH 04/12] Revert "Revert "src/Makefile.am: fix build breaker for xenconfig"" Jim Fehlig
2015-01-10  5:03 ` [PATCH 05/12] Revert "build: fix xenconfig VPATH builds" Jim Fehlig
2015-01-10  5:03 ` [PATCH 06/12] Revert "src/Makefile.am: fix build breaker for xenconfig" Jim Fehlig
2015-01-10  5:03 ` [PATCH 07/12] Revert "libxl: Add support for parsing/formating Xen XL config" Jim Fehlig
2015-01-10  5:03 ` [PATCH 08/12] Revert "tests: Tests for the xen-xl parser" Jim Fehlig
2015-01-10  5:03 ` [PATCH 09/12] Revert "src/xenconfig: Xen-xl parser" Jim Fehlig
2015-01-12 14:51   ` [libvirt] " John Ferlan
     [not found]   ` <54B3DF75.2000406@redhat.com>
2015-01-12 16:07     ` Eric Blake
     [not found]     ` <54B3F12C.1050406@redhat.com>
2015-01-12 17:46       ` Jim Fehlig
2015-01-10  5:03 ` [PATCH 10/12] Introduce support for parsing/formatting Xen xl config format Jim Fehlig
2015-01-12 15:27   ` [libvirt] " John Ferlan
2015-01-12 16:41     ` Jim Fehlig
2015-01-13  1:32     ` Jim Fehlig
2015-01-12 17:49   ` Eric Blake
2015-01-13  1:33     ` Jim Fehlig
2015-01-10  5:03 ` [PATCH 11/12] tests: Tests for the xen-xl parser Jim Fehlig
2015-01-10  5:03 ` [PATCH 12/12] libxl: Add support for parsing/formating Xen XL config Jim Fehlig
2015-01-12 15:06 ` [PATCH 00/12] Replace Xen xl parsing/formatting impl Ian Campbell
     [not found] ` <1421075193.26317.95.camel@citrix.com>
2015-01-12 16:05   ` [libvirt] " Eric Blake
2015-01-12 16:23   ` Jim Fehlig
2015-01-12 16:27     ` Ian Campbell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.