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

Re: [libvirt] [PATCH] Fix bug #611823 prohibit pools with duplicate storage



Hi Lei Li,

This patch doesn't seem to apply for me.  Please recreate the patch
against an up to date git repository.  Make sure to test that you can
apply the patch yourself.

On 07/28/2011 11:34 PM, Lei Li wrote:
> To make sure the unique storage pool defined and created from different
> directory to avoid inconsistent version of volume pool created, I add
> two API be called by storage driver to check for the probable duplicate
> pools and refused the duplicate pool.
> 
> virStoragePoolObjFindByPath() provide a method to find pool object by
> target path in pool list.
> virStoragePoolTargetDuplicate() implement the function to check if there
> is duplicate pool.
> Add judgement for storagePoolCreate&storagePoolDefine by calling
> virStoragePoolTargetDuplicate() to avoid both transient storage pool and
> persistent storage pool be created repeatedly in storage driver.
> 
> 
> Signed-off-by: Lei Li<lilei linux vnet ibm com>
> ---
>  src/conf/storage_conf.c      |   39
> +++++++++++++++++++++++++++++++++++++++
>  src/conf/storage_conf.h      |    4 ++++
>  src/libvirt_private.syms     |    2 ++
>  src/storage/storage_driver.c |    6 ++++++
>  4 files changed, 51 insertions(+), 0 deletions(-)
> 
> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index 995f9a6..a499e82 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -1317,6 +1317,22 @@
> virStoragePoolObjFindByName(virStoragePoolObjListPtr pools,
>      return NULL;
>  }
> 
> +virStoragePoolObjPtr
> +virStoragePoolObjFindByPath(virStoragePoolObjListPtr pools,
> +                            const char *path)
> +{
> +    unsigned int i;
> +
> +    for (i = 0 ; i<     pools->count ; i++) {

You have some whitespace damage and coding style problems here.

for (i = 0; i < pools->count; i++) {

would be better.

> +        virStoragePoolObjLock(pools->objs[i]);
> +    if (STREQ(pools->objs[i]->def->target.path, path))
> +        return pools->objs[i];

Here you have some more problems with indentation.

> +    virStoragePoolObjUnlock(pools->objs[i]);
> +    }
> +
> +    return NULL;
> +}
> +
>  void
>  virStoragePoolObjClearVols(virStoragePoolObjPtr pool)
>  {
> @@ -1707,6 +1723,29 @@ cleanup:
>      return ret;
>  }
> 
> +int virStoragePoolTargetDuplicate(virStoragePoolObjListPtr pools,
> +                                  virStoragePoolDefPtr def)
> +{
> +    int ret = 1;
> +    virStoragePoolObjPtr pool = NULL;
> +
> +    /*check pool list if defined target path already exist*/

Spaces needed between opening /* and the actual comment.  Same with the
closing */.

> +    pool = virStoragePoolObjFindByPath(pools, def->target.path);
> +
> +    if (pool) {
> +        virStorageReportError(VIR_ERR_OPERATION_FAILED,
> +                              _("target path '%s' is already in use"),
> +                              pool->def->target.path);
> +        ret = -1;
> +        goto cleanup;

You can optimize this a bit by placing the virStoragePoolObjUnlock()
call in here and getting rid of the cleanup: label altogether.

> +    }
> +
> +
> +cleanup:
> +    if (pool)
> +        virStoragePoolObjUnlock(pool);
> +    return ret;   
> +}
> 
>  void virStoragePoolObjLock(virStoragePoolObjPtr obj)
>  {
> diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
> index 271441a..454c43d 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
> virStoragePoolObjFindByPath(virStoragePoolObjListPtr pools,
> +                                                 const char *path);
> 
>  virStorageVolDefPtr virStorageVolDefFindByKey(virStoragePoolObjPtr pool,
>                                                const char *key);
> @@ -387,6 +389,8 @@ char
> *virStoragePoolSourceListFormat(virStoragePoolSourceListPtr def);
>  int virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools,
>                                   virStoragePoolDefPtr def,
>                                   unsigned int check_active);
> +int virStoragePoolTargetDuplicate(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 853ee62..ef323f5 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -930,7 +930,9 @@ virStoragePoolObjClearVols;
>  virStoragePoolObjDeleteDef;
>  virStoragePoolObjFindByName;
>  virStoragePoolObjFindByUUID;
> +virStoragePoolObjFindByPath;
>  virStoragePoolObjIsDuplicate;
> +virStoragePoolTargetDuplicate;
>  virStoragePoolObjListFree;
>  virStoragePoolObjLock;
>  virStoragePoolObjRemove;
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index 9c353e3..8ee63f6 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 (virStoragePoolTargetDuplicate(&driver->pools, def)<     0)
> +        goto cleanup;
> +
>      if ((backend = virStorageBackendForType(def->type)) == NULL)
>          goto cleanup;

More whitespace problems here.

> @@ -589,6 +592,9 @@ storagePoolDefine(virConnectPtr conn,
>      if (virStoragePoolObjIsDuplicate(&driver->pools, def, 0)<     0)
>          goto cleanup;
> 
> +    if (virStoragePoolTargetDuplicate(&driver->pools, def)<     0)
> +        goto cleanup;
> +
>      if (virStorageBackendForType(def->type) == NULL)
>          goto cleanup;
> 

... and here.

-- 
Adam Litke
IBM Linux Technology Center


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