diff mbox

[COLO-Frame,v5,11/29] COLO VMstate: Load VM state into qsb before restore it

Message ID 1432196001-10352-12-git-send-email-zhang.zhanghailiang@huawei.com
State New
Headers show

Commit Message

Zhanghailiang May 21, 2015, 8:13 a.m. UTC
We should cache the device state before restore it,
besides, we should call qemu_system_reset() before load VM state,
which can ensure the data is intact.

Note: If we discard qemu_system_reset(), there will be some odd error,
For exmple, qemu in slave side crashes and reports:

KVM: entry failed, hardware error 0x7
EAX=00000000 EBX=0000e000 ECX=00009578 EDX=0000434f
ESI=0000fc10 EDI=0000434f EBP=00000000 ESP=00001fca
EIP=00009594 EFL=00010246 [---Z-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0
ES =0040 00000400 0000ffff 00009300
CS =f000 000f0000 0000ffff 00009b00
SS =434f 000434f0 0000ffff 00009300
DS =434f 000434f0 0000ffff 00009300
FS =0000 00000000 0000ffff 00009300
GS =0000 00000000 0000ffff 00009300
LDT=0000 00000000 0000ffff 00008200
TR =0000 00000000 0000ffff 00008b00
GDT=     0002dcc8 00000047
IDT=     00000000 0000ffff
CR0=00000010 CR2=ffffffff CR3=00000000 CR4=00000000
DR0=0000000000000000 DR1=0000000000000000 DR2=0000000000000000 DR3=0000000000000000
DR6=00000000ffff0ff0 DR7=0000000000000400
EFER=0000000000000000
Code=c0 74 0f 66 b9 78 95 00 00 66 31 d2 66 31 c0 e9 47 e0 fb 90 <f3> 90 fa fc 66 c3 66 53 66 89 c3 66 e8 9d e8 ff ff 66 01 c3 66 89 d8 66 e8 40 e9 ff ff 66
ERROR: invalid runstate transition: 'internal-error' -> 'colo'

The reason is, some of the device state will be ignored when saving device state to slave,
if the corresponding data is in its initial value, such as 0.
But the device state in slave maybe in initialized value, after a loop of checkpoint,
there will be inconsistent for the value of device state.
This will happen when the PVM reboot or SVM run ahead of PVM in the startup process.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 migration/colo.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 50 insertions(+), 3 deletions(-)

Comments

Dr. David Alan Gilbert June 5, 2015, 6:02 p.m. UTC | #1
* zhanghailiang (zhang.zhanghailiang@huawei.com) wrote:
> We should cache the device state before restore it,
> besides, we should call qemu_system_reset() before load VM state,
> which can ensure the data is intact.

I think the description could be better; to me the important
point is not that it's a 'cache', but the important point is that you
don't destroy the state of the secondary until you are sure that you can
read the whole state from the primary, just in case the primary fails
in the middle of sending the state.

However, other than that:

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

(I suspect you'll need updates as the qemu migration code updates)

Dave

> Note: If we discard qemu_system_reset(), there will be some odd error,
> For exmple, qemu in slave side crashes and reports:
> 
> KVM: entry failed, hardware error 0x7
> EAX=00000000 EBX=0000e000 ECX=00009578 EDX=0000434f
> ESI=0000fc10 EDI=0000434f EBP=00000000 ESP=00001fca
> EIP=00009594 EFL=00010246 [---Z-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0
> ES =0040 00000400 0000ffff 00009300
> CS =f000 000f0000 0000ffff 00009b00
> SS =434f 000434f0 0000ffff 00009300
> DS =434f 000434f0 0000ffff 00009300
> FS =0000 00000000 0000ffff 00009300
> GS =0000 00000000 0000ffff 00009300
> LDT=0000 00000000 0000ffff 00008200
> TR =0000 00000000 0000ffff 00008b00
> GDT=     0002dcc8 00000047
> IDT=     00000000 0000ffff
> CR0=00000010 CR2=ffffffff CR3=00000000 CR4=00000000
> DR0=0000000000000000 DR1=0000000000000000 DR2=0000000000000000 DR3=0000000000000000
> DR6=00000000ffff0ff0 DR7=0000000000000400
> EFER=0000000000000000
> Code=c0 74 0f 66 b9 78 95 00 00 66 31 d2 66 31 c0 e9 47 e0 fb 90 <f3> 90 fa fc 66 c3 66 53 66 89 c3 66 e8 9d e8 ff ff 66 01 c3 66 89 d8 66 e8 40 e9 ff ff 66
> ERROR: invalid runstate transition: 'internal-error' -> 'colo'
> 
> The reason is, some of the device state will be ignored when saving device state to slave,
> if the corresponding data is in its initial value, such as 0.
> But the device state in slave maybe in initialized value, after a loop of checkpoint,
> there will be inconsistent for the value of device state.
> This will happen when the PVM reboot or SVM run ahead of PVM in the startup process.
> 
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  migration/colo.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 50 insertions(+), 3 deletions(-)
> 
> diff --git a/migration/colo.c b/migration/colo.c
> index 39cd698..0f61786 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -309,8 +309,10 @@ void *colo_process_incoming_checkpoints(void *opaque)
>      struct colo_incoming *colo_in = opaque;
>      QEMUFile *f = colo_in->file;
>      int fd = qemu_get_fd(f);
> -    QEMUFile *ctl = NULL;
> +    QEMUFile *ctl = NULL, *fb = NULL;
>      int ret;
> +    uint64_t total_size;
> +
>      colo = qemu_coroutine_self();
>      assert(colo != NULL);
>  
> @@ -325,10 +327,17 @@ void *colo_process_incoming_checkpoints(void *opaque)
>          goto out;
>      }
>  
> +    colo_buffer = qsb_create(NULL, COLO_BUFFER_BASE_SIZE);
> +    if (colo_buffer == NULL) {
> +        error_report("Failed to allocate colo buffer!");
> +        goto out;
> +    }
> +
>      ret = colo_ctl_put(ctl, COLO_CHECPOINT_READY);
>      if (ret < 0) {
>          goto out;
>      }
> +
>      qemu_mutex_lock_iothread();
>      /* in COLO mode, slave is runing, so start the vm */
>      vm_start();
> @@ -364,7 +373,18 @@ void *colo_process_incoming_checkpoints(void *opaque)
>          }
>          trace_colo_receive_message("COLO_CHECKPOINT_SEND");
>  
> -        /*TODO Load VM state */
> +        /* read the VM state total size first */
> +        ret = colo_ctl_get_value(f, &total_size);
> +        if (ret < 0) {
> +            goto out;
> +        }
> +
> +        /* read vm device state into colo buffer */
> +        ret = qsb_fill_buffer(colo_buffer, f, total_size);
> +        if (ret != total_size) {
> +            error_report("can't get all migration data");
> +            goto out;
> +        }
>  
>          ret = colo_ctl_put(ctl, COLO_CHECKPOINT_RECEIVED);
>          if (ret < 0) {
> @@ -372,6 +392,22 @@ void *colo_process_incoming_checkpoints(void *opaque)
>          }
>          trace_colo_receive_message("COLO_CHECKPOINT_RECEIVED");
>  
> +        /* open colo buffer for read */
> +        fb = qemu_bufopen("r", colo_buffer);
> +        if (!fb) {
> +            error_report("can't open colo buffer for read");
> +            goto out;
> +        }
> +
> +        qemu_mutex_lock_iothread();
> +        qemu_system_reset(VMRESET_SILENT);
> +        if (qemu_loadvm_state(fb) < 0) {
> +            error_report("COLO: loadvm failed");
> +            qemu_mutex_unlock_iothread();
> +            goto out;
> +        }
> +        qemu_mutex_unlock_iothread();
> +
>          /* TODO: flush vm state */
>  
>          ret = colo_ctl_put(ctl, COLO_CHECKPOINT_LOADED);
> @@ -384,14 +420,25 @@ void *colo_process_incoming_checkpoints(void *opaque)
>          vm_start();
>          qemu_mutex_unlock_iothread();
>          trace_colo_vm_state_change("stop", "start");
> -}
> +
> +        qemu_fclose(fb);
> +        fb = NULL;
> +    }
>  
>  out:
>      colo = NULL;
> +
> +    if (fb) {
> +        qemu_fclose(fb);
> +    }
> +
>      release_ram_cache();
>      if (ctl) {
>          qemu_fclose(ctl);
>      }
> +
> +    qsb_free(colo_buffer);
> +
>      loadvm_exit_colo();
>  
>      return NULL;
> -- 
> 1.7.12.4
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Zhanghailiang June 9, 2015, 2:19 a.m. UTC | #2
On 2015/6/6 2:02, Dr. David Alan Gilbert wrote:
> * zhanghailiang (zhang.zhanghailiang@huawei.com) wrote:
>> We should cache the device state before restore it,
>> besides, we should call qemu_system_reset() before load VM state,
>> which can ensure the data is intact.
>
> I think the description could be better; to me the important
> point is not that it's a 'cache', but the important point is that you
> don't destroy the state of the secondary until you are sure that you can
> read the whole state from the primary, just in case the primary fails
> in the middle of sending the state.
>

OK, I will fix this description.

> However, other than that:
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> (I suspect you'll need updates as the qemu migration code updates)
>

Yes, thanks.

> Dave
>
>> Note: If we discard qemu_system_reset(), there will be some odd error,
>> For exmple, qemu in slave side crashes and reports:
>>
>> KVM: entry failed, hardware error 0x7
>> EAX=00000000 EBX=0000e000 ECX=00009578 EDX=0000434f
>> ESI=0000fc10 EDI=0000434f EBP=00000000 ESP=00001fca
>> EIP=00009594 EFL=00010246 [---Z-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0
>> ES =0040 00000400 0000ffff 00009300
>> CS =f000 000f0000 0000ffff 00009b00
>> SS =434f 000434f0 0000ffff 00009300
>> DS =434f 000434f0 0000ffff 00009300
>> FS =0000 00000000 0000ffff 00009300
>> GS =0000 00000000 0000ffff 00009300
>> LDT=0000 00000000 0000ffff 00008200
>> TR =0000 00000000 0000ffff 00008b00
>> GDT=     0002dcc8 00000047
>> IDT=     00000000 0000ffff
>> CR0=00000010 CR2=ffffffff CR3=00000000 CR4=00000000
>> DR0=0000000000000000 DR1=0000000000000000 DR2=0000000000000000 DR3=0000000000000000
>> DR6=00000000ffff0ff0 DR7=0000000000000400
>> EFER=0000000000000000
>> Code=c0 74 0f 66 b9 78 95 00 00 66 31 d2 66 31 c0 e9 47 e0 fb 90 <f3> 90 fa fc 66 c3 66 53 66 89 c3 66 e8 9d e8 ff ff 66 01 c3 66 89 d8 66 e8 40 e9 ff ff 66
>> ERROR: invalid runstate transition: 'internal-error' -> 'colo'
>>
>> The reason is, some of the device state will be ignored when saving device state to slave,
>> if the corresponding data is in its initial value, such as 0.
>> But the device state in slave maybe in initialized value, after a loop of checkpoint,
>> there will be inconsistent for the value of device state.
>> This will happen when the PVM reboot or SVM run ahead of PVM in the startup process.
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
>> ---
>>   migration/colo.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++---
>>   1 file changed, 50 insertions(+), 3 deletions(-)
>>
>> diff --git a/migration/colo.c b/migration/colo.c
>> index 39cd698..0f61786 100644
>> --- a/migration/colo.c
>> +++ b/migration/colo.c
>> @@ -309,8 +309,10 @@ void *colo_process_incoming_checkpoints(void *opaque)
>>       struct colo_incoming *colo_in = opaque;
>>       QEMUFile *f = colo_in->file;
>>       int fd = qemu_get_fd(f);
>> -    QEMUFile *ctl = NULL;
>> +    QEMUFile *ctl = NULL, *fb = NULL;
>>       int ret;
>> +    uint64_t total_size;
>> +
>>       colo = qemu_coroutine_self();
>>       assert(colo != NULL);
>>
>> @@ -325,10 +327,17 @@ void *colo_process_incoming_checkpoints(void *opaque)
>>           goto out;
>>       }
>>
>> +    colo_buffer = qsb_create(NULL, COLO_BUFFER_BASE_SIZE);
>> +    if (colo_buffer == NULL) {
>> +        error_report("Failed to allocate colo buffer!");
>> +        goto out;
>> +    }
>> +
>>       ret = colo_ctl_put(ctl, COLO_CHECPOINT_READY);
>>       if (ret < 0) {
>>           goto out;
>>       }
>> +
>>       qemu_mutex_lock_iothread();
>>       /* in COLO mode, slave is runing, so start the vm */
>>       vm_start();
>> @@ -364,7 +373,18 @@ void *colo_process_incoming_checkpoints(void *opaque)
>>           }
>>           trace_colo_receive_message("COLO_CHECKPOINT_SEND");
>>
>> -        /*TODO Load VM state */
>> +        /* read the VM state total size first */
>> +        ret = colo_ctl_get_value(f, &total_size);
>> +        if (ret < 0) {
>> +            goto out;
>> +        }
>> +
>> +        /* read vm device state into colo buffer */
>> +        ret = qsb_fill_buffer(colo_buffer, f, total_size);
>> +        if (ret != total_size) {
>> +            error_report("can't get all migration data");
>> +            goto out;
>> +        }
>>
>>           ret = colo_ctl_put(ctl, COLO_CHECKPOINT_RECEIVED);
>>           if (ret < 0) {
>> @@ -372,6 +392,22 @@ void *colo_process_incoming_checkpoints(void *opaque)
>>           }
>>           trace_colo_receive_message("COLO_CHECKPOINT_RECEIVED");
>>
>> +        /* open colo buffer for read */
>> +        fb = qemu_bufopen("r", colo_buffer);
>> +        if (!fb) {
>> +            error_report("can't open colo buffer for read");
>> +            goto out;
>> +        }
>> +
>> +        qemu_mutex_lock_iothread();
>> +        qemu_system_reset(VMRESET_SILENT);
>> +        if (qemu_loadvm_state(fb) < 0) {
>> +            error_report("COLO: loadvm failed");
>> +            qemu_mutex_unlock_iothread();
>> +            goto out;
>> +        }
>> +        qemu_mutex_unlock_iothread();
>> +
>>           /* TODO: flush vm state */
>>
>>           ret = colo_ctl_put(ctl, COLO_CHECKPOINT_LOADED);
>> @@ -384,14 +420,25 @@ void *colo_process_incoming_checkpoints(void *opaque)
>>           vm_start();
>>           qemu_mutex_unlock_iothread();
>>           trace_colo_vm_state_change("stop", "start");
>> -}
>> +
>> +        qemu_fclose(fb);
>> +        fb = NULL;
>> +    }
>>
>>   out:
>>       colo = NULL;
>> +
>> +    if (fb) {
>> +        qemu_fclose(fb);
>> +    }
>> +
>>       release_ram_cache();
>>       if (ctl) {
>>           qemu_fclose(ctl);
>>       }
>> +
>> +    qsb_free(colo_buffer);
>> +
>>       loadvm_exit_colo();
>>
>>       return NULL;
>> --
>> 1.7.12.4
>>
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
> .
>
diff mbox

Patch

diff --git a/migration/colo.c b/migration/colo.c
index 39cd698..0f61786 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -309,8 +309,10 @@  void *colo_process_incoming_checkpoints(void *opaque)
     struct colo_incoming *colo_in = opaque;
     QEMUFile *f = colo_in->file;
     int fd = qemu_get_fd(f);
-    QEMUFile *ctl = NULL;
+    QEMUFile *ctl = NULL, *fb = NULL;
     int ret;
+    uint64_t total_size;
+
     colo = qemu_coroutine_self();
     assert(colo != NULL);
 
@@ -325,10 +327,17 @@  void *colo_process_incoming_checkpoints(void *opaque)
         goto out;
     }
 
+    colo_buffer = qsb_create(NULL, COLO_BUFFER_BASE_SIZE);
+    if (colo_buffer == NULL) {
+        error_report("Failed to allocate colo buffer!");
+        goto out;
+    }
+
     ret = colo_ctl_put(ctl, COLO_CHECPOINT_READY);
     if (ret < 0) {
         goto out;
     }
+
     qemu_mutex_lock_iothread();
     /* in COLO mode, slave is runing, so start the vm */
     vm_start();
@@ -364,7 +373,18 @@  void *colo_process_incoming_checkpoints(void *opaque)
         }
         trace_colo_receive_message("COLO_CHECKPOINT_SEND");
 
-        /*TODO Load VM state */
+        /* read the VM state total size first */
+        ret = colo_ctl_get_value(f, &total_size);
+        if (ret < 0) {
+            goto out;
+        }
+
+        /* read vm device state into colo buffer */
+        ret = qsb_fill_buffer(colo_buffer, f, total_size);
+        if (ret != total_size) {
+            error_report("can't get all migration data");
+            goto out;
+        }
 
         ret = colo_ctl_put(ctl, COLO_CHECKPOINT_RECEIVED);
         if (ret < 0) {
@@ -372,6 +392,22 @@  void *colo_process_incoming_checkpoints(void *opaque)
         }
         trace_colo_receive_message("COLO_CHECKPOINT_RECEIVED");
 
+        /* open colo buffer for read */
+        fb = qemu_bufopen("r", colo_buffer);
+        if (!fb) {
+            error_report("can't open colo buffer for read");
+            goto out;
+        }
+
+        qemu_mutex_lock_iothread();
+        qemu_system_reset(VMRESET_SILENT);
+        if (qemu_loadvm_state(fb) < 0) {
+            error_report("COLO: loadvm failed");
+            qemu_mutex_unlock_iothread();
+            goto out;
+        }
+        qemu_mutex_unlock_iothread();
+
         /* TODO: flush vm state */
 
         ret = colo_ctl_put(ctl, COLO_CHECKPOINT_LOADED);
@@ -384,14 +420,25 @@  void *colo_process_incoming_checkpoints(void *opaque)
         vm_start();
         qemu_mutex_unlock_iothread();
         trace_colo_vm_state_change("stop", "start");
-}
+
+        qemu_fclose(fb);
+        fb = NULL;
+    }
 
 out:
     colo = NULL;
+
+    if (fb) {
+        qemu_fclose(fb);
+    }
+
     release_ram_cache();
     if (ctl) {
         qemu_fclose(ctl);
     }
+
+    qsb_free(colo_buffer);
+
     loadvm_exit_colo();
 
     return NULL;