[Qemu-devel] [PATCH 0/3] Migration Fixes

[Qemu-devel] [PATCH 0/3] Migration Fixes

[Glauber Costa]

This is a refactored version of a bunch of migration fixes sent last week
by Yaniv Kamay to kvm@vger. Turns out that most of the changes are not kvm-specific,
and can be applied to qemu as well. So here they are, with a little refactor,
and a bit of lemon.

Serve yourself.

[Qemu-devel] [PATCH 1/3] stop dirty tracking just at the end of migration

[Glauber Costa]

If there is still work to do, it is not safe to assume we
can end the dirty tracking. Specifically, kvm can update the dirty
tracking log inside ram_save_block(), leaving pages still out of sync
if we go with the current code.

Based on a patch by Yaniv Kamay

Signed-off-by: Glauber Costa <glommer@redhat.com>
CC: Yaniv Kamay <ykamay@redhat.com>
CC: Dor Laor <dlaor@redhat.com>
---
 vl.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/vl.c b/vl.c
index 5e6c621..1626f8a 100644
--- a/vl.c
+++ b/vl.c
@@ -3249,10 +3249,10 @@ static int ram_save_live(QEMUFile *f, int stage, void *opaque)
     /* try transferring iterative blocks of memory */
 
     if (stage == 3) {
-        cpu_physical_memory_set_dirty_tracking(0);
 
         /* flush all remaining blocks regardless of rate limiting */
         while (ram_save_block(f) != 0);
+        cpu_physical_memory_set_dirty_tracking(0);
     }
 
     qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
-- 
1.5.6.6

[Qemu-devel] [PATCH 2/3] create qemu_file_set_error

[Glauber Costa]

This is mainly for consistency, since we don't want
anything outside of savevm setting it explicitly. There
are current no users of that in qemu tree, but there
are potential candidates on kvm-userspace. And avi
is a nice guy, let's be nice with him.

Based on a patch by Yaniv Kamay

Signed-off-by: Glauber Costa <glommer@redhat.com>
CC: Yaniv Kamay <ykamay@redhat.com>
CC: Dor Laor <dlaor@redhat.com>
---
 hw/hw.h  |    1 +
 savevm.c |    5 +++++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/hw/hw.h b/hw/hw.h
index eab7bb4..e9628d4 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -67,6 +67,7 @@ unsigned int qemu_get_be32(QEMUFile *f);
 uint64_t qemu_get_be64(QEMUFile *f);
 int qemu_file_rate_limit(QEMUFile *f);
 int qemu_file_has_error(QEMUFile *f);
+void qemu_file_set_error(QEMUFile *f);
 
 /* Try to send any outstanding data.  This function is useful when output is
  * halted due to rate limiting or EAGAIN errors occur as it can be used to
diff --git a/savevm.c b/savevm.c
index ce12628..9f81d07 100644
--- a/savevm.c
+++ b/savevm.c
@@ -370,6 +370,11 @@ int qemu_file_has_error(QEMUFile *f)
     return f->has_error;
 }
 
+void qemu_file_set_error(QEMUFile *f)
+{
+    f->has_error = 1;
+}
+
 void qemu_fflush(QEMUFile *f)
 {
     if (!f->put_buffer)
-- 
1.5.6.6

[Qemu-devel] [PATCH 3/3] propagate error on failed completion

[Glauber Costa]

migrate_fd_put_ready() calls qemu_savevm_state_complete(),
but the later can fail.

If it happens, re-start the vm and propagate the error up

Based on a patch by Yaniv Kamay

Signed-off-by: Glauber Costa <glommer@redhat.com>
CC: Yaniv Kamay <ykamay@redhat.com>
CC: Dor Laor <dlaor@redhat.com>
---
 migration.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/migration.c b/migration.c
index b3904b2..4574830 100644
--- a/migration.c
+++ b/migration.c
@@ -224,7 +224,11 @@ void migrate_fd_put_ready(void *opaque)
         vm_stop(0);
 
         bdrv_flush_all();
-        qemu_savevm_state_complete(s->file);
+        if (qemu_savevm_state_complete(s->file) < 0);
+            vm_start();
+            s->done(s, MIG_STATE_ERROR);
+            return;
+        }
         s->state = MIG_STATE_COMPLETED;
         migrate_fd_cleanup(s);
     }
-- 
1.5.6.6

Re: [Qemu-devel] [PATCH 3/3] propagate error on failed completion

[Edgar E. Iglesias]

On Tue, Mar 31, 2009 at 12:51:15PM -0400, Glauber Costa wrote:
> migrate_fd_put_ready() calls qemu_savevm_state_complete(),
> but the later can fail.
> 
> If it happens, re-start the vm and propagate the error up
> 
> Based on a patch by Yaniv Kamay
> 
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> CC: Yaniv Kamay <ykamay@redhat.com>
> CC: Dor Laor <dlaor@redhat.com>
> ---
>  migration.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/migration.c b/migration.c
> index b3904b2..4574830 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -224,7 +224,11 @@ void migrate_fd_put_ready(void *opaque)
>          vm_stop(0);
>  
>          bdrv_flush_all();
> -        qemu_savevm_state_complete(s->file);
> +        if (qemu_savevm_state_complete(s->file) < 0);
                                                      ^^^
Forgot something?

> +            vm_start();
> +            s->done(s, MIG_STATE_ERROR);
> +            return;
> +        }
>          s->state = MIG_STATE_COMPLETED;
>          migrate_fd_cleanup(s);
>      }

Cheers

Re: [Qemu-devel] [PATCH 3/3] propagate error on failed completion

[Blue Swirl]

On 3/31/09, Glauber Costa <glommer@redhat.com> wrote:
> migrate_fd_put_ready() calls qemu_savevm_state_complete(),
>  but the later can fail.
>
>  If it happens, re-start the vm and propagate the error up

>  +        if (qemu_savevm_state_complete(s->file) < 0);

Did you try to compile this?

Re: [Qemu-devel] [PATCH 3/3] propagate error on failed completion

[Glauber Costa]

On Tue, Mar 31, 2009 at 2:06 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
> On 3/31/09, Glauber Costa <glommer@redhat.com> wrote:
>> migrate_fd_put_ready() calls qemu_savevm_state_complete(),
>>  but the later can fail.
>>
>>  If it happens, re-start the vm and propagate the error up
>
>>  +        if (qemu_savevm_state_complete(s->file) < 0);
>
> Did you try to compile this?
>
ouch. Last hour editing, of course it will work, famous last words...
Shame on me.


-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

[Qemu-devel] Re: [PATCH 1/3] stop dirty tracking just at the end of migration

[Yaniv Kamay]

We need to update dirty map before ram_save_block() loop.

Glauber Costa wrote:
> If there is still work to do, it is not safe to assume we
> can end the dirty tracking. Specifically, kvm can update the dirty
> tracking log inside ram_save_block(), leaving pages still out of sync
> if we go with the current code.
>
> Based on a patch by Yaniv Kamay
>
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> CC: Yaniv Kamay <ykamay@redhat.com>
> CC: Dor Laor <dlaor@redhat.com>
> ---
>  vl.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index 5e6c621..1626f8a 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3249,10 +3249,10 @@ static int ram_save_live(QEMUFile *f, int stage, void *opaque)
>      /* try transferring iterative blocks of memory */
>  
>      if (stage == 3) {
> -        cpu_physical_memory_set_dirty_tracking(0);
>  
>          /* flush all remaining blocks regardless of rate limiting */
>          while (ram_save_block(f) != 0);
> +        cpu_physical_memory_set_dirty_tracking(0);
>      }
>  
>      qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
>   

[Qemu-devel] Re: [PATCH 1/3] stop dirty tracking just at the end of migration

[Glauber Costa]

On Tue, Mar 31, 2009 at 10:04:52PM +0300, Yaniv Kamay wrote:
> We need to update dirty map before ram_save_block() loop.
>From what I understand, this is needed in kvm only.
This patch applies to qemu.

>
> Glauber Costa wrote:
>> If there is still work to do, it is not safe to assume we
>> can end the dirty tracking. Specifically, kvm can update the dirty
>> tracking log inside ram_save_block(), leaving pages still out of sync
>> if we go with the current code.
>>
>> Based on a patch by Yaniv Kamay
>>
>> Signed-off-by: Glauber Costa <glommer@redhat.com>
>> CC: Yaniv Kamay <ykamay@redhat.com>
>> CC: Dor Laor <dlaor@redhat.com>
>> ---
>>  vl.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/vl.c b/vl.c
>> index 5e6c621..1626f8a 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -3249,10 +3249,10 @@ static int ram_save_live(QEMUFile *f, int stage, void *opaque)
>>      /* try transferring iterative blocks of memory */
>>       if (stage == 3) {
>> -        cpu_physical_memory_set_dirty_tracking(0);
>>           /* flush all remaining blocks regardless of rate limiting */
>>          while (ram_save_block(f) != 0);
>> +        cpu_physical_memory_set_dirty_tracking(0);
>>      }
>>       qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
>>   
>

[Qemu-devel] Re: [PATCH 1/3] stop dirty tracking just at the end of migration

[Yaniv Kamay]

Glauber Costa wrote:
> On Tue, Mar 31, 2009 at 10:04:52PM +0300, Yaniv Kamay wrote:
>   
>> We need to update dirty map before ram_save_block() loop.
>>     
> From what I understand, this is needed in kvm only.
> This patch applies to qemu.
>   
OK, my mistake. sorry.
>   
>> Glauber Costa wrote:
>>     
>>> If there is still work to do, it is not safe to assume we
>>> can end the dirty tracking. Specifically, kvm can update the dirty
>>> tracking log inside ram_save_block(), leaving pages still out of sync
>>> if we go with the current code.
>>>
>>> Based on a patch by Yaniv Kamay
>>>
>>> Signed-off-by: Glauber Costa <glommer@redhat.com>
>>> CC: Yaniv Kamay <ykamay@redhat.com>
>>> CC: Dor Laor <dlaor@redhat.com>
>>> ---
>>>  vl.c |    2 +-
>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/vl.c b/vl.c
>>> index 5e6c621..1626f8a 100644
>>> --- a/vl.c
>>> +++ b/vl.c
>>> @@ -3249,10 +3249,10 @@ static int ram_save_live(QEMUFile *f, int stage, void *opaque)
>>>      /* try transferring iterative blocks of memory */
>>>       if (stage == 3) {
>>> -        cpu_physical_memory_set_dirty_tracking(0);
>>>           /* flush all remaining blocks regardless of rate limiting */
>>>          while (ram_save_block(f) != 0);
>>> +        cpu_physical_memory_set_dirty_tracking(0);
>>>      }
>>>       qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
>>>   
>>>