On Mon, 2010-06-28 at 05:43 -0400, Feng Yang wrote:
> ----- "Michael Goldish" <mgold...@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?
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
