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(a)linux.vnet.ibm.com&gt; --- 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