On Mon, Oct 07, 2013 at 10:48:30AM +0200, Michal Privoznik wrote:
On 07.10.2013 07:20, Hongwei Bi wrote:
> I created a storage volume(eg: test) from a storage pool(eg:vg10) using
> the following command:"virsh vol-create-as --pool vg10 --name test --capacity
300M."
> When I re-executed the above command, the output was as the following:
> "error: Failed to create vol test
> error: Storage volume not found: storage vol 'test' already exists"
>
> I think the output "Storage volume not found" is not appropriate. Because
in fact storage
> vol test has been found at this time. And then I think virErrorNumber should
includes
> VIR_ERR_STORAGE_EXIST which can also be used elsewhere. So I make this patch. The
result
> is as following:
> "error: Failed to create vol test
> error: storage volume 'test' exists already"
>
> ---
> include/libvirt/virterror.h | 1 +
> src/storage/storage_driver.c | 4 ++--
> src/util/virerror.c | 6 ++++++
> 3 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
> index c1960c8..28ef30a 100644
> --- a/include/libvirt/virterror.h
> +++ b/include/libvirt/virterror.h
> @@ -296,6 +296,7 @@ typedef enum {
> VIR_ERR_ACCESS_DENIED = 88, /* operation on the object/resource
> was denied */
> VIR_ERR_DBUS_SERVICE = 89, /* error from a dbus service */
> + VIR_ERR_STORAGE_EXIST = 90, /* the storage vol already exist */
I'd suggest this should be VIR_ERR_STORAGE_VOL_EXISTS.
What about instead of inventing new error code re-use the old VIR_ERR_OPERATION_FAILED?
Something like we have in virDomainObjListAddLocked:
if (STRNEQ(vm->def->name, def->name)) {
virUUIDFormat(vm->def->uuid, uuidstr);
virReportError(VIR_ERR_OPERATION_FAILED,
_("domain '%s' is already defined with uuid
%s"),
vm->def->name, uuidstr);
goto error;
}
I'd actually suggest that we should add a new error code for domains
and other object types too. I think it is useful to be explicit about
duplicate / clashing names/ids.
Regards,
Daniel
--
|:
http://berrange.com -o-
http://www.flickr.com/photos/dberrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|:
http://entangle-photo.org -o-
http://live.gnome.org/gtk-vnc :|