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: > > This patch refactors the JSON parsing function that extracts the block > > IO tuning parameters from qemu's output. The most impacting change > > concerns the error message that is returned if the reply from qemu does > > not contain the needed data. The data for IO parameter tuning were added > > in qemu 1.1 and the previous error message was confusing. > > > > This patch also breaks long lines and extracts a multiple time used code > > pattern to a macro. > > --- > > Old error message looks like: > > # virsh blkdeviotune asdf hda > > error: Unable to get block I/O throttle parameters > > error: internal error cannot read total_bytes_sec > > > > and the new: > > # 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 Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list