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

[Peter Krempa]

On Tue, Oct 13, 2015 at 16:58:53 -0400, John Ferlan wrote:
> On 10/13/2015 08:39 AM, Peter Krempa wrote:
> > On Fri, Oct 09, 2015 at 09:34:11 -0400, John Ferlan wrote:

...

> > 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.
> > 
> 
> OK, I left:     /* buildVol handles deleting volume on failure */
> 
> Since a few lines later the refreshVol calls the DeleteVol - I just
> wanted to make it clear that the same delete call is not necessary.

Ok, that makes sense.

> 
> >> +            storageVolRemoveFromPool(pool, voldef);
> >>              voldef = NULL;
> >>              goto cleanup;
> > 
> > ACK if you move the comment to a reasonable place.
> 
> I hope src/storage/storage_backend.h is your idea of the reasonable
> place - I added the following text there right above the typedef:
> 
> /* 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,
> 
> 
> If you had a different location in mind, then let me know.
> 

I had in mind a few lines below where the storage driver struct is
defined but this place is equally good since anyone implementing the API
will have to look at the function type.

Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20151014/fa115b61/attachment-0001.sig>