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
