[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4] qom, qmp, hmp, qapi: create qom-type-prop-li
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v4] qom, qmp, hmp, qapi: create qom-type-prop-list for class properties |
Date: |
Mon, 25 Jan 2016 10:43:49 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 |
On 01/22/2016 01:20 PM, Valentin Rakush wrote:
> Hi Eric, hi Daniel,
>
> Re dashes in the command name
>
> AFAICC, the QOM related command in HMP use dash "-". For example, qom-list
> and qom-set. If we will change dash "-" to underscore "_" then
> qom_type_prop_list will be consistent with old HMP commands (created before
> year 2013), but will _not_ be consistent with QOM commands created after
> 2013. Which is not nice and may be misleading.
Well, HMP is not set in stone. We can rename the HMP command to qom_list
and qom_set, if we want consistency so that _all_ HMP commands prefer _.
>
> If we want to have consistent naming of all HMP commands, then we should
> rename all QOM commands to replace "-" to "_". But in this case we can
> brake compatibility in possible scripts that already use these commands.
> For example, scripts/qmp/qom-fuse
Any script using HMP is already broken, because HMP is not designed for
scriptability. Scripts should be using QMP, and QMP has stricter rules
about not arbitrarily changing names.
>
> I would leave name qom-type-prop-list with dashes, so it will be consistent
> with other two QOM commands and would refactor all QOM commands names if
> possible.
I'm not asking you to change the QMP name, just the HMP name.
>
> Re subcommand of the info command
>
> Also from hmp-command.hx I can see that info command shows various
> information about the _system_state_ The qom-type-prop-list shows
> properties of the class type. They can be changed during runtime, but I am
> not sure if they can be referred as a system state. From another side
> command like "info class x86_64-cpu" could take less typing, but this will
> be inconsistent with QMP version of the command.
We aren't aiming for equivalence between QMP and HMP. It's fine for the
HMP command to be higher-level, have more smarts, and have more
consistency with other HMP commands.
>
> For this reasons I would leave qom-type-prop-list as it is right now.
Since HMP is not scriptable, I am not going to hold up the patch on
bikeshedding how the HMP command is spelled. I just wanted to point out
the difference in conventions, and that you are adding an exception to
the conventions.
>
> Daniel have reviewed this patch already but found my error in the
> parameters of the HMP command. I have fixed this error and tested command
> with monitor. But would it be ok to add QMP and HMP tests to this patch?
> Or may be submit tests with another patch, because this one is already
> reviewed? I do not see much QMP/HMP tests so I am hesitating if this is a
> good idea.
Another idea is to add ONLY a QMP command, and omit the functionality
from HMP altogether. If someone finds the HMP interface important, they
can submit a later patch to add it on top of the QMP command.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v4] qom, qmp, hmp, qapi: create qom-type-prop-list for class properties, Valentin Rakush, 2016/01/22
- Re: [Qemu-devel] [PATCH v4] qom, qmp, hmp, qapi: create qom-type-prop-list for class properties, Daniel P. Berrange, 2016/01/22
- Re: [Qemu-devel] [PATCH v4] qom, qmp, hmp, qapi: create qom-type-prop-list for class properties, Denis V. Lunev, 2016/01/22
- Re: [Qemu-devel] [PATCH v4] qom, qmp, hmp, qapi: create qom-type-prop-list for class properties, Eric Blake, 2016/01/22