[PATCH v2 0/3] Optimized some code for COLO

[PATCH v2 0/3] Optimized some code for COLO

[leirao]

Optimized some code for COLO and fixed a qemu crash when guest
shutdown in COLO mode.

leirao (3):
  Optimize seq_sorter function for colo-compare
  Reduce the time of checkpoint for COLO
  Fix the qemu crash when guest shutdown in COLO mode

 migration/ram.c    | 14 +++++++++++++-
 net/colo-compare.c |  6 +-----
 softmmu/vl.c       |  1 +
 3 files changed, 15 insertions(+), 6 deletions(-)

--
1.8.3.1

[PATCH v2 1/3] Optimize seq_sorter function for colo-compare

[leirao]

The seq of tcp has been filled in fill_pkt_tcp_info, it
can be used directly here.

Signed-off-by: leirao <lei.rao@intel.com>
---
 net/colo-compare.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 3a45d64..86980ce 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -196,11 +196,7 @@ static void colo_compare_inconsistency_notify(CompareState *s)
 
 static gint seq_sorter(Packet *a, Packet *b, gpointer data)
 {
-    struct tcp_hdr *atcp, *btcp;
-
-    atcp = (struct tcp_hdr *)(a->transport_header);
-    btcp = (struct tcp_hdr *)(b->transport_header);
-    return ntohl(atcp->th_seq) - ntohl(btcp->th_seq);
+    return a->tcp_seq - b->tcp_seq;
 }
 
 static void fill_pkt_tcp_info(void *data, uint32_t *max_ack)
-- 
1.8.3.1

RE: [PATCH v2 1/3] Optimize seq_sorter function for colo-compare

[Zhang, Chen]

> -----Original Message-----
> From: Rao, Lei <lei.rao@intel.com>
> Sent: Tuesday, September 22, 2020 5:25 PM
> To: Zhang, Chen <chen.zhang@intel.com>; lizhijian@cn.fujitsu.com;
> jasowang@redhat.com; quintela@redhat.com; dgilbert@redhat.com;
> pbonzini@redhat.com
> Cc: qemu-devel@nongnu.org; Rao, Lei <lei.rao@intel.com>
> Subject: [PATCH v2 1/3] Optimize seq_sorter function for colo-compare
> 
> The seq of tcp has been filled in fill_pkt_tcp_info, it can be used directly here.
> 
> Signed-off-by: leirao <lei.rao@intel.com>

Reviewed-by: Zhang Chen <chen.zhang@intel.com>
By the way, please add Zhijian's reviewed-by in new version of patches.

Thanks
Zhang Chen

> ---
>  net/colo-compare.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/net/colo-compare.c b/net/colo-compare.c index
> 3a45d64..86980ce 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -196,11 +196,7 @@ static void
> colo_compare_inconsistency_notify(CompareState *s)
> 
>  static gint seq_sorter(Packet *a, Packet *b, gpointer data)  {
> -    struct tcp_hdr *atcp, *btcp;
> -
> -    atcp = (struct tcp_hdr *)(a->transport_header);
> -    btcp = (struct tcp_hdr *)(b->transport_header);
> -    return ntohl(atcp->th_seq) - ntohl(btcp->th_seq);
> +    return a->tcp_seq - b->tcp_seq;
>  }
> 
>  static void fill_pkt_tcp_info(void *data, uint32_t *max_ack)
> --
> 1.8.3.1

[PATCH v2 2/3] Reduce the time of checkpoint for COLO

[leirao]

we should set ram_bulk_stage to false after ram_state_init,
otherwise the bitmap will be unused in migration_bitmap_find_dirty.
all pages in ram cache will be flushed to the ram of secondary guest
for each checkpoint.

Signed-off-by: leirao <lei.rao@intel.com>
---
 migration/ram.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/migration/ram.c b/migration/ram.c
index 76d4fee..59ff0cf 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3018,6 +3018,18 @@ static void decompress_data_with_multi_threads(QEMUFile *f,
     qemu_mutex_unlock(&decomp_done_lock);
 }
 
+ /*
+  * we must set ram_bulk_stage to fasle, otherwise in
+  * migation_bitmap_find_dirty the bitmap will be unused and
+  * all the pages in ram cache wil be flushed to the ram of
+  * secondary VM.
+  */
+static void colo_init_ram_state(void)
+{
+    ram_state_init(&ram_state);
+    ram_state->ram_bulk_stage = false;
+}
+
 /*
  * colo cache: this is for secondary VM, we cache the whole
  * memory of the secondary VM, it is need to hold the global lock
@@ -3061,7 +3073,7 @@ int colo_init_ram_cache(void)
         }
     }
 
-    ram_state_init(&ram_state);
+    colo_init_ram_state();
     return 0;
 }
 
-- 
1.8.3.1

RE: [PATCH v2 2/3] Reduce the time of checkpoint for COLO

[Zhang, Chen]

> -----Original Message-----
> From: Rao, Lei <lei.rao@intel.com>
> Sent: Tuesday, September 22, 2020 5:25 PM
> To: Zhang, Chen <chen.zhang@intel.com>; lizhijian@cn.fujitsu.com;
> jasowang@redhat.com; quintela@redhat.com; dgilbert@redhat.com;
> pbonzini@redhat.com
> Cc: qemu-devel@nongnu.org; Rao, Lei <lei.rao@intel.com>
> Subject: [PATCH v2 2/3] Reduce the time of checkpoint for COLO
> 
> we should set ram_bulk_stage to false after ram_state_init, otherwise the
> bitmap will be unused in migration_bitmap_find_dirty.
> all pages in ram cache will be flushed to the ram of secondary guest for each
> checkpoint.
> 
> Signed-off-by: leirao <lei.rao@intel.com>

Reviewed-by: Zhang Chen <chen.zhang@intel.com>

Thanks
Zhang Chen

> ---
>  migration/ram.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c index 76d4fee..59ff0cf 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -3018,6 +3018,18 @@ static void
> decompress_data_with_multi_threads(QEMUFile *f,
>      qemu_mutex_unlock(&decomp_done_lock);
>  }
> 
> + /*
> +  * we must set ram_bulk_stage to fasle, otherwise in
> +  * migation_bitmap_find_dirty the bitmap will be unused and
> +  * all the pages in ram cache wil be flushed to the ram of
> +  * secondary VM.
> +  */
> +static void colo_init_ram_state(void)
> +{
> +    ram_state_init(&ram_state);
> +    ram_state->ram_bulk_stage = false;
> +}
> +
>  /*
>   * colo cache: this is for secondary VM, we cache the whole
>   * memory of the secondary VM, it is need to hold the global lock @@ -
> 3061,7 +3073,7 @@ int colo_init_ram_cache(void)
>          }
>      }
> 
> -    ram_state_init(&ram_state);
> +    colo_init_ram_state();
>      return 0;
>  }
> 
> --
> 1.8.3.1

Re: [PATCH v2 2/3] Reduce the time of checkpoint for COLO

[Li Zhijian]

On 9/22/20 5:24 PM, leirao wrote:
> we should set ram_bulk_stage to false after ram_state_init,
> otherwise the bitmap will be unused in migration_bitmap_find_dirty.
> all pages in ram cache will be flushed to the ram of secondary guest
> for each checkpoint.
>
> Signed-off-by: leirao <lei.rao@intel.com>
> ---
>   migration/ram.c | 14 +++++++++++++-
>   1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index 76d4fee..59ff0cf 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -3018,6 +3018,18 @@ static void decompress_data_with_multi_threads(QEMUFile *f,
>       qemu_mutex_unlock(&decomp_done_lock);
>   }
>   
> + /*
> +  * we must set ram_bulk_stage to fasle, otherwise in
a typo: s/fasle/false

Reviewed-by: Li Zhijian <lizhijian@cn.fujitsu.com>


> +  * migation_bitmap_find_dirty the bitmap will be unused and
> +  * all the pages in ram cache wil be flushed to the ram of
> +  * secondary VM.
> +  */
> +static void colo_init_ram_state(void)
> +{
> +    ram_state_init(&ram_state);
> +    ram_state->ram_bulk_stage = false;
> +}
> +
>   /*
>    * colo cache: this is for secondary VM, we cache the whole
>    * memory of the secondary VM, it is need to hold the global lock
> @@ -3061,7 +3073,7 @@ int colo_init_ram_cache(void)
>           }
>       }
>   
> -    ram_state_init(&ram_state);
> +    colo_init_ram_state();
>       return 0;
>   }
>   

[PATCH v2 3/3] Fix the qemu crash when guest shutdown in COLO mode

[leirao]

In COLO mode, if the startup parameters of QEMU include "no-shutdown",
QEMU will crash when the guest shutdown. The root cause is when the
guest shutdown, the state of VM will switch COLO to SHUTDOWN. When do
checkpoint again, the state will be changed to COLO. But the state
switch is undefined in runstate_transitions_def, we should add it.
This patch fixes the following:
qemu-system-x86_64: invalid runstate transition: 'shutdown' -> 'colo'
Aborted

Signed-off-by: leirao <lei.rao@intel.com>
---
 softmmu/vl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/softmmu/vl.c b/softmmu/vl.c
index f7b1034..c21606c 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -631,6 +631,7 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_SHUTDOWN, RUN_STATE_PAUSED },
     { RUN_STATE_SHUTDOWN, RUN_STATE_FINISH_MIGRATE },
     { RUN_STATE_SHUTDOWN, RUN_STATE_PRELAUNCH },
+    { RUN_STATE_SHUTDOWN, RUN_STATE_COLO },
 
     { RUN_STATE_DEBUG, RUN_STATE_SUSPENDED },
     { RUN_STATE_RUNNING, RUN_STATE_SUSPENDED },
-- 
1.8.3.1

RE: [PATCH v2 3/3] Fix the qemu crash when guest shutdown in COLO mode

[Zhang, Chen]

> -----Original Message-----
> From: Rao, Lei <lei.rao@intel.com>
> Sent: Tuesday, September 22, 2020 5:25 PM
> To: Zhang, Chen <chen.zhang@intel.com>; lizhijian@cn.fujitsu.com;
> jasowang@redhat.com; quintela@redhat.com; dgilbert@redhat.com;
> pbonzini@redhat.com
> Cc: qemu-devel@nongnu.org; Rao, Lei <lei.rao@intel.com>
> Subject: [PATCH v2 3/3] Fix the qemu crash when guest shutdown in COLO
> mode
> 
> In COLO mode, if the startup parameters of QEMU include "no-shutdown",
> QEMU will crash when the guest shutdown. The root cause is when the guest
> shutdown, the state of VM will switch COLO to SHUTDOWN. When do
> checkpoint again, the state will be changed to COLO. But the state switch is
> undefined in runstate_transitions_def, we should add it.
> This patch fixes the following:
> qemu-system-x86_64: invalid runstate transition: 'shutdown' -> 'colo'
> Aborted
> 
> Signed-off-by: leirao <lei.rao@intel.com>

Reviewed-by: Zhang Chen <chen.zhang@intel.com>

Thanks
Zhang Chen

> ---
>  softmmu/vl.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/softmmu/vl.c b/softmmu/vl.c index f7b1034..c21606c 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -631,6 +631,7 @@ static const RunStateTransition
> runstate_transitions_def[] = {
>      { RUN_STATE_SHUTDOWN, RUN_STATE_PAUSED },
>      { RUN_STATE_SHUTDOWN, RUN_STATE_FINISH_MIGRATE },
>      { RUN_STATE_SHUTDOWN, RUN_STATE_PRELAUNCH },
> +    { RUN_STATE_SHUTDOWN, RUN_STATE_COLO },
> 
>      { RUN_STATE_DEBUG, RUN_STATE_SUSPENDED },
>      { RUN_STATE_RUNNING, RUN_STATE_SUSPENDED },
> --
> 1.8.3.1