[libvirt] [PATCH] redefine pool after pool creation failure

[Daniel P. Berrange]

On Wed, Nov 23, 2011 at 11:09:16AM +0800, Wen Ruo Lv wrote:
> Redefine pool after pool creation failure,give out err msg for non-exsisted vg when starting pool.
> ref:
> http://www.redhat.com/archives/libvir-list/2011-November/msg01152.html
> http://www.redhat.com/archives/libvir-list/2011-November/msg01152.html
> 
> src/storage/storage_backend_logical.c:just give out err msg for non-exsisted vg,not combine pool-build in pool-create ,since undo build for kinds of pool(iscsi,multipath) when err occours cause a lot of trouble.
> 
> src/storage/storage_driver.c:add redefinition of pool,pool may change in pool-start,redefine it after free the pool
> 
> Signed-off-by: Wen Ruo Lv <lvroyce at linux.vnet.ibm.com>
> ---
>  src/storage/storage_backend_logical.c |   10 ++++++++++
>  src/storage/storage_driver.c          |   29 +++++++++++++++++++++++++----
>  2 files changed, 35 insertions(+), 4 deletions(-)
> 
> diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
> index 3c3e736..994c792 100644
> --- a/src/storage/storage_backend_logical.c
> +++ b/src/storage/storage_backend_logical.c
> @@ -50,6 +50,16 @@ virStorageBackendLogicalSetActive(virStoragePoolObjPtr pool,
>  {
>      const char *cmdargv[4];
>  
> +    cmdargv[0] = VGS;
> +    cmdargv[1] = pool->def->source.name;
> +    cmdargv[2] = NULL;
> +
> +    if (virRun(cmdargv, NULL) < 0) {
> +        virStorageReportError(VIR_ERR_INTERNAL_ERROR,
> +                                  _("vg '%s' does not exsist,try pool-build"), pool->def->source.name);
> +        return -1;
> +    }
> +
>      cmdargv[0] = VGCHANGE;
>      cmdargv[1] = on ? "-ay" : "-an";
>      cmdargv[2] = pool->def->source.name;
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index 8c2d6e1..a5cbafe 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -526,6 +526,7 @@ storagePoolCreate(virConnectPtr conn,
>      virStoragePoolObjPtr pool = NULL;
>      virStoragePoolPtr ret = NULL;
>      virStorageBackendPtr backend;
> +    int duppool;
>  
>      virCheckFlags(0, NULL);
>  
> @@ -533,7 +534,7 @@ storagePoolCreate(virConnectPtr conn,
>      if (!(def = virStoragePoolDefParseString(xml)))
>          goto cleanup;
>  
> -    if (virStoragePoolObjIsDuplicate(&driver->pools, def, 1) < 0)
> +    if ((duppool = virStoragePoolObjIsDuplicate(&driver->pools, def, 1)) < 0)
>          goto cleanup;
>  
>      if (virStoragePoolSourceFindDuplicate(&driver->pools, def) < 0)
> @@ -544,26 +545,46 @@ storagePoolCreate(virConnectPtr conn,
>  
>      if (!(pool = virStoragePoolObjAssignDef(&driver->pools, def)))
>          goto cleanup;
> -    def = NULL;
>  
>      if (backend->startPool &&
>          backend->startPool(conn, pool) < 0) {
>          virStoragePoolObjRemove(&driver->pools, pool);
> -        pool = NULL;
> +
> +        if (duppool) {
> +            if (!(def = virStoragePoolDefParseString(xml)))
> +                goto cleanup;
> +
> +            pool = virStoragePoolObjAssignDef(&driver->pools, def);
> +        }
> +        else
> +            pool = NULL;
> +
> +        def = NULL;
>          goto cleanup;
>      }
>  
>      if (backend->refreshPool(conn, pool) < 0) {
>          if (backend->stopPool)
>              backend->stopPool(conn, pool);
> +
>          virStoragePoolObjRemove(&driver->pools, pool);
> -        pool = NULL;
> +
> +        if (duppool) {
> +            if (!(def = virStoragePoolDefParseString(xml)))
> +                goto cleanup;
> +            pool = virStoragePoolObjAssignDef(&driver->pools, def);
> +        }
> +        else
> +            pool = NULL;
> +
> +        def = NULL;
>          goto cleanup;
>      }
>      VIR_INFO("Creating storage pool '%s'", pool->def->name);
>      pool->active = 1;
>  
>      ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid);
> +    def = NULL;
>  

I don't really like this approach of re-parsing XML & recreating the
object, because I don't think this guarentees we get back to the
original state. The real fix is to not delete the original object in
the first place.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|