Re: [PATCH 0/3] Launch other test during migration

From: Michael Goldish <mgoldish@redhat.com>
To: Jason Wang <jasowang@redhat.com>
Cc: autotest@test.kernel.org, kvm@vger.kernel.org, mst@redhat.com
Subject: Re: [PATCH 0/3] Launch other test during migration
Date: Tue, 02 Nov 2010 10:14:29 +0200	[thread overview]
Message-ID: <4CCFC865.2000606@redhat.com> (raw)
In-Reply-To: <19663.41680.966555.913847@gargle.gargle.HOWL>

On 11/02/2010 07:34 AM, Jason Wang wrote:
> Michael Goldish writes:
>  > On 09/25/2010 11:36 AM, Jason Wang wrote:
>  > > We could give a further test of migration by launch test during migartion. So
>  > > the following series implements:
>  > > 
>  > > - A simple class to run a specified test in the background which could be used
>  > > to launch other test during migartion. Its design is rather simple and its usage
>  > > is a little tricky, but it work well.
>  > > - Two sample tests which take advantages of the background class: Test reboot
>  > > during guest migration and test file_transfer during guest migration.
>  > > 
>  > > In the future, we could even lauch autotest client test during guest migation.
>  > > 
>  > > ---
>  > > 
>  > > Jason Wang (3):
>  > >       KVM Test: Introduce a helper class to run a test in the background
>  > >       KVM test: Test reboot during migration
>  > >       KVM test: Test the file transfer during migartion
>  > > 
>  > > 
>  > >  client/tests/kvm/kvm_test_utils.py                 |   44 +++++++++++++++
>  > >  .../kvm/tests/migration_with_file_transfer.py      |   59 ++++++++++++++++++++
>  > >  client/tests/kvm/tests/migration_with_reboot.py    |   45 +++++++++++++++
>  > >  client/tests/kvm/tests_base.cfg.sample             |   12 ++++
>  > >  4 files changed, 159 insertions(+), 1 deletions(-)
>  > >  create mode 100644 client/tests/kvm/tests/migration_with_file_transfer.py
>  > >  create mode 100644 client/tests/kvm/tests/migration_with_reboot.py
>  > 
>  > It seems to me that this method is only applicable to tests/functions
>  > that don't require a VM object (i.e. that require only a shell session
>  > object).  kvm_test_utils.reboot() operates on a VM object, and the same
>  > VM is destroyed by migrate() which runs in the background, so eventually
>  > reboot() tries logging into a destroyed VM, which fails because
>  > vm.get_address() fails.  Any monitor operation will also fail.  If the
>  > autotest wrapper requires a VM object (currently it does) then it can't
>  > be used either.
>  > 
> 
> You are right and that's an issue when running test in parallel with
> migration, but reboot through shell should work. The aim of this kind
> of test is just to add some stress ( such as run_autotest) during
> migartion, so most (probably all) of the tests only needs a
> session. So I think it's not a very big issue in this situation.

Many tests need to be modified in order to require only a session.  I've
tried reboot and it doesn't work, and I can see that run_autotest() also
uses a VM.  Reboot needs the VM object in order to log in to make sure
the VM goes back up, and run_autotest() needs it for SCP and probably
is_alive().  I agree that some tests don't require a VM object, but the
ones that do are also interesting.

>  > An alternative (somewhat ugly) way to migrate in the background is to
>  > pass a boolean 'migrate' flag to various functions/tests, such as
>  > reboot() and run_autotest().  If 'migrate' is True, these functions will
>  > do something like
>  > 
>  > vm = kvm_test_utils.migrate(vm, ...)
>  > 
>  > in their waiting loops, where wait_for() is normally used.  This
>  > guarantees that 'vm' is always a valid VM object.  For example:
>  > 
>  > # Log in after reboot
>  > while time.time() < end_time:
>  >     if migrate_in_bg:
>  >         vm = kvm_test_utils.migrate(vm, ...)
>  >     session = vm.remote_login()
>  >     if session:
>  >         break
>  >     time.sleep(2)
>  > 
>  > This is much longer than the usual wait_for(...) but it does the job.
>  > What do you think?
> 
> This makes sense but it would let testcases need to care about the
> migration and it's also hard to put all related codes into a wrapper
> which would complicate the codes.
> 
> Despite the issue of vm object, all tests that only depends on the
> shell session should works well with my method and it's more easy to

We should also find a solution for tests that require a VM object.

Which other tests do you think it would be interesting to run during
migration, in addition to reboot(), run_autotest() and file_transfer()?

> be extended. Maybe we could just warn the user of its usage and adapt
> my method?

I think it's cleaner to just avoid passing a VM object to BackgroundTest.