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@xxxxxxxxxx> > --- > 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list