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

[Michal Privoznik]

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.  Moreover, what I think we should
do, is extend virStorageBackendBuildVol for boolean argument to tell if
volume was created (e.g. a file, a dir) and hence storageVolumeDelete()
needs to be called (I am talking about storageVolumeCreateXML(), lines
1428-1445). Or maybe we can lose the new argument and distinguish these
states by different return values, e.g. -1 = okay, something went wrong
but no file/dir/whatever was created; -2 = well, it's completely messed
up, call storageVolumeDelete().

Michal