[libvirt] [PATCH 1/7] storage: Remove unused attribute conn from 'checkPool' callback

John Ferlan jferlan at redhat.com
Wed Mar 25 16:19:18 UTC 2015



On 03/24/2015 06:06 AM, Erik Skultety wrote:
> The checkPool callback should be used when updating states of all pools
> during storageStateInitialize, not storageDriverAutostart, otherwise we
> can't start a domain which mounts a volume after daemons restarted. This
> is because qemuProcessReconnect is called earlier than
> storageDriverAutostart which marks the pool as active, so that the
> domain can mount a volume from this pool successfully.
> In order to fix this, conn attribute has to be discarded from the
> callback, because storageStateInitialize doesn't have a connection yet.
> (it's not used anyway)
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733
> ---
>  src/storage/storage_backend.h         | 3 +--
>  src/storage/storage_backend_fs.c      | 3 +--
>  src/storage/storage_backend_iscsi.c   | 3 +--
>  src/storage/storage_backend_logical.c | 3 +--
>  src/storage/storage_backend_mpath.c   | 3 +--
>  src/storage/storage_backend_scsi.c    | 3 +--
>  src/storage/storage_backend_zfs.c     | 3 +--
>  src/storage/storage_driver.c          | 2 +-
>  8 files changed, 8 insertions(+), 15 deletions(-)
> 

I'm not convinced we can remove 'conn'. Something about backend or
backward compatibility. Since it's not used by any backend anyway,
there's (more or less) no harm in keeping it.

Perhaps others can pipe in on this...

John
> diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
> index 9f1db60..fd2451c 100644
> --- a/src/storage/storage_backend.h
> +++ b/src/storage/storage_backend.h
> @@ -34,8 +34,7 @@
>  typedef char * (*virStorageBackendFindPoolSources)(virConnectPtr conn,
>                                                     const char *srcSpec,
>                                                     unsigned int flags);
> -typedef int (*virStorageBackendCheckPool)(virConnectPtr conn,
> -                                          virStoragePoolObjPtr pool,
> +typedef int (*virStorageBackendCheckPool)(virStoragePoolObjPtr pool,
>                                            bool *active);
>  typedef int (*virStorageBackendStartPool)(virConnectPtr conn,
>                                            virStoragePoolObjPtr pool);
> diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
> index 35385db..d4d65bc 100644
> --- a/src/storage/storage_backend_fs.c
> +++ b/src/storage/storage_backend_fs.c
> @@ -533,8 +533,7 @@ virStorageBackendFileSystemUnmount(virStoragePoolObjPtr pool)
>  
>  
>  static int
> -virStorageBackendFileSystemCheck(virConnectPtr conn ATTRIBUTE_UNUSED,
> -                                 virStoragePoolObjPtr pool,
> +virStorageBackendFileSystemCheck(virStoragePoolObjPtr pool,
>                                   bool *isActive)
>  {
>      if (pool->def->type == VIR_STORAGE_POOL_DIR) {
> diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c
> index 079c767..497a71b 100644
> --- a/src/storage/storage_backend_iscsi.c
> +++ b/src/storage/storage_backend_iscsi.c
> @@ -236,8 +236,7 @@ virStorageBackendISCSIFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED,
>  }
>  
>  static int
> -virStorageBackendISCSICheckPool(virConnectPtr conn ATTRIBUTE_UNUSED,
> -                                virStoragePoolObjPtr pool,
> +virStorageBackendISCSICheckPool(virStoragePoolObjPtr pool,
>                                  bool *isActive)
>  {
>      char *session = NULL;
> diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
> index 7ba8ded..11c5683 100644
> --- a/src/storage/storage_backend_logical.c
> +++ b/src/storage/storage_backend_logical.c
> @@ -479,8 +479,7 @@ virStorageBackendLogicalFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED,
>  
>  
>  static int
> -virStorageBackendLogicalCheckPool(virConnectPtr conn ATTRIBUTE_UNUSED,
> -                                  virStoragePoolObjPtr pool,
> +virStorageBackendLogicalCheckPool(virStoragePoolObjPtr pool,
>                                    bool *isActive)
>  {
>      *isActive = virFileExists(pool->def->target.path);
> diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c
> index 44bcd60..971408a 100644
> --- a/src/storage/storage_backend_mpath.c
> +++ b/src/storage/storage_backend_mpath.c
> @@ -245,8 +245,7 @@ virStorageBackendGetMaps(virStoragePoolObjPtr pool)
>  }
>  
>  static int
> -virStorageBackendMpathCheckPool(virConnectPtr conn ATTRIBUTE_UNUSED,
> -                                virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
> +virStorageBackendMpathCheckPool(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
>                                  bool *isActive)
>  {
>      *isActive = virFileExists("/dev/mpath");
> diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
> index 58e7e6d..66e0846 100644
> --- a/src/storage/storage_backend_scsi.c
> +++ b/src/storage/storage_backend_scsi.c
> @@ -844,8 +844,7 @@ deleteVport(virConnectPtr conn,
>  
>  
>  static int
> -virStorageBackendSCSICheckPool(virConnectPtr conn ATTRIBUTE_UNUSED,
> -                               virStoragePoolObjPtr pool,
> +virStorageBackendSCSICheckPool(virStoragePoolObjPtr pool,
>                                 bool *isActive)
>  {
>      char *path = NULL;
> diff --git a/src/storage/storage_backend_zfs.c b/src/storage/storage_backend_zfs.c
> index 9482706..cb2662a 100644
> --- a/src/storage/storage_backend_zfs.c
> +++ b/src/storage/storage_backend_zfs.c
> @@ -41,8 +41,7 @@ VIR_LOG_INIT("storage.storage_backend_zfs");
>  
>  
>  static int
> -virStorageBackendZFSCheckPool(virConnectPtr conn ATTRIBUTE_UNUSED,
> -                              virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
> +virStorageBackendZFSCheckPool(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
>                                bool *isActive)
>  {
>      char *devpath;
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index b95506f..64ea770 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -100,7 +100,7 @@ storageDriverAutostart(void)
>          }
>  
>          if (backend->checkPool &&
> -            backend->checkPool(conn, pool, &started) < 0) {
> +            backend->checkPool(pool, &started) < 0) {
>              virErrorPtr err = virGetLastError();
>              VIR_ERROR(_("Failed to initialize storage pool '%s': %s"),
>                        pool->def->name, err ? err->message :
> 




More information about the libvir-list mailing list