On Mon, Jan 25, 2010 at 05:15:00AM -0500, Michael Goldish wrote:
>
> ----- "Yolkfull Chow" <[email protected]> wrote:
>
> > On Thu, Jan 21, 2010 at 09:14:26PM +0800, Jason Wang wrote:
> > > Yolkfull Chow wrote:
> > > >Using 'wait_for' for logging into migrated guest repeats the work
> > of
> > > >'wait_for_login' which exists already. We just need to change the
> > name
> > > >of 'dest_vm'.
> > > >
> > > >Signed-off-by: Yolkfull Chow <[email protected]>
> > > >---
> > > > client/tests/kvm/kvm_test_utils.py | 1 +
> > > > client/tests/kvm/tests/migration.py | 7 ++-----
> > > > 2 files changed, 3 insertions(+), 5 deletions(-)
> > > >
> > > >diff --git a/client/tests/kvm/kvm_test_utils.py
> > b/client/tests/kvm/kvm_test_utils.py
> > > >index 02ec0cf..13af8e1 100644
> > > >--- a/client/tests/kvm/kvm_test_utils.py
> > > >+++ b/client/tests/kvm/kvm_test_utils.py
> > > >@@ -135,6 +135,7 @@ def migrate(vm, env=None):
> > > > # Clone the source VM and ask the clone to wait for incoming
> > migration
> > > > dest_vm = vm.clone()
> > > >+ dest_vm.name = "migrated_guest"
> > > > dest_vm.create(for_migration=True)
> > > What's the use of this attribute in this case?
> >
> > We could see the difference in message printed from wait_for_login:
> >
> > Try to log into guest '%s' % vm.name
> >
> > It will then tell user after migration: Try to log into guest
> > 'migrated_guest'
>
> This is not the intended usage of the name attribute. It's supposed to
> be pretty much read only, and it's always supposed to match the name
> registered in 'env'.
Of course I knew. But should everything has only one usage?
We can see from following code fragment in migration.py that, it obviously
re-did the work of wait_for_login() in kvm_test_utils.py which looks logically
redundant.
...
# Log into the guest again
logging.info("Logging into guest after migration...")
session2 = kvm_utils.wait_for(dest_vm.remote_login, 30, 0, 2)
if not session2:
raise error.TestFail("Could not log into guest after migration")
logging.info("Logged in after migration")
...
> If this patch is applied stuff might break.
I wonder why and how it can break stuff...
As I had tested this patch, works pretty good.
> I think we should either leave the test the way it is, or find a different
> solution. This one seems a little "kludgy".
Cannot agree if no any other reason.
>
> (BTW, you could do
> dest_vm = vm.clone(name="migrated_guest")
Yes, this is not bad. I can modify accordingly.
Thanks for comments.
> which is OK, but then you'd have to register the VM as "migrated_guest"
> instead of "vm1", which would make the following test kill the migrated
> VM and start a new "vm1").
>
> > > > try:
> > > >diff --git a/client/tests/kvm/tests/migration.py
> > b/client/tests/kvm/tests/migration.py
> > > >index b8f171c..b65064b 100644
> > > >--- a/client/tests/kvm/tests/migration.py
> > > >+++ b/client/tests/kvm/tests/migration.py
> > > >@@ -46,11 +46,8 @@ def run_migration(test, params, env):
> > > > dest_vm = kvm_test_utils.migrate(vm, env)
> > > > # Log into the guest again
> > > >- logging.info("Logging into guest after migration...")
> > > >- session2 = kvm_utils.wait_for(dest_vm.remote_login, 30, 0,
> > 2)
> > > >- if not session2:
> > > >- raise error.TestFail("Could not log into guest after
> > migration")
> > > >- logging.info("Logged in after migration")
> > > >+ session2 = kvm_test_utils.wait_for_login(dest_vm,
> > timeout=30, start=0,
> > > >+ step=2)
> > > > # Make sure the background process is still running
> > > > if session2.get_command_status(check_command, timeout=30)
> > != 0:
> > _______________________________________________
> > Autotest mailing list
> > [email protected]
> > http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
_______________________________________________
Autotest mailing list
[email protected]
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
