Re: [Qemu-devel] [PATCH 05/27] qapi: add SIZE type parser to string_input_visitor

[Paolo Bonzini]

Il 19/12/2013 15:40, Michael S. Tsirkin ha scritto:
> On Mon, Nov 25, 2013 at 09:43:48AM -0700, Eric Blake wrote:
>> On 11/25/2013 09:32 AM, Paolo Bonzini wrote:
>>
>>>> Yes please. Firing up a calculator to figure out how much is 1G is not
>>>> friendly, neither is firing it up to figure out what did management do
>>>> with QMP. It should be a text based interface not a binary one.
>>
>> Right now, QMP takes an 'int', which does not allow a suffix.  Libvirt
>> prefers passing a value in 'bytes', rather than risking confusion on
>> whether a value in G was rounded (up, down? to nearest power of 10 or
>> power of 2?).  Unfortunately, yes, that means you need a calculator when
>> parsing QMP logs to see whether the 1073741824 passed by libvirt is the
>> 1G you had in mind.
>>
>> HMP, qtest, and any other decent shell around raw QMP is more than
>> welcome to provide human-usable wrappers that takes "1G" as a string and
>> turns it into the raw int used by the underlying QMP.  In fact, I
>> encourage it.
> 
> How will it know 1G is not e.g. an ID?

Because all properties are associated to a name.  In the case of a
memory device, the name could be "size" or "id".  When requesting the
id, QEMU will use visit_type_str.  When requesting the size, QEMU will
use visit_type_size.  Different functions are called, and the different
functions accept different input.

To explain further what Markus dismissed as "against the design of QMP",
there is another way to choose the way your input is parsed, and that is
simply by choosing a different visitor.

QMP uses strongly-typed JSON, so it uses a QmpInputVisitor.  HMP and
command-line just parse text, so it uses an OptsVisitor.

So we have:

   -object memory-ram,size=1M,id=foo           (command-line)
   object_add memory-ram,size=1M,id=foo        (HMP)

that use OptsVisitor and converts 1M to 1048576.  For QMP instead

   {'command': 'object-add', 'arguments': {
        'qom-type': 'memory-ram', 'id': 'foo',
        'props': {'size':1048576}}}

uses QmpInputVisitor which rejects any 'size' that is not a JSON
integer.  We definitely do not want to allow QMP input such as this:

   {'command': 'object-add', 'arguments': {
        'qom-type': 'memory-ram', 'id': 'foo',
        'props': {'size':'1M'}}}

That's against the idea of making QMP as strongly-typed as possible.

Paolo

> We can invent rules like "IDs should not start with a number", but these
> rules are better enforced in a single place for consistency, and it's
> likely too late to enforce that in HMP.
> 
>>> This is unfortunately a counter-example to the rule that HMP commands
>>> should always be implemented in terms of their QMP counterparts.  I do
>>> not believe this is really a problem.  It can be fixed later; for now, I
>>> think "perfect is the enemy of good" applies.
>>
>> Hey - I just realized that now that we have anonymous unions, we could
>> theoretically extend QMP to allow a union between 'int' and 'string' -
>> if an 'int' is passed, it is in bytes; if a 'string' is passed, then
>> parse it the way HMP would (so the string "1G" would be equivalent to
>> the raw int 1073741824).  But I don't know if it will help you (libvirt
>> will still prefer to use raw ints in any QMP log you read off of libvirt
>> interactions).
> 
> Yes, I think that would address the issue.
> 
>> Eric Blake   eblake redhat com    +1-919-301-3266
>> Libvirt virtualization library http://libvirt.org
>>
> 
>