Re: [libvirt] [PATCH] virterror: Add error message for unsupported operations.
On 08/09/12 15:38, Eric Blake wrote:
Peter
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.
Thanks for reviewing.