Re: [Qemu-devel] [PATCH 3/4] migration: set bytes_xfer_* outside of autoconverge logic

[Juan Quintela]

Felipe Franciosi  wrote:
> The bytes_xfer_now/prev counters are only used by the auto convergence
> logic. However, they are used alongside the dirty_pages_rate counter,
> which is calculated (and required) outside of this logic. The problem
> with this approach is that if the auto convergence capability is changed
> while a migration is ongoing, the relationship of the counters will be
> broken.
>
> This moves the management of bytes_xfer_now/prev counters outside of the
> auto convergence logic to address this issue.
>
> Signed-off-by: Felipe Franciosi 

Reviewed-by: Juan Quintela 

Re: [Qemu-devel] [PATCH 3/4] migration: set bytes_xfer_* outside of autoconverge logic

[Peter Xu]

On Wed, May 24, 2017 at 05:10:02PM +0100, Felipe Franciosi wrote:
> The bytes_xfer_now/prev counters are only used by the auto convergence
> logic. However, they are used alongside the dirty_pages_rate counter,
> which is calculated (and required) outside of this logic. The problem
> with this approach is that if the auto convergence capability is changed
> while a migration is ongoing, the relationship of the counters will be
> broken.
> 
> This moves the management of bytes_xfer_now/prev counters outside of the
> auto convergence logic to address this issue.
> 
> Signed-off-by: Felipe Franciosi 

Reviewed-by: Peter Xu 

> ---
>  migration/ram.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 495ecbe..1a3d9e6 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -697,6 +697,7 @@ static void migration_bitmap_sync(RAMState *rs)
>  /* calculate period counters */
>  rs->dirty_pages_rate = rs->num_dirty_pages_period * 1000
>  / (end_time - rs->time_last_bitmap_sync);
> +bytes_xfer_now = ram_bytes_transferred();
>  
>  if (migrate_auto_converge()) {
>  /* The following detection logic can be refined later. For now:
> @@ -704,7 +705,6 @@ static void migration_bitmap_sync(RAMState *rs)
> amount of bytes that just got transferred since the last time 
> we
> were in this routine. If that happens twice, start or increase
> throttling */
> -bytes_xfer_now = ram_bytes_transferred();
>  
>  if ((rs->num_dirty_pages_period * TARGET_PAGE_SIZE >
> (bytes_xfer_now - rs->bytes_xfer_prev) / 2) &&
> @@ -713,7 +713,6 @@ static void migration_bitmap_sync(RAMState *rs)
>  rs->dirty_rate_high_cnt = 0;
>  mig_throttle_guest_down();
>  }
> -rs->bytes_xfer_prev = bytes_xfer_now;
>  }
>  
>  if (migrate_use_xbzrle()) {
> @@ -730,6 +729,7 @@ static void migration_bitmap_sync(RAMState *rs)
>  /* reset period counters */
>  rs->time_last_bitmap_sync = end_time;
>  rs->num_dirty_pages_period = 0;
> +rs->bytes_xfer_prev = bytes_xfer_now;
>  }
>  if (migrate_use_events()) {
>  qapi_event_send_migration_pass(rs->bitmap_sync_count, NULL);
> -- 
> 1.9.5
> 

-- 
Peter Xu

[Qemu-devel] [PATCH 3/4] migration: set bytes_xfer_* outside of autoconverge logic

[Felipe Franciosi]

The bytes_xfer_now/prev counters are only used by the auto convergence
logic. However, they are used alongside the dirty_pages_rate counter,
which is calculated (and required) outside of this logic. The problem
with this approach is that if the auto convergence capability is changed
while a migration is ongoing, the relationship of the counters will be
broken.

This moves the management of bytes_xfer_now/prev counters outside of the
auto convergence logic to address this issue.

Signed-off-by: Felipe Franciosi 
---
 migration/ram.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 495ecbe..1a3d9e6 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -697,6 +697,7 @@ static void migration_bitmap_sync(RAMState *rs)
 /* calculate period counters */
 rs->dirty_pages_rate = rs->num_dirty_pages_period * 1000
 / (end_time - rs->time_last_bitmap_sync);
+bytes_xfer_now = ram_bytes_transferred();
 
 if (migrate_auto_converge()) {
 /* The following detection logic can be refined later. For now:
@@ -704,7 +705,6 @@ static void migration_bitmap_sync(RAMState *rs)
amount of bytes that just got transferred since the last time we
were in this routine. If that happens twice, start or increase
throttling */
-bytes_xfer_now = ram_bytes_transferred();
 
 if ((rs->num_dirty_pages_period * TARGET_PAGE_SIZE >
(bytes_xfer_now - rs->bytes_xfer_prev) / 2) &&
@@ -713,7 +713,6 @@ static void migration_bitmap_sync(RAMState *rs)
 rs->dirty_rate_high_cnt = 0;
 mig_throttle_guest_down();
 }
-rs->bytes_xfer_prev = bytes_xfer_now;
 }
 
 if (migrate_use_xbzrle()) {
@@ -730,6 +729,7 @@ static void migration_bitmap_sync(RAMState *rs)
 /* reset period counters */
 rs->time_last_bitmap_sync = end_time;
 rs->num_dirty_pages_period = 0;
+rs->bytes_xfer_prev = bytes_xfer_now;
 }
 if (migrate_use_events()) {
 qapi_event_send_migration_pass(rs->bitmap_sync_count, NULL);
-- 
1.9.5