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