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  
Sent: Tuesday, September 22, 2020 2:58 PM
To: Rao, Lei ; Zhang, Chen ; 
jasow...@redhat.com; quint...@redhat.com; dgilb...@redhat.com; 
pbonz...@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 
> ---
>   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

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