[libvirt] [PATCH 12/12] storage: On 'buildVol' failure don't delete the volume

[John Ferlan]

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

Commit id 'fdda3760' only managed a symptom where it was possible to
create a file in a pool without libvirt's knowledge, so it was reverted.

The real fix is to have all the createVol API's which actually create
a volume (disk, logical, zfs) and the buildVol API's which handle the
real creation of some volume file (fs, rbd, sheepdog) manage deleting
any volume which they create when there is some sort of error in
processing the volume.

This way the onus isn't left up to the storage_driver to determine whether
the buildVol failure was due to some failure as a result of adjustments
made to the volume after creation such as getting sizes, changing ownership,
changing volume protections, etc. or simple a failure in creation.

Without needing to consider that the volume has to be removed, the
buildVol failure path only needs to remove the volume from the pool.
This way if a creation failed due to duplicate name, libvirt wouldn't
remove a volume that it didn't create in the pool target.

Signed-off-by: John Ferlan <jferlan at redhat.com>
---
 src/storage/storage_driver.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 58bc4bd..0c70482 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1869,8 +1869,17 @@ storageVolCreateXML(virStoragePoolPtr obj,
         pool->asyncjobs--;
 
         if (buildret < 0) {
-            storageVolDeleteInternal(volobj, backend, pool, voldef,
-                                     0, false);
+            /* NB: A 'buildVol' backend must remove any volume created
+             * on error since this code does not distinguish whether the
+             * failure is to create the volume, reserve any space necessary
+             * for the volume, get data about the volume, change it's
+             * accessibility, etc. All that should be done at this point
+             * is remove the volume from the pool. This avoids issues
+             * arising from a creation failure due to some external action
+             * which created a volume of the same name that libvirt was
+             * not aware of between the time checked and create attempt.
+             */
+            storageVolRemoveFromPool(pool, voldef);
             voldef = NULL;
             goto cleanup;
         }
-- 
2.1.0