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

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