* [Qemu-devel] [PATCH 0/2] Migration fixes @ 2010-03-09 23:10 Juan Quintela 2010-03-09 23:10 ` [Qemu-devel] [PATCH 1/2] migration: Clear fd also in error cases Juan Quintela 2010-03-09 23:10 ` [Qemu-devel] [PATCH 2/2] migration: unix migration should obey autostart are the other ones Juan Quintela 0 siblings, 2 replies; 6+ messages in thread From: Juan Quintela @ 2010-03-09 23:10 UTC (permalink / raw) To: qemu-devel This series: - fix 100% cpu use when incoming migration fails - makes unix migration to obey autostart, like the others Please, apply Juan Quintela (2): migration: Clear fd also in error cases migration: unix migration should obey autostart are the other ones migration-exec.c | 4 ++-- migration-fd.c | 4 ++-- migration-tcp.c | 5 ++--- migration-unix.c | 7 ++++--- 4 files changed, 10 insertions(+), 10 deletions(-) ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 1/2] migration: Clear fd also in error cases 2010-03-09 23:10 [Qemu-devel] [PATCH 0/2] Migration fixes Juan Quintela @ 2010-03-09 23:10 ` Juan Quintela 2010-03-17 16:57 ` Anthony Liguori 2010-03-09 23:10 ` [Qemu-devel] [PATCH 2/2] migration: unix migration should obey autostart are the other ones Juan Quintela 1 sibling, 1 reply; 6+ messages in thread From: Juan Quintela @ 2010-03-09 23:10 UTC (permalink / raw) To: qemu-devel Not clearing the fd and closing the file makes qemu spin using 100%CPU after incoming migration error. See for instance bug: https://bugzilla.redhat.com/show_bug.cgi?id=518032 Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration-exec.c | 4 ++-- migration-fd.c | 4 ++-- migration-tcp.c | 5 ++--- migration-unix.c | 5 ++--- 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/migration-exec.c b/migration-exec.c index 3edc026..6ff8449 100644 --- a/migration-exec.c +++ b/migration-exec.c @@ -120,12 +120,12 @@ static void exec_accept_incoming_migration(void *opaque) } qemu_announce_self(); DPRINTF("successfully loaded vm state\n"); - /* we've successfully migrated, close the fd */ - qemu_set_fd_handler2(qemu_stdio_fd(f), NULL, NULL, NULL, NULL); + if (autostart) vm_start(); err: + qemu_set_fd_handler2(qemu_stdio_fd(f), NULL, NULL, NULL, NULL); qemu_fclose(f); } diff --git a/migration-fd.c b/migration-fd.c index 0cc74ad..9cf52ce 100644 --- a/migration-fd.c +++ b/migration-fd.c @@ -113,12 +113,12 @@ static void fd_accept_incoming_migration(void *opaque) } qemu_announce_self(); DPRINTF("successfully loaded vm state\n"); - /* we've successfully migrated, close the fd */ - qemu_set_fd_handler2(qemu_stdio_fd(f), NULL, NULL, NULL, NULL); + if (autostart) vm_start(); err: + qemu_set_fd_handler2(qemu_stdio_fd(f), NULL, NULL, NULL, NULL); qemu_fclose(f); } diff --git a/migration-tcp.c b/migration-tcp.c index e7f307c..95ce722 100644 --- a/migration-tcp.c +++ b/migration-tcp.c @@ -170,15 +170,14 @@ static void tcp_accept_incoming_migration(void *opaque) qemu_announce_self(); DPRINTF("successfully loaded vm state\n"); - /* we've successfully migrated, close the server socket */ - qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL); - close(s); if (autostart) vm_start(); out_fopen: qemu_fclose(f); out: + qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL); + close(s); close(c); } diff --git a/migration-unix.c b/migration-unix.c index b7aab38..ce59a2a 100644 --- a/migration-unix.c +++ b/migration-unix.c @@ -176,13 +176,12 @@ static void unix_accept_incoming_migration(void *opaque) qemu_announce_self(); DPRINTF("successfully loaded vm state\n"); - /* we've successfully migrated, close the server socket */ - qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL); - close(s); out_fopen: qemu_fclose(f); out: + qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL); + close(s); close(c); } -- 1.6.6.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] migration: Clear fd also in error cases 2010-03-09 23:10 ` [Qemu-devel] [PATCH 1/2] migration: Clear fd also in error cases Juan Quintela @ 2010-03-17 16:57 ` Anthony Liguori 0 siblings, 0 replies; 6+ messages in thread From: Anthony Liguori @ 2010-03-17 16:57 UTC (permalink / raw) To: Juan Quintela; +Cc: qemu-devel On 03/09/2010 05:10 PM, Juan Quintela wrote: > Not clearing the fd and closing the file makes qemu spin using 100%CPU > after incoming migration error. > > See for instance bug: > https://bugzilla.redhat.com/show_bug.cgi?id=518032 > > Signed-off-by: Juan Quintela<quintela@redhat.com> > Applied all. Thanks. Regards, Anthony Liguori > --- > migration-exec.c | 4 ++-- > migration-fd.c | 4 ++-- > migration-tcp.c | 5 ++--- > migration-unix.c | 5 ++--- > 4 files changed, 8 insertions(+), 10 deletions(-) > > diff --git a/migration-exec.c b/migration-exec.c > index 3edc026..6ff8449 100644 > --- a/migration-exec.c > +++ b/migration-exec.c > @@ -120,12 +120,12 @@ static void exec_accept_incoming_migration(void *opaque) > } > qemu_announce_self(); > DPRINTF("successfully loaded vm state\n"); > - /* we've successfully migrated, close the fd */ > - qemu_set_fd_handler2(qemu_stdio_fd(f), NULL, NULL, NULL, NULL); > + > if (autostart) > vm_start(); > > err: > + qemu_set_fd_handler2(qemu_stdio_fd(f), NULL, NULL, NULL, NULL); > qemu_fclose(f); > } > > diff --git a/migration-fd.c b/migration-fd.c > index 0cc74ad..9cf52ce 100644 > --- a/migration-fd.c > +++ b/migration-fd.c > @@ -113,12 +113,12 @@ static void fd_accept_incoming_migration(void *opaque) > } > qemu_announce_self(); > DPRINTF("successfully loaded vm state\n"); > - /* we've successfully migrated, close the fd */ > - qemu_set_fd_handler2(qemu_stdio_fd(f), NULL, NULL, NULL, NULL); > + > if (autostart) > vm_start(); > > err: > + qemu_set_fd_handler2(qemu_stdio_fd(f), NULL, NULL, NULL, NULL); > qemu_fclose(f); > } > > diff --git a/migration-tcp.c b/migration-tcp.c > index e7f307c..95ce722 100644 > --- a/migration-tcp.c > +++ b/migration-tcp.c > @@ -170,15 +170,14 @@ static void tcp_accept_incoming_migration(void *opaque) > qemu_announce_self(); > DPRINTF("successfully loaded vm state\n"); > > - /* we've successfully migrated, close the server socket */ > - qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL); > - close(s); > if (autostart) > vm_start(); > > out_fopen: > qemu_fclose(f); > out: > + qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL); > + close(s); > close(c); > } > > diff --git a/migration-unix.c b/migration-unix.c > index b7aab38..ce59a2a 100644 > --- a/migration-unix.c > +++ b/migration-unix.c > @@ -176,13 +176,12 @@ static void unix_accept_incoming_migration(void *opaque) > qemu_announce_self(); > DPRINTF("successfully loaded vm state\n"); > > - /* we've successfully migrated, close the server socket */ > - qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL); > - close(s); > > out_fopen: > qemu_fclose(f); > out: > + qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL); > + close(s); > close(c); > } > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 2/2] migration: unix migration should obey autostart are the other ones 2010-03-09 23:10 [Qemu-devel] [PATCH 0/2] Migration fixes Juan Quintela 2010-03-09 23:10 ` [Qemu-devel] [PATCH 1/2] migration: Clear fd also in error cases Juan Quintela @ 2010-03-09 23:10 ` Juan Quintela 2010-03-11 20:27 ` Anthony Liguori 1 sibling, 1 reply; 6+ messages in thread From: Juan Quintela @ 2010-03-09 23:10 UTC (permalink / raw) To: qemu-devel This was the only incoming migration without autostart check Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration-unix.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/migration-unix.c b/migration-unix.c index ce59a2a..49de1b9 100644 --- a/migration-unix.c +++ b/migration-unix.c @@ -176,6 +176,8 @@ static void unix_accept_incoming_migration(void *opaque) qemu_announce_self(); DPRINTF("successfully loaded vm state\n"); + if (autostart) + vm_start(); out_fopen: qemu_fclose(f); -- 1.6.6.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] migration: unix migration should obey autostart are the other ones 2010-03-09 23:10 ` [Qemu-devel] [PATCH 2/2] migration: unix migration should obey autostart are the other ones Juan Quintela @ 2010-03-11 20:27 ` Anthony Liguori 2010-03-11 20:42 ` [Qemu-devel] " Juan Quintela 0 siblings, 1 reply; 6+ messages in thread From: Anthony Liguori @ 2010-03-11 20:27 UTC (permalink / raw) To: Juan Quintela; +Cc: qemu-devel On 03/09/2010 05:10 PM, Juan Quintela wrote: > This was the only incoming migration without autostart check > > Signed-off-by: Juan Quintela<quintela@redhat.com> > --- > migration-unix.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/migration-unix.c b/migration-unix.c > index ce59a2a..49de1b9 100644 > --- a/migration-unix.c > +++ b/migration-unix.c > @@ -176,6 +176,8 @@ static void unix_accept_incoming_migration(void *opaque) > qemu_announce_self(); > DPRINTF("successfully loaded vm state\n"); > > + if (autostart) > + vm_start(); > Maybe a better thing to do is make accept_incoming_migration return an error code and do the vm_start in common code. Regards, Anthony Liguori > out_fopen: > qemu_fclose(f); > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] Re: [PATCH 2/2] migration: unix migration should obey autostart are the other ones 2010-03-11 20:27 ` Anthony Liguori @ 2010-03-11 20:42 ` Juan Quintela 0 siblings, 0 replies; 6+ messages in thread From: Juan Quintela @ 2010-03-11 20:42 UTC (permalink / raw) To: Anthony Liguori; +Cc: qemu-devel Anthony Liguori <anthony@codemonkey.ws> wrote: > On 03/09/2010 05:10 PM, Juan Quintela wrote: >> This was the only incoming migration without autostart check >> >> Signed-off-by: Juan Quintela<quintela@redhat.com> >> --- >> migration-unix.c | 2 ++ >> 1 files changed, 2 insertions(+), 0 deletions(-) >> >> diff --git a/migration-unix.c b/migration-unix.c >> index ce59a2a..49de1b9 100644 >> --- a/migration-unix.c >> +++ b/migration-unix.c >> @@ -176,6 +176,8 @@ static void unix_accept_incoming_migration(void *opaque) >> qemu_announce_self(); >> DPRINTF("successfully loaded vm state\n"); >> >> + if (autostart) >> + vm_start(); >> > > Maybe a better thing to do is make accept_incoming_migration return an > error code and do the vm_start in common code. It is uglier than that (got the ones from migration-fd but idea is the same, removed uinteresting bits) static void fd_accept_incoming_migration(void *opaque) { ret = qemu_loadvm_state(f); .... if (autostart) vm_start(); .... } int fd_start_incoming_migration(const char *infd) { .... qemu_set_fd_handler2(fd, NULL, fd_accept_incoming_migration, NULL, (void *)(unsigned long)f); return 0; } function call chain is: main() -> qemu_start_incoming_migration() -> fd_start_migration() -> here they got an fd, and call fd_accept_incoming_migration asyncronously on that fd. We can't call vm_start() until migration finish. And we have gone async a long time ago. As a related note, you can see that exec_accept_incoming_migration() is identical to fd_start_incoming_migration(). I was going to share the code for all migrations until I found that unix & tcp migration do the accept also asynchronously, breaking the common function. My plan was to make the accept in {unix,tcp}_start_incoming_migration() and then have a shared qemu_accept_incoming_migration() for all four kinds of migration. Notice that it don't make sense to make the accept async, because qemu is not going to do _anything_ until incoming migration is finished, anyways. Just didn't want to merge the bugfix with the other change. Later, Juan. > Regards, > > Anthony Liguori > >> out_fopen: >> qemu_fclose(f); >> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-03-17 16:58 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-03-09 23:10 [Qemu-devel] [PATCH 0/2] Migration fixes Juan Quintela 2010-03-09 23:10 ` [Qemu-devel] [PATCH 1/2] migration: Clear fd also in error cases Juan Quintela 2010-03-17 16:57 ` Anthony Liguori 2010-03-09 23:10 ` [Qemu-devel] [PATCH 2/2] migration: unix migration should obey autostart are the other ones Juan Quintela 2010-03-11 20:27 ` Anthony Liguori 2010-03-11 20:42 ` [Qemu-devel] " Juan Quintela
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).