On Fri, Oct 09, 2015 at 09:34:11 -0400, John Ferlan wrote: > 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@xxxxxxxxxx> > --- > 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. > + */ This comment should document the required behavior at the point where we define the callback structure. Otherwise somebody might miss it as it'd be stashed in a random function. > + storageVolRemoveFromPool(pool, voldef); > voldef = NULL; > goto cleanup; ACK if you move the comment to a reasonable place. Peter
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list