Re: [Qemu-devel] [PATCH v4] qom, qmp, hmp, qapi: create qom-type-prop-list for class properties

[Eric Blake]

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