[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: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 05/27] qapi: add SIZE type parser to string_input_visitor |
Date: |
Wed, 27 Nov 2013 15:15:07 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux) |
Eric Blake <address@hidden> writes:
> 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.
This is by design.
> 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.
Me too, as long as the units suffixes are used consistently.
>> 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.
I don't get why there's a counter-example.
The rules are
1. A QMP command handler (which needs to be of type int (*)(Monitor *,
const QDict *params, QObject **ret_data)) should be a thin wrapper
around a function with a type appropriate for C that contains all the
logic. The wrapper merely translates.
2. HMP commands are to be built on top of these functions, not their
handler wrappers.
> 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).
Please do not complicate QMP's wire format just to help humans deal with
it. It's for machines. We intentionally picked a machine-readable
syntax that is still readable and writable for humans (feature!), but
that's as far as we should go in supporting human use.
- Re: [Qemu-devel] [PATCH 02/27] rename pci_hotplug_fn to hotplug_fn and make it available for other devices, (continued)
- [Qemu-devel] [PATCH 05/27] qapi: add SIZE type parser to string_input_visitor, Igor Mammedov, 2013/11/20
- Re: [Qemu-devel] [PATCH 05/27] qapi: add SIZE type parser to string_input_visitor, Markus Armbruster, 2013/11/21
- Re: [Qemu-devel] [PATCH 05/27] qapi: add SIZE type parser to string_input_visitor, Igor Mammedov, 2013/11/25
- Re: [Qemu-devel] [PATCH 05/27] qapi: add SIZE type parser to string_input_visitor, Michael S. Tsirkin, 2013/11/25
- Re: [Qemu-devel] [PATCH 05/27] qapi: add SIZE type parser to string_input_visitor, Paolo Bonzini, 2013/11/25
- Re: [Qemu-devel] [PATCH 05/27] qapi: add SIZE type parser to string_input_visitor, Eric Blake, 2013/11/25
- Re: [Qemu-devel] [PATCH 05/27] qapi: add SIZE type parser to string_input_visitor, Paolo Bonzini, 2013/11/25
- Re: [Qemu-devel] [PATCH 05/27] qapi: add SIZE type parser to string_input_visitor,
Markus Armbruster <=
- Re: [Qemu-devel] [PATCH 05/27] qapi: add SIZE type parser to string_input_visitor, Paolo Bonzini, 2013/11/27
- Re: [Qemu-devel] [PATCH 05/27] qapi: add SIZE type parser to string_input_visitor, Markus Armbruster, 2013/11/27
- Re: [Qemu-devel] [PATCH 05/27] qapi: add SIZE type parser to string_input_visitor, Paolo Bonzini, 2013/11/27
[Qemu-devel] [PATCH 06/27] get reference to /backend container via qemu_get_backend(), Igor Mammedov, 2013/11/20
[Qemu-devel] [PATCH 07/27] add memdev backend infrastructure, Igor Mammedov, 2013/11/20