Re: [libvirt] [PATCH 09/12] storage: Cleanup failures in virStorageBackendCreateRaw
On 10/14/2015 08:16 AM, Peter Krempa wrote:
On Tue, Oct 13, 2015 at 16:12:46 -0400, John Ferlan wrote:
> On 10/13/2015 08:43 AM, Peter Krempa wrote:
>> On Fri, Oct 09, 2015 at 09:34:08 -0400, John Ferlan wrote:
...
>>> @@ -520,6 +522,8 @@ virStorageBackendCreateRaw(virConnectPtr conn
ATTRIBUTE_UNUSED,
>>> if (vol->target.perms->mode != (mode_t) -1)
>>> open_mode = vol->target.perms->mode;
>>>
>>> + if (virFileExists(vol->target.path))
>>> + exists = true;
>>
>> Why even bother checking? Shouldn't this function fail right away if the
>> target file exists?
>>
>
> Paranoia. Which function? I assume you mean *CreateRaw. Nothing in the
> function checks for existence. And while there isn't a caller currently
> that wouldn't be creating, I just wanted to be sure and perhaps future
> proof.
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.
John