Thanks for your comments!! Any comment is welcome.
I will carefully consider all comments
Thanks very much!
Feng Yang
----- "Michael Goldish" <mgoldish@xxxxxxxxxx> wrote:
> From: "Michael Goldish" <mgoldish@xxxxxxxxxx>
> To: "Feng Yang" <fyang@xxxxxxxxxx>
> Cc: autotest@xxxxxxxxxxxxxxx, kvm@xxxxxxxxxxxxxxx
> Sent: Wednesday, May 26, 2010 6:22:54 PM GMT +08:00 Beijing / Chongqing / Hong Kong / Urumqi
> Subject: Re: [Autotest][PATCH] KVM Test: Extend migration test to test unix, exec and migrate_cancel
>
> On 05/26/2010 07:41 AM, Feng Yang wrote:
> > Update migrate() in kvm_test_utils.py to support unix, exec
> protocol
> > and migration cancel test.
> > Add four migration sub test. There are tcp, unix, exec and
> mig_cancel.
> > migrate_cancel only work in tcp protocol.
> >
> > Signed-off-by: Feng Yang <fyang@xxxxxxxxxx>
> > ---
> > client/tests/kvm/kvm_test_utils.py | 96
> +++++++++++++++++++++++++------
> > client/tests/kvm/tests/migration.py | 7 ++-
> > client/tests/kvm/tests_base.cfg.sample | 11 ++++
> > 3 files changed, 94 insertions(+), 20 deletions(-)
> >
> > diff --git a/client/tests/kvm/kvm_test_utils.py
> b/client/tests/kvm/kvm_test_utils.py
> > index 24e2bf5..a3d6217 100644
> > --- a/client/tests/kvm/kvm_test_utils.py
> > +++ b/client/tests/kvm/kvm_test_utils.py
> > @@ -108,13 +108,17 @@ def reboot(vm, session, method="shell",
> sleep_before_reset=10, nic_index=0,
> > return session
> >
> >
> > -def migrate(vm, env=None):
> > +def migrate(vm, env=None, mig_timeout=3600, mig_protocol="tcp",
> > + mig_cancel=False):
> > """
> > Migrate a VM locally and re-register it in the environment.
> >
> > @param vm: The VM to migrate.
> > @param env: The environment dictionary. If omitted, the
> migrated VM will
> > not be registered.
> > + @param mig_timeout: timeout value for migration.
> > + @param mig_protocol: migration protocol
> > + @param mig_cancel: Test migrate_cancel or not when protocol is
> tcp.
> > @return: The post-migration VM.
> > """
> > # Helper functions
> > @@ -126,6 +130,10 @@ def migrate(vm, env=None):
> > s, o = vm.send_monitor_cmd("info migrate")
> > return s == 0 and "Migration status: completed" in o
> >
> > + def mig_canceled():
> > + s, o = vm.send_monitor_cmd("info migrate")
> > + return s == 0 and "Migration status: cancelled" in o
> > +
> > def mig_failed():
> > s, o = vm.send_monitor_cmd("info migrate")
> > return s == 0 and "Migration status: failed" in o
> > @@ -135,29 +143,73 @@ def migrate(vm, env=None):
> > if not "info migrate" in o:
> > raise error.TestError("Migration is not supported")
> >
> > - # Clone the source VM and ask the clone to wait for incoming
> migration
> > - dest_vm = vm.clone()
> > - if not dest_vm.create(for_migration=True):
> > - raise error.TestError("Could not create dest VM")
> > -
> > - try:
> > + if mig_protocol == "tcp":
> > + migration_port = kvm_utils.find_free_port(5200, 6000)
> > + mig_extra_params = " -incoming tcp:0:%d" % migration_port
> > # Define the migration command
> > - cmd = "migrate -d tcp:localhost:%d" %
> dest_vm.migration_port
> > - logging.debug("Migrating with command: %s" % cmd)
> > -
> > - # Migrate
> > - s, o = vm.send_monitor_cmd(cmd)
> > - if s:
> > - logging.error("Migration command failed (command: %r,
> output: %r)"
> > - % (cmd, o))
> > - raise error.TestFail("Migration command failed")
> > -
> > - # Wait for migration to finish
> > - if not kvm_utils.wait_for(mig_finished, 90, 2, 2,
> > + mig_cmd = "migrate -d tcp:localhost:%d" % migration_port
> > + if mig_protocol == "unix":
> > + file = os.path.join("/tmp/", mig_protocol +
> > +
> time.strftime("%Y%m%d-%H%M%S"))
> > + mig_extra_params = " -incoming unix:%s" % file
> > + mig_cmd = "migrate unix:%s" % file
> > +
> > + if mig_protocol == "exec":
> > + file = os.path.join("/tmp/", mig_protocol +
> > +
> time.strftime("%Y%m%d-%H%M%S"))
> > + mig_extra_params = " -incoming \"exec: gzip -c -d %s\"" %
> file
> > + mig_cmd = "migrate \"exec:gzip -c > %s\"" % file
> > +
> > + vm.send_monitor_cmd("stop")
> > + vm.send_monitor_cmd(mig_cmd, timeout=mig_timeout)
> > + if not kvm_utils.wait_for(mig_finished, mig_timeout, 2, 2,
> > "Waiting for migration to
> finish..."):
> > raise error.TestFail("Timeout elapsed while waiting for
> migration "
> > "to finish")
> >
> > + # Clone the source VM and ask the clone to wait for incoming
> migration
> > + params = vm.params
> > + if params.has_key("extra_params"):
> > + params["extra_params"] += mig_extra_params
> > + else:
> > + params["extra_params"] = mig_extra_params
>
> This looks like a good patch. I'd like to point out some things
> though:
>
> 1. It is preferable not to add migration params to extra_params. If
> we
> do that, the preprocessor of the next test will look at the qemu
> command
> (which contains extra_params) to decide if the VM should be
> restarted,
> and the VM will be restarted even when it shouldn't. If the
> parameters
> of the VM of the next test are exactly identical to those of the
> current
> test, except for migration parameters, then there's no need to
> restart
> the VM. Including migration parameters in extra_params will cause
> the
> VM to be restarted anyway.
>
> To clarify, an example: let's say there's a migration test with a VM:
> image_name = img
> mem = 512
> smp = 2
>
> and then a reboot test with an identical VM:
> image_name = img
> mem = 512
> smp = 2
>
> There's no need to restart the VM because the qemu commands are
> identical. However, if the first VM has "extra_params = -incoming
> ...",
> the qemu commands will be different, even though the VMs are
> identical
> (except for the fact that the first VM is a migrated one).
>
> We need to think of a way to pass these migration parameters without
> putting them in extra_params or in anything else that
> make_qemu_command() uses.
>
> 2. It is preferable to choose ports in VM.create(), where vnc and
> redir
> ports are chosen. This is especially important for running in
> parallel,
> and also for consistency.
>
> I haven't read the patch thoroughly yet, so I'll post more comments
> after I do that.
>
> Thanks,
> Michael
>
> > + dest_vm = vm.clone(params=params)
> > + if not dest_vm.create():
> > + raise error.TestError("Could not create dest VM")
> > + try:
> > + if mig_protocol != "exec":
> > + logging.debug("Migrating with command: %s" % mig_cmd)
> > +
> > + # Migrate
> > + s, o = vm.send_monitor_cmd(mig_cmd,
> timeout=mig_timeout)
> > + if s:
> > + logging.error("Migration command failed (command:
> %r, output:"
> > + " %r)" % (mig_cmd, o))
> > + raise error.TestFail("Migration command failed")
> > +
> > + if mig_protocol == "tcp" and mig_cancel:
> > + # Sleep two seconds before send migrate_cancel
> command.
> > + time.sleep(2)
> > + s, o = vm.send_monitor_cmd("migrate_cancel")
> > + if not kvm_utils.wait_for(mig_canceled, 60, 2, 2,
> > + "Waiting for migration
> cancel"):
> > + raise error.TestFail("Fail to cancel
> migration")
> > + s, o = dest_vm.send_monitor_cmd("info status")
> > + if "paused" not in o:
> > + raise error.TestFail("Fail to cancel migration,
> dest VM"
> > + "is not paused")
> > + s, o = vm.send_monitor_cmd("info status")
> > + if "running" not in o:
> > + raise error.TestFail("VM status is not running
> after"
> > + " migration canceled")
> > + return vm
> > +
> > + # Wait for migration to finish
> > + if not kvm_utils.wait_for(mig_finished, mig_timeout, 2,
> 2,
> > + "Waiting for migration to
> finish..."):
> > + raise error.TestFail("Timeout elapsed while waiting
> for "
> > + "migration to finish")
> > +
> > # Report migration status
> > if mig_succeeded():
> > logging.info("Migration finished successfully")
> > @@ -166,6 +218,12 @@ def migrate(vm, env=None):
> > else:
> > raise error.TestFail("Migration ended with unknown
> status")
> >
> > + if mig_protocol == "exec":
> > + s, o = dest_vm.send_monitor_cmd("info status")
> > + if "paused" in o:
> > + logging.debug("Dest VM is in paused status")
> > + dest_vm.send_monitor_cmd("c")
> > +
> > # Kill the source VM
> > vm.destroy(gracefully=False)
> >
> > diff --git a/client/tests/kvm/tests/migration.py
> b/client/tests/kvm/tests/migration.py
> > index b8f171c..9cdafc9 100644
> > --- a/client/tests/kvm/tests/migration.py
> > +++ b/client/tests/kvm/tests/migration.py
> > @@ -22,6 +22,10 @@ def run_migration(test, params, env):
> > vm = kvm_test_utils.get_living_vm(env, params.get("main_vm"))
> > session = kvm_test_utils.wait_for_login(vm)
> >
> > + mig_timeout = float(params.get("mig_timeout", "3600"))
> > + mig_protocol = params.get("migration_protocol", "tcp")
> > + mig_cancel = bool(params.get("mig_cancel"))
> > +
> > # Get the output of migration_test_command
> > test_command = params.get("migration_test_command")
> > reference_output = session.get_command_output(test_command)
> > @@ -43,7 +47,8 @@ def run_migration(test, params, env):
> > session2.close()
> >
> > # Migrate the VM
> > - dest_vm = kvm_test_utils.migrate(vm, env)
> > + dest_vm = kvm_test_utils.migrate(vm, env,mig_timeout,
> mig_protocol,
> > + mig_cancel)
> >
> > # Log into the guest again
> > logging.info("Logging into guest after migration...")
> > diff --git a/client/tests/kvm/tests_base.cfg.sample
> b/client/tests/kvm/tests_base.cfg.sample
> > index 3a56b65..509d8eb 100644
> > --- a/client/tests/kvm/tests_base.cfg.sample
> > +++ b/client/tests/kvm/tests_base.cfg.sample
> > @@ -105,6 +105,17 @@ variants:
> > kill_vm_on_error = yes
> > iterations = 2
> > used_mem = 1024
> > + mig_timeout = 3600
> > + variants:
> > + - tcp:
> > + migration_protocol = "tcp"
> > + - unix:
> > + migration_protocol = "unix"
> > + - exec:
> > + migration_protocol = "exec"
> > + - mig_cancel:
> > + migration_protocol = "tcp"
> > + mig_cancel = True
> >
> > - boot_savevm: install setup unattended_install
> > type = boot_savevm
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
