[libvirt] [PATCH v2] Source control for storage pool

[Daniel Veillard]

On Sat, Sep 03, 2011 at 02:59:18AM +0800, Lei Li wrote:
> Fix bug #611823 storage driver should prohibit pools with duplicate underlying
> storage.
> 
> Add API virStoragePoolSourceFindDuplicate() to do uniqueness check based on
> source location infomation for pool type.

  Okay I tweaked the subject and description a little bit

> 
> Signed-off-by: Lei Li <lilei at linux.vnet.ibm.com>
> ---
>  src/conf/storage_conf.c      |   80 ++++++++++++++++++++++++++++++++++++++++++
>  src/conf/storage_conf.h      |    5 +++
>  src/libvirt_private.syms     |    2 +
>  src/storage/storage_driver.c |    6 +++
>  4 files changed, 93 insertions(+), 0 deletions(-)
> 
> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index 8d14e87..1e7da69 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -1311,6 +1311,21 @@ virStoragePoolObjFindByName(virStoragePoolObjListPtr pools,
>      return NULL;
>  }
>  
> +virStoragePoolObjPtr
> +virStoragePoolSourceFindDuplicateDevices(virStoragePoolObjPtr pool,
> +                                         virStoragePoolDefPtr def) {
> +    unsigned int i, j;
> +
> +    for (i = 0; i < pool->def->source.ndevice; i++) {
> +        for (j = 0; j < def->source.ndevice; j++) {
> +            if (STREQ(pool->def->source.devices[i].path, def->source.devices[j].path))
> +                return pool;
> +        }
> +    }
> +
> +    return NULL;
> +}
> +
>  void
>  virStoragePoolObjClearVols(virStoragePoolObjPtr pool)
>  {
> @@ -1701,6 +1716,71 @@ cleanup:
>      return ret;
>  }
>  
> +int virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools,
> +                                      virStoragePoolDefPtr def)
> +{
> +    int i;
> +    int ret = 1;
> +    virStoragePoolObjPtr pool = NULL;
> +    virStoragePoolObjPtr matchpool = NULL;
> +
> +    /* Check the pool list for duplicate underlying storage */
> +    for (i = 0; i < pools->count; i++) {
> +        pool = pools->objs[i];
> +        if (def->type != pool->def->type)
> +            continue;
> +
> +        virStoragePoolObjLock(pool);
> +
> +        switch (pool->def->type) {
> +        case VIR_STORAGE_POOL_DIR:
> +            if (STREQ(pool->def->target.path, def->target.path))
> +                matchpool = pool;
> +            break;
> +        case VIR_STORAGE_POOL_NETFS:
> +            if ((STREQ(pool->def->source.dir, def->source.dir)) \
> +                && (STREQ(pool->def->source.host.name, def->source.host.name)))
> +                matchpool = pool;
> +            break;
> +        case VIR_STORAGE_POOL_SCSI:
> +            if (STREQ(pool->def->source.adapter, def->source.adapter))
> +                matchpool = pool;
> +            break;
> +        case VIR_STORAGE_POOL_ISCSI:
> +        {
> +            matchpool = virStoragePoolSourceFindDuplicateDevices(pool, def);
> +            if (matchpool) {
> +                if (STREQ(matchpool->def->source.host.name, def->source.host.name)) {
> +                    if ((matchpool->def->source.initiator.iqn) && (def->source.initiator.iqn)) {
> +                        if (STREQ(matchpool->def->source.initiator.iqn, def->source.initiator.iqn))
> +                            break;
> +                        matchpool = NULL;
> +                    }
> +                    break;
> +                }
> +                matchpool = NULL;
> +            }
> +            break;
> +        }
> +        case VIR_STORAGE_POOL_FS:
> +        case VIR_STORAGE_POOL_LOGICAL:
> +        case VIR_STORAGE_POOL_DISK:
> +            matchpool = virStoragePoolSourceFindDuplicateDevices(pool, def);
> +            break;
> +        default:
> +            break;
> +        }
> +        virStoragePoolObjUnlock(pool);
> +    }
> +
> +    if (matchpool) {
> +        virStorageReportError(VIR_ERR_OPERATION_FAILED,
> +                              _("Storage source conflict with pool: '%s'"),
> +                              matchpool->def->name);
> +        ret = -1;
> +    }
> +    return ret;
> +}
>  
>  void virStoragePoolObjLock(virStoragePoolObjPtr obj)
>  {
> diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
> index 271441a..d115a15 100644
> --- a/src/conf/storage_conf.h
> +++ b/src/conf/storage_conf.h
> @@ -335,6 +335,8 @@ virStoragePoolObjPtr virStoragePoolObjFindByUUID(virStoragePoolObjListPtr pools,
>                                                   const unsigned char *uuid);
>  virStoragePoolObjPtr virStoragePoolObjFindByName(virStoragePoolObjListPtr pools,
>                                                   const char *name);
> +virStoragePoolObjPtr virStoragePoolSourceFindDuplicateDevices(virStoragePoolObjPtr pool,
> +                                                              virStoragePoolDefPtr def);
>  
>  virStorageVolDefPtr virStorageVolDefFindByKey(virStoragePoolObjPtr pool,
>                                                const char *key);
> @@ -388,6 +390,9 @@ int virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools,
>                                   virStoragePoolDefPtr def,
>                                   unsigned int check_active);
>  
> +int virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools,
> +                                      virStoragePoolDefPtr def);
> +
>  void virStoragePoolObjLock(virStoragePoolObjPtr obj);
>  void virStoragePoolObjUnlock(virStoragePoolObjPtr obj);
>  
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 9f03e30..316eb7e 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -947,7 +947,9 @@ virStoragePoolObjClearVols;
>  virStoragePoolObjDeleteDef;
>  virStoragePoolObjFindByName;
>  virStoragePoolObjFindByUUID;
> +virStoragePoolSourceFindDuplicateDevices;
>  virStoragePoolObjIsDuplicate;
> +virStoragePoolSourceFindDuplicate;
>  virStoragePoolObjListFree;
>  virStoragePoolObjLock;
>  virStoragePoolObjRemove;
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index 68cac1f..c05b74e 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -536,6 +536,9 @@ storagePoolCreate(virConnectPtr conn,
>      if (virStoragePoolObjIsDuplicate(&driver->pools, def, 1) < 0)
>          goto cleanup;
>  
> +    if (virStoragePoolSourceFindDuplicate(&driver->pools, def) < 0)
> +        goto cleanup;
> +
>      if ((backend = virStorageBackendForType(def->type)) == NULL)
>          goto cleanup;
>  
> @@ -589,6 +592,9 @@ storagePoolDefine(virConnectPtr conn,
>      if (virStoragePoolObjIsDuplicate(&driver->pools, def, 0) < 0)
>          goto cleanup;
>  
> +    if (virStoragePoolSourceFindDuplicate(&driver->pools, def) < 0)
> +        goto cleanup;
> +
>      if (virStorageBackendForType(def->type) == NULL)
>          goto cleanup;
>  

  ACK, the two comments from Dan have been addressed in v2, and patch
looks fine so applied,

  I also added you to the AUTHORS before commiting,

   thanks a lot,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/