Re: [Autotest] [PATCH 1/2] KVM Test: Update cmd() help function in kvm_monitor.py to support parameters.

[Michael Goldish <mgoldish@redhat.com>]

On 07/12/2010 10:57 PM, Lucas Meneghel Rodrigues wrote:
> On Mon, 2010-06-28 at 05:43 -0400, Feng Yang wrote:
>> ----- "Michael Goldish" <mgoldish@redhat.com> wrote: 
>>> Why not add a wrapper for the command you're interested in?
>>> If we do it your way, a test that uses cmd() with parameters will
>>> have
>>> to handle the human case and the QMP case separately.  For example, if
>>> a
>>> human monitor is used the test will have to use
>>>
>>>   vm.monitor.cmd("screendump scr.ppm")
>>>
>>> and if QMP is used the test will have to use
>>>
>>>   vm.monitor.cmd("screendump filename=scr.ppm").
>>>
>>> but if we use a wrapper,
>>>
>>>   vm.monitor.screendump("scr.ppm")
>>>
>>> will work in both cases.
>> Thanks for your command.
>>
>> But in your way, we will have to add wrapper function for every command. I think a general function to run all qmp command during test script design is must. Or It is difficult to design a common process to test all qmp command.
>>
>> I still think make cmd command support parameters is better.  Even we have to handle the human case and the QMP case separately.
> 
> I've thought about what would be the best approach in this situation,
> and here are my findings:
> 
>  * Wrappers are nice, but indeed a systematic test of all monitor
> commands would be made unnecessarily complicated, since not all
> functions have enough usage to justify a separate wrapper for them.
> 
>  * On the other hand, it also sucks having to handle qmp and human
> monitor separately.
> 
> It seems a reasonable alternative to implement the cmd() method in a way
> that it picks **kwargs and then passes the appropriate params to the
> underlying monitor. For example:
> 
> vm.monitor.cmd(command="screendump", filename="scr.ppm")
> 
> Would pass "screendump scr.ppm" to a human monitor, and "screendump
> filename=scr.ppm" to a qmp monitor (we'd just have slightly different
> implementations that handle the same **kwargs differently for each type
> of monitor).
> 
> What do you guys think?

Human monitor commands that take more than a single parameter will have
a problem because dicts (kwargs) are unordered, while human monitor
parameters are.

We could use a string like Feng Yang suggested both for QMP and the
human monitor.  In the human monitor case we'll just have to ignore the
keys and consider only the values in the order provided.  However, that
will only work for some of the commands.  What do we do about commands
like migrate which takes a -d flag?

The way I see it, the solution is either wrappers, or separate cmd()
syntaxes for the human monitor and QMP.  The QMP cmd() will take kwargs
(e.g. cmd(command="screendump", filename="scr.ppm")), and the human
cmd() will remain as it is today.