[libvirt] [PATCHv3] avoid false creation discard pool definition

27 Nov 2011

False pool creation will clear previous definition.
This patch roll back to previous definition after pool-creat fails
ref:
http://www.redhat.com/archives/libvir-list/2011-November/msg01152.html
http://www.redhat.com/archives/libvir-list/2011-November/msg01152.html

Signed-off-by: Wen Ruo Lv <lvroyce@linux.vnet.ibm.com>
---
 src/conf/storage_conf.c      |   30 +++++++++++++++++++++++++-----
 src/conf/storage_conf.h      |    4 ++--
 src/libvirt_private.syms     |    1 +
 src/storage/storage_driver.c |   36 ++++++++++++++++++++++++------------
 src/test/test_driver.c       |   18 +++++++++---------
 5 files changed, 61 insertions(+), 28 deletions(-)

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index dadc115..bf82eb6 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1385,17 +1385,20 @@ virStorageVolDefFindByName(virStoragePoolObjPtr pool,
 
 virStoragePoolObjPtr
 virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
-                           virStoragePoolDefPtr def) {
+                           virStoragePoolDefPtr *pdef) {
     virStoragePoolObjPtr pool;
+    virStoragePoolDefPtr lastDef;
+    virStoragePoolDefPtr def = *pdef;
 
     if ((pool = virStoragePoolObjFindByName(pools, def->name))) {
         if (!virStoragePoolObjIsActive(pool)) {
-            virStoragePoolDefFree(pool->def);
+            lastDef = pool->def;
             pool->def = def;
         } else {
-            virStoragePoolDefFree(pool->newDef);
+            lastDef = pool->newDef;
             pool->newDef = def;
         }
+        *pdef = lastDef;
         return pool;
     }
 
@@ -1421,11 +1424,26 @@ virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
         virReportOOMError();
         return NULL;
     }
+    *pdef = NULL;
     pools->objs[pools->count++] = pool;
 
     return pool;
 }
 
+void virStoragePoolObjDefRollBack(virStoragePoolObjPtr pool, virStoragePoolDefPtr *lastDef)
+{
+    virStoragePoolDefPtr tmpDef;
+
+    if (pool->active) {
+        tmpDef = pool->newDef;
+        pool->newDef = *lastDef;
+    }
+    else {
+        tmpDef = pool->def;
+        pool->def = *lastDef;
+    }
+    *lastDef = tmpDef;
+}
 static virStoragePoolObjPtr
 virStoragePoolObjLoad(virStoragePoolObjListPtr pools,
                       const char *file,
@@ -1446,7 +1464,7 @@ virStoragePoolObjLoad(virStoragePoolObjListPtr pools,
         return NULL;
     }
 
-    if (!(pool = virStoragePoolObjAssignDef(pools, def))) {
+    if (!(pool = virStoragePoolObjAssignDef(pools, &def))) {
         virStoragePoolDefFree(def);
         return NULL;
     }
@@ -1455,6 +1473,7 @@ virStoragePoolObjLoad(virStoragePoolObjListPtr pools,
     pool->configFile = strdup(path);
     if (pool->configFile == NULL) {
         virReportOOMError();
+        virStoragePoolObjDefRollBack(pool, &def);
         virStoragePoolDefFree(def);
         return NULL;
     }
@@ -1462,13 +1481,14 @@ virStoragePoolObjLoad(virStoragePoolObjListPtr pools,
     pool->autostartLink = strdup(autostartLink);
     if (pool->autostartLink == NULL) {
         virReportOOMError();
+        virStoragePoolObjDefRollBack(pool, &def);
         virStoragePoolDefFree(def);
         return NULL;
     }
 
     pool->autostart = virFileLinkPointsTo(pool->autostartLink,
                                           pool->configFile);
-
+    virStoragePoolDefFree(def);
     return pool;
 }
 
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 19bbd2c..dc9dc05 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -364,8 +364,8 @@ char *virStorageVolDefFormat(virStoragePoolDefPtr pool,
                              virStorageVolDefPtr def);
 
 virStoragePoolObjPtr virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
-                                                virStoragePoolDefPtr def);
-
+                                                virStoragePoolDefPtr *def);
+void virStoragePoolObjDefRollBack(virStoragePoolObjPtr pool,virStoragePoolDefPtr *lastDef);
 int virStoragePoolObjSaveDef(virStorageDriverStatePtr driver,
                              virStoragePoolObjPtr pool,
                              virStoragePoolDefPtr def);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0b21cdc..391998c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -975,6 +975,7 @@ virStoragePoolLoadAllConfigs;
 virStoragePoolObjAssignDef;
 virStoragePoolObjClearVols;
 virStoragePoolObjDeleteDef;
+virStoragePoolObjDefRollBack;
 virStoragePoolObjFindByName;
 virStoragePoolObjFindByUUID;
 virStoragePoolObjIsDuplicate;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 8c2d6e1..4ea22d5 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)
@@ -542,22 +543,29 @@ storagePoolCreate(virConnectPtr conn,
     if ((backend = virStorageBackendForType(def->type)) == NULL)
         goto cleanup;
 
-    if (!(pool = virStoragePoolObjAssignDef(&driver->pools, def)))
+    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)
+            virStoragePoolObjDefRollBack(pool, &def);
+        else {
+            virStoragePoolObjRemove(&driver->pools, pool);
+            pool = NULL;
+        }
         goto cleanup;
     }
 
     if (backend->refreshPool(conn, pool) < 0) {
         if (backend->stopPool)
             backend->stopPool(conn, pool);
-        virStoragePoolObjRemove(&driver->pools, pool);
-        pool = NULL;
+        if (dupPool)
+            virStoragePoolObjDefRollBack(pool, &def);
+        else {
+            virStoragePoolObjRemove(&driver->pools, pool);
+            pool = NULL;
+        }
         goto cleanup;
     }
     VIR_INFO("Creating storage pool '%s'", pool->def->name);
@@ -582,6 +590,7 @@ storagePoolDefine(virConnectPtr conn,
     virStoragePoolDefPtr def;
     virStoragePoolObjPtr pool = NULL;
     virStoragePoolPtr ret = NULL;
+    int dupPool;
 
     virCheckFlags(0, NULL);
 
@@ -589,7 +598,7 @@ storagePoolDefine(virConnectPtr conn,
     if (!(def = virStoragePoolDefParseString(xml)))
         goto cleanup;
 
-    if (virStoragePoolObjIsDuplicate(&driver->pools, def, 0) < 0)
+    if ((dupPool = virStoragePoolObjIsDuplicate(&driver->pools, def, 0)) < 0)
         goto cleanup;
 
     if (virStoragePoolSourceFindDuplicate(&driver->pools, def) < 0)
@@ -598,15 +607,18 @@ storagePoolDefine(virConnectPtr conn,
     if (virStorageBackendForType(def->type) == NULL)
         goto cleanup;
 
-    if (!(pool = virStoragePoolObjAssignDef(&driver->pools, def)))
+    if (!(pool = virStoragePoolObjAssignDef(&driver->pools, &def)))
         goto cleanup;
 
     if (virStoragePoolObjSaveDef(driver, pool, def) < 0) {
-        virStoragePoolObjRemove(&driver->pools, pool);
-        def = NULL;
+        if (dupPool)
+            virStoragePoolObjDefRollBack(pool, &def);
+        else {
+            virStoragePoolObjRemove(&driver->pools, pool);
+            pool = NULL;
+        }
         goto cleanup;
     }
-    def = NULL;
 
     VIR_INFO("Defining storage pool '%s'", pool->def->name);
     ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid);
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index ce94a17..65dd606 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -594,10 +594,8 @@ static int testOpenDefault(virConnectPtr conn) {
         goto error;
 
     if (!(poolobj = virStoragePoolObjAssignDef(&privconn->pools,
-                                               pooldef))) {
-        virStoragePoolDefFree(pooldef);
+                                               &pooldef)))
         goto error;
-    }
 
     if (testStoragePoolObjSetDefaults(poolobj) == -1) {
         virStoragePoolObjUnlock(poolobj);
@@ -614,13 +612,14 @@ static int testOpenDefault(virConnectPtr conn) {
         virNodeDeviceDefFree(nodedef);
         goto error;
     }
+    virStoragePoolDefFree(pooldef);
     virNodeDeviceObjUnlock(nodeobj);
 
     testDriverUnlock(privconn);
 
     return VIR_DRV_OPEN_SUCCESS;
-
 error:
+    virStoragePoolDefFree(pooldef);
     virDomainObjListDeinit(&privconn->domains);
     virNetworkObjListFree(&privconn->networks);
     virInterfaceObjListFree(&privconn->ifaces);
@@ -1016,12 +1015,13 @@ static int testOpenFromFile(virConnectPtr conn,
         }
 
         if (!(pool = virStoragePoolObjAssignDef(&privconn->pools,
-                                                def))) {
+                                                &def))) {
             virStoragePoolDefFree(def);
             goto error;
         }
 
         if (testStoragePoolObjSetDefaults(pool) == -1) {
+            virStoragePoolDefFree(def);
             virStoragePoolObjUnlock(pool);
             goto error;
         }
@@ -1029,10 +1029,12 @@ static int testOpenFromFile(virConnectPtr conn,
 
         /* Find storage volumes */
         if (testOpenVolumesForPool(xml, ctxt, file, pool, i+1) < 0) {
+            virStoragePoolDefFree(def);
             virStoragePoolObjUnlock(pool);
             goto error;
         }
 
+        virStoragePoolDefFree(def);
         virStoragePoolObjUnlock(pool);
     }
     VIR_FREE(pools);
@@ -4123,9 +4125,8 @@ testStoragePoolCreate(virConnectPtr conn,
         goto cleanup;
     }
 
-    if (!(pool = virStoragePoolObjAssignDef(&privconn->pools, def)))
+    if (!(pool = virStoragePoolObjAssignDef(&privconn->pools, &def)))
         goto cleanup;
-    def = NULL;
 
     if (testStoragePoolObjSetDefaults(pool) == -1) {
         virStoragePoolObjRemove(&privconn->pools, pool);
@@ -4164,9 +4165,8 @@ testStoragePoolDefine(virConnectPtr conn,
     def->allocation = defaultPoolAlloc;
     def->available = defaultPoolCap - defaultPoolAlloc;
 
-    if (!(pool = virStoragePoolObjAssignDef(&privconn->pools, def)))
+    if (!(pool = virStoragePoolObjAssignDef(&privconn->pools, &def)))
         goto cleanup;
-    def = NULL;
 
     if (testStoragePoolObjSetDefaults(pool) == -1) {
         virStoragePoolObjRemove(&privconn->pools, pool);
-- 
1.7.4.1

    

Wen Ruo Lv

tags

participants (1)