[Qemu-devel] [PATCH v2 0/2] Make info migrate output consistent

[Qemu-devel] [PATCH v2 0/2] Make info migrate output consistent

[Juan Quintela]

Hi

Changes for v2:
- patches 1 & 2 in previous pull requset
- create function populate_disk_info (dave suggestion)

Please, review.

Thanks, Juan.

[v1]
This series make the output of info migrate be printed in a single
place for the states ACTIVE, CANCELLING and POSTCOPY_ACTIVE.

The code was duplicated, just use the same code.  Once done that, fix
the inconsistences that were not needed.


Juan Quintela (2):
  ram: Print block stats also in the complete case
  ram: Now POSTCOPY_ACTIVE is the same that STATUS_ACTIVE

 migration/migration.c | 43 +++++++++++++------------------------------
 1 file changed, 13 insertions(+), 30 deletions(-)

-- 
2.9.4

[Qemu-devel] [PATCH v2 1/2] ram: Print block stats also in the complete case

[Juan Quintela]

Once there, create populate_disk_info.

Signed-off-by: Juan Quintela <quintela@redhat.com>

--

- create populate_disk_info instead of "abusing" populate_ram_info
---
 migration/migration.c | 31 +++++++++++++------------------
 1 file changed, 13 insertions(+), 18 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 4dbc4f2..cfa266b 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -515,6 +515,17 @@ static void populate_ram_info(MigrationInfo *info, MigrationState *s)
     }
 }
 
+static void populate_disk_info(MigrationInfo *info)
+{
+    if (blk_mig_active()) {
+        info->has_disk = true;
+        info->disk = g_malloc0(sizeof(*info->disk));
+        info->disk->transferred = blk_mig_bytes_transferred();
+        info->disk->remaining = blk_mig_bytes_remaining();
+        info->disk->total = blk_mig_bytes_total();
+    }
+}
+
 MigrationInfo *qmp_query_migrate(Error **errp)
 {
     MigrationInfo *info = g_malloc0(sizeof(*info));
@@ -540,15 +551,7 @@ MigrationInfo *qmp_query_migrate(Error **errp)
         info->setup_time = s->setup_time;
 
         populate_ram_info(info, s);
-
-        if (blk_mig_active()) {
-            info->has_disk = true;
-            info->disk = g_malloc0(sizeof(*info->disk));
-            info->disk->transferred = blk_mig_bytes_transferred();
-            info->disk->remaining = blk_mig_bytes_remaining();
-            info->disk->total = blk_mig_bytes_total();
-        }
-
+        populate_disk_info(info);
         break;
     case MIGRATION_STATUS_POSTCOPY_ACTIVE:
         /* Mostly the same as active; TODO add some postcopy stats */
@@ -562,15 +565,7 @@ MigrationInfo *qmp_query_migrate(Error **errp)
         info->setup_time = s->setup_time;
 
         populate_ram_info(info, s);
-
-        if (blk_mig_active()) {
-            info->has_disk = true;
-            info->disk = g_malloc0(sizeof(*info->disk));
-            info->disk->transferred = blk_mig_bytes_transferred();
-            info->disk->remaining = blk_mig_bytes_remaining();
-            info->disk->total = blk_mig_bytes_total();
-        }
-
+        populate_disk_info(info);
         break;
     case MIGRATION_STATUS_COLO:
         info->has_status = true;
-- 
2.9.4

Re: [Qemu-devel] [PATCH v2 1/2] ram: Print block stats also in the complete case

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> Once there, create populate_disk_info.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>

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

> 
> --
> 
> - create populate_disk_info instead of "abusing" populate_ram_info
> ---
>  migration/migration.c | 31 +++++++++++++------------------
>  1 file changed, 13 insertions(+), 18 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 4dbc4f2..cfa266b 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -515,6 +515,17 @@ static void populate_ram_info(MigrationInfo *info, MigrationState *s)
>      }
>  }
>  
> +static void populate_disk_info(MigrationInfo *info)
> +{
> +    if (blk_mig_active()) {
> +        info->has_disk = true;
> +        info->disk = g_malloc0(sizeof(*info->disk));
> +        info->disk->transferred = blk_mig_bytes_transferred();
> +        info->disk->remaining = blk_mig_bytes_remaining();
> +        info->disk->total = blk_mig_bytes_total();
> +    }
> +}
> +
>  MigrationInfo *qmp_query_migrate(Error **errp)
>  {
>      MigrationInfo *info = g_malloc0(sizeof(*info));
> @@ -540,15 +551,7 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>          info->setup_time = s->setup_time;
>  
>          populate_ram_info(info, s);
> -
> -        if (blk_mig_active()) {
> -            info->has_disk = true;
> -            info->disk = g_malloc0(sizeof(*info->disk));
> -            info->disk->transferred = blk_mig_bytes_transferred();
> -            info->disk->remaining = blk_mig_bytes_remaining();
> -            info->disk->total = blk_mig_bytes_total();
> -        }
> -
> +        populate_disk_info(info);
>          break;
>      case MIGRATION_STATUS_POSTCOPY_ACTIVE:
>          /* Mostly the same as active; TODO add some postcopy stats */
> @@ -562,15 +565,7 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>          info->setup_time = s->setup_time;
>  
>          populate_ram_info(info, s);
> -
> -        if (blk_mig_active()) {
> -            info->has_disk = true;
> -            info->disk = g_malloc0(sizeof(*info->disk));
> -            info->disk->transferred = blk_mig_bytes_transferred();
> -            info->disk->remaining = blk_mig_bytes_remaining();
> -            info->disk->total = blk_mig_bytes_total();
> -        }
> -
> +        populate_disk_info(info);
>          break;
>      case MIGRATION_STATUS_COLO:
>          info->has_status = true;
> -- 
> 2.9.4
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 1/2] ram: Print block stats also in the complete case

[Laurent Vivier]

On 07/06/2017 12:58, Juan Quintela wrote:
> Once there, create populate_disk_info.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

> 
> --
> 
> - create populate_disk_info instead of "abusing" populate_ram_info
> ---
>  migration/migration.c | 31 +++++++++++++------------------
>  1 file changed, 13 insertions(+), 18 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 4dbc4f2..cfa266b 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -515,6 +515,17 @@ static void populate_ram_info(MigrationInfo *info, MigrationState *s)
>      }
>  }
>  
> +static void populate_disk_info(MigrationInfo *info)
> +{
> +    if (blk_mig_active()) {
> +        info->has_disk = true;
> +        info->disk = g_malloc0(sizeof(*info->disk));
> +        info->disk->transferred = blk_mig_bytes_transferred();
> +        info->disk->remaining = blk_mig_bytes_remaining();
> +        info->disk->total = blk_mig_bytes_total();
> +    }
> +}
> +
>  MigrationInfo *qmp_query_migrate(Error **errp)
>  {
>      MigrationInfo *info = g_malloc0(sizeof(*info));
> @@ -540,15 +551,7 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>          info->setup_time = s->setup_time;
>  
>          populate_ram_info(info, s);
> -
> -        if (blk_mig_active()) {
> -            info->has_disk = true;
> -            info->disk = g_malloc0(sizeof(*info->disk));
> -            info->disk->transferred = blk_mig_bytes_transferred();
> -            info->disk->remaining = blk_mig_bytes_remaining();
> -            info->disk->total = blk_mig_bytes_total();
> -        }
> -
> +        populate_disk_info(info);
>          break;
>      case MIGRATION_STATUS_POSTCOPY_ACTIVE:
>          /* Mostly the same as active; TODO add some postcopy stats */
> @@ -562,15 +565,7 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>          info->setup_time = s->setup_time;
>  
>          populate_ram_info(info, s);
> -
> -        if (blk_mig_active()) {
> -            info->has_disk = true;
> -            info->disk = g_malloc0(sizeof(*info->disk));
> -            info->disk->transferred = blk_mig_bytes_transferred();
> -            info->disk->remaining = blk_mig_bytes_remaining();
> -            info->disk->total = blk_mig_bytes_total();
> -        }
> -
> +        populate_disk_info(info);
>          break;
>      case MIGRATION_STATUS_COLO:
>          info->has_status = true;
> 

[Qemu-devel] [PATCH v2 2/2] ram: Now POSTCOPY_ACTIVE is the same that STATUS_ACTIVE

[Juan Quintela]

Merge them.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 migration/migration.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index cfa266b..b31a195 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -541,20 +541,8 @@ MigrationInfo *qmp_query_migrate(Error **errp)
         break;
     case MIGRATION_STATUS_ACTIVE:
     case MIGRATION_STATUS_CANCELLING:
-        info->has_status = true;
-        info->has_total_time = true;
-        info->total_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME)
-            - s->total_time;
-        info->has_expected_downtime = true;
-        info->expected_downtime = s->expected_downtime;
-        info->has_setup_time = true;
-        info->setup_time = s->setup_time;
-
-        populate_ram_info(info, s);
-        populate_disk_info(info);
-        break;
     case MIGRATION_STATUS_POSTCOPY_ACTIVE:
-        /* Mostly the same as active; TODO add some postcopy stats */
+         /* TODO add some postcopy stats */
         info->has_status = true;
         info->has_total_time = true;
         info->total_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME)
-- 
2.9.4

Re: [Qemu-devel] [PATCH v2 2/2] ram: Now POSTCOPY_ACTIVE is the same that STATUS_ACTIVE

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> Merge them.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>

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

> ---
>  migration/migration.c | 14 +-------------
>  1 file changed, 1 insertion(+), 13 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index cfa266b..b31a195 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -541,20 +541,8 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>          break;
>      case MIGRATION_STATUS_ACTIVE:
>      case MIGRATION_STATUS_CANCELLING:
> -        info->has_status = true;
> -        info->has_total_time = true;
> -        info->total_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME)
> -            - s->total_time;
> -        info->has_expected_downtime = true;
> -        info->expected_downtime = s->expected_downtime;
> -        info->has_setup_time = true;
> -        info->setup_time = s->setup_time;
> -
> -        populate_ram_info(info, s);
> -        populate_disk_info(info);
> -        break;
>      case MIGRATION_STATUS_POSTCOPY_ACTIVE:
> -        /* Mostly the same as active; TODO add some postcopy stats */
> +         /* TODO add some postcopy stats */
>          info->has_status = true;
>          info->has_total_time = true;
>          info->total_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME)
> -- 
> 2.9.4
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 2/2] ram: Now POSTCOPY_ACTIVE is the same that STATUS_ACTIVE

[Laurent Vivier]

On 07/06/2017 12:58, Juan Quintela wrote:
> Merge them.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

> ---
>  migration/migration.c | 14 +-------------
>  1 file changed, 1 insertion(+), 13 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index cfa266b..b31a195 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -541,20 +541,8 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>          break;
>      case MIGRATION_STATUS_ACTIVE:
>      case MIGRATION_STATUS_CANCELLING:
> -        info->has_status = true;
> -        info->has_total_time = true;
> -        info->total_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME)
> -            - s->total_time;
> -        info->has_expected_downtime = true;
> -        info->expected_downtime = s->expected_downtime;
> -        info->has_setup_time = true;
> -        info->setup_time = s->setup_time;
> -
> -        populate_ram_info(info, s);
> -        populate_disk_info(info);
> -        break;
>      case MIGRATION_STATUS_POSTCOPY_ACTIVE:
> -        /* Mostly the same as active; TODO add some postcopy stats */
> +         /* TODO add some postcopy stats */
>          info->has_status = true;
>          info->has_total_time = true;
>          info->total_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME)
> 

Re: [Qemu-devel] [PATCH v2 0/2] Make info migrate output consistent

[Peter Xu]

On Wed, Jun 07, 2017 at 12:58:08PM +0200, Juan Quintela wrote:
> Hi
> 
> Changes for v2:
> - patches 1 & 2 in previous pull requset
> - create function populate_disk_info (dave suggestion)
> 
> Please, review.
> 
> Thanks, Juan.
> 
> [v1]
> This series make the output of info migrate be printed in a single
> place for the states ACTIVE, CANCELLING and POSTCOPY_ACTIVE.
> 
> The code was duplicated, just use the same code.  Once done that, fix
> the inconsistences that were not needed.

I feel that putting cpu throttling info into populate_ram_info() is
still slightly weird, but hmm it's okay and that's trivial.

Reviewed-by: Peter Xu <peterx@redhat.com>

Thanks,

-- 
Peter Xu