[libvirt] [PATCH] virterror: Add error message for unsupported operations.

[Laine Stump]

On 08/09/2012 09:44 AM, Peter Krempa wrote:
> On 08/09/12 15:38, Eric Blake wrote:
>> On 08/09/2012 07:31 AM, Peter Krempa wrote:
>>> This patch introduces a new error code VIR_ERR_OPERATION_UNSUPPORTED to
>>> mark error messages regarding operations that failed due to lack of
>>> support on the hypervisor or other than libvirt issues.
>>>
>>> The code is first used in reporting error if qemu does not support
>>> block
>>> IO tuning variables yielding error message:
>>> error: Unable to get block I/O throttle parameters
>>> error: Operation not supported: block_io_throttle field
>>> 'total_bytes_sec' missing in qemu's output
>>>
>>> instead of:
>>> error: Unable to get block I/O throttle parameters
>>> error: internal error cannot read total_bytes_sec
>>> ---
>>
>> In the past, we have used VIR_ERR_CONFIG_UNSUPPORTED for messages about
>> a qemu binary that doesn't support something; would that be any better
>> than inventing a new error here?  Or are all of those errors worth
>> switching over to this new code?
>
> Using VIR_ERR_CONFIG_UNSUPPORTED seems reasonable to me in cases where
> the unsupported feature is requested by the user. Eg. when setting
> something or requesting some weird configuration or even if the
> hypervisor doesn't support it.
>
> On the other hand It doesn't make sense to me to use it on getter-type
> APIs where the user isn't setting anything just wants some information
> back. In this case I'd rather like to see a new message, as stating
> that config isn't supported is a little bit strange.
>
>>
>> As written, your patch seems fine, but only if we agree that a new error
>> is the way to go.

There was already a short discussion about this related to another patch:

https://www.redhat.com/archives/libvir-list/2012-August/msg00589.html

Peter wrote this patch in response to that discussion.

My vote is in favor of the new code.

<knows-a-bandwagon-when-he-sees-it/>