[libvirt] [PATCH] daemon: Do not lose storage driver
Per WITH_STORAGE_DIR is always defined as long as libvirtd is built, it's fine to use it as the condition. --- daemon/libvirtd.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index c74cd43..7db02e3 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -366,7 +366,7 @@ static void daemonInitialize(void) # ifdef WITH_NETWORK virDriverLoadModule("network"); # endif -# ifdef WITH_STORAGE +# ifdef WITH_STORAGE_DIR virDriverLoadModule("storage"); # endif # ifdef WITH_NODE_DEVICES -- 1.7.7.3
On 06/05/2012 04:12 AM, Osier Yang wrote:
Per WITH_STORAGE_DIR is always defined as long as libvirtd is built,
s/Per/Since/
it's fine to use it as the condition. --- daemon/libvirtd.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index c74cd43..7db02e3 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -366,7 +366,7 @@ static void daemonInitialize(void) # ifdef WITH_NETWORK virDriverLoadModule("network"); # endif -# ifdef WITH_STORAGE +# ifdef WITH_STORAGE_DIR virDriverLoadModule("storage"); # endif # ifdef WITH_NODE_DEVICES
Incomplete. Grepping the sources for uses of 'WITH_STORAGE*', I see: tests/virdrivermoduletest.c:#ifdef WITH_STORAGE as another instance of an undefined macro usage. Also, configure.ac _does_ allow you to disable WITH_STORAGE_DIR but still build libvirtd. I think the real solution is to add something like this to configure.ac: for tmp in dir fd lvm iscsi scsi mpath rbd disk; do if eval test \$with_storage_$tmp = yes; then AC_DEFINE([WITH_STORAGE], [1], [Define to 1 if at least one storage backend is in use]) break fi done At which point, WITH_STORAGE will really be what we want to know that at least one storage driver was built and not explicitly disabled by configure options. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Commit 1c275e9a accidentally dropped the storage driver from libvirtd, because it depended on a C preprocessor macro that was not defined. Furthermore, if you do './configure --without-storage-dir --with-storage-disk' or any other combination where you explicitly build a subset of storage backends excluding the dir backend, then the build is broken. Based on analysis by Osier Yang. * configure.ac (WITH_STORAGE): Define top-level conditional. * src/Makefile.am (mod_LTLIBRARIES): Build driver even when storage_dir is disabled. * daemon/libvirtd.c: Pick up storage driver for any backend, not just dir. * daemon/Makefile.am (libvirtd_LDADD): Likewise. --- v2: incorporate my suggestions on Osier's v1. configure.ac | 13 +++++++++++++ daemon/Makefile.am | 2 +- daemon/libvirtd.c | 4 ++-- src/Makefile.am | 2 +- 4 files changed, 17 insertions(+), 4 deletions(-) diff --git a/configure.ac b/configure.ac index 7d8d26b..0c65279 100644 --- a/configure.ac +++ b/configure.ac @@ -2076,6 +2076,19 @@ fi AC_SUBST([DEVMAPPER_CFLAGS]) AC_SUBST([DEVMAPPER_LIBS]) +with_storage=no +for backend in dir fd lvm iscsi scsi mpath rbd disk; do + if eval test \$with_storage_$backend = yes; then + with_storage=yes + break + fi +done +if test $with_storage = yes; then + AC_DEFINE([WITH_STORAGE], [1], + [Define to 1 if at least one storage backend is in use]) +fi +AM_CONDITIONAL([WITH_STORAGE], [test "$with_storage" = "yes"]) + dnl dnl check for libcurl (ESX/XenAPI) dnl diff --git a/daemon/Makefile.am b/daemon/Makefile.am index b1518c5..fbb0ae1 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -140,7 +140,7 @@ if WITH_UML libvirtd_LDADD += ../src/libvirt_driver_uml.la endif -if WITH_STORAGE_DIR +if WITH_STORAGE libvirtd_LDADD += ../src/libvirt_driver_storage.la endif diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index c74cd43..d5ad05e 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -77,7 +77,7 @@ # ifdef WITH_NETCF # include "interface/netcf_driver.h" # endif -# ifdef WITH_STORAGE_DIR +# ifdef WITH_STORAGE # include "storage/storage_driver.h" # endif # ifdef WITH_NODE_DEVICES @@ -403,7 +403,7 @@ static void daemonInitialize(void) # ifdef WITH_NETCF interfaceRegister(); # endif -# ifdef WITH_STORAGE_DIR +# ifdef WITH_STORAGE storageRegister(); # endif # ifdef WITH_NODE_DEVICES diff --git a/src/Makefile.am b/src/Makefile.am index dd1af65..2d737af 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -967,7 +967,7 @@ if HAVE_LIBBLKID libvirt_driver_storage_la_CFLAGS += $(BLKID_CFLAGS) libvirt_driver_storage_la_LIBADD += $(BLKID_LIBS) endif -if WITH_STORAGE_DIR +if WITH_STORAGE if WITH_DRIVER_MODULES mod_LTLIBRARIES += libvirt_driver_storage.la else -- 1.7.10.2
On 2012年06月06日 00:28, Eric Blake wrote:
Commit 1c275e9a accidentally dropped the storage driver from libvirtd, because it depended on a C preprocessor macro that was not defined. Furthermore, if you do './configure --without-storage-dir --with-storage-disk' or any other combination where you explicitly build a subset of storage backends excluding the dir backend, then the build is broken.
Based on analysis by Osier Yang.
* configure.ac (WITH_STORAGE): Define top-level conditional. * src/Makefile.am (mod_LTLIBRARIES): Build driver even when storage_dir is disabled. * daemon/libvirtd.c: Pick up storage driver for any backend, not just dir. * daemon/Makefile.am (libvirtd_LDADD): Likewise. ---
v2: incorporate my suggestions on Osier's v1.
configure.ac | 13 +++++++++++++ daemon/Makefile.am | 2 +- daemon/libvirtd.c | 4 ++-- src/Makefile.am | 2 +- 4 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/configure.ac b/configure.ac index 7d8d26b..0c65279 100644 --- a/configure.ac +++ b/configure.ac @@ -2076,6 +2076,19 @@ fi AC_SUBST([DEVMAPPER_CFLAGS]) AC_SUBST([DEVMAPPER_LIBS])
+with_storage=no +for backend in dir fd lvm iscsi scsi mpath rbd disk; do
s/fd/fs/
+ if eval test \$with_storage_$backend = yes; then + with_storage=yes + break + fi +done +if test $with_storage = yes; then + AC_DEFINE([WITH_STORAGE], [1], + [Define to 1 if at least one storage backend is in use]) +fi +AM_CONDITIONAL([WITH_STORAGE], [test "$with_storage" = "yes"]) + dnl dnl check for libcurl (ESX/XenAPI) dnl diff --git a/daemon/Makefile.am b/daemon/Makefile.am index b1518c5..fbb0ae1 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -140,7 +140,7 @@ if WITH_UML libvirtd_LDADD += ../src/libvirt_driver_uml.la endif
-if WITH_STORAGE_DIR +if WITH_STORAGE libvirtd_LDADD += ../src/libvirt_driver_storage.la endif
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index c74cd43..d5ad05e 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -77,7 +77,7 @@ # ifdef WITH_NETCF # include "interface/netcf_driver.h" # endif -# ifdef WITH_STORAGE_DIR +# ifdef WITH_STORAGE # include "storage/storage_driver.h" # endif # ifdef WITH_NODE_DEVICES @@ -403,7 +403,7 @@ static void daemonInitialize(void) # ifdef WITH_NETCF interfaceRegister(); # endif -# ifdef WITH_STORAGE_DIR +# ifdef WITH_STORAGE storageRegister(); # endif # ifdef WITH_NODE_DEVICES diff --git a/src/Makefile.am b/src/Makefile.am index dd1af65..2d737af 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -967,7 +967,7 @@ if HAVE_LIBBLKID libvirt_driver_storage_la_CFLAGS += $(BLKID_CFLAGS) libvirt_driver_storage_la_LIBADD += $(BLKID_LIBS) endif -if WITH_STORAGE_DIR +if WITH_STORAGE if WITH_DRIVER_MODULES mod_LTLIBRARIES += libvirt_driver_storage.la else
I missed the other use case of WITH_STORAGE, thanks for the v2, ACK and pushed with the nit fixed. (it blocks my process on the storage stuff, :-) Osier
participants (2)
-
Eric Blake -
Osier Yang