[libvirt] [PATCH 0/8] Refactored SEXPR and XM code
Hi, this is a reworked version of my patch that refactors the XM and SEXPR parsing routines from the xen-unified driver into a seperate directory (xenxs; x for xm and s for sexpr). This way different xen-drivers besides xen-unified are able to use the parsing functionality. For example the upcoming XenLight (libxl) driver. Because the XM functions reuse code from the SEXPR parsing functions I had to move the SEXPR functions too. To use the XM parsing functions one includes "xen_xm.h" and for SEXPR parsing "xen_sxpr.h". The first patch just moves the sexpr.c.h files to src/util. The second patch moves some general sexpr functions from xen-unified to the sexpr unit. With the patches 3 and 4 the SEXPR parsing/formatting functions from xen-unified are moved to a new unit xen_sxpr.c. Patches 5 and 6 do the same for the XM functions. Patch 7 removes driver-references from the function names in xenxs. Some parsing functions required a driver object to fetch the tty path and vncport. I removed all references to a specific driver and added additional parameters as a replacement. The tests sexpr2xml, xmconfig and xml2sexpr are adapted and show no error. Thanks in advance for your comments about this. Markus Groß (8): Moved SEXPR unit to utils Moved some SEXPR functions from xen-unified Moved SEXPR parsing functions to xenxs Moved SEXPR formatting functions to xenxs Moved XM parsing functions to xenxs Moved XM formatting functions to xenxs Renamed functions in xenxs Added to AUTHORS AUTHORS | 1 + po/POTFILES.in | 2 + src/Makefile.am | 16 +- src/{xen => util}/sexpr.c | 64 + src/{xen => util}/sexpr.h | 6 + src/xen/xen_driver.c | 19 +- src/xen/xend_internal.c | 6668 +++++++++++++++------------------------------ src/xen/xend_internal.h | 30 - src/xen/xm_internal.c | 1702 +------------ src/xen/xm_internal.h | 3 - src/xenxs/xen_sxpr.c | 2197 +++++++++++++++ src/xenxs/xen_sxpr.h | 66 + src/xenxs/xen_xm.c | 1724 ++++++++++++ src/xenxs/xen_xm.h | 39 + src/xenxs/xenxs_private.h | 63 + tests/sexpr2xmltest.c | 12 +- tests/xmconfigtest.c | 5 +- tests/xml2sexprtest.c | 3 +- 18 files changed, 6437 insertions(+), 6183 deletions(-) rename src/{xen => util}/sexpr.c (90%) rename src/{xen => util}/sexpr.h (88%) create mode 100644 src/xenxs/xen_sxpr.c create mode 100644 src/xenxs/xen_sxpr.h create mode 100644 src/xenxs/xen_xm.c create mode 100644 src/xenxs/xen_xm.h create mode 100644 src/xenxs/xenxs_private.h -- 1.7.4.1
--- src/Makefile.am | 2 +- src/{xen => util}/sexpr.c | 0 src/{xen => util}/sexpr.h | 0 3 files changed, 1 insertions(+), 1 deletions(-) rename src/{xen => util}/sexpr.c (100%) rename src/{xen => util}/sexpr.h (100%)
On Mon, Feb 21, 2011 at 02:40:06PM +0100, Markus Groß wrote:
--- src/Makefile.am | 2 +- src/{xen => util}/sexpr.c | 0 src/{xen => util}/sexpr.h | 0 3 files changed, 1 insertions(+), 1 deletions(-) rename src/{xen => util}/sexpr.c (100%) rename src/{xen => util}/sexpr.h (100%)
diff --git a/src/Makefile.am b/src/Makefile.am index 30c6cd3..5c9b019 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -68,6 +68,7 @@ UTIL_SOURCES = \ util/network.c util/network.h \ util/interface.c util/interface.h \ util/qparams.c util/qparams.h \ + util/sexpr.c util/sexpr.h \ util/stats_linux.c util/stats_linux.h \ util/storage_file.c util/storage_file.h \ util/sysinfo.c util/sysinfo.h \ @@ -221,7 +222,6 @@ TEST_DRIVER_SOURCES = \
# Now the Hypervisor specific drivers XEN_DRIVER_SOURCES = \ - xen/sexpr.c xen/sexpr.h \ xen/block_stats.c xen/block_stats.h \ xen/xen_hypervisor.c xen/xen_hypervisor.h \ xen/xen_driver.c xen/xen_driver.h \ diff --git a/src/xen/sexpr.c b/src/util/sexpr.c similarity index 100% rename from src/xen/sexpr.c rename to src/util/sexpr.c diff --git a/src/xen/sexpr.h b/src/util/sexpr.h similarity index 100% rename from src/xen/sexpr.h rename to src/util/sexpr.h
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--- src/util/sexpr.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/sexpr.h | 6 ++++ src/xen/xend_internal.c | 65 ----------------------------------------------- 3 files changed, 70 insertions(+), 65 deletions(-)
On Mon, Feb 21, 2011 at 02:40:07PM +0100, Markus Groß wrote:
--- src/util/sexpr.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/sexpr.h | 6 ++++ src/xen/xend_internal.c | 65 ----------------------------------------------- 3 files changed, 70 insertions(+), 65 deletions(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--- po/POTFILES.in | 1 + src/Makefile.am | 13 + src/xen/xen_driver.c | 12 +- src/xen/xend_internal.c | 1329 +------------------------------------------- src/xen/xend_internal.h | 18 - src/xen/xm_internal.c | 1 + src/xenxs/xen_sxpr.c | 1351 +++++++++++++++++++++++++++++++++++++++++++++ src/xenxs/xen_sxpr.h | 62 ++ src/xenxs/xenxs_private.h | 37 ++ tests/sexpr2xmltest.c | 13 +- 10 files changed, 1512 insertions(+), 1325 deletions(-) create mode 100644 src/xenxs/xen_sxpr.c create mode 100644 src/xenxs/xen_sxpr.h create mode 100644 src/xenxs/xenxs_private.h
On Mon, Feb 21, 2011 at 02:40:08PM +0100, Markus Groß wrote:
--- po/POTFILES.in | 1 + src/Makefile.am | 13 + src/xen/xen_driver.c | 12 +- src/xen/xend_internal.c | 1329 +------------------------------------------- src/xen/xend_internal.h | 18 - src/xen/xm_internal.c | 1 + src/xenxs/xen_sxpr.c | 1351 +++++++++++++++++++++++++++++++++++++++++++++ src/xenxs/xen_sxpr.h | 62 ++ src/xenxs/xenxs_private.h | 37 ++ tests/sexpr2xmltest.c | 13 +- 10 files changed, 1512 insertions(+), 1325 deletions(-) create mode 100644 src/xenxs/xen_sxpr.c create mode 100644 src/xenxs/xen_sxpr.h create mode 100644 src/xenxs/xenxs_private.h
diff --git a/po/POTFILES.in b/po/POTFILES.in index 2256cb2..7fbbe8f 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -119,6 +119,7 @@ src/xen/xm_internal.c src/xen/xs_internal.c src/xenapi/xenapi_driver.c src/xenapi/xenapi_utils.c +src/xenxs/xen_sxpr.c tools/console.c tools/libvirt-guests.init.sh tools/virsh.c diff --git a/src/Makefile.am b/src/Makefile.am index 5c9b019..1e670e5 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -423,6 +423,10 @@ CPU_SOURCES = \ VMX_SOURCES = \ vmx/vmx.c vmx/vmx.h
+XENXS_SOURCES = \ + xenxs/xenxs_private.h \ + xenxs/xen_sxpr.c xenxs/xen_sxpr.h + pkgdata_DATA = cpu/cpu_map.xml
EXTRA_DIST += $(pkgdata_DATA) @@ -464,6 +468,14 @@ libvirt_vmx_la_CFLAGS = \ libvirt_vmx_la_SOURCES = $(VMX_SOURCES) endif
+if WITH_XEN +noinst_LTLIBRARIES += libvirt_xenxs.la +libvirt_la_BUILT_LIBADD += libvirt_xenxs.la +libvirt_xenxs_la_CFLAGS = \ + -I@top_srcdir@/src/conf $(AM_CFLAGS) +libvirt_xenxs_la_SOURCES = $(XENXS_SOURCES) +endif +
noinst_LTLIBRARIES += libvirt_driver.la libvirt_la_BUILT_LIBADD += libvirt_driver.la @@ -583,6 +595,7 @@ endif libvirt_driver_xen_la_CFLAGS = \ $(XEN_CFLAGS) \ -I@top_srcdir@/src/conf \ + -I@top_srcdir@/src/xenxs \ $(AM_CFLAGS) libvirt_driver_xen_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_xen_la_LIBADD = $(XEN_LIBS)
You also need to add $(XENXS_SOURCES) to the huge EXTRA_DIST entry later, so that the files get included in the tar.gz even if 'make dist' is run on a system without xen. ACK to the rest of the patch if that bit is fixed Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 02/21/2011 10:19 AM, Daniel P. Berrange wrote:
On Mon, Feb 21, 2011 at 02:40:08PM +0100, Markus Groß wrote:
+XENXS_SOURCES = \ + xenxs/xenxs_private.h \ + xenxs/xen_sxpr.c xenxs/xen_sxpr.h + pkgdata_DATA = cpu/cpu_map.xml
You also need to add $(XENXS_SOURCES) to the huge EXTRA_DIST entry later, so that the files get included in the tar.gz even if 'make dist' is run on a system without xen.
ACK to the rest of the patch if that bit is fixed
I'm pushing, after squashing this in: diff --git i/src/Makefile.am w/src/Makefile.am index 1e670e5..4a64eb1 100644 --- i/src/Makefile.am +++ w/src/Makefile.am @@ -1008,7 +1008,8 @@ EXTRA_DIST += \ $(SECURITY_DRIVER_APPARMOR_SOURCES) \ $(SECRET_DRIVER_SOURCES) \ $(VBOX_DRIVER_EXTRA_DIST) \ - $(VMWARE_DRIVER_SOURCES) + $(VMWARE_DRIVER_SOURCES) \ + $(XENXS_SOURCES) check-local: augeas-check diff --git i/src/xenxs/xenxs_private.h w/src/xenxs/xenxs_private.h index 02fd1a3..1acabfb 100644 --- i/src/xenxs/xenxs_private.h +++ w/src/xenxs/xenxs_private.h @@ -28,9 +28,9 @@ # include "internal.h" -#define VIR_FROM_THIS VIR_FROM_NONE +# define VIR_FROM_THIS VIR_FROM_NONE -#define XENXS_ERROR(code, ...) \ +# define XENXS_ERROR(code, ...) \ virReportErrorHelper(NULL, VIR_FROM_NONE, code, __FILE__, __FUNCTION__, \ __LINE__, __VA_ARGS__) -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
--- src/xen/xend_internal.c | 876 --------------------------------------------- src/xen/xend_internal.h | 12 - src/xenxs/xen_sxpr.c | 850 +++++++++++++++++++++++++++++++++++++++++++- src/xenxs/xen_sxpr.h | 32 ++ src/xenxs/xenxs_private.h | 18 + tests/xml2sexprtest.c | 1 + 6 files changed, 900 insertions(+), 889 deletions(-)
On Mon, Feb 21, 2011 at 02:40:09PM +0100, Markus Groß wrote:
--- src/xen/xend_internal.c | 876 --------------------------------------------- src/xen/xend_internal.h | 12 - src/xenxs/xen_sxpr.c | 850 +++++++++++++++++++++++++++++++++++++++++++- src/xenxs/xen_sxpr.h | 32 ++ src/xenxs/xenxs_private.h | 18 + tests/xml2sexprtest.c | 1 + 6 files changed, 900 insertions(+), 889 deletions(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 02/21/2011 06:40 AM, Markus Groß wrote:
--- src/xen/xend_internal.c | 876 --------------------------------------------- src/xen/xend_internal.h | 12 - src/xenxs/xen_sxpr.c | 850 +++++++++++++++++++++++++++++++++++++++++++- src/xenxs/xen_sxpr.h | 32 ++ src/xenxs/xenxs_private.h | 18 + tests/xml2sexprtest.c | 1 + 6 files changed, 900 insertions(+), 889 deletions(-)
$ git am -3 ../xenxs.eml Applying: Moved SEXPR formatting functions to xenxs fatal: sha1 information is lacking or useless (src/xen/xend_internal.c). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Moved SEXPR formatting functions to xenxs Would you mind rebasing and fixing this? 1-3 and 8 are in, but 4-7 could use some more attention. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 02/21/2011 11:01 AM, Eric Blake wrote:
On 02/21/2011 06:40 AM, Markus Groß wrote:
--- src/xen/xend_internal.c | 876 --------------------------------------------- src/xen/xend_internal.h | 12 - src/xenxs/xen_sxpr.c | 850 +++++++++++++++++++++++++++++++++++++++++++- src/xenxs/xen_sxpr.h | 32 ++ src/xenxs/xenxs_private.h | 18 + tests/xml2sexprtest.c | 1 + 6 files changed, 900 insertions(+), 889 deletions(-)
$ git am -3 ../xenxs.eml Applying: Moved SEXPR formatting functions to xenxs fatal: sha1 information is lacking or useless (src/xen/xend_internal.c). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Moved SEXPR formatting functions to xenxs
Would you mind rebasing and fixing this? 1-3 and 8 are in, but 4-7 could use some more attention.
Never mind - I figured it out. My squashed change for keeping cppi happy in patch 3 caused one of the conflicts, and my global DEBUG->VIR_DEBUG patch pushed earlier today caused the other. I've gone ahead and pushed 4-7 with the trivial conflict resolutions, but you may want to double-check things. Thanks again for tackling this patch series! -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
--- po/POTFILES.in | 1 + src/Makefile.am | 3 +- src/xen/xen_driver.c | 3 +- src/xen/xm_internal.c | 976 +------------------------------------------- src/xen/xm_internal.h | 1 - src/xenxs/xen_xm.c | 1010 +++++++++++++++++++++++++++++++++++++++++++++ src/xenxs/xen_xm.h | 36 ++ src/xenxs/xenxs_private.h | 2 + tests/xmconfigtest.c | 3 +- 9 files changed, 1058 insertions(+), 977 deletions(-) create mode 100644 src/xenxs/xen_xm.c create mode 100644 src/xenxs/xen_xm.h
On Mon, Feb 21, 2011 at 02:40:10PM +0100, Markus Groß wrote:
--- po/POTFILES.in | 1 + src/Makefile.am | 3 +- src/xen/xen_driver.c | 3 +- src/xen/xm_internal.c | 976 +------------------------------------------- src/xen/xm_internal.h | 1 - src/xenxs/xen_xm.c | 1010 +++++++++++++++++++++++++++++++++++++++++++++ src/xenxs/xen_xm.h | 36 ++ src/xenxs/xenxs_private.h | 2 + tests/xmconfigtest.c | 3 +- 9 files changed, 1058 insertions(+), 977 deletions(-) create mode 100644 src/xenxs/xen_xm.c create mode 100644 src/xenxs/xen_xm.h
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--- src/xen/xen_driver.c | 2 +- src/xen/xm_internal.c | 723 +-------------------------------------------- src/xen/xm_internal.h | 2 - src/xenxs/xen_sxpr.h | 6 - src/xenxs/xen_xm.c | 715 ++++++++++++++++++++++++++++++++++++++++++++ src/xenxs/xen_xm.h | 3 + src/xenxs/xenxs_private.h | 6 + tests/xmconfigtest.c | 2 +- 8 files changed, 728 insertions(+), 731 deletions(-)
On Mon, Feb 21, 2011 at 02:40:11PM +0100, Markus Groß wrote:
--- src/xen/xen_driver.c | 2 +- src/xen/xm_internal.c | 723 +-------------------------------------------- src/xen/xm_internal.h | 2 - src/xenxs/xen_sxpr.h | 6 - src/xenxs/xen_xm.c | 715 ++++++++++++++++++++++++++++++++++++++++++++ src/xenxs/xen_xm.h | 3 + src/xenxs/xenxs_private.h | 6 + tests/xmconfigtest.c | 2 +- 8 files changed, 728 insertions(+), 731 deletions(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--- src/xen/xen_driver.c | 8 +- src/xen/xend_internal.c | 54 +++++++------- src/xen/xm_internal.c | 6 +- src/xenxs/xen_sxpr.c | 184 +++++++++++++++++++++++------------------------ src/xenxs/xen_sxpr.h | 60 +++++----------- src/xenxs/xen_xm.c | 35 +++++----- src/xenxs/xen_xm.h | 8 +- tests/sexpr2xmltest.c | 3 +- tests/xmconfigtest.c | 4 +- tests/xml2sexprtest.c | 2 +- 10 files changed, 168 insertions(+), 196 deletions(-)
On Mon, Feb 21, 2011 at 02:40:12PM +0100, Markus Groß wrote:
--- src/xen/xen_driver.c | 8 +- src/xen/xend_internal.c | 54 +++++++------- src/xen/xm_internal.c | 6 +- src/xenxs/xen_sxpr.c | 184 +++++++++++++++++++++++------------------------ src/xenxs/xen_sxpr.h | 60 +++++----------- src/xenxs/xen_xm.c | 35 +++++----- src/xenxs/xen_xm.h | 8 +- tests/sexpr2xmltest.c | 3 +- tests/xmconfigtest.c | 4 +- tests/xml2sexprtest.c | 2 +- 10 files changed, 168 insertions(+), 196 deletions(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--- AUTHORS | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
On 02/21/2011 06:40 AM, Markus Groß wrote:
--- AUTHORS | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
Technically, for bisection purposes of 'make syntax-check', this should be squashed into patch 1/8. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Markus Groß