[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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
-- 
|: 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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]