Re: [KVM-AUTOTEST] [PATCH] support for remote migration

[Michael Goldish <mgoldish@redhat.com>]

----- "yogi" <anantyog@linux.vnet.ibm.com> wrote:

> Hello everyone,
> 
> I like to submit patch to add support for "remote migration" in
> kvm-autotest.

Sounds like a good idea.

Also, the patch isn't too big, which I personally appreciate very much (makes it easier to read).

> To use this patch the following four parameters should be added to
> the
> existing migration test
> 
>         remote = dst
>         hostip = <localhost ip or name>
>         remoteip = <remote host ip or name>
>         remuser = root
>         rempassword = <password>
> 
> the field remote=dst indicates the VM "dst" should be created on
> remote
> machine.
> For example:
>     - migrate:      install setup
>         type = migration
>         vms += " dst"
>         migration_test_command = help
>         kill_vm_on_error = yes
>         remote = dst
>         hostip = 192.168.1.2
>         remoteip = 192.168.1.3
>         remuser = root
>         rempassword = 123456
>         variants:
> 
> Three files r being modified in this patch kvm_utils.py, kvm_tests.py
> and kvm_vm.py.
> kvm_utils.py - if the  ssh-keys have been exchanged between the test
> machines,then remote login fails with message "Got unexpected login
> prompt", to prevent this, have made it return a session rather then
> None
> 
> kvm_tests.py - the host address used in migration is made dynamic
> 
> kvm_vm.py -    have replaced unix sockets with tcp sockets for
> monitor,
> in both remote and local VM. Added two new variables(remote,ssh_port)
> to
> class VM,remote set to True if the VM is on a  remote
> machine,ssh_port
> contains the redirection port, funtion get_address() returns the ip
> of
> the host whr the VM is(local or remote).

I've only looked at the code briefly, and it looks very good overall, but I have a few comments/questions:

Regarding remote_login:

- Why should remote_login return a session when it gets an unexpected login prompt? If you get a login prompt doesn't that mean something went wrong? The username is always provided in the ssh command line, so we shouldn't expect to receive a login prompt -- or am I missing something? I am pretty confident this is true in the general case, but maybe it's different when ssh keys have been exchanged between the hosts.

- I think it makes little sense to return a session object when you see a login prompt because that session will be useless. You can't send any commands to it because you don't have a shell prompt yet. Any command you send will be interpreted as a username, and will most likely be the wrong username.

- When a guest is in the process of booting and we try to log into it, remote_login sometimes fails because it gets an unexpected login prompt. This is good, as far as I understand, because it means the guest isn't ready yet (still booting). The next time remote_login attempts to log in, it usually succeeds. If we consider an unexpected login prompt OK, we pass login attempts that actually should have failed (and the resulting sessions will be useless anyway).

Other things:

- If I understand correctly, remote migration will only work if the remote qemu binary path is exactly the same as the local one. Maybe we should receive a qemu path parameter that will allow for some flexibility.

- In VM.make_qemu_command(), in the code that handles redirections, you add 'self.ssh_port = host_port'. I don't think this is correct because there can be multiple redirections, unrelated to SSH, so you certainly shouldn't assume that the only redirection is an SSH one. When you want the host port redirected to the guest's SSH port, you should use self.get_port(int(self.params.get("ssh_port"))). This will also work if for some reason 'ssh_port' changes while the guest is alive.

- It seems that the purpose of 'remote = dst' is to indicate to 'dst' that it should be started as a remote VM. The preferred way to do this is to pass something like 'remote_dst = yes' and then in VM.create() you can test for params.get("remote") == "yes". See "Addressing objects" in the wiki (http://www.linux-kvm.org/page/KVM-Autotest/Parameters#Addressing_objects_.28VMs.2C_images.2C_NICs_etc.29).
In general, any parameter you want to pass to a specific VM, you pass using <param>_<vmname> = <value>, e.g. 'mem_dst = 128', and then in VM.create() the parameter is accessible without the VM name extension (e.g. self.params.get("mem") will equal "128").

Thanks,
Michael