Re: [PATCH 1/2 RESEND] KVM Test: Introduce qmp_monitor property in 'kvm_vm.VM'

[Lucas Meneghel Rodrigues <lmr@redhat.com>]

On Mon, 2011-01-10 at 19:52 +0200, Michael Goldish wrote:
> On 01/10/2011 12:47 PM, qzhou@redhat.com wrote:
> > From: Qingtang Zhou<qzhou@redhat.com>
> >
> > Introduce qmp_monitor property can help us easily get a qmp monitor
> > of guest.
> >
> > CC to Luiz and kvm@vger.kernel.org
> >
> > Signed-off-by: Qingtang Zhou<qzhou@redhat.com>
> 
> The monitor property returns the main monitor.  Isn't it fair to assume 
> that qmp_basic is always run with main_monitor set to a qmp monitor?

^ Well, with this patch we have one less requirement, which is to set
main_monitor as the primary one, kvm autotest can just find the first
appropriate monitor and return it. It's mostly a usability patch, makes
easier to figure out when errors happen and put less configuration
strain on users. The fact the user will see the message:

> +        raise error.TestError("Could not find a QMP monitor, abort test.")

In case no QMP monitor was found is very helpful.

> > ---
> >   client/tests/kvm/kvm_vm.py          |   10 ++++++++++
> >   client/tests/kvm/tests/qmp_basic.py |   15 +++++++++------
> >   2 files changed, 19 insertions(+), 6 deletions(-)
> >
> > diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
> > index f6f1684..0ba2db4 100755
> > --- a/client/tests/kvm/kvm_vm.py
> > +++ b/client/tests/kvm/kvm_vm.py
> > @@ -870,6 +870,16 @@ class VM:
> >           if self.monitors and not self.params.get("main_monitor"):
> >               return self.monitors[0]
> >
> > +    @property
> > +    def qmp_monitor(self):
> > +        """
> > +        Return the first QMP monitor.
> > +        If no QMP monitor exist, return None.
> > +        """
> > +        for m in self.monitors:
> > +            if isinstance(m, kvm_monitor.QMPMonitor):
> > +                return m
> > +        return None
> >
> >       def is_alive(self):
> >           """
> > diff --git a/client/tests/kvm/tests/qmp_basic.py b/client/tests/kvm/tests/qmp_basic.py
> > index 985ad15..36cbd78 100644
> > --- a/client/tests/kvm/tests/qmp_basic.py
> > +++ b/client/tests/kvm/tests/qmp_basic.py
> > @@ -383,13 +383,16 @@ def run_qmp_basic(test, params, env):
> >
> >       vm = kvm_test_utils.get_living_vm(env, params.get("main_vm"))
> >
> > +    if vm.qmp_monitor is None:
> > +        raise error.TestError("Could not find a QMP monitor, abort test.")
> > +
> >       # Run all suites
> > -    greeting_suite(vm.monitor)
> > -    input_object_suite(vm.monitor)
> > -    argument_checker_suite(vm.monitor)
> > -    unknown_commands_suite(vm.monitor)
> > -    json_parsing_errors_suite(vm.monitor)
> > +    greeting_suite(vm.qmp_monitor)
> > +    input_object_suite(vm.qmp_monitor)
> > +    argument_checker_suite(vm.qmp_monitor)
> > +    unknown_commands_suite(vm.qmp_monitor)
> > +    json_parsing_errors_suite(vm.qmp_monitor)
> >
> >       # check if QMP is still alive
> > -    if not vm.monitor.is_responsive():
> > +    if not vm.qmp_monitor.is_responsive():
> >           raise error.TestFail('QEMU is not alive after QMP testing')
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html