[PATCH 0/3] Optimized some code for COLO

[PATCH 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    | 12 ++++++++++++
 net/colo-compare.c |  6 +-----
 softmmu/vl.c       |  1 +
 3 files changed, 14 insertions(+), 5 deletions(-)

--
1.8.3.1

[PATCH 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 1/3] Optimize seq_sorter function for colo-compare

[Li Zhijian]

On 9/19/20 11:10 AM, leirao wrote:
> 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: Li Zhijian <lizhijian@cn.fujitsu.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)

[PATCH 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 | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/migration/ram.c b/migration/ram.c
index 76d4fee..6a2b6c1 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3019,6 +3019,17 @@ static void decompress_data_with_multi_threads(QEMUFile *f,
 }
 
 /*
+ * 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_set_ram_state(RAMState *rsp)
+{
+    rsp->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
  * to call this helper.
@@ -3062,6 +3073,7 @@ int colo_init_ram_cache(void)
     }
 
     ram_state_init(&ram_state);
+    colo_set_ram_state(ram_state);
     return 0;
 }
 
-- 
1.8.3.1

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

[Li Zhijian]

On 9/19/20 11:10 AM, 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 | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index 76d4fee..6a2b6c1 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -3019,6 +3019,17 @@ static void decompress_data_with_multi_threads(QEMUFile *f,
>   }
>   
>   /*
> + * 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_set_ram_state(RAMState *rsp)
this function name is too general, how about

colo_init_ram_state(ram_state)
{
     ram_state_init(&ram_state);
     ram_state->ram_bulk_stage = false;
}

Thanks
Zhijian

> +{
> +    rsp->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
>    * to call this helper.
> @@ -3062,6 +3073,7 @@ int colo_init_ram_cache(void)
>       }
>   
>       ram_state_init(&ram_state);
> +    colo_set_ram_state(ram_state);
>       return 0;
>   }
>   

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

[Rao, Lei]

Got it.
It looks more reasonable. Will be changed in V2.

Thanks,
Lei.

-----Original Message-----
From: Li Zhijian <lizhijian@cn.fujitsu.com> 
Sent: Tuesday, September 22, 2020 2:58 PM
To: Rao, Lei <lei.rao@intel.com>; Zhang, Chen <chen.zhang@intel.com>; jasowang@redhat.com; quintela@redhat.com; dgilbert@redhat.com; pbonzini@redhat.com
Cc: qemu-devel@nongnu.org
Subject: Re: [PATCH 2/3] Reduce the time of checkpoint for COLO



On 9/19/20 11:10 AM, 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 | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
>
> diff --git a/migration/ram.c b/migration/ram.c index 76d4fee..6a2b6c1 
> 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -3019,6 +3019,17 @@ static void decompress_data_with_multi_threads(QEMUFile *f,
>   }
>   
>   /*
> + * 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_set_ram_state(RAMState *rsp)
this function name is too general, how about

colo_init_ram_state(ram_state)
{
     ram_state_init(&ram_state);
     ram_state->ram_bulk_stage = false;
}

Thanks
Zhijian

> +{
> +    rsp->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
>    * to call this helper.
> @@ -3062,6 +3073,7 @@ int colo_init_ram_cache(void)
>       }
>   
>       ram_state_init(&ram_state);
> +    colo_set_ram_state(ram_state);
>       return 0;
>   }
>   

[PATCH 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