On 02.10.2015 15:41, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1233003
Although perhaps bordering on a don't do that type scenario, if
someone creates a volume in a pool outside of libvirt, then uses that
same name to create a volume in the pool via libvirt, then the creation
will fail and in some cases cause the same name volume to be deleted.
This patch will refresh the pool just prior to checking whether the
named volume exists prior to creating the volume in the pool. While
it's still possible to have a timing window to create a file after the
check - at least we tried. At that point, someone is being malicious.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_driver.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 7aaa060..ddf4405 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1796,6 +1796,15 @@ storageVolCreateXML(virStoragePoolPtr obj,
if (virStorageVolCreateXMLEnsureACL(obj->conn, pool->def, voldef) < 0)
goto cleanup;
+ /* While not perfect, refresh the list of volumes in the pool and
+ * then check that the incoming name isn't already in the pool.
+ */
+ if (backend->refreshPool) {
+ virStoragePoolObjClearVols(pool);
+ if (backend->refreshPool(obj->conn, pool) < 0)
+ goto cleanup;
+ }
+
if (virStorageVolDefFindByName(pool, voldef->name)) {
virReportError(VIR_ERR_STORAGE_VOL_EXIST,
_("'%s'"), voldef->name);
Okay, this makes sense. Not only from the POV you are presenting, but I'd expect my
pool to be refreshed after I create new volume in it.
And I think we have a way to eliminate even the little window - just track if we
successfully built the volume or not. Something among these lines:
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index ddf4405..dd28f3f 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1857,7 +1857,7 @@ storageVolCreateXML(virStoragePoolPtr obj,
voldef->building = true;
virStoragePoolObjUnlock(pool);
- buildret = backend->buildVol(obj->conn, pool, buildvoldef, flags);
+ buildret = backend->buildVol(obj->conn, pool, buildvoldef, &created,
flags);
storageDriverLock();
virStoragePoolObjLock(pool);
@@ -1866,7 +1866,7 @@ storageVolCreateXML(virStoragePoolPtr obj,
voldef->building = false;
pool->asyncjobs--;
- if (buildret < 0) {
+ if (buildret < 0 && created) {
VIR_FREE(buildvoldef);
storageVolDeleteInternal(volobj, backend, pool, voldef,
0, false);
At any rate, ACK to this patch as is.
Michal