[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 05/27] qapi: add SIZE type parser to string_inpu
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH 05/27] qapi: add SIZE type parser to string_input_visitor |
Date: |
Thu, 19 Dec 2013 17:40:36 +0200 |
On Thu, Dec 19, 2013 at 04:31:41PM +0100, Paolo Bonzini wrote:
> 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.
QEMU can do this, but if you want a frontend with QMP as backend,
it won't.
So you end up re-implementing this logic in all frontends:
instead of just passing on properties they need to know
where 1G is a number and where it's a name.
> 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
> >>
> >
> >
>