[Qemu-devel] [PATCH 0/2] Migration fixes

[Qemu-devel] [PATCH 0/2] Migration fixes

[Juan Quintela]

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(-)

[Qemu-devel] [PATCH 1/2] migration: Clear fd also in error cases

[Juan Quintela]

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

[Qemu-devel] [PATCH 2/2] migration: unix migration should obey autostart are the other ones

[Juan Quintela]

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

Re: [Qemu-devel] [PATCH 2/2] migration: unix migration should obey autostart are the other ones

[Anthony Liguori]

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);
>    

[Qemu-devel] Re: [PATCH 2/2] migration: unix migration should obey autostart are the other ones

[Juan Quintela]

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);
>>    

Re: [Qemu-devel] [PATCH 1/2] migration: Clear fd also in error cases

[Anthony Liguori]

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);
>   }
>
>