Re: [PATCH v2] fix a ambiguous output of the command:'virsh vol-create-as'

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2013/10/8 Eric Blake <eblake@xxxxxxxxxx>:
> On 10/07/2013 11:03 AM, Michal Privoznik wrote:
>> On 07.10.2013 16:04, 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..fd14237 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_VOL_EXISTS = 90,         /* the storage vol already exists */
>
> Indentation is odd.  Furthermore, existing practice is VIR_ERR_DOM_EXIST
> and VIR_ERR_NETWORK_EXIST; we should use the singular form here.
> Please, let's fix this up before a release bakes in the inconsistent naming.
>
> Also, your subject line isn't grammatically correct (should have been
> 'an ambiguous') - but that's too late to change now.
>
>>
>> ACKed and pushed.
>>
>
> While at it, should we add VIR_ERR_*_EXIST for all the other libvirt
> objects that can cause problems when trying to create a new object of an
> already-existing name?
>
> --
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>

As I understand it, it's necessary to add more error codes to
represent specific meanings.
And I'm sorry about the grammatically correct. I would avoid it from now on.

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]