[libvirt] [PATCH 00/11] storage: modularize the storage driver backends

Split up the storage driver backends into loadable modules so that binary distributions don't have to compromise on shipping the storage driver with all backends which may pull in too many dependencies. Peter Krempa (11): configure: Fix configure output for RBD storage backend tests: storagepoolxml2xml: Remove compile conditionals tests: drivermodule: Drop unused macro arguments driver: Split/refactor driver module loading daemon: Refactor connection driver module loading storage: backend: Refactor registration of the backend drivers storage: Turn driver backends into (static) modules storage: Turn storage backends into dynamic modules tests: drivermodule: Make sure that all compiled storage backends can be loaded spec: Modularize the storage driver news: Mention storage driver split daemon/libvirtd.c | 136 +++++++++------------- docs/news.xml | 10 ++ libvirt.spec.in | 185 +++++++++++++++++++++++++----- m4/virt-storage-rbd.m4 | 2 +- src/Makefile.am | 199 ++++++++++++++++++++++++++++++--- src/driver.c | 136 ++++++++++++++-------- src/driver.h | 6 +- src/libvirt_driver_modules.syms | 1 + src/storage/storage_backend.c | 151 +++++++++++++++++++------ src/storage/storage_backend.h | 5 + src/storage/storage_backend_disk.c | 7 ++ src/storage/storage_backend_disk.h | 4 +- src/storage/storage_backend_fs.c | 27 +++++ src/storage/storage_backend_fs.h | 11 +- src/storage/storage_backend_gluster.c | 13 ++- src/storage/storage_backend_gluster.h | 5 +- src/storage/storage_backend_iscsi.c | 7 ++ src/storage/storage_backend_iscsi.h | 4 +- src/storage/storage_backend_logical.c | 7 ++ src/storage/storage_backend_logical.h | 4 +- src/storage/storage_backend_mpath.c | 8 ++ src/storage/storage_backend_mpath.h | 4 +- src/storage/storage_backend_rbd.c | 7 ++ src/storage/storage_backend_rbd.h | 4 +- src/storage/storage_backend_scsi.c | 7 ++ src/storage/storage_backend_scsi.h | 4 +- src/storage/storage_backend_sheepdog.c | 7 ++ src/storage/storage_backend_sheepdog.h | 4 +- src/storage/storage_backend_vstorage.c | 7 ++ src/storage/storage_backend_vstorage.h | 4 +- src/storage/storage_backend_zfs.c | 7 ++ src/storage/storage_backend_zfs.h | 4 +- src/storage/storage_driver.c | 19 +++- src/storage/storage_driver.h | 1 + tests/Makefile.am | 4 +- tests/storagepoolxml2xmltest.c | 6 - tests/virdrivermoduletest.c | 52 +++++---- tests/virstoragetest.c | 4 + 38 files changed, 788 insertions(+), 285 deletions(-) -- 2.11.0

We'd print status for the 'dir' backend instead of the correct one. --- m4/virt-storage-rbd.m4 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/m4/virt-storage-rbd.m4 b/m4/virt-storage-rbd.m4 index 0104b7cfc..48522a6a7 100644 --- a/m4/virt-storage-rbd.m4 +++ b/m4/virt-storage-rbd.m4 @@ -41,7 +41,7 @@ AC_DEFUN([LIBVIRT_STORAGE_CHECK_RBD], [ ]) AC_DEFUN([LIBVIRT_STORAGE_RESULT_RBD], [ - LIBVIRT_RESULT([RBD], [$with_storage_dir]) + LIBVIRT_RESULT([RBD], [$with_storage_rbd]) ]) AC_DEFUN([LIBVIRT_RESULT_RBD], [ -- 2.11.0

On 02/08/2017 11:27 AM, Peter Krempa wrote:
We'd print status for the 'dir' backend instead of the correct one. --- m4/virt-storage-rbd.m4 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
ACK John

The XML2XML test should work properly even if the storage backend is disabled, since it does not use it. --- tests/storagepoolxml2xmltest.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c index 98a844926..79bdc26da 100644 --- a/tests/storagepoolxml2xmltest.c +++ b/tests/storagepoolxml2xmltest.c @@ -97,16 +97,10 @@ mymain(void) DO_TEST("pool-gluster"); DO_TEST("pool-gluster-sub"); DO_TEST("pool-scsi-type-scsi-host-stable"); -#ifdef WITH_STORAGE_ZFS DO_TEST("pool-zfs"); DO_TEST("pool-zfs-sourcedev"); -#endif -#ifdef WITH_STORAGE_RBD DO_TEST("pool-rbd"); -#endif -#ifdef WITH_STORAGE_VSTORAGE DO_TEST("pool-vstorage"); -#endif return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.11.0

On 02/08/2017 11:27 AM, Peter Krempa wrote:
The XML2XML test should work properly even if the storage backend is disabled, since it does not use it. --- tests/storagepoolxml2xmltest.c | 6 ------ 1 file changed, 6 deletions(-)
ACK John

Refactors of the test resulted into the second argument of the 'TEST' macro to be unused. Drop them. --- tests/virdrivermoduletest.c | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/tests/virdrivermoduletest.c b/tests/virdrivermoduletest.c index 870a07de6..09c1a614b 100644 --- a/tests/virdrivermoduletest.c +++ b/tests/virdrivermoduletest.c @@ -47,53 +47,50 @@ mymain(void) { int ret = 0; -#define TEST(name, dep1) \ +#define TEST(name) \ do { \ if (virTestRun("Test driver " # name, testDriverModule, name) < 0) \ ret = -1; \ } while (0) #ifdef WITH_NETWORK -# define USE_NETWORK "network" - TEST("network", NULL); -#else -# define USE_NETWORK NULL + TEST("network"); #endif #ifdef WITH_INTERFACE - TEST("interface", NULL); + TEST("interface"); #endif #ifdef WITH_STORAGE - TEST("storage", NULL); + TEST("storage"); #endif #ifdef WITH_NODE_DEVICES - TEST("nodedev", NULL); + TEST("nodedev"); #endif #ifdef WITH_SECRETS - TEST("secret", NULL); + TEST("secret"); #endif #ifdef WITH_NWFILTER - TEST("nwfilter", NULL); + TEST("nwfilter"); #endif #ifdef WITH_XEN - TEST("xen", NULL); + TEST("xen"); #endif #ifdef WITH_LIBXL - TEST("libxl", NULL); + TEST("libxl"); #endif #ifdef WITH_QEMU - TEST("qemu", USE_NETWORK); + TEST("qemu"); #endif #ifdef WITH_LXC - TEST("lxc", USE_NETWORK); + TEST("lxc"); #endif #ifdef WITH_UML - TEST("uml", NULL); + TEST("uml"); #endif #ifdef WITH_VBOX - TEST("vbox", NULL); + TEST("vbox"); #endif #ifdef WITH_BHYVE - TEST("bhyve", NULL); + TEST("bhyve"); #endif return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; -- 2.11.0

On 02/08/2017 11:27 AM, Peter Krempa wrote:
Refactors of the test resulted into the second argument of the 'TEST' macro to be unused. Drop them. --- tests/virdrivermoduletest.c | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-)
ACK John

Split the convoluted driver loader function into simpler parts which will potentially allow reuse. --- src/driver.c | 133 +++++++++++++++++++++++++++++----------- src/driver.h | 3 + src/libvirt_driver_modules.syms | 1 + 3 files changed, 101 insertions(+), 36 deletions(-) diff --git a/src/driver.c b/src/driver.c index 67ac02006..783e08a28 100644 --- a/src/driver.c +++ b/src/driver.c @@ -43,15 +43,105 @@ VIR_LOG_INIT("driver"); # include <dlfcn.h> # define DEFAULT_DRIVER_DIR LIBDIR "/libvirt/connection-driver" + +static void * +virDriverLoadModuleFile(const char *file) +{ + void *handle = NULL; + + VIR_DEBUG("Load module file '%s'", file); + + if (access(file, R_OK) < 0) { + VIR_INFO("Module %s not accessible", file); + return NULL; + } + + virUpdateSelfLastChanged(file); + + if (!(handle = dlopen(file, RTLD_NOW | RTLD_GLOBAL))) + VIR_ERROR(_("failed to load module %s %s"), file, dlerror()); + + return handle; +} + + +static void * +virDriverLoadModuleFunc(void *handle, + const char *funcname) +{ + void *regsym; + + VIR_DEBUG("Lookup function '%s'", funcname); + + if (!(regsym = dlsym(handle, funcname))) + VIR_ERROR(_("Missing module registration symbol %s"), funcname); + + return regsym; +} + + +/** + * virDriverLoadModuleFull: + * @path: filename of module to load + * @regfunc: name of the function that registers the module + * @handle: Returns handle of the loaded library if not NULL + * + * Loads a loadable module named @path and calls the + * registration function @regfunc. If @handle is not NULL the handle is returned + * in the variable. Otherwise the handle is leaked so that the module stays + * loaded forever. + * + * The module is automatically looked up in the appropriate place (git or + * installed directory). + * + * Returns 0 on success, 1 if the module was not found and -1 on any error. + */ +int +virDriverLoadModuleFull(const char *path, + const char *regfunc, + void **handle) +{ + void *rethandle = NULL; + int (*regsym)(void); + int ret = -1; + + VIR_DEBUG("Module load %s", path); + + if (!(rethandle = virDriverLoadModuleFile(path))) { + ret = 1; + goto cleanup; + } + + if (!(regsym = virDriverLoadModuleFunc(rethandle, regfunc))) + goto cleanup; + + if ((*regsym)() < 0) { + VIR_ERROR(_("Failed module registration %s"), regfunc); + goto cleanup; + } + + if (handle) + VIR_STEAL_PTR(*handle, rethandle); + else + rethandle = NULL; + + ret = 0; + + cleanup: + if (rethandle) + dlclose(rethandle); + return ret; +} + + void * virDriverLoadModule(const char *name) { - char *modfile = NULL, *regfunc = NULL, *fixedname = NULL; + char *modfile = NULL; + char *fixedname = NULL; + char *regfunc = NULL; char *tmp; void *handle = NULL; - int (*regsym)(void); - - VIR_DEBUG("Module load %s", name); if (!(modfile = virFileFindResourceFull(name, "libvirt_driver_", @@ -61,19 +151,6 @@ virDriverLoadModule(const char *name) "LIBVIRT_DRIVER_DIR"))) return NULL; - if (access(modfile, R_OK) < 0) { - VIR_INFO("Module %s not accessible", modfile); - goto cleanup; - } - - virUpdateSelfLastChanged(modfile); - - handle = dlopen(modfile, RTLD_NOW | RTLD_GLOBAL); - if (!handle) { - VIR_ERROR(_("failed to load module %s %s"), modfile, dlerror()); - goto cleanup; - } - if (VIR_STRDUP_QUIET(fixedname, name) < 0) { VIR_ERROR(_("out of memory")); goto cleanup; @@ -88,29 +165,13 @@ virDriverLoadModule(const char *name) if (virAsprintfQuiet(®func, "%sRegister", fixedname) < 0) goto cleanup; - regsym = dlsym(handle, regfunc); - if (!regsym) { - VIR_ERROR(_("Missing module registration symbol %s"), regfunc); - goto cleanup; - } - - if ((*regsym)() < 0) { - VIR_ERROR(_("Failed module registration %s"), regfunc); - goto cleanup; - } - - VIR_FREE(modfile); - VIR_FREE(regfunc); - VIR_FREE(fixedname); - return handle; + virDriverLoadModuleFull(modfile, regfunc, &handle); cleanup: VIR_FREE(modfile); - VIR_FREE(regfunc); VIR_FREE(fixedname); - if (handle) - dlclose(handle); - return NULL; + VIR_FREE(regfunc); + return handle; } diff --git a/src/driver.h b/src/driver.h index e4e382b63..885e8843e 100644 --- a/src/driver.h +++ b/src/driver.h @@ -100,5 +100,8 @@ int virSetSharedSecretDriver(virSecretDriverPtr driver) ATTRIBUTE_RETURN_CHECK; int virSetSharedStorageDriver(virStorageDriverPtr driver) ATTRIBUTE_RETURN_CHECK; void *virDriverLoadModule(const char *name); +int virDriverLoadModuleFull(const char *name, + const char *regfunc, + void **handle); #endif /* __VIR_DRIVER_H__ */ diff --git a/src/libvirt_driver_modules.syms b/src/libvirt_driver_modules.syms index f9d0ee9b9..bd9bf1c31 100644 --- a/src/libvirt_driver_modules.syms +++ b/src/libvirt_driver_modules.syms @@ -4,6 +4,7 @@ # driver.h virDriverLoadModule; +virDriverLoadModuleFull; # Let emacs know we want case-insensitive sorting # Local Variables: -- 2.11.0

On 02/08/2017 11:27 AM, Peter Krempa wrote:
Split the convoluted driver loader function into simpler parts which will potentially allow reuse. --- src/driver.c | 133 +++++++++++++++++++++++++++++----------- src/driver.h | 3 + src/libvirt_driver_modules.syms | 1 + 3 files changed, 101 insertions(+), 36 deletions(-)
diff --git a/src/driver.c b/src/driver.c index 67ac02006..783e08a28 100644 --- a/src/driver.c +++ b/src/driver.c @@ -43,15 +43,105 @@ VIR_LOG_INIT("driver"); # include <dlfcn.h> # define DEFAULT_DRIVER_DIR LIBDIR "/libvirt/connection-driver"
+ +static void * +virDriverLoadModuleFile(const char *file) +{ + void *handle = NULL; + + VIR_DEBUG("Load module file '%s'", file);
NIT: Moving this here removes from the more general [1] location, no big deal. Still should the virFileFindResourceFull failure path get VIR_ERROR too or would the virReportOOMErrorFull be "good enough"? ACK in general - just trying to think about odd failures... John

Pass the registration function name to virDriverLoadModule so that we can later call specific functions if necessary (e.g. for testing purposes). This gets rid of the rather ugly automatic name generator and unifies the code to load/initialize the modules. It's also clear which registration function gets called. --- daemon/libvirtd.c | 136 ++++++++++++++++---------------------------- src/driver.c | 37 +++--------- src/driver.h | 3 +- tests/virdrivermoduletest.c | 23 ++++++-- 4 files changed, 77 insertions(+), 122 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index b6d76ed84..2f9f5c77d 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -341,6 +341,14 @@ static int daemonErrorLogFilter(virErrorPtr err, int priority) return priority; } + +#ifdef WITH_DRIVER_MODULES +# define VIR_DAEMON_LOAD_MODULE(func, module) \ + virDriverLoadModule(module, #func) +#else +# define VIR_DAEMON_LOAD_MODULE(func, module) \ + func() +#endif static void daemonInitialize(void) { /* @@ -350,99 +358,55 @@ static void daemonInitialize(void) * driver, since their resources must be auto-started before any * domains can be auto-started. */ -#ifdef WITH_DRIVER_MODULES /* We don't care if any of these fail, because the whole point * is to allow users to only install modules they want to use. * If they try to open a connection for a module that * is not loaded they'll get a suitable error at that point */ -# ifdef WITH_NETWORK - virDriverLoadModule("network"); -# endif -# ifdef WITH_INTERFACE - virDriverLoadModule("interface"); -# endif -# ifdef WITH_STORAGE - virDriverLoadModule("storage"); -# endif -# ifdef WITH_NODE_DEVICES - virDriverLoadModule("nodedev"); -# endif -# ifdef WITH_SECRETS - virDriverLoadModule("secret"); -# endif -# ifdef WITH_NWFILTER - virDriverLoadModule("nwfilter"); -# endif -# ifdef WITH_XEN - virDriverLoadModule("xen"); -# endif -# ifdef WITH_LIBXL - virDriverLoadModule("libxl"); -# endif -# ifdef WITH_QEMU - virDriverLoadModule("qemu"); -# endif -# ifdef WITH_LXC - virDriverLoadModule("lxc"); -# endif -# ifdef WITH_UML - virDriverLoadModule("uml"); -# endif -# ifdef WITH_VBOX - virDriverLoadModule("vbox"); -# endif -# ifdef WITH_BHYVE - virDriverLoadModule("bhyve"); -# endif -# ifdef WITH_VZ - virDriverLoadModule("vz"); -# endif -#else -# ifdef WITH_NETWORK - networkRegister(); -# endif -# ifdef WITH_INTERFACE - interfaceRegister(); -# endif -# ifdef WITH_STORAGE - storageRegister(); -# endif -# ifdef WITH_NODE_DEVICES - nodedevRegister(); -# endif -# ifdef WITH_SECRETS - secretRegister(); -# endif -# ifdef WITH_NWFILTER - nwfilterRegister(); -# endif -# ifdef WITH_XEN - xenRegister(); -# endif -# ifdef WITH_LIBXL - libxlRegister(); -# endif -# ifdef WITH_QEMU - qemuRegister(); -# endif -# ifdef WITH_LXC - lxcRegister(); -# endif -# ifdef WITH_UML - umlRegister(); -# endif -# ifdef WITH_VBOX - vboxRegister(); -# endif -# ifdef WITH_BHYVE - bhyveRegister(); -# endif -# ifdef WITH_VZ - vzRegister(); -# endif +#ifdef WITH_NETWORK + VIR_DAEMON_LOAD_MODULE(networkRegister, "network"); +#endif +#ifdef WITH_INTERFACE + VIR_DAEMON_LOAD_MODULE(interfaceRegister, "interface"); +#endif +#ifdef WITH_STORAGE + VIR_DAEMON_LOAD_MODULE(storageRegister, "storage"); +#endif +#ifdef WITH_NODE_DEVICES + VIR_DAEMON_LOAD_MODULE(nodedevRegister, "nodedev"); +#endif +#ifdef WITH_SECRETS + VIR_DAEMON_LOAD_MODULE(secretRegister, "secret"); +#endif +#ifdef WITH_NWFILTER + VIR_DAEMON_LOAD_MODULE(nwfilterRegister, "nwfilter"); +#endif +#ifdef WITH_XEN + VIR_DAEMON_LOAD_MODULE(xenRegister, "xen"); +#endif +#ifdef WITH_LIBXL + VIR_DAEMON_LOAD_MODULE(libxlRegister, "libxl"); +#endif +#ifdef WITH_QEMU + VIR_DAEMON_LOAD_MODULE(qemuRegister, "qemu"); +#endif +#ifdef WITH_LXC + VIR_DAEMON_LOAD_MODULE(lxcRegister, "lxc"); +#endif +#ifdef WITH_UML + VIR_DAEMON_LOAD_MODULE(umlRegister, "uml"); +#endif +#ifdef WITH_VBOX + VIR_DAEMON_LOAD_MODULE(vboxRegister, "vbox"); +#endif +#ifdef WITH_BHYVE + VIR_DAEMON_LOAD_MODULE(bhyveRegister, "bhyve"); +#endif +#ifdef WITH_VZ + VIR_DAEMON_LOAD_MODULE(vzRegister, "vz"); #endif } +#undef VIR_DAEMON_LOAD_MODULE static int ATTRIBUTE_NONNULL(3) diff --git a/src/driver.c b/src/driver.c index 783e08a28..f47bea0fb 100644 --- a/src/driver.c +++ b/src/driver.c @@ -23,15 +23,12 @@ #include <config.h> #include <unistd.h> -#include <c-ctype.h> #include "driver.h" #include "viralloc.h" #include "virfile.h" #include "virlog.h" -#include "virutil.h" #include "configmake.h" -#include "virstring.h" VIR_LOG_INIT("driver"); @@ -134,14 +131,12 @@ virDriverLoadModuleFull(const char *path, } -void * -virDriverLoadModule(const char *name) +int +virDriverLoadModule(const char *name, + const char *regfunc) { char *modfile = NULL; - char *fixedname = NULL; - char *regfunc = NULL; - char *tmp; - void *handle = NULL; + int ret; if (!(modfile = virFileFindResourceFull(name, "libvirt_driver_", @@ -149,29 +144,13 @@ virDriverLoadModule(const char *name) abs_topbuilddir "/src/.libs", DEFAULT_DRIVER_DIR, "LIBVIRT_DRIVER_DIR"))) - return NULL; - - if (VIR_STRDUP_QUIET(fixedname, name) < 0) { - VIR_ERROR(_("out of memory")); - goto cleanup; - } - - /* convert something_like_this into somethingLikeThis */ - while ((tmp = strchr(fixedname, '_'))) { - memmove(tmp, tmp + 1, strlen(tmp)); - *tmp = c_toupper(*tmp); - } - - if (virAsprintfQuiet(®func, "%sRegister", fixedname) < 0) - goto cleanup; + return 1; - virDriverLoadModuleFull(modfile, regfunc, &handle); + ret = virDriverLoadModuleFull(modfile, regfunc, NULL); - cleanup: VIR_FREE(modfile); - VIR_FREE(fixedname); - VIR_FREE(regfunc); - return handle; + + return ret; } diff --git a/src/driver.h b/src/driver.h index 885e8843e..420f6455d 100644 --- a/src/driver.h +++ b/src/driver.h @@ -99,7 +99,8 @@ int virSetSharedNWFilterDriver(virNWFilterDriverPtr driver) ATTRIBUTE_RETURN_CHE int virSetSharedSecretDriver(virSecretDriverPtr driver) ATTRIBUTE_RETURN_CHECK; int virSetSharedStorageDriver(virStorageDriverPtr driver) ATTRIBUTE_RETURN_CHECK; -void *virDriverLoadModule(const char *name); +int virDriverLoadModule(const char *name, + const char *regfunc); int virDriverLoadModuleFull(const char *name, const char *regfunc, void **handle); diff --git a/tests/virdrivermoduletest.c b/tests/virdrivermoduletest.c index 09c1a614b..e440350c2 100644 --- a/tests/virdrivermoduletest.c +++ b/tests/virdrivermoduletest.c @@ -30,12 +30,18 @@ VIR_LOG_INIT("tests.drivermoduletest"); +struct testDriverModuleData { + const char *module; + const char *regfunc; +}; + + static int testDriverModule(const void *args) { - const char *name = args; + const struct testDriverModuleData *data = args; /* coverity[leaked_storage] */ - if (!virDriverLoadModule(name)) + if (virDriverLoadModule(data->module, data->regfunc) != 0) return -1; return 0; @@ -46,13 +52,18 @@ static int mymain(void) { int ret = 0; + struct testDriverModuleData data; -#define TEST(name) \ - do { \ - if (virTestRun("Test driver " # name, testDriverModule, name) < 0) \ - ret = -1; \ +#define TEST_FULL(name, fnc) \ + do { \ + data.module = name; \ + data.regfunc = fnc; \ + if (virTestRun("Test driver " # name, testDriverModule, &data) < 0) \ + ret = -1; \ } while (0) +#define TEST(name) TEST_FULL(name, name "Register") + #ifdef WITH_NETWORK TEST("network"); #endif -- 2.11.0

On 02/08/2017 11:27 AM, Peter Krempa wrote:
Pass the registration function name to virDriverLoadModule so that we can later call specific functions if necessary (e.g. for testing purposes). This gets rid of the rather ugly automatic name generator and unifies the code to load/initialize the modules.
It's also clear which registration function gets called. --- daemon/libvirtd.c | 136 ++++++++++++++++---------------------------- src/driver.c | 37 +++--------- src/driver.h | 3 +- tests/virdrivermoduletest.c | 23 ++++++-- 4 files changed, 77 insertions(+), 122 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index b6d76ed84..2f9f5c77d 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -341,6 +341,14 @@ static int daemonErrorLogFilter(virErrorPtr err, int priority) return priority; }
+ +#ifdef WITH_DRIVER_MODULES +# define VIR_DAEMON_LOAD_MODULE(func, module) \ + virDriverLoadModule(module, #func) +#else +# define VIR_DAEMON_LOAD_MODULE(func, module) \ + func() +#endif
I found the ordering of arguments to the macro in a different order than to the virDriverLoadModule to be confusing. Your call to change, Also wasn't sure if there was a "compelling enough" reason to check status from virDriverLoadModule and perhaps spit out a VIR_{DEBUG|INFO|WARN} type message... ACK for the change though - your call on the debug stuff. John

Add APIs that allow to dynamically register driver backends so that the list of available drivers does not need to be known during compile time. This will allow us to modularize the storage driver on runtime. --- src/storage/storage_backend.c | 111 ++++++++++++++++++++++----------- src/storage/storage_backend.h | 5 ++ src/storage/storage_backend_disk.c | 7 +++ src/storage/storage_backend_disk.h | 4 +- src/storage/storage_backend_fs.c | 27 ++++++++ src/storage/storage_backend_fs.h | 11 +--- src/storage/storage_backend_gluster.c | 13 +++- src/storage/storage_backend_gluster.h | 5 +- src/storage/storage_backend_iscsi.c | 7 +++ src/storage/storage_backend_iscsi.h | 4 +- src/storage/storage_backend_logical.c | 7 +++ src/storage/storage_backend_logical.h | 4 +- src/storage/storage_backend_mpath.c | 8 +++ src/storage/storage_backend_mpath.h | 4 +- src/storage/storage_backend_rbd.c | 7 +++ src/storage/storage_backend_rbd.h | 4 +- src/storage/storage_backend_scsi.c | 7 +++ src/storage/storage_backend_scsi.h | 4 +- src/storage/storage_backend_sheepdog.c | 7 +++ src/storage/storage_backend_sheepdog.h | 4 +- src/storage/storage_backend_vstorage.c | 7 +++ src/storage/storage_backend_vstorage.h | 4 +- src/storage/storage_backend_zfs.c | 7 +++ src/storage/storage_backend_zfs.h | 4 +- src/storage/storage_driver.c | 2 + tests/virstoragetest.c | 4 ++ 26 files changed, 200 insertions(+), 78 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 500d7567d..d8099be36 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -72,67 +72,106 @@ VIR_LOG_INIT("storage.storage_backend"); -static virStorageBackendPtr backends[] = { -#if WITH_STORAGE_DIR - &virStorageBackendDirectory, -#endif -#if WITH_STORAGE_FS - &virStorageBackendFileSystem, - &virStorageBackendNetFileSystem, +#define VIR_STORAGE_BACKENDS_MAX 20 + +static virStorageBackendPtr virStorageBackends[VIR_STORAGE_BACKENDS_MAX]; +static size_t virStorageBackendsCount; +static virStorageFileBackendPtr virStorageFileBackends[VIR_STORAGE_BACKENDS_MAX]; +static size_t virStorageFileBackendsCount; + +#define VIR_STORAGE_BACKEND_REGISTER(name) \ + if (name() < 0) \ + return -1 + +int +virStorageBackendDriversRegister(void) +{ +#if WITH_STORAGE_DIR || WITH_STORAGE_FS + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendFsRegister); #endif #if WITH_STORAGE_LVM - &virStorageBackendLogical, + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendLogicalRegister); #endif #if WITH_STORAGE_ISCSI - &virStorageBackendISCSI, + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendISCSIRegister); #endif #if WITH_STORAGE_SCSI - &virStorageBackendSCSI, + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendSCSIRegister); #endif #if WITH_STORAGE_MPATH - &virStorageBackendMpath, + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendMpathRegister); #endif #if WITH_STORAGE_DISK - &virStorageBackendDisk, + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendDiskRegister); #endif #if WITH_STORAGE_RBD - &virStorageBackendRBD, + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendRBDRegister); #endif #if WITH_STORAGE_SHEEPDOG - &virStorageBackendSheepdog, + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendSheepdogRegister); #endif #if WITH_STORAGE_GLUSTER - &virStorageBackendGluster, + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendGlusterRegister); #endif #if WITH_STORAGE_ZFS - &virStorageBackendZFS, + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendZFSRegister); #endif #if WITH_STORAGE_VSTORAGE - &virStorageBackendVstorage, + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendVstorageRegister); #endif - NULL -}; + return 0; +} +#undef VIR_STORAGE_BACKEND_REGISTER -static virStorageFileBackendPtr fileBackends[] = { -#if WITH_STORAGE_FS - &virStorageFileBackendFile, - &virStorageFileBackendBlock, -#endif -#if WITH_STORAGE_GLUSTER - &virStorageFileBackendGluster, -#endif - NULL -}; + +int +virStorageBackendRegister(virStorageBackendPtr backend) +{ + VIR_DEBUG("Registering storage backend '%s'", + virStorageTypeToString(backend->type)); + + if (virStorageBackendsCount >= VIR_STORAGE_BACKENDS_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Too many drivers, cannot register storage backend '%s'"), + virStorageTypeToString(backend->type)); + return -1; + } + + virStorageBackends[virStorageBackendsCount] = backend; + virStorageBackendsCount++; + return 0; +} + + +int +virStorageBackendFileRegister(virStorageFileBackendPtr backend) +{ + VIR_DEBUG("Registering storage file backend '%s' protocol '%s'", + virStorageTypeToString(backend->type), + virStorageNetProtocolTypeToString(backend->protocol)); + + if (virStorageFileBackendsCount >= VIR_STORAGE_BACKENDS_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Too many drivers, cannot register storage file " + "backend '%s'"), + virStorageTypeToString(backend->type)); + return -1; + } + + virStorageFileBackends[virStorageFileBackendsCount] = backend; + virStorageFileBackendsCount++; + return 0; +} virStorageBackendPtr virStorageBackendForType(int type) { size_t i; - for (i = 0; backends[i]; i++) - if (backends[i]->type == type) - return backends[i]; + for (i = 0; i < virStorageBackendsCount; i++) + if (virStorageBackends[i]->type == type) + return virStorageBackends[i]; virReportError(VIR_ERR_INTERNAL_ERROR, _("missing backend for pool type %d (%s)"), @@ -148,13 +187,13 @@ virStorageFileBackendForTypeInternal(int type, { size_t i; - for (i = 0; fileBackends[i]; i++) { - if (fileBackends[i]->type == type) { + for (i = 0; i < virStorageFileBackendsCount; i++) { + if (virStorageFileBackends[i]->type == type) { if (type == VIR_STORAGE_TYPE_NETWORK && - fileBackends[i]->protocol != protocol) + virStorageFileBackends[i]->protocol != protocol) continue; - return fileBackends[i]; + return virStorageFileBackends[i]; } } diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index b8fb368bb..ca6c19c45 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -198,4 +198,9 @@ struct _virStorageFileBackend { virStorageFileBackendChown storageFileChown; }; +int virStorageBackendDriversRegister(void); + +int virStorageBackendRegister(virStorageBackendPtr backend); +int virStorageBackendFileRegister(virStorageFileBackendPtr backend); + #endif /* __VIR_STORAGE_BACKEND_H__ */ diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 819f1e5c4..50bdd3646 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -937,3 +937,10 @@ virStorageBackend virStorageBackendDisk = { .downloadVol = virStorageBackendVolDownloadLocal, .wipeVol = virStorageBackendDiskVolWipe, }; + + +int +virStorageBackendDiskRegister(void) +{ + return virStorageBackendRegister(&virStorageBackendDisk); +} diff --git a/src/storage/storage_backend_disk.h b/src/storage/storage_backend_disk.h index aaabe6224..e614ca278 100644 --- a/src/storage/storage_backend_disk.h +++ b/src/storage/storage_backend_disk.h @@ -24,8 +24,6 @@ #ifndef __VIR_STORAGE_BACKEND_DISK_H__ # define __VIR_STORAGE_BACKEND_DISK_H__ -# include "storage_backend.h" - -extern virStorageBackend virStorageBackendDisk; +int virStorageBackendDiskRegister(void); #endif /* __VIR_STORAGE_BACKEND_DISK_H__ */ diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 54bcc5777..551fd945d 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -877,3 +877,30 @@ virStorageFileBackend virStorageFileBackendDir = { .storageFileGetUniqueIdentifier = virStorageFileBackendFileGetUniqueIdentifier, }; + + +int +virStorageBackendFsRegister(void) +{ + if (virStorageBackendRegister(&virStorageBackendDirectory) < 0) + return -1; + +#if WITH_STORAGE_FS + if (virStorageBackendRegister(&virStorageBackendFileSystem) < 0) + return -1; + + if (virStorageBackendRegister(&virStorageBackendNetFileSystem) < 0) + return -1; +#endif /* WITH_STORAGE_FS */ + + if (virStorageBackendFileRegister(&virStorageFileBackendFile) < 0) + return -1; + + if (virStorageBackendFileRegister(&virStorageFileBackendBlock) < 0) + return -1; + + if (virStorageBackendFileRegister(&virStorageFileBackendDir) < 0) + return -1; + + return 0; +} diff --git a/src/storage/storage_backend_fs.h b/src/storage/storage_backend_fs.h index 94fe11138..8f381352c 100644 --- a/src/storage/storage_backend_fs.h +++ b/src/storage/storage_backend_fs.h @@ -24,15 +24,6 @@ #ifndef __VIR_STORAGE_BACKEND_FS_H__ # define __VIR_STORAGE_BACKEND_FS_H__ -# include "storage_backend.h" +int virStorageBackendFsRegister(void); -# if WITH_STORAGE_FS -extern virStorageBackend virStorageBackendFileSystem; -extern virStorageBackend virStorageBackendNetFileSystem; -# endif - -extern virStorageBackend virStorageBackendDirectory; - -extern virStorageFileBackend virStorageFileBackendFile; -extern virStorageFileBackend virStorageFileBackendBlock; #endif /* __VIR_STORAGE_BACKEND_FS_H__ */ diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 7be2d9e81..52c9ee372 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -843,6 +843,17 @@ virStorageFileBackend virStorageFileBackendGluster = { .storageFileChown = virStorageFileBackendGlusterChown, .storageFileGetUniqueIdentifier = virStorageFileBackendGlusterGetUniqueIdentifier, +}; -}; +int +virStorageBackendGlusterRegister(void) +{ + if (virStorageBackendRegister(&virStorageBackendGluster) < 0) + return -1; + + if (virStorageBackendFileRegister(&virStorageFileBackendGluster) < 0) + return -1; + + return 0; +} diff --git a/src/storage/storage_backend_gluster.h b/src/storage/storage_backend_gluster.h index 679601624..91b8d8275 100644 --- a/src/storage/storage_backend_gluster.h +++ b/src/storage/storage_backend_gluster.h @@ -22,9 +22,6 @@ #ifndef __VIR_STORAGE_BACKEND_GLUSTER_H__ # define __VIR_STORAGE_BACKEND_GLUSTER_H__ -# include "storage_backend.h" - -extern virStorageBackend virStorageBackendGluster; -extern virStorageFileBackend virStorageFileBackendGluster; +int virStorageBackendGlusterRegister(void); #endif /* __VIR_STORAGE_BACKEND_GLUSTER_H__ */ diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index 281334124..866fa7415 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -437,3 +437,10 @@ virStorageBackend virStorageBackendISCSI = { .downloadVol = virStorageBackendVolDownloadLocal, .wipeVol = virStorageBackendVolWipeLocal, }; + + +int +virStorageBackendISCSIRegister(void) +{ + return virStorageBackendRegister(&virStorageBackendISCSI); +} diff --git a/src/storage/storage_backend_iscsi.h b/src/storage/storage_backend_iscsi.h index da3b22c44..98d2b3ef2 100644 --- a/src/storage/storage_backend_iscsi.h +++ b/src/storage/storage_backend_iscsi.h @@ -24,8 +24,6 @@ #ifndef __VIR_STORAGE_BACKEND_ISCSI_H__ # define __VIR_STORAGE_BACKEND_ISCSI_H__ -# include "storage_backend.h" - -extern virStorageBackend virStorageBackendISCSI; +int virStorageBackendISCSIRegister(void); #endif /* __VIR_STORAGE_BACKEND_ISCSI_H__ */ diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index b0191aa45..756c62e90 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -1112,3 +1112,10 @@ virStorageBackend virStorageBackendLogical = { .downloadVol = virStorageBackendVolDownloadLocal, .wipeVol = virStorageBackendLogicalVolWipe, }; + + +int +virStorageBackendLogicalRegister(void) +{ + return virStorageBackendRegister(&virStorageBackendLogical); +} diff --git a/src/storage/storage_backend_logical.h b/src/storage/storage_backend_logical.h index c646fd6a6..c0f62cd18 100644 --- a/src/storage/storage_backend_logical.h +++ b/src/storage/storage_backend_logical.h @@ -24,8 +24,6 @@ #ifndef __VIR_STORAGE_BACKEND_LOGICAL_H__ # define __VIR_STORAGE_BACKEND_LOGICAL_H__ -# include "storage_backend.h" - -extern virStorageBackend virStorageBackendLogical; +int virStorageBackendLogicalRegister(void); #endif /* __VIR_STORAGE_BACKEND_LOGICAL_H__ */ diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c index a5d692a07..4bb38bb52 100644 --- a/src/storage/storage_backend_mpath.c +++ b/src/storage/storage_backend_mpath.c @@ -32,6 +32,7 @@ #include "virerror.h" #include "storage_conf.h" #include "storage_backend.h" +#include "storage_backend_mpath.h" #include "viralloc.h" #include "virlog.h" #include "virfile.h" @@ -278,3 +279,10 @@ virStorageBackend virStorageBackendMpath = { .downloadVol = virStorageBackendVolDownloadLocal, .wipeVol = virStorageBackendVolWipeLocal, }; + + +int +virStorageBackendMpathRegister(void) +{ + return virStorageBackendRegister(&virStorageBackendMpath); +} diff --git a/src/storage/storage_backend_mpath.h b/src/storage/storage_backend_mpath.h index b66664576..c14dcc3cf 100644 --- a/src/storage/storage_backend_mpath.h +++ b/src/storage/storage_backend_mpath.h @@ -24,8 +24,6 @@ #ifndef __VIR_STORAGE_BACKEND_MPATH_H__ # define __VIR_STORAGE_BACKEND_MPATH_H__ -# include "storage_backend.h" - -extern virStorageBackend virStorageBackendMpath; +int virStorageBackendMpathRegister(void); #endif /* __VIR_STORAGE_BACKEND_MPATH_H__ */ diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 45beb107a..c806d6d30 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -1294,3 +1294,10 @@ virStorageBackend virStorageBackendRBD = { .resizeVol = virStorageBackendRBDResizeVol, .wipeVol = virStorageBackendRBDVolWipe }; + + +int +virStorageBackendRBDRegister(void) +{ + return virStorageBackendRegister(&virStorageBackendRBD); +} diff --git a/src/storage/storage_backend_rbd.h b/src/storage/storage_backend_rbd.h index e60caa957..21a43fd51 100644 --- a/src/storage/storage_backend_rbd.h +++ b/src/storage/storage_backend_rbd.h @@ -23,8 +23,6 @@ #ifndef __VIR_STORAGE_BACKEND_RBD_H__ # define __VIR_STORAGE_BACKEND_RBD_H__ -# include "storage_backend.h" - -extern virStorageBackend virStorageBackendRBD; +int virStorageBackendRBDRegister(void); #endif /* __VIR_STORAGE_BACKEND_RBD_H__ */ diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 0cc11486b..6e2e22a8c 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -531,3 +531,10 @@ virStorageBackend virStorageBackendSCSI = { .downloadVol = virStorageBackendVolDownloadLocal, .wipeVol = virStorageBackendVolWipeLocal, }; + + +int +virStorageBackendSCSIRegister(void) +{ + return virStorageBackendRegister(&virStorageBackendSCSI); +} diff --git a/src/storage/storage_backend_scsi.h b/src/storage/storage_backend_scsi.h index 1ba53a57c..efd01658b 100644 --- a/src/storage/storage_backend_scsi.h +++ b/src/storage/storage_backend_scsi.h @@ -24,8 +24,6 @@ #ifndef __VIR_STORAGE_BACKEND_SCSI_H__ # define __VIR_STORAGE_BACKEND_SCSI_H__ -# include "storage_backend.h" - -extern virStorageBackend virStorageBackendSCSI; +int virStorageBackendSCSIRegister(void); #endif /* __VIR_STORAGE_BACKEND_SCSI_H__ */ diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index 36458a562..a9a2301e6 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -417,3 +417,10 @@ virStorageBackend virStorageBackendSheepdog = { .deleteVol = virStorageBackendSheepdogDeleteVol, .resizeVol = virStorageBackendSheepdogResizeVol, }; + + +int +virStorageBackendSheepdogRegister(void) +{ + return virStorageBackendRegister(&virStorageBackendSheepdog); +} diff --git a/src/storage/storage_backend_sheepdog.h b/src/storage/storage_backend_sheepdog.h index df2ead5ed..e96832309 100644 --- a/src/storage/storage_backend_sheepdog.h +++ b/src/storage/storage_backend_sheepdog.h @@ -27,8 +27,6 @@ #ifndef __VIR_STORAGE_BACKEND_SHEEPDOG_H__ # define __VIR_STORAGE_BACKEND_SHEEPDOG_H__ -# include "storage_backend.h" - -extern virStorageBackend virStorageBackendSheepdog; +int virStorageBackendSheepdogRegister(void); #endif /* __VIR_STORAGE_BACKEND_SHEEPDOG_H__ */ diff --git a/src/storage/storage_backend_vstorage.c b/src/storage/storage_backend_vstorage.c index ac1fa756c..fb0613853 100644 --- a/src/storage/storage_backend_vstorage.c +++ b/src/storage/storage_backend_vstorage.c @@ -183,3 +183,10 @@ virStorageBackend virStorageBackendVstorage = { .downloadVol = virStorageBackendVolDownloadLocal, .wipeVol = virStorageBackendVolWipeLocal, }; + + +int +virStorageBackendVstorageRegister(void) +{ + return virStorageBackendRegister(&virStorageBackendVstorage); +} diff --git a/src/storage/storage_backend_vstorage.h b/src/storage/storage_backend_vstorage.h index 262e454c0..0a29c597f 100644 --- a/src/storage/storage_backend_vstorage.h +++ b/src/storage/storage_backend_vstorage.h @@ -21,8 +21,6 @@ #ifndef __VIR_STORAGE_BACKEND_VSTORAGE_H__ # define __VIR_STORAGE_BACKEND_VSTORAGE_H__ -# include "storage_backend.h" - -extern virStorageBackend virStorageBackendVstorage; +int virStorageBackendVstorageRegister(void); #endif /* __VIR_STORAGE_BACKEND_VSTORAGE_H__ */ diff --git a/src/storage/storage_backend_zfs.c b/src/storage/storage_backend_zfs.c index 70c533a7f..004d95a53 100644 --- a/src/storage/storage_backend_zfs.c +++ b/src/storage/storage_backend_zfs.c @@ -467,3 +467,10 @@ virStorageBackend virStorageBackendZFS = { .uploadVol = virStorageBackendVolUploadLocal, .downloadVol = virStorageBackendVolDownloadLocal, }; + + +int +virStorageBackendZFSRegister(void) +{ + return virStorageBackendRegister(&virStorageBackendZFS); +} diff --git a/src/storage/storage_backend_zfs.h b/src/storage/storage_backend_zfs.h index 4c34b5909..076ff2799 100644 --- a/src/storage/storage_backend_zfs.h +++ b/src/storage/storage_backend_zfs.h @@ -22,8 +22,6 @@ #ifndef __VIR_STORAGE_BACKEND_ZFS_H__ # define __VIR_STORAGE_BACKEND_ZFS_H__ -# include "storage_backend.h" - -extern virStorageBackend virStorageBackendZFS; +int virStorageBackendZFSRegister(void); #endif /* __VIR_STORAGE_BACKEND_ZFS_H__ */ diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index ed4772ad9..7fafbcf75 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2840,6 +2840,8 @@ static virStateDriver stateDriver = { int storageRegister(void) { + if (virStorageBackendDriversRegister() < 0) + return -1; if (virSetSharedStorageDriver(&storageDriver) < 0) return -1; if (virRegisterStateDriver(&stateDriver) < 0) diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index f766df115..d715fd762 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -32,6 +32,7 @@ #include "dirname.h" #include "storage/storage_driver.h" +#include "storage/storage_backend.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -731,6 +732,9 @@ mymain(void) virStorageSourcePtr chain2; /* short for chain->backingStore */ virStorageSourcePtr chain3; /* short for chain2->backingStore */ + if (virStorageBackendDriversRegister() < 0) + return -1; + /* Prep some files with qemu-img; if that is not found on PATH, or * if it lacks support for qcow2 and qed, skip this test. */ if ((ret = testPrepImages()) != 0) -- 2.11.0

On 02/08/2017 11:27 AM, Peter Krempa wrote:
Add APIs that allow to dynamically register driver backends so that the list of available drivers does not need to be known during compile time.
This will allow us to modularize the storage driver on runtime. --- src/storage/storage_backend.c | 111 ++++++++++++++++++++++----------- src/storage/storage_backend.h | 5 ++ src/storage/storage_backend_disk.c | 7 +++ src/storage/storage_backend_disk.h | 4 +- src/storage/storage_backend_fs.c | 27 ++++++++ src/storage/storage_backend_fs.h | 11 +--- src/storage/storage_backend_gluster.c | 13 +++- src/storage/storage_backend_gluster.h | 5 +- src/storage/storage_backend_iscsi.c | 7 +++ src/storage/storage_backend_iscsi.h | 4 +- src/storage/storage_backend_logical.c | 7 +++ src/storage/storage_backend_logical.h | 4 +- src/storage/storage_backend_mpath.c | 8 +++ src/storage/storage_backend_mpath.h | 4 +- src/storage/storage_backend_rbd.c | 7 +++ src/storage/storage_backend_rbd.h | 4 +- src/storage/storage_backend_scsi.c | 7 +++ src/storage/storage_backend_scsi.h | 4 +- src/storage/storage_backend_sheepdog.c | 7 +++ src/storage/storage_backend_sheepdog.h | 4 +- src/storage/storage_backend_vstorage.c | 7 +++ src/storage/storage_backend_vstorage.h | 4 +- src/storage/storage_backend_zfs.c | 7 +++ src/storage/storage_backend_zfs.h | 4 +- src/storage/storage_driver.c | 2 + tests/virstoragetest.c | 4 ++ 26 files changed, 200 insertions(+), 78 deletions(-)
[1] The one difference I note with these patches is that virStorageFileBackendDir *is* included for the virStorageFileBackends; whereas, prior to this patch it was not included in fileBackends. It's not a problem per se, but just wanted to make sure it was intentional... ACK for what's here though John
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 500d7567d..d8099be36 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -72,67 +72,106 @@
VIR_LOG_INIT("storage.storage_backend");
-static virStorageBackendPtr backends[] = { -#if WITH_STORAGE_DIR - &virStorageBackendDirectory, -#endif -#if WITH_STORAGE_FS - &virStorageBackendFileSystem, - &virStorageBackendNetFileSystem, +#define VIR_STORAGE_BACKENDS_MAX 20 + +static virStorageBackendPtr virStorageBackends[VIR_STORAGE_BACKENDS_MAX]; +static size_t virStorageBackendsCount; +static virStorageFileBackendPtr virStorageFileBackends[VIR_STORAGE_BACKENDS_MAX]; +static size_t virStorageFileBackendsCount; + +#define VIR_STORAGE_BACKEND_REGISTER(name) \ + if (name() < 0) \ + return -1 + +int +virStorageBackendDriversRegister(void) +{ +#if WITH_STORAGE_DIR || WITH_STORAGE_FS + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendFsRegister); #endif #if WITH_STORAGE_LVM - &virStorageBackendLogical, + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendLogicalRegister); #endif #if WITH_STORAGE_ISCSI - &virStorageBackendISCSI, + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendISCSIRegister); #endif #if WITH_STORAGE_SCSI - &virStorageBackendSCSI, + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendSCSIRegister); #endif #if WITH_STORAGE_MPATH - &virStorageBackendMpath, + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendMpathRegister); #endif #if WITH_STORAGE_DISK - &virStorageBackendDisk, + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendDiskRegister); #endif #if WITH_STORAGE_RBD - &virStorageBackendRBD, + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendRBDRegister); #endif #if WITH_STORAGE_SHEEPDOG - &virStorageBackendSheepdog, + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendSheepdogRegister); #endif #if WITH_STORAGE_GLUSTER - &virStorageBackendGluster, + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendGlusterRegister); #endif #if WITH_STORAGE_ZFS - &virStorageBackendZFS, + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendZFSRegister); #endif #if WITH_STORAGE_VSTORAGE - &virStorageBackendVstorage, + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendVstorageRegister); #endif - NULL -};
+ return 0; +} +#undef VIR_STORAGE_BACKEND_REGISTER
-static virStorageFileBackendPtr fileBackends[] = { -#if WITH_STORAGE_FS - &virStorageFileBackendFile, - &virStorageFileBackendBlock, -#endif -#if WITH_STORAGE_GLUSTER - &virStorageFileBackendGluster, -#endif - NULL -};
[1] the virStorageFileBackendDir is not in this list...
+ +int +virStorageBackendRegister(virStorageBackendPtr backend) +{ + VIR_DEBUG("Registering storage backend '%s'", + virStorageTypeToString(backend->type)); + + if (virStorageBackendsCount >= VIR_STORAGE_BACKENDS_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Too many drivers, cannot register storage backend '%s'"), + virStorageTypeToString(backend->type)); + return -1; + } + + virStorageBackends[virStorageBackendsCount] = backend; + virStorageBackendsCount++; + return 0; +} + + +int +virStorageBackendFileRegister(virStorageFileBackendPtr backend) +{ + VIR_DEBUG("Registering storage file backend '%s' protocol '%s'", + virStorageTypeToString(backend->type), + virStorageNetProtocolTypeToString(backend->protocol)); + + if (virStorageFileBackendsCount >= VIR_STORAGE_BACKENDS_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Too many drivers, cannot register storage file " + "backend '%s'"), + virStorageTypeToString(backend->type)); + return -1; + } + + virStorageFileBackends[virStorageFileBackendsCount] = backend; + virStorageFileBackendsCount++; + return 0; +}
[...]
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 54bcc5777..551fd945d 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -877,3 +877,30 @@ virStorageFileBackend virStorageFileBackendDir = {
.storageFileGetUniqueIdentifier = virStorageFileBackendFileGetUniqueIdentifier, }; + + +int +virStorageBackendFsRegister(void) +{ + if (virStorageBackendRegister(&virStorageBackendDirectory) < 0) + return -1; + +#if WITH_STORAGE_FS + if (virStorageBackendRegister(&virStorageBackendFileSystem) < 0) + return -1; + + if (virStorageBackendRegister(&virStorageBackendNetFileSystem) < 0) + return -1; +#endif /* WITH_STORAGE_FS */ + + if (virStorageBackendFileRegister(&virStorageFileBackendFile) < 0) + return -1; + + if (virStorageBackendFileRegister(&virStorageFileBackendBlock) < 0) + return -1; + + if (virStorageBackendFileRegister(&virStorageFileBackendDir) < 0) + return -1;
[1] now will be added in the file backend table.
+ + return 0; +}
[...]

On Fri, Feb 10, 2017 at 08:19:33 -0500, John Ferlan wrote:
On 02/08/2017 11:27 AM, Peter Krempa wrote:
Add APIs that allow to dynamically register driver backends so that the list of available drivers does not need to be known during compile time.
This will allow us to modularize the storage driver on runtime. --- src/storage/storage_backend.c | 111 ++++++++++++++++++++++----------- src/storage/storage_backend.h | 5 ++ src/storage/storage_backend_disk.c | 7 +++ src/storage/storage_backend_disk.h | 4 +- src/storage/storage_backend_fs.c | 27 ++++++++ src/storage/storage_backend_fs.h | 11 +--- src/storage/storage_backend_gluster.c | 13 +++- src/storage/storage_backend_gluster.h | 5 +- src/storage/storage_backend_iscsi.c | 7 +++ src/storage/storage_backend_iscsi.h | 4 +- src/storage/storage_backend_logical.c | 7 +++ src/storage/storage_backend_logical.h | 4 +- src/storage/storage_backend_mpath.c | 8 +++ src/storage/storage_backend_mpath.h | 4 +- src/storage/storage_backend_rbd.c | 7 +++ src/storage/storage_backend_rbd.h | 4 +- src/storage/storage_backend_scsi.c | 7 +++ src/storage/storage_backend_scsi.h | 4 +- src/storage/storage_backend_sheepdog.c | 7 +++ src/storage/storage_backend_sheepdog.h | 4 +- src/storage/storage_backend_vstorage.c | 7 +++ src/storage/storage_backend_vstorage.h | 4 +- src/storage/storage_backend_zfs.c | 7 +++ src/storage/storage_backend_zfs.h | 4 +- src/storage/storage_driver.c | 2 + tests/virstoragetest.c | 4 ++ 26 files changed, 200 insertions(+), 78 deletions(-)
[1] The one difference I note with these patches is that virStorageFileBackendDir *is* included for the virStorageFileBackends; whereas, prior to this patch it was not included in fileBackends.
It's not a problem per se, but just wanted to make sure it was intentional...
The initial omission was unintentional. Thankfully directory storage sources are very uncommon since the drivers to expose them in the guest are rather old. I'll keep it here as is.

Compile the storage driver into modules rather than by compiling all files together. All modules are still linked together statically. --- src/Makefile.am | 114 ++++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 98 insertions(+), 16 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 2f32d4197..b71209a9d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1674,52 +1674,134 @@ noinst_LTLIBRARIES += libvirt_driver_storage.la #libvirt_la_BUILT_LIBADD += libvirt_driver_storage.la endif ! WITH_DRIVER_MODULES libvirt_driver_storage_impl_la_SOURCES += $(STORAGE_DRIVER_SOURCES) -libvirt_driver_storage_impl_la_SOURCES += $(STORAGE_DRIVER_FS_SOURCES) + + +libvirt_storage_backend_fs_la_SOURCES = $(STORAGE_DRIVER_FS_SOURCES) +libvirt_storage_backend_fs_la_CFLAGS = \ + -I$(srcdir)/conf \ + $(AM_CFLAGS) + +noinst_LTLIBRARIES += libvirt_storage_backend_fs.la +libvirt_driver_storage_impl_la_LIBADD += libvirt_storage_backend_fs.la endif WITH_STORAGE if WITH_STORAGE_LVM -libvirt_driver_storage_impl_la_SOURCES += $(STORAGE_DRIVER_LVM_SOURCES) +libvirt_storage_backend_logical_la_SOURCES = \ + $(STORAGE_DRIVER_LVM_SOURCES) +libvirt_storage_backend_logical_la_CFLAGS = \ + -I$(srcdir)/conf \ + $(AM_CFLAGS) + +noinst_LTLIBRARIES += libvirt_storage_backend_logical.la +libvirt_driver_storage_impl_la_LIBADD += \ + libvirt_storage_backend_logical.la endif WITH_STORAGE_LVM if WITH_STORAGE_ISCSI -libvirt_driver_storage_impl_la_SOURCES += $(STORAGE_DRIVER_ISCSI_SOURCES) +libvirt_storage_backend_iscsi_la_SOURCES = \ + $(STORAGE_DRIVER_ISCSI_SOURCES) +libvirt_storage_backend_iscsi_la_CFLAGS = \ + -I$(srcdir)/conf \ + -I$(srcdir)/secret \ + $(AM_CFLAGS) + +noinst_LTLIBRARIES += libvirt_storage_backend_iscsi.la +libvirt_driver_storage_impl_la_LIBADD += \ + libvirt_storage_backend_iscsi.la endif WITH_STORAGE_ISCSI if WITH_STORAGE_SCSI -libvirt_driver_storage_impl_la_SOURCES += $(STORAGE_DRIVER_SCSI_SOURCES) +libvirt_storage_backend_scsi_la_SOURCES = $(STORAGE_DRIVER_SCSI_SOURCES) +libvirt_storage_backend_scsi_la_CFLAGS = \ + -I$(srcdir)/conf \ + $(AM_CFLAGS) + +noinst_LTLIBRARIES += libvirt_storage_backend_scsi.la +libvirt_driver_storage_impl_la_LIBADD += libvirt_storage_backend_scsi.la endif WITH_STORAGE_SCSI if WITH_STORAGE_MPATH -libvirt_driver_storage_impl_la_SOURCES += $(STORAGE_DRIVER_MPATH_SOURCES) -libvirt_driver_storage_impl_la_CFLAGS += $(DEVMAPPER_CFLAGS) -libvirt_driver_storage_impl_la_LIBADD += $(DEVMAPPER_LIBS) +libvirt_storage_backend_mpath_la_SOURCES = \ + $(STORAGE_DRIVER_MPATH_SOURCES) +libvirt_storage_backend_mpath_la_LIBADD = $(DEVMAPPER_LIBS) +libvirt_storage_backend_mpath_la_CFLAGS = \ + -I$(srcdir)/conf \ + $(DEVMAPPER_CFLAGS) \ + $(AM_CFLAGS) + +noinst_LTLIBRARIES += libvirt_storage_backend_mpath.la +libvirt_driver_storage_impl_la_LIBADD += \ + libvirt_storage_backend_mpath.la endif WITH_STORAGE_MPATH if WITH_STORAGE_DISK -libvirt_driver_storage_impl_la_SOURCES += $(STORAGE_DRIVER_DISK_SOURCES) +libvirt_storage_backend_disk_la_SOURCES = $(STORAGE_DRIVER_DISK_SOURCES) +libvirt_storage_backend_disk_la_CFLAGS = \ + -I$(srcdir)/conf \ + $(AM_CFLAGS) + +noinst_LTLIBRARIES += libvirt_storage_backend_disk.la +libvirt_driver_storage_impl_la_LIBADD += libvirt_storage_backend_disk.la endif WITH_STORAGE_DISK if WITH_STORAGE_RBD -libvirt_driver_storage_impl_la_SOURCES += $(STORAGE_DRIVER_RBD_SOURCES) -libvirt_driver_storage_impl_la_LIBADD += $(LIBRBD_LIBS) +libvirt_storage_backend_rbd_la_SOURCES = $(STORAGE_DRIVER_RBD_SOURCES) +libvirt_storage_backend_rbd_la_LIBADD = $(LIBRBD_LIBS) +libvirt_storage_backend_rbd_la_CFLAGS = \ + -I$(srcdir)/conf \ + -I$(srcdir)/secret \ + $(AM_CFLAGS) + +noinst_LTLIBRARIES += libvirt_storage_backend_rbd.la +libvirt_driver_storage_impl_la_LIBADD += libvirt_storage_backend_rbd.la endif WITH_STORAGE_RBD if WITH_STORAGE_SHEEPDOG -libvirt_driver_storage_impl_la_SOURCES += $(STORAGE_DRIVER_SHEEPDOG_SOURCES) +libvirt_storage_backend_sheepdog_la_SOURCES = \ + $(STORAGE_DRIVER_SHEEPDOG_SOURCES) +libvirt_storage_backend_sheepdog_la_CFLAGS = \ + -I$(srcdir)/conf \ + $(AM_CFLAGS) + +noinst_LTLIBRARIES += libvirt_storage_backend_sheepdog.la +libvirt_driver_storage_impl_la_LIBADD += \ + libvirt_storage_backend_sheepdog.la endif WITH_STORAGE_SHEEPDOG if WITH_STORAGE_GLUSTER -libvirt_driver_storage_impl_la_SOURCES += $(STORAGE_DRIVER_GLUSTER_SOURCES) -libvirt_driver_storage_impl_la_CFLAGS += $(GLUSTERFS_CFLAGS) -libvirt_driver_storage_impl_la_LIBADD += $(GLUSTERFS_LIBS) +libvirt_storage_backend_gluster_la_SOURCES = \ + $(STORAGE_DRIVER_GLUSTER_SOURCES) +libvirt_storage_backend_gluster_la_LIBADD = $(GLUSTERFS_LIBS) +libvirt_storage_backend_gluster_la_CFLAGS = \ + -I$(srcdir)/conf \ + $(GLUSTERFS_CFLAGS) \ + $(AM_CFLAGS) + +noinst_LTLIBRARIES += libvirt_storage_backend_gluster.la +libvirt_driver_storage_impl_la_LIBADD += \ + libvirt_storage_backend_gluster.la endif WITH_STORAGE_GLUSTER if WITH_STORAGE_ZFS -libvirt_driver_storage_impl_la_SOURCES += $(STORAGE_DRIVER_ZFS_SOURCES) +libvirt_storage_backend_zfs_la_SOURCES = $(STORAGE_DRIVER_ZFS_SOURCES) +libvirt_storage_backend_zfs_la_CFLAGS = \ + -I$(srcdir)/conf \ + $(AM_CFLAGS) + +noinst_LTLIBRARIES += libvirt_storage_backend_zfs.la +libvirt_driver_storage_impl_la_LIBADD += libvirt_storage_backend_zfs.la endif WITH_STORAGE_ZFS if WITH_STORAGE_VSTORAGE -libvirt_driver_storage_impl_la_SOURCES += $(STORAGE_DRIVER_VSTORAGE_SOURCES) +libvirt_storage_backend_vstorage_la_SOURCES = \ + $(STORAGE_DRIVER_VSTORAGE_SOURCES) +libvirt_storage_backend_vstorage_la_CFLAGS = \ + -I$(srcdir)/conf \ + $(AM_CFLAGS) + +noinst_LTLIBRARIES += libvirt_storage_backend_vstorage.la +libvirt_driver_storage_impl_la_LIBADD += \ + libvirt_storage_backend_vstorage.la endif WITH_STORAGE_VSTORAGE if WITH_NODE_DEVICES -- 2.11.0

On 02/08/2017 11:27 AM, Peter Krempa wrote:
Compile the storage driver into modules rather than by compiling all files together. All modules are still linked together statically. --- src/Makefile.am | 114 ++++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 98 insertions(+), 16 deletions(-)
Makefile adjustments are not exactly in my wheelhouse, but this does compile and does "look" right and "is" comparable to hypervisor drivers, so consider it a weak ACK. If you want a second opinion on this I'm sure you can ask those in the office around you for an opinion too ;-) John

If driver modules are enabled turn storage driver backends into dynamically loadable objects. This will allow greater modularity for binary distributions, where heavyweight dependencies as rbd and gluster can be avoided by selecting only a subset of drivers if the rest is not necessary. The storage modules are installed into 'LIBDIR/libvirt/storage-backend/' and users can't override the location by using 'LIBVIRT_STORAGE_BACKEND_DIR' environment variable. rpm based distros will at this point install all the backends when libvirt-daemon-driver-storage package is installed. --- libvirt.spec.in | 17 +++++++++ src/Makefile.am | 85 ++++++++++++++++++++++++++++++++++++++++++- src/storage/storage_backend.c | 60 +++++++++++++++++++++++------- tests/Makefile.am | 4 +- 4 files changed, 151 insertions(+), 15 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index e8c272bd7..3098ed2dd 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1234,6 +1234,8 @@ rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/lock-driver/*.la rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/lock-driver/*.a rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/connection-driver/*.la rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/connection-driver/*.a +rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/storage-backend/*.la +rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/storage-backend/*.a %if %{with_wireshark} %if 0%{fedora} >= 24 rm -f $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/libvirt.la @@ -1689,6 +1691,21 @@ exit 0 %files daemon-driver-storage %attr(0755, root, root) %{_libexecdir}/libvirt_parthelper %{_libdir}/%{name}/connection-driver/libvirt_driver_storage.so +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_fs.so +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_disk.so +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_logical.so +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_scsi.so +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_iscsi.so +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_mpath.so +%if %{with_storage_gluster} +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_gluster.so +%endif +%if %{with_storage_rbd} +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_rbd.so +%endif +%if %{with_storage_sheepdog} +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_sheepdog.so +%endif %if %{with_qemu} %files daemon-driver-qemu diff --git a/src/Makefile.am b/src/Makefile.am index b71209a9d..cdac7a1b5 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -970,9 +970,12 @@ SECRET_DRIVER_SOURCES = \ secret/secret_driver.h secret/secret_driver.c # Storage backend specific impls +STORAGE_DRIVER_BACKEND_SOURCES = \ + storage/storage_backend.h storage/storage_backend.c + STORAGE_DRIVER_SOURCES = \ storage/storage_driver.h storage/storage_driver.c \ - storage/storage_backend.h storage/storage_backend.c \ + $(STORAGE_DRIVER_BACKEND_SOURCES) \ storage/storage_util.h storage/storage_util.c STORAGE_DRIVER_FS_SOURCES = \ @@ -1660,6 +1663,12 @@ if WITH_BLKID libvirt_driver_storage_impl_la_CFLAGS += $(BLKID_CFLAGS) libvirt_driver_storage_impl_la_LIBADD += $(BLKID_LIBS) endif WITH_BLKID + +if WITH_DRIVER_MODULES +storagebackenddir = $(libdir)/libvirt/storage-backend +storagebackend_LTLIBRARIES = +endif WITH_DRIVER_MODULES + if WITH_STORAGE noinst_LTLIBRARIES += libvirt_driver_storage_impl.la libvirt_driver_storage_la_SOURCES = @@ -1681,8 +1690,14 @@ libvirt_storage_backend_fs_la_CFLAGS = \ -I$(srcdir)/conf \ $(AM_CFLAGS) +if WITH_DRIVER_MODULES +storagebackend_LTLIBRARIES += libvirt_storage_backend_fs.la +libvirt_storage_backend_fs_la_LDFLAGS = \ + -module -avoid-version $(AM_LDFLAGS) +else ! WITH_DRIVER_MODULES noinst_LTLIBRARIES += libvirt_storage_backend_fs.la libvirt_driver_storage_impl_la_LIBADD += libvirt_storage_backend_fs.la +endif ! WITH_DRIVER_MODULES endif WITH_STORAGE if WITH_STORAGE_LVM @@ -1692,9 +1707,15 @@ libvirt_storage_backend_logical_la_CFLAGS = \ -I$(srcdir)/conf \ $(AM_CFLAGS) +if WITH_DRIVER_MODULES +storagebackend_LTLIBRARIES += libvirt_storage_backend_logical.la +libvirt_storage_backend_logical_la_LDFLAGS = \ + -module -avoid-version $(AM_LDFLAGS) +else ! WITH_DRIVER_MODULES noinst_LTLIBRARIES += libvirt_storage_backend_logical.la libvirt_driver_storage_impl_la_LIBADD += \ libvirt_storage_backend_logical.la +endif ! WITH_DRIVER_MODULES endif WITH_STORAGE_LVM if WITH_STORAGE_ISCSI @@ -1705,9 +1726,15 @@ libvirt_storage_backend_iscsi_la_CFLAGS = \ -I$(srcdir)/secret \ $(AM_CFLAGS) +if WITH_DRIVER_MODULES +storagebackend_LTLIBRARIES += libvirt_storage_backend_iscsi.la +libvirt_storage_backend_iscsi_la_LDFLAGS = \ + -module -avoid-version $(AM_LDFLAGS) +else ! WITH_DRIVER_MODULES noinst_LTLIBRARIES += libvirt_storage_backend_iscsi.la libvirt_driver_storage_impl_la_LIBADD += \ libvirt_storage_backend_iscsi.la +endif ! WITH_DRIVER_MODULES endif WITH_STORAGE_ISCSI if WITH_STORAGE_SCSI @@ -1716,8 +1743,14 @@ libvirt_storage_backend_scsi_la_CFLAGS = \ -I$(srcdir)/conf \ $(AM_CFLAGS) +if WITH_DRIVER_MODULES +storagebackend_LTLIBRARIES += libvirt_storage_backend_scsi.la +libvirt_storage_backend_scsi_la_LDFLAGS = \ + -module -avoid-version $(AM_LDFLAGS) +else ! WITH_DRIVER_MODULES noinst_LTLIBRARIES += libvirt_storage_backend_scsi.la libvirt_driver_storage_impl_la_LIBADD += libvirt_storage_backend_scsi.la +endif ! WITH_DRIVER_MODULES endif WITH_STORAGE_SCSI if WITH_STORAGE_MPATH @@ -1729,9 +1762,15 @@ libvirt_storage_backend_mpath_la_CFLAGS = \ $(DEVMAPPER_CFLAGS) \ $(AM_CFLAGS) +if WITH_DRIVER_MODULES +storagebackend_LTLIBRARIES += libvirt_storage_backend_mpath.la +libvirt_storage_backend_mpath_la_LDFLAGS = \ + -module -avoid-version $(AM_LDFLAGS) +else ! WITH_DRIVER_MODULES noinst_LTLIBRARIES += libvirt_storage_backend_mpath.la libvirt_driver_storage_impl_la_LIBADD += \ libvirt_storage_backend_mpath.la +endif ! WITH_DRIVER_MODULES endif WITH_STORAGE_MPATH if WITH_STORAGE_DISK @@ -1740,8 +1779,14 @@ libvirt_storage_backend_disk_la_CFLAGS = \ -I$(srcdir)/conf \ $(AM_CFLAGS) +if WITH_DRIVER_MODULES +storagebackend_LTLIBRARIES += libvirt_storage_backend_disk.la +libvirt_storage_backend_disk_la_LDFLAGS = \ + -module -avoid-version $(AM_LDFLAGS) +else ! WITH_DRIVER_MODULES noinst_LTLIBRARIES += libvirt_storage_backend_disk.la libvirt_driver_storage_impl_la_LIBADD += libvirt_storage_backend_disk.la +endif ! WITH_DRIVER_MODULES endif WITH_STORAGE_DISK if WITH_STORAGE_RBD @@ -1752,8 +1797,14 @@ libvirt_storage_backend_rbd_la_CFLAGS = \ -I$(srcdir)/secret \ $(AM_CFLAGS) +if WITH_DRIVER_MODULES +storagebackend_LTLIBRARIES += libvirt_storage_backend_rbd.la +libvirt_storage_backend_rbd_la_LDFLAGS = \ + -module -avoid-version $(AM_LDFLAGS) +else ! WITH_DRIVER_MODULES noinst_LTLIBRARIES += libvirt_storage_backend_rbd.la libvirt_driver_storage_impl_la_LIBADD += libvirt_storage_backend_rbd.la +endif ! WITH_DRIVER_MODULES endif WITH_STORAGE_RBD if WITH_STORAGE_SHEEPDOG @@ -1763,9 +1814,23 @@ libvirt_storage_backend_sheepdog_la_CFLAGS = \ -I$(srcdir)/conf \ $(AM_CFLAGS) +libvirt_storage_backend_sheepdog_priv_la_SOURCES = \ + $(STORAGE_DRIVER_SHEEPDOG_SOURCES) \ + $(STORAGE_DRIVER_BACKEND_SOURCES) +libvirt_storage_backend_sheepdog_priv_la_CFLAGS = \ + -I$(srcdir)/conf \ + $(AM_CFLAGS) +noinst_LTLIBRARIES += libvirt_storage_backend_sheepdog_priv.la + +if WITH_DRIVER_MODULES +storagebackend_LTLIBRARIES += libvirt_storage_backend_sheepdog.la +libvirt_storage_backend_sheepdog_la_LDFLAGS = \ + -module -avoid-version $(AM_LDFLAGS) +else ! WITH_DRIVER_MODULES noinst_LTLIBRARIES += libvirt_storage_backend_sheepdog.la libvirt_driver_storage_impl_la_LIBADD += \ libvirt_storage_backend_sheepdog.la +endif ! WITH_DRIVER_MODULES endif WITH_STORAGE_SHEEPDOG if WITH_STORAGE_GLUSTER @@ -1777,9 +1842,15 @@ libvirt_storage_backend_gluster_la_CFLAGS = \ $(GLUSTERFS_CFLAGS) \ $(AM_CFLAGS) +if WITH_DRIVER_MODULES +storagebackend_LTLIBRARIES += libvirt_storage_backend_gluster.la +libvirt_storage_backend_gluster_la_LDFLAGS = \ + -module -avoid-version $(AM_LDFLAGS) +else ! WITH_DRIVER_MODULES noinst_LTLIBRARIES += libvirt_storage_backend_gluster.la libvirt_driver_storage_impl_la_LIBADD += \ libvirt_storage_backend_gluster.la +endif ! WITH_DRIVER_MODULES endif WITH_STORAGE_GLUSTER if WITH_STORAGE_ZFS @@ -1788,8 +1859,14 @@ libvirt_storage_backend_zfs_la_CFLAGS = \ -I$(srcdir)/conf \ $(AM_CFLAGS) +if WITH_DRIVER_MODULES +storagebackend_LTLIBRARIES += libvirt_storage_backend_zfs.la +libvirt_storage_backend_zfs_la_LDFLAGS = \ + -module -avoid-version $(AM_LDFLAGS) +else ! WITH_DRIVER_MODULES noinst_LTLIBRARIES += libvirt_storage_backend_zfs.la libvirt_driver_storage_impl_la_LIBADD += libvirt_storage_backend_zfs.la +endif ! WITH_DRIVER_MODULES endif WITH_STORAGE_ZFS if WITH_STORAGE_VSTORAGE @@ -1799,9 +1876,15 @@ libvirt_storage_backend_vstorage_la_CFLAGS = \ -I$(srcdir)/conf \ $(AM_CFLAGS) +if WITH_DRIVER_MODULES +storagebackend_LTLIBRARIES += libvirt_storage_backend_vstorage.la +libvirt_storage_backend_vstorage_la_LDFLAGS = \ + -module -avoid-version $(AM_LDFLAGS) +else ! WITH_DRIVER_MODULES noinst_LTLIBRARIES += libvirt_storage_backend_vstorage.la libvirt_driver_storage_impl_la_LIBADD += \ libvirt_storage_backend_vstorage.la +endif ! WITH_DRIVER_MODULES endif WITH_STORAGE_VSTORAGE if WITH_NODE_DEVICES diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index d8099be36..32f45e841 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -33,6 +33,8 @@ #include "virstoragefile.h" #include "storage_backend.h" #include "virlog.h" +#include "virfile.h" +#include "configmake.h" #if WITH_STORAGE_LVM # include "storage_backend_logical.h" @@ -79,45 +81,77 @@ static size_t virStorageBackendsCount; static virStorageFileBackendPtr virStorageFileBackends[VIR_STORAGE_BACKENDS_MAX]; static size_t virStorageFileBackendsCount; -#define VIR_STORAGE_BACKEND_REGISTER(name) \ - if (name() < 0) \ +#if WITH_DRIVER_MODULES + +# define STORAGE_BACKEND_MODULE_DIR LIBDIR "/libvirt/storage-backend" + +static int +virStorageDriverLoadBackendModule(const char *name, + const char *regfunc) +{ + char *modfile = NULL; + int ret; + + if (!(modfile = virFileFindResourceFull(name, + "libvirt_storage_backend_", + ".so", + abs_topbuilddir "/src/.libs", + STORAGE_BACKEND_MODULE_DIR, + "LIBVIRT_STORAGE_BACKEND_DIR"))) + return 1; + + ret = virDriverLoadModuleFull(modfile, regfunc, NULL); + + VIR_FREE(modfile); + + return ret; +} + + +# define VIR_STORAGE_BACKEND_REGISTER(func, module) \ + if (virStorageDriverLoadBackendModule(module, #func) < 0) \ + return -1 +#else +# define VIR_STORAGE_BACKEND_REGISTER(func, module) \ + if (func() < 0) \ return -1 +#endif int virStorageBackendDriversRegister(void) { #if WITH_STORAGE_DIR || WITH_STORAGE_FS - VIR_STORAGE_BACKEND_REGISTER(virStorageBackendFsRegister); + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendFsRegister, "fs"); #endif #if WITH_STORAGE_LVM - VIR_STORAGE_BACKEND_REGISTER(virStorageBackendLogicalRegister); + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendLogicalRegister, "logical"); #endif #if WITH_STORAGE_ISCSI - VIR_STORAGE_BACKEND_REGISTER(virStorageBackendISCSIRegister); + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendISCSIRegister, "iscsi"); #endif #if WITH_STORAGE_SCSI - VIR_STORAGE_BACKEND_REGISTER(virStorageBackendSCSIRegister); + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendSCSIRegister, "scsi"); #endif #if WITH_STORAGE_MPATH - VIR_STORAGE_BACKEND_REGISTER(virStorageBackendMpathRegister); + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendMpathRegister, "mpath"); #endif #if WITH_STORAGE_DISK - VIR_STORAGE_BACKEND_REGISTER(virStorageBackendDiskRegister); + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendDiskRegister, "disk"); #endif #if WITH_STORAGE_RBD - VIR_STORAGE_BACKEND_REGISTER(virStorageBackendRBDRegister); + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendRBDRegister, "rbd"); #endif #if WITH_STORAGE_SHEEPDOG - VIR_STORAGE_BACKEND_REGISTER(virStorageBackendSheepdogRegister); + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendSheepdogRegister, "sheepdog"); #endif #if WITH_STORAGE_GLUSTER - VIR_STORAGE_BACKEND_REGISTER(virStorageBackendGlusterRegister); + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendGlusterRegister, "gluster"); #endif #if WITH_STORAGE_ZFS - VIR_STORAGE_BACKEND_REGISTER(virStorageBackendZFSRegister); + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendZFSRegister, "zfs"); #endif #if WITH_STORAGE_VSTORAGE - VIR_STORAGE_BACKEND_REGISTER(virStorageBackendVstorageRegister); + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendVstorageRegister, "vstorage"); #endif return 0; diff --git a/tests/Makefile.am b/tests/Makefile.am index e923178f2..401253da3 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -804,7 +804,9 @@ storagebackendsheepdogtest_SOURCES = \ storagebackendsheepdogtest.c \ testutils.c testutils.h storagebackendsheepdogtest_LDADD = \ - ../src/libvirt_driver_storage_impl.la $(LDADDS) + ../src/libvirt_driver_storage_impl.la \ + ../src/libvirt_storage_backend_sheepdog_priv.la \ + $(LDADDS) else ! WITH_STORAGE_SHEEPDOG EXTRA_DIST += storagebackendsheepdogtest.c endif ! WITH_STORAGE_SHEEPDOG -- 2.11.0

On 02/08/2017 11:27 AM, Peter Krempa wrote:
If driver modules are enabled turn storage driver backends into dynamically loadable objects. This will allow greater modularity for binary distributions, where heavyweight dependencies as rbd and gluster can be avoided by selecting only a subset of drivers if the rest is not necessary.
The storage modules are installed into 'LIBDIR/libvirt/storage-backend/' and users can't override the location by using 'LIBVIRT_STORAGE_BACKEND_DIR' environment variable.
can or can't?
rpm based distros will at this point install all the backends when libvirt-daemon-driver-storage package is installed. --- libvirt.spec.in | 17 +++++++++ src/Makefile.am | 85 ++++++++++++++++++++++++++++++++++++++++++- src/storage/storage_backend.c | 60 +++++++++++++++++++++++------- tests/Makefile.am | 4 +- 4 files changed, 151 insertions(+), 15 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in index e8c272bd7..3098ed2dd 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1234,6 +1234,8 @@ rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/lock-driver/*.la rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/lock-driver/*.a rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/connection-driver/*.la rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/connection-driver/*.a +rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/storage-backend/*.la +rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/storage-backend/*.a %if %{with_wireshark} %if 0%{fedora} >= 24 rm -f $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/libvirt.la @@ -1689,6 +1691,21 @@ exit 0 %files daemon-driver-storage %attr(0755, root, root) %{_libexecdir}/libvirt_parthelper %{_libdir}/%{name}/connection-driver/libvirt_driver_storage.so +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_fs.so +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_disk.so +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_logical.so +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_scsi.so +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_iscsi.so +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_mpath.so +%if %{with_storage_gluster} +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_gluster.so +%endif +%if %{with_storage_rbd} +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_rbd.so +%endif +%if %{with_storage_sheepdog} +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_sheepdog.so +%endif
What about zfs and vstorage? The changes seem OK... they build, etc., but this is out of my wheelhouse. Hopefully someone else can chime in.... A very weak ACK ;-) John
%if %{with_qemu} %files daemon-driver-qemu diff --git a/src/Makefile.am b/src/Makefile.am index b71209a9d..cdac7a1b5 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -970,9 +970,12 @@ SECRET_DRIVER_SOURCES = \ secret/secret_driver.h secret/secret_driver.c
# Storage backend specific impls +STORAGE_DRIVER_BACKEND_SOURCES = \ + storage/storage_backend.h storage/storage_backend.c + STORAGE_DRIVER_SOURCES = \ storage/storage_driver.h storage/storage_driver.c \ - storage/storage_backend.h storage/storage_backend.c \ + $(STORAGE_DRIVER_BACKEND_SOURCES) \ storage/storage_util.h storage/storage_util.c
STORAGE_DRIVER_FS_SOURCES = \ @@ -1660,6 +1663,12 @@ if WITH_BLKID libvirt_driver_storage_impl_la_CFLAGS += $(BLKID_CFLAGS) libvirt_driver_storage_impl_la_LIBADD += $(BLKID_LIBS) endif WITH_BLKID + +if WITH_DRIVER_MODULES +storagebackenddir = $(libdir)/libvirt/storage-backend +storagebackend_LTLIBRARIES = +endif WITH_DRIVER_MODULES + if WITH_STORAGE noinst_LTLIBRARIES += libvirt_driver_storage_impl.la libvirt_driver_storage_la_SOURCES = @@ -1681,8 +1690,14 @@ libvirt_storage_backend_fs_la_CFLAGS = \ -I$(srcdir)/conf \ $(AM_CFLAGS)
+if WITH_DRIVER_MODULES +storagebackend_LTLIBRARIES += libvirt_storage_backend_fs.la +libvirt_storage_backend_fs_la_LDFLAGS = \ + -module -avoid-version $(AM_LDFLAGS) +else ! WITH_DRIVER_MODULES noinst_LTLIBRARIES += libvirt_storage_backend_fs.la libvirt_driver_storage_impl_la_LIBADD += libvirt_storage_backend_fs.la +endif ! WITH_DRIVER_MODULES endif WITH_STORAGE
if WITH_STORAGE_LVM @@ -1692,9 +1707,15 @@ libvirt_storage_backend_logical_la_CFLAGS = \ -I$(srcdir)/conf \ $(AM_CFLAGS)
+if WITH_DRIVER_MODULES +storagebackend_LTLIBRARIES += libvirt_storage_backend_logical.la +libvirt_storage_backend_logical_la_LDFLAGS = \ + -module -avoid-version $(AM_LDFLAGS) +else ! WITH_DRIVER_MODULES noinst_LTLIBRARIES += libvirt_storage_backend_logical.la libvirt_driver_storage_impl_la_LIBADD += \ libvirt_storage_backend_logical.la +endif ! WITH_DRIVER_MODULES endif WITH_STORAGE_LVM
if WITH_STORAGE_ISCSI @@ -1705,9 +1726,15 @@ libvirt_storage_backend_iscsi_la_CFLAGS = \ -I$(srcdir)/secret \ $(AM_CFLAGS)
+if WITH_DRIVER_MODULES +storagebackend_LTLIBRARIES += libvirt_storage_backend_iscsi.la +libvirt_storage_backend_iscsi_la_LDFLAGS = \ + -module -avoid-version $(AM_LDFLAGS) +else ! WITH_DRIVER_MODULES noinst_LTLIBRARIES += libvirt_storage_backend_iscsi.la libvirt_driver_storage_impl_la_LIBADD += \ libvirt_storage_backend_iscsi.la +endif ! WITH_DRIVER_MODULES endif WITH_STORAGE_ISCSI
if WITH_STORAGE_SCSI @@ -1716,8 +1743,14 @@ libvirt_storage_backend_scsi_la_CFLAGS = \ -I$(srcdir)/conf \ $(AM_CFLAGS)
+if WITH_DRIVER_MODULES +storagebackend_LTLIBRARIES += libvirt_storage_backend_scsi.la +libvirt_storage_backend_scsi_la_LDFLAGS = \ + -module -avoid-version $(AM_LDFLAGS) +else ! WITH_DRIVER_MODULES noinst_LTLIBRARIES += libvirt_storage_backend_scsi.la libvirt_driver_storage_impl_la_LIBADD += libvirt_storage_backend_scsi.la +endif ! WITH_DRIVER_MODULES endif WITH_STORAGE_SCSI
if WITH_STORAGE_MPATH @@ -1729,9 +1762,15 @@ libvirt_storage_backend_mpath_la_CFLAGS = \ $(DEVMAPPER_CFLAGS) \ $(AM_CFLAGS)
+if WITH_DRIVER_MODULES +storagebackend_LTLIBRARIES += libvirt_storage_backend_mpath.la +libvirt_storage_backend_mpath_la_LDFLAGS = \ + -module -avoid-version $(AM_LDFLAGS) +else ! WITH_DRIVER_MODULES noinst_LTLIBRARIES += libvirt_storage_backend_mpath.la libvirt_driver_storage_impl_la_LIBADD += \ libvirt_storage_backend_mpath.la +endif ! WITH_DRIVER_MODULES endif WITH_STORAGE_MPATH
if WITH_STORAGE_DISK @@ -1740,8 +1779,14 @@ libvirt_storage_backend_disk_la_CFLAGS = \ -I$(srcdir)/conf \ $(AM_CFLAGS)
+if WITH_DRIVER_MODULES +storagebackend_LTLIBRARIES += libvirt_storage_backend_disk.la +libvirt_storage_backend_disk_la_LDFLAGS = \ + -module -avoid-version $(AM_LDFLAGS) +else ! WITH_DRIVER_MODULES noinst_LTLIBRARIES += libvirt_storage_backend_disk.la libvirt_driver_storage_impl_la_LIBADD += libvirt_storage_backend_disk.la +endif ! WITH_DRIVER_MODULES endif WITH_STORAGE_DISK
if WITH_STORAGE_RBD @@ -1752,8 +1797,14 @@ libvirt_storage_backend_rbd_la_CFLAGS = \ -I$(srcdir)/secret \ $(AM_CFLAGS)
+if WITH_DRIVER_MODULES +storagebackend_LTLIBRARIES += libvirt_storage_backend_rbd.la +libvirt_storage_backend_rbd_la_LDFLAGS = \ + -module -avoid-version $(AM_LDFLAGS) +else ! WITH_DRIVER_MODULES noinst_LTLIBRARIES += libvirt_storage_backend_rbd.la libvirt_driver_storage_impl_la_LIBADD += libvirt_storage_backend_rbd.la +endif ! WITH_DRIVER_MODULES endif WITH_STORAGE_RBD
if WITH_STORAGE_SHEEPDOG @@ -1763,9 +1814,23 @@ libvirt_storage_backend_sheepdog_la_CFLAGS = \ -I$(srcdir)/conf \ $(AM_CFLAGS)
+libvirt_storage_backend_sheepdog_priv_la_SOURCES = \ + $(STORAGE_DRIVER_SHEEPDOG_SOURCES) \ + $(STORAGE_DRIVER_BACKEND_SOURCES) +libvirt_storage_backend_sheepdog_priv_la_CFLAGS = \ + -I$(srcdir)/conf \ + $(AM_CFLAGS) +noinst_LTLIBRARIES += libvirt_storage_backend_sheepdog_priv.la + +if WITH_DRIVER_MODULES +storagebackend_LTLIBRARIES += libvirt_storage_backend_sheepdog.la +libvirt_storage_backend_sheepdog_la_LDFLAGS = \ + -module -avoid-version $(AM_LDFLAGS) +else ! WITH_DRIVER_MODULES noinst_LTLIBRARIES += libvirt_storage_backend_sheepdog.la libvirt_driver_storage_impl_la_LIBADD += \ libvirt_storage_backend_sheepdog.la +endif ! WITH_DRIVER_MODULES endif WITH_STORAGE_SHEEPDOG
if WITH_STORAGE_GLUSTER @@ -1777,9 +1842,15 @@ libvirt_storage_backend_gluster_la_CFLAGS = \ $(GLUSTERFS_CFLAGS) \ $(AM_CFLAGS)
+if WITH_DRIVER_MODULES +storagebackend_LTLIBRARIES += libvirt_storage_backend_gluster.la +libvirt_storage_backend_gluster_la_LDFLAGS = \ + -module -avoid-version $(AM_LDFLAGS) +else ! WITH_DRIVER_MODULES noinst_LTLIBRARIES += libvirt_storage_backend_gluster.la libvirt_driver_storage_impl_la_LIBADD += \ libvirt_storage_backend_gluster.la +endif ! WITH_DRIVER_MODULES endif WITH_STORAGE_GLUSTER
if WITH_STORAGE_ZFS @@ -1788,8 +1859,14 @@ libvirt_storage_backend_zfs_la_CFLAGS = \ -I$(srcdir)/conf \ $(AM_CFLAGS)
+if WITH_DRIVER_MODULES +storagebackend_LTLIBRARIES += libvirt_storage_backend_zfs.la +libvirt_storage_backend_zfs_la_LDFLAGS = \ + -module -avoid-version $(AM_LDFLAGS) +else ! WITH_DRIVER_MODULES noinst_LTLIBRARIES += libvirt_storage_backend_zfs.la libvirt_driver_storage_impl_la_LIBADD += libvirt_storage_backend_zfs.la +endif ! WITH_DRIVER_MODULES endif WITH_STORAGE_ZFS
if WITH_STORAGE_VSTORAGE @@ -1799,9 +1876,15 @@ libvirt_storage_backend_vstorage_la_CFLAGS = \ -I$(srcdir)/conf \ $(AM_CFLAGS)
+if WITH_DRIVER_MODULES +storagebackend_LTLIBRARIES += libvirt_storage_backend_vstorage.la +libvirt_storage_backend_vstorage_la_LDFLAGS = \ + -module -avoid-version $(AM_LDFLAGS) +else ! WITH_DRIVER_MODULES noinst_LTLIBRARIES += libvirt_storage_backend_vstorage.la libvirt_driver_storage_impl_la_LIBADD += \ libvirt_storage_backend_vstorage.la +endif ! WITH_DRIVER_MODULES endif WITH_STORAGE_VSTORAGE
if WITH_NODE_DEVICES diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index d8099be36..32f45e841 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -33,6 +33,8 @@ #include "virstoragefile.h" #include "storage_backend.h" #include "virlog.h" +#include "virfile.h" +#include "configmake.h"
#if WITH_STORAGE_LVM # include "storage_backend_logical.h" @@ -79,45 +81,77 @@ static size_t virStorageBackendsCount; static virStorageFileBackendPtr virStorageFileBackends[VIR_STORAGE_BACKENDS_MAX]; static size_t virStorageFileBackendsCount;
-#define VIR_STORAGE_BACKEND_REGISTER(name) \ - if (name() < 0) \ +#if WITH_DRIVER_MODULES + +# define STORAGE_BACKEND_MODULE_DIR LIBDIR "/libvirt/storage-backend" + +static int +virStorageDriverLoadBackendModule(const char *name, + const char *regfunc) +{ + char *modfile = NULL; + int ret; + + if (!(modfile = virFileFindResourceFull(name, + "libvirt_storage_backend_", + ".so", + abs_topbuilddir "/src/.libs", + STORAGE_BACKEND_MODULE_DIR, + "LIBVIRT_STORAGE_BACKEND_DIR"))) + return 1; + + ret = virDriverLoadModuleFull(modfile, regfunc, NULL); + + VIR_FREE(modfile); + + return ret; +} + + +# define VIR_STORAGE_BACKEND_REGISTER(func, module) \ + if (virStorageDriverLoadBackendModule(module, #func) < 0) \ + return -1 +#else +# define VIR_STORAGE_BACKEND_REGISTER(func, module) \ + if (func() < 0) \ return -1 +#endif
int virStorageBackendDriversRegister(void) { #if WITH_STORAGE_DIR || WITH_STORAGE_FS - VIR_STORAGE_BACKEND_REGISTER(virStorageBackendFsRegister); + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendFsRegister, "fs"); #endif #if WITH_STORAGE_LVM - VIR_STORAGE_BACKEND_REGISTER(virStorageBackendLogicalRegister); + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendLogicalRegister, "logical"); #endif #if WITH_STORAGE_ISCSI - VIR_STORAGE_BACKEND_REGISTER(virStorageBackendISCSIRegister); + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendISCSIRegister, "iscsi"); #endif #if WITH_STORAGE_SCSI - VIR_STORAGE_BACKEND_REGISTER(virStorageBackendSCSIRegister); + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendSCSIRegister, "scsi"); #endif #if WITH_STORAGE_MPATH - VIR_STORAGE_BACKEND_REGISTER(virStorageBackendMpathRegister); + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendMpathRegister, "mpath"); #endif #if WITH_STORAGE_DISK - VIR_STORAGE_BACKEND_REGISTER(virStorageBackendDiskRegister); + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendDiskRegister, "disk"); #endif #if WITH_STORAGE_RBD - VIR_STORAGE_BACKEND_REGISTER(virStorageBackendRBDRegister); + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendRBDRegister, "rbd"); #endif #if WITH_STORAGE_SHEEPDOG - VIR_STORAGE_BACKEND_REGISTER(virStorageBackendSheepdogRegister); + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendSheepdogRegister, "sheepdog"); #endif #if WITH_STORAGE_GLUSTER - VIR_STORAGE_BACKEND_REGISTER(virStorageBackendGlusterRegister); + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendGlusterRegister, "gluster"); #endif #if WITH_STORAGE_ZFS - VIR_STORAGE_BACKEND_REGISTER(virStorageBackendZFSRegister); + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendZFSRegister, "zfs"); #endif #if WITH_STORAGE_VSTORAGE - VIR_STORAGE_BACKEND_REGISTER(virStorageBackendVstorageRegister); + VIR_STORAGE_BACKEND_REGISTER(virStorageBackendVstorageRegister, "vstorage"); #endif
return 0; diff --git a/tests/Makefile.am b/tests/Makefile.am index e923178f2..401253da3 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -804,7 +804,9 @@ storagebackendsheepdogtest_SOURCES = \ storagebackendsheepdogtest.c \ testutils.c testutils.h storagebackendsheepdogtest_LDADD = \ - ../src/libvirt_driver_storage_impl.la $(LDADDS) + ../src/libvirt_driver_storage_impl.la \ + ../src/libvirt_storage_backend_sheepdog_priv.la \ + $(LDADDS) else ! WITH_STORAGE_SHEEPDOG EXTRA_DIST += storagebackendsheepdogtest.c endif ! WITH_STORAGE_SHEEPDOG

On Fri, Feb 10, 2017 at 08:34:11 -0500, John Ferlan wrote:
On 02/08/2017 11:27 AM, Peter Krempa wrote:
If driver modules are enabled turn storage driver backends into dynamically loadable objects. This will allow greater modularity for binary distributions, where heavyweight dependencies as rbd and gluster can be avoided by selecting only a subset of drivers if the rest is not necessary.
The storage modules are installed into 'LIBDIR/libvirt/storage-backend/' and users can't override the location by using 'LIBVIRT_STORAGE_BACKEND_DIR' environment variable.
can or can't?
oops
rpm based distros will at this point install all the backends when libvirt-daemon-driver-storage package is installed. --- libvirt.spec.in | 17 +++++++++ src/Makefile.am | 85 ++++++++++++++++++++++++++++++++++++++++++- src/storage/storage_backend.c | 60 +++++++++++++++++++++++------- tests/Makefile.am | 4 +- 4 files changed, 151 insertions(+), 15 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in index e8c272bd7..3098ed2dd 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1234,6 +1234,8 @@ rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/lock-driver/*.la rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/lock-driver/*.a rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/connection-driver/*.la rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/connection-driver/*.a +rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/storage-backend/*.la +rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt/storage-backend/*.a %if %{with_wireshark} %if 0%{fedora} >= 24 rm -f $RPM_BUILD_ROOT%{_libdir}/wireshark/plugins/libvirt.la @@ -1689,6 +1691,21 @@ exit 0 %files daemon-driver-storage %attr(0755, root, root) %{_libexecdir}/libvirt_parthelper %{_libdir}/%{name}/connection-driver/libvirt_driver_storage.so +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_fs.so +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_disk.so +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_logical.so +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_scsi.so +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_iscsi.so +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_mpath.so +%if %{with_storage_gluster} +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_gluster.so +%endif +%if %{with_storage_rbd} +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_rbd.so +%endif +%if %{with_storage_sheepdog} +%{_libdir}/%{name}/storage-backend/libvirt_storage_backend_sheepdog.so +%endif
What about zfs and vstorage?
Neither of those is built by the spec file. Since the spec file does not specify any build requirements that are necessary for the ZFS or vstorage driver rpmbuild does not install them and thus such drivers are never going to be built unless somebody adds them to the spec file.

Add a new storage driver registration function that will force the backend code to fail if any of the storage backend modules can't be loaded. This will make sure that they work and are present. --- src/storage/storage_backend.c | 16 ++++++++++++---- src/storage/storage_backend.h | 2 +- src/storage/storage_driver.c | 19 +++++++++++++++++-- src/storage/storage_driver.h | 1 + tests/virdrivermoduletest.c | 2 +- tests/virstoragetest.c | 2 +- 6 files changed, 33 insertions(+), 9 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 32f45e841..ce278b99c 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -87,7 +87,8 @@ static size_t virStorageFileBackendsCount; static int virStorageDriverLoadBackendModule(const char *name, - const char *regfunc) + const char *regfunc, + bool forceload) { char *modfile = NULL; int ret; @@ -100,7 +101,14 @@ virStorageDriverLoadBackendModule(const char *name, "LIBVIRT_STORAGE_BACKEND_DIR"))) return 1; - ret = virDriverLoadModuleFull(modfile, regfunc, NULL); + if ((ret = virDriverLoadModuleFull(modfile, regfunc, NULL)) != 0) { + if (forceload) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to load storage backend module '%s'"), + name); + ret = -1; + } + } VIR_FREE(modfile); @@ -109,7 +117,7 @@ virStorageDriverLoadBackendModule(const char *name, # define VIR_STORAGE_BACKEND_REGISTER(func, module) \ - if (virStorageDriverLoadBackendModule(module, #func) < 0) \ + if (virStorageDriverLoadBackendModule(module, #func, allbackends) < 0) \ return -1 #else # define VIR_STORAGE_BACKEND_REGISTER(func, module) \ @@ -118,7 +126,7 @@ virStorageDriverLoadBackendModule(const char *name, #endif int -virStorageBackendDriversRegister(void) +virStorageBackendDriversRegister(bool allbackends ATTRIBUTE_UNUSED) { #if WITH_STORAGE_DIR || WITH_STORAGE_FS VIR_STORAGE_BACKEND_REGISTER(virStorageBackendFsRegister, "fs"); diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index ca6c19c45..f433071f4 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -198,7 +198,7 @@ struct _virStorageFileBackend { virStorageFileBackendChown storageFileChown; }; -int virStorageBackendDriversRegister(void); +int virStorageBackendDriversRegister(bool allmodules); int virStorageBackendRegister(virStorageBackendPtr backend); int virStorageBackendFileRegister(virStorageFileBackendPtr backend); diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 7fafbcf75..0bc047f23 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2838,9 +2838,10 @@ static virStateDriver stateDriver = { .stateReload = storageStateReload, }; -int storageRegister(void) +static int +storageRegisterFull(bool allbackends) { - if (virStorageBackendDriversRegister() < 0) + if (virStorageBackendDriversRegister(allbackends) < 0) return -1; if (virSetSharedStorageDriver(&storageDriver) < 0) return -1; @@ -2850,6 +2851,20 @@ int storageRegister(void) } +int +storageRegister(void) +{ + return storageRegisterFull(false); +} + + +int +storageRegisterAll(void) +{ + return storageRegisterFull(true); +} + + /* ----------- file handlers cooperating with storage driver --------------- */ static bool virStorageFileIsInitialized(const virStorageSource *src) diff --git a/src/storage/storage_driver.h b/src/storage/storage_driver.h index 682c9ff82..f0aca3671 100644 --- a/src/storage/storage_driver.h +++ b/src/storage/storage_driver.h @@ -70,5 +70,6 @@ char *virStoragePoolObjBuildTempFilePath(virStoragePoolObjPtr pool, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; int storageRegister(void); +int storageRegisterAll(void); #endif /* __VIR_STORAGE_DRIVER_H__ */ diff --git a/tests/virdrivermoduletest.c b/tests/virdrivermoduletest.c index e440350c2..13d51a8e9 100644 --- a/tests/virdrivermoduletest.c +++ b/tests/virdrivermoduletest.c @@ -71,7 +71,7 @@ mymain(void) TEST("interface"); #endif #ifdef WITH_STORAGE - TEST("storage"); + TEST_FULL("storage", "storageRegisterAll"); #endif #ifdef WITH_NODE_DEVICES TEST("nodedev"); diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index d715fd762..36567753b 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -732,7 +732,7 @@ mymain(void) virStorageSourcePtr chain2; /* short for chain->backingStore */ virStorageSourcePtr chain3; /* short for chain2->backingStore */ - if (virStorageBackendDriversRegister() < 0) + if (virStorageBackendDriversRegister(false) < 0) return -1; /* Prep some files with qemu-img; if that is not found on PATH, or -- 2.11.0

On 02/08/2017 11:27 AM, Peter Krempa wrote:
Add a new storage driver registration function that will force the backend code to fail if any of the storage backend modules can't be loaded. This will make sure that they work and are present. --- src/storage/storage_backend.c | 16 ++++++++++++---- src/storage/storage_backend.h | 2 +- src/storage/storage_driver.c | 19 +++++++++++++++++-- src/storage/storage_driver.h | 1 + tests/virdrivermoduletest.c | 2 +- tests/virstoragetest.c | 2 +- 6 files changed, 33 insertions(+), 9 deletions(-)
ACK John

Create a new set of sub-packages containing the new storage driver modules so that certain heavy-weight backends (gluster, rbd) can be installed separately only if required. To keep backward compatibility the 'libvirt-driver-storage' package will be turned into a virtual package pulling in all the new storage backend sub-packages. The storage driver module will be moved into libvirt-driver-storage-core including the filesystem backend which is mandatory. This then allows to make libvirt-daemon-driver-qemu depend only on the core of the storage driver. All other meta-packages still depend on the full storage driver and thus pull in all the backends. --- libvirt.spec.in | 168 +++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 143 insertions(+), 25 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 3098ed2dd..9b2ca1c42 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -583,35 +583,13 @@ Requires: libvirt-daemon = %{version}-%{release} The secret driver plugin for the libvirtd daemon, providing an implementation of the secret key APIs. - -%package daemon-driver-storage -Summary: Storage driver plugin for the libvirtd daemon +%package daemon-driver-storage-core +Summary: Storage driver plugin including base backends for the libvirtd daemon Group: Development/Libraries Requires: libvirt-daemon = %{version}-%{release} Requires: nfs-utils # For mkfs Requires: util-linux -# For glusterfs -%if 0%{?fedora} -Requires: glusterfs-client >= 2.0.1 -%endif -# gluster cli tool for pool discovery -%if (0%{?fedora} || 0%{?with_storage_gluster}) -Requires: /usr/sbin/gluster -%endif -# For LVM drivers -Requires: lvm2 -# For ISCSI driver -Requires: iscsi-initiator-utils -# For disk driver -Requires: parted -Requires: device-mapper -# For multipath support -Requires: device-mapper -%if %{with_storage_sheepdog} -# For Sheepdog support -Requires: sheepdog -%endif %if %{with_qemu} # From QEMU RPMs Requires: /usr/bin/qemu-img @@ -622,6 +600,128 @@ Requires: /usr/sbin/qcow-create %endif %endif +%description daemon-driver-storage-core +The storage driver plugin for the libvirtd daemon, providing +an implementation of the storage APIs using files, local disks, LVM, SCSI, +iSCSI, and multipath storage. + +%package daemon-driver-storage-logical +Summary: Storage driver plugin for lvm volumes +Group: Development/Libraries +Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} +Requires: lvm2 + +%description daemon-driver-storage-logical +The storage driver backend adding implementation of the storage APIs for block +volumes using lvm. + + +%package daemon-driver-storage-disk +Summary: Storage driver plugin for disk +Group: Development/Libraries +Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} +Requires: parted +Requires: device-mapper + +%description daemon-driver-storage-disk +The storage driver backend adding implementation of the storage APIs for block +volumes using the host disks. + + +%package daemon-driver-storage-scsi +Summary: Storage driver plugin for local scsi devices +Group: Development/Libraries +Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} + +%description daemon-driver-storage-scsi +The storage driver backend adding implementation of the storage APIs for scsi +host devices. + + +%package daemon-driver-storage-iscsi +Summary: Storage driver plugin for iscsi +Group: Development/Libraries +Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} +Requires: iscsi-initiator-utils + +%description daemon-driver-storage-iscsi +The storage driver backend adding implementation of the storage APIs for iscsi +volumes using the host iscsi stack. + + +%package daemon-driver-storage-mpath +Summary: Storage driver plugin for multipath volumes +Group: Development/Libraries +Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} +Requires: device-mapper + +%description daemon-driver-storage-mpath +The storage driver backend adding implementation of the storage APIs for +multipath storage using device mapper. + + +%if %{with_storage_gluster} +%package daemon-driver-storage-gluster +Summary: Storage driver plugin for gluster +Group: Development/Libraries +Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} + %if 0%{?fedora} +Requires: glusterfs-client >= 2.0.1 + %endif + %if (0%{?fedora} || 0%{?with_storage_gluster}) +Requires: /usr/sbin/gluster + %endif + +%description daemon-driver-storage-gluster +The storage driver backend adding implementation of the storage APIs for gluster +volumes using libgfapi. +%endif + + +%if %{with_storage_rbd} +%package daemon-driver-storage-rbd +Summary: Storage driver plugin for rbd +Group: Development/Libraries +Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} + +%description daemon-driver-storage-rbd +The storage driver backend adding implementation of the storage APIs for rbd +volumes using the ceph protocol. +%endif + + +%if %{with_storage_sheepdog} +%package daemon-driver-storage-sheepdog +Summary: Storage driver plugin for sheepdog +Group: Development/Libraries +Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} +Requires: sheepdog + +%description daemon-driver-storage-sheepdog +The storage driver backend adding implementation of the storage APIs for +sheepdog volumes using. +%endif + + +%package daemon-driver-storage +Summary: Storage driver plugin including all backends for the libvirtd daemon +Group: Development/Libraries +Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} +Requires: libvirt-daemon-driver-storage-disk = %{version}-%{release} +Requires: libvirt-daemon-driver-storage-logical = %{version}-%{release} +Requires: libvirt-daemon-driver-storage-scsi = %{version}-%{release} +Requires: libvirt-daemon-driver-storage-iscsi = %{version}-%{release} +Requires: libvirt-daemon-driver-storage-mpath = %{version}-%{release} +%if %{with_storage_gluster} +Requires: libvirt-daemon-driver-storage-gluster = %{version}-%{release} +%endif +%if %{with_storage_rbd} +Requires: libvirt-daemon-driver-storage-rbd = %{version}-%{release} +%endif +%if %{with_storage_sheepdog} +Requires: libvirt-daemon-driver-storage-sheepdog = %{version}-%{release} +%endif + %description daemon-driver-storage The storage driver plugin for the libvirtd daemon, providing an implementation of the storage APIs using LVM, iSCSI, @@ -635,7 +735,7 @@ Group: Development/Libraries Requires: libvirt-daemon = %{version}-%{release} # There really is a hard cross-driver dependency here Requires: libvirt-daemon-driver-network = %{version}-%{release} -Requires: libvirt-daemon-driver-storage = %{version}-%{release} +Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} Requires: /usr/bin/qemu-img # For image compression Requires: gzip @@ -1689,21 +1789,39 @@ exit 0 %{_libdir}/%{name}/connection-driver/libvirt_driver_secret.so %files daemon-driver-storage + +%files daemon-driver-storage-core %attr(0755, root, root) %{_libexecdir}/libvirt_parthelper %{_libdir}/%{name}/connection-driver/libvirt_driver_storage.so %{_libdir}/%{name}/storage-backend/libvirt_storage_backend_fs.so + +%files daemon-driver-storage-disk %{_libdir}/%{name}/storage-backend/libvirt_storage_backend_disk.so + +%files daemon-driver-storage-logical %{_libdir}/%{name}/storage-backend/libvirt_storage_backend_logical.so + +%files daemon-driver-storage-scsi %{_libdir}/%{name}/storage-backend/libvirt_storage_backend_scsi.so + +%files daemon-driver-storage-iscsi %{_libdir}/%{name}/storage-backend/libvirt_storage_backend_iscsi.so + +%files daemon-driver-storage-mpath %{_libdir}/%{name}/storage-backend/libvirt_storage_backend_mpath.so + %if %{with_storage_gluster} +%files daemon-driver-storage-gluster %{_libdir}/%{name}/storage-backend/libvirt_storage_backend_gluster.so %endif + %if %{with_storage_rbd} +%files daemon-driver-storage-rbd %{_libdir}/%{name}/storage-backend/libvirt_storage_backend_rbd.so %endif + %if %{with_storage_sheepdog} +%files daemon-driver-storage-sheepdog %{_libdir}/%{name}/storage-backend/libvirt_storage_backend_sheepdog.so %endif -- 2.11.0

On 02/08/2017 11:27 AM, Peter Krempa wrote:
Create a new set of sub-packages containing the new storage driver modules so that certain heavy-weight backends (gluster, rbd) can be installed separately only if required.
To keep backward compatibility the 'libvirt-driver-storage' package will be turned into a virtual package pulling in all the new storage backend sub-packages. The storage driver module will be moved into libvirt-driver-storage-core including the filesystem backend which is mandatory.
This then allows to make libvirt-daemon-driver-qemu depend only on the core of the storage driver.
All other meta-packages still depend on the full storage driver and thus pull in all the backends. --- libvirt.spec.in | 168 +++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 143 insertions(+), 25 deletions(-)
This type of change is further out of my wheelhouse, but I'll leave a few thoughts anyway... I note this too is missing the zfs and vstorage (whether that matters or not). I guess the one question/concern I'd have here would be dealing with various install processing options. I'm assuming the simple case of applying the updated package finds everything correctly. It's a few "edge" cases that I wonder about. It just seems any time we adjust the rpm installation we run into strange issues, but it doesn't happen often enough for me to recall what "common" issues may be ;-) Since I don't typically use rpm installs, this also gets me thinking about 'make install' type environments and if those get everything updated properly too. That is the dependency stuff in Makefile's. John
diff --git a/libvirt.spec.in b/libvirt.spec.in index 3098ed2dd..9b2ca1c42 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -583,35 +583,13 @@ Requires: libvirt-daemon = %{version}-%{release} The secret driver plugin for the libvirtd daemon, providing an implementation of the secret key APIs.
- -%package daemon-driver-storage -Summary: Storage driver plugin for the libvirtd daemon +%package daemon-driver-storage-core +Summary: Storage driver plugin including base backends for the libvirtd daemon Group: Development/Libraries Requires: libvirt-daemon = %{version}-%{release} Requires: nfs-utils # For mkfs Requires: util-linux -# For glusterfs -%if 0%{?fedora} -Requires: glusterfs-client >= 2.0.1 -%endif -# gluster cli tool for pool discovery -%if (0%{?fedora} || 0%{?with_storage_gluster}) -Requires: /usr/sbin/gluster -%endif -# For LVM drivers -Requires: lvm2 -# For ISCSI driver -Requires: iscsi-initiator-utils -# For disk driver -Requires: parted -Requires: device-mapper -# For multipath support -Requires: device-mapper -%if %{with_storage_sheepdog} -# For Sheepdog support -Requires: sheepdog -%endif %if %{with_qemu} # From QEMU RPMs Requires: /usr/bin/qemu-img @@ -622,6 +600,128 @@ Requires: /usr/sbin/qcow-create %endif %endif
+%description daemon-driver-storage-core +The storage driver plugin for the libvirtd daemon, providing +an implementation of the storage APIs using files, local disks, LVM, SCSI, +iSCSI, and multipath storage. + +%package daemon-driver-storage-logical +Summary: Storage driver plugin for lvm volumes +Group: Development/Libraries +Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} +Requires: lvm2 + +%description daemon-driver-storage-logical +The storage driver backend adding implementation of the storage APIs for block +volumes using lvm. + + +%package daemon-driver-storage-disk +Summary: Storage driver plugin for disk +Group: Development/Libraries +Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} +Requires: parted +Requires: device-mapper + +%description daemon-driver-storage-disk +The storage driver backend adding implementation of the storage APIs for block +volumes using the host disks. + + +%package daemon-driver-storage-scsi +Summary: Storage driver plugin for local scsi devices +Group: Development/Libraries +Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} + +%description daemon-driver-storage-scsi +The storage driver backend adding implementation of the storage APIs for scsi +host devices. + + +%package daemon-driver-storage-iscsi +Summary: Storage driver plugin for iscsi +Group: Development/Libraries +Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} +Requires: iscsi-initiator-utils + +%description daemon-driver-storage-iscsi +The storage driver backend adding implementation of the storage APIs for iscsi +volumes using the host iscsi stack. + + +%package daemon-driver-storage-mpath +Summary: Storage driver plugin for multipath volumes +Group: Development/Libraries +Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} +Requires: device-mapper + +%description daemon-driver-storage-mpath +The storage driver backend adding implementation of the storage APIs for +multipath storage using device mapper. + + +%if %{with_storage_gluster} +%package daemon-driver-storage-gluster +Summary: Storage driver plugin for gluster +Group: Development/Libraries +Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} + %if 0%{?fedora} +Requires: glusterfs-client >= 2.0.1 + %endif + %if (0%{?fedora} || 0%{?with_storage_gluster}) +Requires: /usr/sbin/gluster + %endif + +%description daemon-driver-storage-gluster +The storage driver backend adding implementation of the storage APIs for gluster +volumes using libgfapi. +%endif + + +%if %{with_storage_rbd} +%package daemon-driver-storage-rbd +Summary: Storage driver plugin for rbd +Group: Development/Libraries +Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} + +%description daemon-driver-storage-rbd +The storage driver backend adding implementation of the storage APIs for rbd +volumes using the ceph protocol. +%endif + + +%if %{with_storage_sheepdog} +%package daemon-driver-storage-sheepdog +Summary: Storage driver plugin for sheepdog +Group: Development/Libraries +Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} +Requires: sheepdog + +%description daemon-driver-storage-sheepdog +The storage driver backend adding implementation of the storage APIs for +sheepdog volumes using. +%endif + + +%package daemon-driver-storage +Summary: Storage driver plugin including all backends for the libvirtd daemon +Group: Development/Libraries +Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} +Requires: libvirt-daemon-driver-storage-disk = %{version}-%{release} +Requires: libvirt-daemon-driver-storage-logical = %{version}-%{release} +Requires: libvirt-daemon-driver-storage-scsi = %{version}-%{release} +Requires: libvirt-daemon-driver-storage-iscsi = %{version}-%{release} +Requires: libvirt-daemon-driver-storage-mpath = %{version}-%{release} +%if %{with_storage_gluster} +Requires: libvirt-daemon-driver-storage-gluster = %{version}-%{release} +%endif +%if %{with_storage_rbd} +Requires: libvirt-daemon-driver-storage-rbd = %{version}-%{release} +%endif +%if %{with_storage_sheepdog} +Requires: libvirt-daemon-driver-storage-sheepdog = %{version}-%{release} +%endif + %description daemon-driver-storage The storage driver plugin for the libvirtd daemon, providing an implementation of the storage APIs using LVM, iSCSI, @@ -635,7 +735,7 @@ Group: Development/Libraries Requires: libvirt-daemon = %{version}-%{release} # There really is a hard cross-driver dependency here Requires: libvirt-daemon-driver-network = %{version}-%{release} -Requires: libvirt-daemon-driver-storage = %{version}-%{release} +Requires: libvirt-daemon-driver-storage-core = %{version}-%{release} Requires: /usr/bin/qemu-img # For image compression Requires: gzip @@ -1689,21 +1789,39 @@ exit 0 %{_libdir}/%{name}/connection-driver/libvirt_driver_secret.so
%files daemon-driver-storage + +%files daemon-driver-storage-core %attr(0755, root, root) %{_libexecdir}/libvirt_parthelper %{_libdir}/%{name}/connection-driver/libvirt_driver_storage.so %{_libdir}/%{name}/storage-backend/libvirt_storage_backend_fs.so + +%files daemon-driver-storage-disk %{_libdir}/%{name}/storage-backend/libvirt_storage_backend_disk.so + +%files daemon-driver-storage-logical %{_libdir}/%{name}/storage-backend/libvirt_storage_backend_logical.so + +%files daemon-driver-storage-scsi %{_libdir}/%{name}/storage-backend/libvirt_storage_backend_scsi.so + +%files daemon-driver-storage-iscsi %{_libdir}/%{name}/storage-backend/libvirt_storage_backend_iscsi.so + +%files daemon-driver-storage-mpath %{_libdir}/%{name}/storage-backend/libvirt_storage_backend_mpath.so + %if %{with_storage_gluster} +%files daemon-driver-storage-gluster %{_libdir}/%{name}/storage-backend/libvirt_storage_backend_gluster.so %endif + %if %{with_storage_rbd} +%files daemon-driver-storage-rbd %{_libdir}/%{name}/storage-backend/libvirt_storage_backend_rbd.so %endif + %if %{with_storage_sheepdog} +%files daemon-driver-storage-sheepdog %{_libdir}/%{name}/storage-backend/libvirt_storage_backend_sheepdog.so %endif

--- docs/news.xml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 69ed6a75e..041515253 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -43,6 +43,16 @@ and network. </description> </change> + <change> + <summary> + storage: modularize the storage driver + </summary> + <description> + Split up the storage driver backends into loadable modules so that + binary distributions don't have to compromise on shipping the storage + driver with all backends which may pull in too many dependencies. + </description> + </change> </section> <section title="Bug fixes"> <change> -- 2.11.0

On 02/08/2017 11:27 AM, Peter Krempa wrote:
--- docs/news.xml | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml index 69ed6a75e..041515253 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -43,6 +43,16 @@ and network. </description> </change> + <change> + <summary> + storage: modularize the storage driver
storage backends? or storage driver backends
+ </summary> + <description> + Split up the storage driver backends into loadable modules so that + binary distributions don't have to compromise on shipping the storage + driver with all backends which may pull in too many dependencies. + </description> + </change>
seems to be a reasonably short description... I am stuck on the word compromise, but cannot think of a better way to say this. John
</section> <section title="Bug fixes"> <change>

On Wed, Feb 08, 2017 at 05:27:00PM +0100, Peter Krempa wrote:
Split up the storage driver backends into loadable modules so that binary distributions don't have to compromise on shipping the storage driver with all backends which may pull in too many dependencies.
BTW, it has been 9 years since we enabled dlopen for the main drivers. Initially it was tedious to use when running from non-installed git build dir, but we fixed that in 2014 with virFileFindResource, so that it "just works". The only bug we ever hit was when we initially tried to modularize even the non-daemon drivers, and it broke usage from language bindings. Every platform we care about, including Windows, has dlopen() or equivalent functionality. So perhaps it is time to *drop* support for building without modules. It will simplify our makefiles quite a bit to be able to assume everything is always dlopen'd modules, and will slightly simplify the code, and most importantly ensure a single codepath, so we know the behaviour is always the same. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
participants (3)
-
Daniel P. Berrange
-
John Ferlan
-
Peter Krempa