|Subject:||Re: [Qemu-devel] [PATCH v4] qom, qmp, hmp, qapi: create qom-type-prop-list for class properties|
|Date:||Tue, 26 Jan 2016 12:08:53 +0300|
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
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
> 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.
|[Prev in Thread]||Current Thread||[Next in Thread]|