storage: Don't update pool available/allocation if buildVol fails

https://bugzilla.redhat.com/show_bug.cgi?id=1024159

If adding a volume to a storage pool fails during the CreateXML or
CreateXMLFrom API's, we don't want to adjust the available and
allocation values for the storage pool during storageVolDelete
since we haven't adjusted the values for the create.

Refactor storageVolDelete() a bit to create a storageVolDeleteInternal()
which will handle the primary deletion activities.  Add a parameter
updateMeta which will signify whether to update the values or not.

Adjust the calls from CreateXML and CreateXMLFrom to directly call the
DeleteInternal with the pool lock held.  This does bypass the call
to virStorageVolDeleteEnsureACL().

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 2f98f7816d0a6ac3de5c4482618453536368292d..ddd282eb4979f3501b01d376a66e5a37346045d7 100644 (file)
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1552,6 +1552,52 @@ storageVolLookupByPath(virConnectPtr conn,
 }
 
 
+static int
+storageVolDeleteInternal(virStorageVolPtr obj,
+                         virStorageBackendPtr backend,
+                         virStoragePoolObjPtr pool,
+                         virStorageVolDefPtr vol,
+                         unsigned int flags,
+                         bool updateMeta)
+{
+    size_t i;
+    int ret = -1;
+
+    if (!backend->deleteVol) {
+        virReportError(VIR_ERR_NO_SUPPORT,
+                       "%s", _("storage pool does not support vol deletion"));
+
+        goto cleanup;
+    }
+
+    if (backend->deleteVol(obj->conn, pool, vol, flags) < 0)
+        goto cleanup;
+
+    /* Update pool metadata - don't update meta data from error paths
+     * in this module since the allocation/available weren't adjusted yet
+     */
+    if (updateMeta) {
+        pool->def->allocation -= vol->target.allocation;
+        pool->def->available += vol->target.allocation;
+    }
+
+    for (i = 0; i < pool->volumes.count; i++) {
+        if (pool->volumes.objs[i] == vol) {
+            VIR_INFO("Deleting volume '%s' from storage pool '%s'",
+                     vol->name, pool->def->name);
+            virStorageVolDefFree(vol);
+
+            VIR_DELETE_ELEMENT(pool->volumes.objs, i, pool->volumes.count);
+            break;
+        }
+    }
+    ret = 0;
+
+ cleanup:
+    return ret;
+}
+
+
 static int
 storageVolDelete(virStorageVolPtr obj,
                  unsigned int flags)
@@ -1560,7 +1606,6 @@ storageVolDelete(virStorageVolPtr obj,
     virStoragePoolObjPtr pool;
     virStorageBackendPtr backend;
     virStorageVolDefPtr vol = NULL;
-    size_t i;
     int ret = -1;
 
     storageDriverLock(driver);
@@ -1602,30 +1647,9 @@ storageVolDelete(virStorageVolPtr obj,
         goto cleanup;
     }
 
-    if (!backend->deleteVol) {
-        virReportError(VIR_ERR_NO_SUPPORT,
-                       "%s", _("storage pool does not support vol deletion"));
-
-        goto cleanup;
-    }
-
-    if (backend->deleteVol(obj->conn, pool, vol, flags) < 0)
+    if (storageVolDeleteInternal(obj, backend, pool, vol, flags, true) < 0)
         goto cleanup;
 
-    /* Update pool metadata */
-    pool->def->allocation -= vol->target.allocation;
-    pool->def->available += vol->target.allocation;
-
-    for (i = 0; i < pool->volumes.count; i++) {
-        if (pool->volumes.objs[i] == vol) {
-            VIR_INFO("Deleting volume '%s' from storage pool '%s'",
-                     vol->name, pool->def->name);
-            virStorageVolDefFree(vol);
-
-            VIR_DELETE_ELEMENT(pool->volumes.objs, i, pool->volumes.count);
-            break;
-        }
-    }
     ret = 0;
 
  cleanup:
@@ -1738,9 +1762,9 @@ storageVolCreateXML(virStoragePoolPtr obj,
         voldef = NULL;
 
         if (buildret < 0) {
-            virStoragePoolObjUnlock(pool);
-            storageVolDelete(volobj, 0);
-            pool = NULL;
+            storageVolDeleteInternal(volobj, backend, pool, buildvoldef,
+                                     0, false);
+            buildvoldef = NULL;
             goto cleanup;
         }
 
@@ -1908,7 +1932,6 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj,
     origvol->building = 0;
     newvol->building = 0;
     allocation = newvol->target.allocation;
-    newvol = NULL;
     pool->asyncjobs--;
 
     if (origpool) {
@@ -1918,11 +1941,11 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj,
     }
 
     if (buildret < 0) {
-        virStoragePoolObjUnlock(pool);
-        storageVolDelete(volobj, 0);
-        pool = NULL;
+        storageVolDeleteInternal(volobj, backend, pool, newvol, 0, false);
+        newvol = NULL;
         goto cleanup;
     }
+    newvol = NULL;
 
     /* Updating pool metadata */
     pool->def->allocation += allocation;