On 10/14/2015 09:45 AM, Peter Krempa wrote: > On Wed, Oct 14, 2015 at 09:35:24 -0400, John Ferlan wrote: >> On 10/14/2015 08:16 AM, Peter Krempa wrote: >>> On Tue, Oct 13, 2015 at 16:12:46 -0400, John Ferlan wrote: > > ... > >>> Erm, you've patched virFileOpenAs a few patches before this to do the >>> right thing when creating the file so I don't see a reason to make sure >>> again. >>> >> >> I know that, look closer at the code after the call to virFileOpenAs. We >> know that if virOpenFileAs succeeds then we've created the file (if it >> didn't exist prior to calling virOpenFileAs); however, if the following >> fails: >> >> >> if ((ret = createRawFile(fd, vol, inputvol, reflink_copy)) < 0) >> /* createRawFile already reported the exact error. */ >> ret = -1; >> >> we would have left the function creating the file, but not deleting it >> because some subsequent action failed. > > I agree that virStorageBackendCreateRaw needs to delete the file in some > cases. I just don't think that checking if the file exists prior to the > call to virOpenFileAs is necessary or makes sense in any way. > > OK - I'm just making sure since it wasn't clear in my mind... again the 'exists' was merely paranoia in the event that the file didn't exist before virFileCreateAs and then we delete something we didn't create. I'll remove the 'exists' checking then - no problem John -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list