[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 08/11] storage: Turn storage backends into dynamic modules




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
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]