[libvirt] [PATCH] storage: Error out earlier if the volume target path already exists

[Osier Yang]

On 2012年12月05日 18:12, Michal Privoznik wrote:
> On 05.12.2012 05:44, Osier Yang wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=832302
>>
>> It's odd to fall through to buildVol, and the existed file is
>> removed when buildVol fails. This checks if the volume target
>> path already exists in createVol. The reason for not using
>> error like "Volume already exists" is that there isn't volume
>> maintained by libvirt for the path until a operation like
>> pool-refresh, using error like that will just cause confusion.
>>
>> ---
>> BTW, It inspires me again that perhaps we should integrate things
>> like inotify to storage.
>> ---
>>   src/storage/storage_backend_fs.c |    7 +++++++
>>   1 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
>> index 4e6ebbf..6cddad0 100644
>> --- a/src/storage/storage_backend_fs.c
>> +++ b/src/storage/storage_backend_fs.c
>> @@ -1000,6 +1000,13 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn ATTRIBUTE_UNUSED,
>>           return -1;
>>       }
>>
>> +    if (virFileExists(vol->target.path)) {
>> +        virReportError(VIR_ERR_OPERATION_INVALID,
>> +                       _("volume target path '%s' already exists"),
>> +                       vol->target.path);
>> +        return -1;
>> +    }
>> +
>>       VIR_FREE(vol->key);
>>       vol->key = strdup(vol->target.path);
>>       if (vol->key == NULL) {
>>
>
> While this fix works for VIR_STORAGE_FILE_RAW it doesn't work for other
> types, such as VIR_STORAGE_FILE_DIR.

It works for vol of type VIR_STORAGE_FILE_DIR too actually. As access(2)
also works on a directory path. And it should also work for the
volume which will be created using the image tools. To prove:

% mkdir /var/lib/libvirt/images/test
% virsh vol-create default vol.xml
error: Failed to create vol from vol.xml
error: Requested operation is not valid: volume target path 
'/var/lib/libvirt/images/test' already exists

% cat vol.xml
<volume>
   <name>test</name>
   <capacity unit='bytes'>0</capacity>
   <allocation unit='bytes'>0</allocation>
   <target>
     <format type='dir'/>
   </target>
</volume>

Regards,
Osier