Re: [PATCH]: Add a "migrate_incoming" monitor option

["Daniel P. Berrange" <berrange@redhat.com>]

On Thu, Jul 31, 2008 at 04:50:51PM +0200, Chris Lalancette wrote:
> We've been trying to plumb libvirt to do KVM migration.  One of the stumbling
> blocks we are running into, however, is that libvirt expects to be able to use
> the Qemu monitor both before and after migration has taken place, on both the
> source and destination nodes.  After migration has taken place is no problem; we
> return to the main qemu select() loop, and we can run monitor commands.
> However, before migration, on the destination side, when we start qemu with a
> command-line like:
> 
> qemu-kvm -M pc -S blah blah -incoming tcp://0:4444
> 
> we can't run any monitor commands since the migration code is synchronously
> waiting for an incoming tcp connection.  To get around this, the following patch
> adds a new monitor command called "migrate_incoming"; it takes all of the same
> parameters as the command-line option, but just starts it later.  To make sure
> it is safe, you actually have to start with "-incoming monitor"; if you run it
> without that, it will just spit an error at you.  So with this in place, libvirt
> can do the equivalent of:
> 
> qemu-kvm -M pc -S blah blah -incoming monitor
> (qemu) info cpus
> ...other commands
> (qemu) migrate_incoming tcp://0:4444
> ...wait for migration to start, and then complete
> (qemu) info block
> ...etc.
> 
> Signed-off-by: Chris Lalancette <clalance@redhat.com>

> diff --git a/qemu/monitor.c b/qemu/monitor.c
> index 20dcca6..c11b82c 100644
> --- a/qemu/monitor.c
> +++ b/qemu/monitor.c
> @@ -504,6 +504,25 @@ static void do_cont(void)
>      vm_start();
>  }
>  
> +static void do_migrate_incoming(const char *incom)
> +{
> +    extern int incoming_monitor;
> +
> +    if (!incoming_monitor) {
> +        term_printf("FAIL: Can only use the migrate-incoming command with -incoming monitor\n");
> +    }
> +    else {
> +        int rc;
> +
> +        rc = migrate_incoming(incom);

You probably want to set  'incoming_monitor = 0' here, to protect against
someone accidentially running it multiple times.

> +	if (rc != 0) {
> +            fprintf(stderr, "Migration failed rc=%d\n", rc);
> +            exit(rc);
> +	}

And some whitespace issues here..

> +        vm_start();
> +    }
> +}
> +
>  #ifdef CONFIG_GDBSTUB
>  static void do_gdbserver(const char *port)
>  {
> @@ -1486,6 +1505,7 @@ static term_cmd_t term_cmds[] = {
>        "", "cancel the current VM migration" },
>      { "migrate_set_speed", "s", do_migrate_set_speed,
>        "value", "set maximum speed (in bytes) for migrations" },
> +    { "migrate_incoming", "s", do_migrate_incoming, "incom", "incoming string" },
>      { "cpu_set", "is", do_cpu_set_nr, "cpu [online|offline]", "change cpu state" },
>  #if defined(TARGET_I386) || defined(TARGET_X86_64)
>      { "drive_add", "iss", drive_hot_add, "pcibus pcidevfn [file=file][,if=type][,bus=n]\n"
> diff --git a/qemu/vl.c b/qemu/vl.c
> index e1762ee..9b5f113 100644
> --- a/qemu/vl.c
> +++ b/qemu/vl.c
> @@ -229,6 +229,7 @@ int cursor_hide = 1;
>  int graphic_rotate = 0;
>  int daemonize = 0;
>  const char *incoming;
> +int incoming_monitor = 0;
>  const char *option_rom[MAX_OPTION_ROMS];
>  int nb_option_roms;
>  int semihosting_enabled = 0;
> @@ -9673,11 +9675,16 @@ int main(int argc, char **argv)
>      if (incoming) {
>          int rc;
>  
> -        rc = migrate_incoming(incoming);
> -        if (rc != 0) {
> -            fprintf(stderr, "Migration failed rc=%d\n", rc);
> -            exit(rc);
> -	}
> +        if (strncmp(incoming, "monitor", 7) == 0) {
> +            incoming_monitor = 1;
> +        }
> +        else {
> +            rc = migrate_incoming(incoming);
> +            if (rc != 0) {
> +                fprintf(stderr, "Migration failed rc=%d\n", rc);
> +                exit(rc);
> +            }
> +        }

Rather than putting the strncmp("monitor")  into vl.c, I'd just leave
this part as is.  Put the logic into the 'migrate_incoming()' method
so that it just sets the 'incoming_monitor' flag and then returns
immediately. That would allwo the 'incoming_Monitor' flag to be declared
static to the migrate.c file, instead of polluting vl.c

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|