: Add a "migrate_incoming" monitor option
PATCH 1/1 : Add a "migrate_incoming" monitor option
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.
qemu/monitor.c +20 -0
@@ -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); +if (rc !=0 ) { +fprintf (stderr,"Migration failed rc=%d\n" , rc); +exit (rc); + } +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"
qemu/vl.c +11 -5
@@ -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); + } + } } {
Quoted 37 lines · Chris Lalancette
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_incomingtcp://0:4444
...wait for migration to start, and then complete
(qemu) info block
...etc.Signed-off-by Chris Lalancette <[email protected]>qemu/monitor.c +1
-0@@ -504,6 +504,25 @@ static void do_cont(void)vm_start (); } +static void do_migrate_incoming (const char *incom)
qemu/monitor.c static void do_cont(void) · line 517
Quoted 10 lines
qemu/monitor.c +10
-0+{ +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.
qemu/monitor.c static void do_cont(void) · line 521
Quoted 4 lines
qemu/monitor.c +4
-0+if (rc !=0 ) { +fprintf (stderr,"Migration failed rc=%d\n" , rc); +exit (rc); + }
And some whitespace issues here..
Quoted 36 lines
qemu/monitor.c +5
-0+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" qemu/vl.c +1
-5@@ -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); - }
qemu/vl.c int main(int argc, char **argv) · line 9687
Quoted 10 lines
qemu/vl.c +10
-0+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
Signature · 4 lines
|: 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 :|
Quoted 17 lines · Daniel P. Berrange
Quoted 17 lines
qemu/vl.c +8
-5@@ -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);
Quoted 8 lines
Review int main(int argc, char **argv) · line 9687
Quoted 2 lines
(quoted diff) +2
-0+ } + }Rather than putting the
strncmp("monitor")intovl.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 themigrate.cfile, instead of pollutingvl.c
Actually, that won't quite work. We still need to share the incoming_monitor flag between migration.c and monitor.c. However, your suggestion is better in that this is a "migration-specific" flag, so I'll move it over like you suggest.
Chris Lalancette
Quoted 18 lines · Chris Lalancette
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:
Quoted 2 lines
qemu-kvm -M pc -S blah blah -incoming monitor
I think adding a 'nowait' parameter to migration would make more sense than introducing a monitor command.
So:
qemu-kvm -M pc -S blah blah -incoming tcp://0:4444,nowait
From an implementation perspective, it's just a matter of setting a callback for the accept fd I imagine.
Regards,
Anthony Liguori
Quoted 9 lines
(qemu) info cpus
...other commands
(qemu)migrate_incomingtcp://0:4444
...wait for migration to start, and then complete
(qemu) info block
...etc.Signed-off-by Chris Lalancette <[email protected]>
Quoted 35 lines · Anthony Liguori
Quoted 28 lines · Chris Lalancette
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 qemuselect()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:Quoted 2 lines
qemu-kvm -M pc -S blah blah -incoming monitor
I think adding a 'nowait' parameter to migration would make more sense than introducing a monitor command.
Quoted 6 lines
So:
qemu-kvm -M pc -S blah blah -incoming tcp://0:4444,nowait
From an implementation perspective, it's just a matter of setting a callback for the accept fd I imagine.
An accept trick only handles the TCP case though. I know this was Chris' example that we're currently using, but we intend to switch to passing an open file descriptor instead, and proxying the data via a secure channel instead of the plain tcp, or builtin SSH tunnelling.
With an open FD we'll need another way - I guess just registering a event on POLLIN might do the trick. It seems to me that just having an explicit migrate incoming monitor command would be simpler than having to code different triggers to implement 'nowait' for each transport we have.
Daniel
Signature · 4 lines
|: 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 :|
Quoted 10 lines · Daniel P. Berrange
An accept trick only handles the TCP case though. I know this was Chris' example that we're currently using, but we intend to switch to passing an open file descriptor instead, and proxying the data via a secure channel instead of the plain tcp, or builtin SSH tunnelling.
With an open FD we'll need another way - I guess just registering a event on POLLIN might do the trick. It seems to me that just having an explicit migrate incoming monitor command would be simpler than having to code different triggers to implement 'nowait' for each transport we have.
I just finished a patch to do the "nowait" for tcp as suggested by Anthony, and it was actually far less code then I feared. It just needed a little re-factoring of the migrate_incoming_tcp() function. I'll be posting that in a couple of minutes. Given how this code worked out, doing a "nowait" for the fd style is really no big deal.
Both ways (explicit monitor command and "nowait") seem to have their benefits; the monitor command has the benefit of being more flexible, while the nowait has the benefit of being similar to already existing Qemu commands. My preference is for the monitor command since it seems a little more natural for a management tool, but the nowait is clearly an option as well.
I'll also post a cleanup patch with Dan's suggestion for the monitor patch, so both implementations will be available.
Chris Lalancette