Re: [Qemu-devel] [PATCH 7/9] qmp: expose a command to query capabilities of config parser

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2464 bytes --]

On 03/19/2012 02:19 PM, Anthony Liguori wrote:
>>> +# @size: an integer followed by either 'K', 'M', 'G', or 'T'.
>>
>> Not quite - I found three different parsers, but none of them match this
>> description.
> 
> Heh, I was going by the documentation in the code:
> 
>     QEMU_OPT_SIZE,        /* size, accepts (K)ilo, (M)ega, (G)iga,
> (T)era postfix */
> 
> 
>> That is, qemu-option.c:parse_option_size() supports 'k'
>> and 'b', as well as omitting a suffix, but does not support 'm'.
> 
> TBH, I'd prefer to not document 'k' and 'b' and explicitly document that
> the suffix is optional and bytes is implied (which I thought was implied
> in my documentation, but I can see that it may not be).
> 
> I'd rather we support officially support one way of doing things and if
> we support more, do it under the mantra of being liberal in what we accept.

Okay, I can live with documenting less than we accept, but we definitely
need to mention that the suffix is optional, and that when omitted, the
unit is bytes (as originally written, I read it that the suffix was
mandatory, and that there was no way to request bytes).  I'm not sure
whether documenting an explicit 'b' for bytes helps or hurts, and I'm
not sure whether adding support for 'p' and 'e' as documented suffixes
makes sense.

> 
>> Then
>> there's cmd.c:cvtnum(), which is case-insensitive, and adds 'p' and 'e',
>> but omits 'b'.  Then there's the 'o' type in monitor.c that defers to
>> cutils.c:strtosz(), which defaults to bytes but is case-insensitive and
>> supports 'b' but not 'p'.  Why are we using three different parsers,
>> anyway?
> 
> Yes, I am aware of that, and yes, I'm going to fix it.  One bit at a
> time though.

Fair enough :)  It's just that I recently fixed a bug in libvirt where
it was using a human interface with no suffix and getting 'M' behavior,
where an explicit 'B' behavior fixed things to use bytes, and I was
quite put off by the inconsistencies and lack of documentation when I
finally discovered that a 'B' suffix did what I wanted.  I'm glad that
QMP defaults to 'B', not 'M'.  (Libvirt had its own fair share of
inconsistent scaling routines, where some interfaces default to 'k'
instead of bytes, and I recently consolidated libvirt to use a single
scaling routing for much the same reasons.)

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]