[libvirt] [PATCH] qemu: Refactor parsing of block device IO tuning parameters.

[Peter Krempa]

On 08/09/12 13:00, Daniel P. Berrange wrote:
> On Thu, Aug 09, 2012 at 12:52:16PM +0200, Jiri Denemark wrote:
>> On Thu, Aug 09, 2012 at 10:48:43 +0100, Daniel P. Berrange wrote:
>>> On Thu, Aug 09, 2012 at 11:39:57AM +0200, Jiri Denemark wrote:
>>>> On Thu, Aug 09, 2012 at 10:48:42 +0200, Peter Krempa wrote:
>>>>> # virsh blkdeviotune asdf hda
>>>>> error: Unable to get block I/O throttle parameters
>>>>> error: internal error block_io_throttle field 'total_bytes_sec' missing in qemu's output
>>>>
>>>> I think the old/new error messages can be included directly in the commit
>>>> message. Anyway, I wonder if we should take this as an opportunity to fix the
>>>> "internal error", however I'm not sure what is the best code to use. It seems
>>>> we use OPERATION_INVALID in such cases, which could be good enough.
>>>
>>> No, OPERATION_INVALID is for scenarios where the API request cannot
>>> be performed because the object is in the wrong state.
>>>
>>> eg when you request to pause a VM that is shutoff.
>>>
>>>
>>> This is a case where the data we receive back from QEMU is incorrect.
>>> I don't think we have a current error code that is suitable for this
>>> kind of scenario, so we ought to invent a new one.
>>>
>>> Some ideas
>>>
>>>     MISSING_DATA
>>>     DATA_INVALID
>>>     UNEXPECTED_DATA
>>>     ...insert others...
>>>
>>> my favour is probably DATA_INVALID
>>
>> The data is actually correct, just the fields required for this functionality
>> are missing because QEMU is too old to support them. Perhaps
>> OPERATION_UNSUPPORTED (which doesn't currently exist either) could be the
>> right choice.
>
> We do have a VIR_ERR_NO_SUPPORT, but its probably wise to restrict that
> to only the case where the public API is not provided by the current
> driver. So agree that a new OPERATION_UNSUPPORTED would be reasonable
>
> Daniel
>

I've pushed this patch and I'll follow up with a separate one adding 
VIR_ERR_OPERATION_UNSUPPORTED and changing the error message type in 
that particular case.

Peter