[libvirt] [REPOST PATCH 6/6] storage: On 'buildVol' failure don't delete the volume
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(a)redhat.com>
---
src/storage/storage_backend.h | 10 ++++++++++
src/storage/storage_driver.c | 4 ++--
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 39c00b1..96b5f39 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -48,6 +48,16 @@ typedef int (*virStorageBackendStopPool)(virConnectPtr conn,
typedef int (*virStorageBackendDeletePool)(virConnectPtr conn,
virStoragePoolObjPtr pool,
unsigned int flags);
+
+/* A 'buildVol' backend must remove any volume created on error since
+ * the storage driver does not distinguish whether the failure is due
+ * to failure to create the volume, to reserve any space necessary for
+ * the volume, to get data about the volume, to change it's accessibility,
+ * etc. 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 checking the pool and the create attempt. It
+ * also avoids extra round trips to just delete a file.
+ */
typedef int (*virStorageBackendBuildVol)(virConnectPtr conn,
virStoragePoolObjPtr pool,
virStorageVolDefPtr vol,
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 31b7095..ee8b263 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1869,8 +1869,8 @@ storageVolCreateXML(virStoragePoolPtr obj,
pool->asyncjobs--;
if (buildret < 0) {
- storageVolDeleteInternal(volobj, backend, pool, voldef,
- 0, false);
+ /* buildVol handles deleting volume on failure */
+ storageVolRemoveFromPool(pool, voldef);
voldef = NULL;
goto cleanup;
}
--
2.1.0