[libvirt] [PATCH 06/11] storage: backend: Refactor registration of the backend drivers

[John Ferlan]

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;
> +}

[...]