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