| Message ID | 1345288635-1369-4-git-send-email-quintela@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 08/18/2012 05:17 AM, Juan Quintela wrote: > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > hmp.c | 4 ++-- > migration.c | 6 +++--- > qapi-schema.json | 14 +++++++------- > qmp-commands.hx | 6 +++--- > 4 files changed, 15 insertions(+), 15 deletions(-) > > +++ b/qapi-schema.json > @@ -290,10 +290,6 @@ > # > # @total: total amount of bytes involved in the migration process > # > -# @total-time: total amount of ms since migration started. If > -# migration has ended, it returns the total migration > -# time. (since 1.2) > -# > # @duplicate: number of duplicate pages (since 1.2) > # > # @normal : number of normal pages (since 1.2) > @@ -304,8 +300,7 @@ > ## > { 'type': 'MigrationStats', > 'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' , > - 'total-time': 'int', 'duplicate': 'int', 'normal': 'int', > - 'normal-bytes': 'int' } } > + 'duplicate': 'int', 'normal': 'int', 'normal-bytes': 'int' } } > > ## > # @XBZRLECacheStats > @@ -350,12 +345,17 @@ > # migration statistics, only returned if XBZRLE feature is on and > # status is 'active' or 'completed' (since 1.2) > # > +# @total-time: total amount of milliseconds since migration started. > +# If migration has ended, it returns the total migration > +# time. (since 1.2) > +# > # Since: 0.14.0 > ## > { 'type': 'MigrationInfo', > 'data': {'*status': 'str', '*ram': 'MigrationStats', > '*disk': 'MigrationStats', > - '*xbzrle-cache': 'XBZRLECacheStats'} } > + '*xbzrle-cache': 'XBZRLECacheStats', > + 'total-time': 'int'} } Anthony - are you planning on taking this series for 1.2? If we don't get this patch in on time, then taking this for 1.3 would result in changing released QMP interface (right now, there has been no release with the field in the wrong type).
On 08/18/2012 02:17 PM, Juan Quintela wrote: > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > hmp.c | 4 ++-- > migration.c | 6 +++--- > qapi-schema.json | 14 +++++++------- > qmp-commands.hx | 6 +++--- > 4 files changed, 15 insertions(+), 15 deletions(-) > > diff --git a/hmp.c b/hmp.c > index a9d5675..81c8acb 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -149,6 +149,8 @@ void hmp_info_migrate(Monitor *mon) > > if (info->has_status) { > monitor_printf(mon, "Migration status: %s\n", info->status); > + monitor_printf(mon, "total time: %" PRIu64 " milliseconds\n", > + info->total_time); > } > > if (info->has_ram) { > @@ -158,8 +160,6 @@ void hmp_info_migrate(Monitor *mon) > info->ram->remaining >> 10); > monitor_printf(mon, "total ram: %" PRIu64 " kbytes\n", > info->ram->total >> 10); > - monitor_printf(mon, "total time: %" PRIu64 " milliseconds\n", > - info->ram->total_time); > monitor_printf(mon, "duplicate: %" PRIu64 " pages\n", > info->ram->duplicate); > monitor_printf(mon, "normal: %" PRIu64 " pages\n", > diff --git a/migration.c b/migration.c > index 653a3c1..8e4c508 100644 > --- a/migration.c > +++ b/migration.c > @@ -166,14 +166,14 @@ MigrationInfo *qmp_query_migrate(Error **errp) > case MIG_STATE_ACTIVE: > info->has_status = true; > info->status = g_strdup("active"); > + info->total_time = qemu_get_clock_ms(rt_clock) > + - s->total_time; > > info->has_ram = true; > info->ram = g_malloc0(sizeof(*info->ram)); > info->ram->transferred = ram_bytes_transferred(); > info->ram->remaining = ram_bytes_remaining(); > info->ram->total = ram_bytes_total(); > - info->ram->total_time = qemu_get_clock_ms(rt_clock) > - - s->total_time; > info->ram->duplicate = dup_mig_pages_transferred(); > info->ram->normal = norm_mig_pages_transferred(); > info->ram->normal_bytes = norm_mig_bytes_transferred(); > @@ -193,13 +193,13 @@ MigrationInfo *qmp_query_migrate(Error **errp) > > info->has_status = true; > info->status = g_strdup("completed"); > + info->total_time = s->total_time; > > info->has_ram = true; > info->ram = g_malloc0(sizeof(*info->ram)); > info->ram->transferred = ram_bytes_transferred(); > info->ram->remaining = 0; > info->ram->total = ram_bytes_total(); > - info->ram->total_time = s->total_time; > info->ram->duplicate = dup_mig_pages_transferred(); > info->ram->normal = norm_mig_pages_transferred(); > info->ram->normal_bytes = norm_mig_bytes_transferred(); > diff --git a/qapi-schema.json b/qapi-schema.json > index 3d2b2d1..f83cf22 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -290,10 +290,6 @@ > # > # @total: total amount of bytes involved in the migration process > # > -# @total-time: total amount of ms since migration started. If > -# migration has ended, it returns the total migration > -# time. (since 1.2) > -# > # @duplicate: number of duplicate pages (since 1.2) > # > # @normal : number of normal pages (since 1.2) > @@ -304,8 +300,7 @@ > ## > { 'type': 'MigrationStats', > 'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' , > - 'total-time': 'int', 'duplicate': 'int', 'normal': 'int', > - 'normal-bytes': 'int' } } > + 'duplicate': 'int', 'normal': 'int', 'normal-bytes': 'int' } } > > ## > # @XBZRLECacheStats > @@ -350,12 +345,17 @@ > # migration statistics, only returned if XBZRLE feature is on and > # status is 'active' or 'completed' (since 1.2) > # > +# @total-time: total amount of milliseconds since migration started. > +# If migration has ended, it returns the total migration > +# time. (since 1.2) > +# > # Since: 0.14.0 > ## > { 'type': 'MigrationInfo', > 'data': {'*status': 'str', '*ram': 'MigrationStats', > '*disk': 'MigrationStats', > - '*xbzrle-cache': 'XBZRLECacheStats'} } > + '*xbzrle-cache': 'XBZRLECacheStats', > + 'total-time': 'int'} } > > ## > # @query-migrate > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 2ce4ce6..8671bf3 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -2239,14 +2239,14 @@ The main json-object contains the following: > > - "status": migration status (json-string) > - Possible values: "active", "completed", "failed", "cancelled" > +- "total-time": total amount of ms since migration started. If > + migration has ended, it returns the total migration > + time (json-int) > - "ram": only present if "status" is "active", it is a json-object with the > following RAM information (in bytes): > - "transferred": amount transferred (json-int) > - "remaining": amount remaining (json-int) > - "total": total (json-int) > - - "total-time": total amount of ms since migration started. If > - migration has ended, it returns the total migration time > - (json-int) > - "duplicate": number of duplicated pages (json-int) > - "normal" : number of normal pages transferred (json-int) > - "normal-bytes" : number of normal bytes transferred (json-int) > Reviewed-by: Orit Wasserman <owasserm@redhat.com>
On Sat, 18 Aug 2012 07:02:50 -0600 Eric Blake <eblake@redhat.com> wrote: > On 08/18/2012 05:17 AM, Juan Quintela wrote: > > Signed-off-by: Juan Quintela <quintela@redhat.com> > > --- > > hmp.c | 4 ++-- > > migration.c | 6 +++--- > > qapi-schema.json | 14 +++++++------- > > qmp-commands.hx | 6 +++--- > > 4 files changed, 15 insertions(+), 15 deletions(-) > > > > > +++ b/qapi-schema.json > > @@ -290,10 +290,6 @@ > > # > > # @total: total amount of bytes involved in the migration process > > # > > -# @total-time: total amount of ms since migration started. If > > -# migration has ended, it returns the total migration > > -# time. (since 1.2) > > -# > > # @duplicate: number of duplicate pages (since 1.2) > > # > > # @normal : number of normal pages (since 1.2) > > @@ -304,8 +300,7 @@ > > ## > > { 'type': 'MigrationStats', > > 'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' , > > - 'total-time': 'int', 'duplicate': 'int', 'normal': 'int', > > - 'normal-bytes': 'int' } } > > + 'duplicate': 'int', 'normal': 'int', 'normal-bytes': 'int' } } > > > > ## > > # @XBZRLECacheStats > > @@ -350,12 +345,17 @@ > > # migration statistics, only returned if XBZRLE feature is on and > > # status is 'active' or 'completed' (since 1.2) > > # > > +# @total-time: total amount of milliseconds since migration started. > > +# If migration has ended, it returns the total migration > > +# time. (since 1.2) Field is optional, needs to be marked as such and the has_total_time field should be set appropriately. > > +# > > # Since: 0.14.0 > > ## > > { 'type': 'MigrationInfo', > > 'data': {'*status': 'str', '*ram': 'MigrationStats', > > '*disk': 'MigrationStats', > > - '*xbzrle-cache': 'XBZRLECacheStats'} } > > + '*xbzrle-cache': 'XBZRLECacheStats', > > + 'total-time': 'int'} } > > Anthony - are you planning on taking this series for 1.2? If we don't > get this patch in on time, then taking this for 1.3 would result in > changing released QMP interface (right now, there has been no release > with the field in the wrong type). I can cherry-pick this into the qmp branch. Juan, I can also fix myself the problem I pointed out above if that works for you.
Luiz Capitulino <lcapitulino@redhat.com> wrote: > On Sat, 18 Aug 2012 07:02:50 -0600 > Eric Blake <eblake@redhat.com> wrote: > > I can cherry-pick this into the qmp branch. > > Juan, I can also fix myself the problem I pointed out above if that > works for you. I very much preffer that. But what to do with the other ones? They are really simple, and it makes things better for users? Anthony? Later, Juan.
On Tue, 21 Aug 2012 17:00:20 +0200 Juan Quintela <quintela@redhat.com> wrote: > Luiz Capitulino <lcapitulino@redhat.com> wrote: > > On Sat, 18 Aug 2012 07:02:50 -0600 > > Eric Blake <eblake@redhat.com> wrote: > > > > I can cherry-pick this into the qmp branch. > > > > Juan, I can also fix myself the problem I pointed out above if that > > works for you. > > I very much preffer that. Done, and applied to the qmp branch for 1.2. > But what to do with the other ones? They look good to me, but I think it's too late for 1.2.
Eric Blake <eblake@redhat.com> writes: > On 08/18/2012 05:17 AM, Juan Quintela wrote: >> Signed-off-by: Juan Quintela <quintela@redhat.com> >> --- >> hmp.c | 4 ++-- >> migration.c | 6 +++--- >> qapi-schema.json | 14 +++++++------- >> qmp-commands.hx | 6 +++--- >> 4 files changed, 15 insertions(+), 15 deletions(-) >> > >> +++ b/qapi-schema.json >> @@ -290,10 +290,6 @@ >> # >> # @total: total amount of bytes involved in the migration process >> # >> -# @total-time: total amount of ms since migration started. If >> -# migration has ended, it returns the total migration >> -# time. (since 1.2) >> -# >> # @duplicate: number of duplicate pages (since 1.2) >> # >> # @normal : number of normal pages (since 1.2) >> @@ -304,8 +300,7 @@ >> ## >> { 'type': 'MigrationStats', >> 'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' , >> - 'total-time': 'int', 'duplicate': 'int', 'normal': 'int', >> - 'normal-bytes': 'int' } } >> + 'duplicate': 'int', 'normal': 'int', 'normal-bytes': 'int' } } >> >> ## >> # @XBZRLECacheStats >> @@ -350,12 +345,17 @@ >> # migration statistics, only returned if XBZRLE feature is on and >> # status is 'active' or 'completed' (since 1.2) >> # >> +# @total-time: total amount of milliseconds since migration started. >> +# If migration has ended, it returns the total migration >> +# time. (since 1.2) >> +# >> # Since: 0.14.0 >> ## >> { 'type': 'MigrationInfo', >> 'data': {'*status': 'str', '*ram': 'MigrationStats', >> '*disk': 'MigrationStats', >> - '*xbzrle-cache': 'XBZRLECacheStats'} } >> + '*xbzrle-cache': 'XBZRLECacheStats', >> + 'total-time': 'int'} } > > Anthony - are you planning on taking this series for 1.2? No. This is a new feature and we're past freeze. > If we don't > get this patch in on time, then taking this for 1.3 would result in > changing released QMP interface (right now, there has been no release > with the field in the wrong type). Ack. We need to preserve compat with the 1.2 interface. Regards, Anthony Liguori > > -- > Eric Blake eblake@redhat.com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org
On 08/22/2012 07:22 AM, Anthony Liguori wrote: Just restating things to make sure I'm clear... >>> { 'type': 'MigrationInfo', >>> 'data': {'*status': 'str', '*ram': 'MigrationStats', >>> '*disk': 'MigrationStats', >>> - '*xbzrle-cache': 'XBZRLECacheStats'} } >>> + '*xbzrle-cache': 'XBZRLECacheStats', >>> + 'total-time': 'int'} } >> >> Anthony - are you planning on taking this series for 1.2? > > No. This is a new feature and we're past freeze. No, the overall series (patches 1,2,4-8) is not appropriate at this time. > >> If we don't >> get this patch in on time, then taking this for 1.3 would result in >> changing released QMP interface (right now, there has been no release >> with the field in the wrong type). > > Ack. We need to preserve compat with the 1.2 interface. Yes, this particular patch 3 is a bug fix in order to prevent a future regression when 1.3 takes the rest of the series, and must therefore be part of the 1.2 release (and Luiz is on top of that, via the qmp branch).
diff --git a/hmp.c b/hmp.c index a9d5675..81c8acb 100644 --- a/hmp.c +++ b/hmp.c @@ -149,6 +149,8 @@ void hmp_info_migrate(Monitor *mon) if (info->has_status) { monitor_printf(mon, "Migration status: %s\n", info->status); + monitor_printf(mon, "total time: %" PRIu64 " milliseconds\n", + info->total_time); } if (info->has_ram) { @@ -158,8 +160,6 @@ void hmp_info_migrate(Monitor *mon) info->ram->remaining >> 10); monitor_printf(mon, "total ram: %" PRIu64 " kbytes\n", info->ram->total >> 10); - monitor_printf(mon, "total time: %" PRIu64 " milliseconds\n", - info->ram->total_time); monitor_printf(mon, "duplicate: %" PRIu64 " pages\n", info->ram->duplicate); monitor_printf(mon, "normal: %" PRIu64 " pages\n", diff --git a/migration.c b/migration.c index 653a3c1..8e4c508 100644 --- a/migration.c +++ b/migration.c @@ -166,14 +166,14 @@ MigrationInfo *qmp_query_migrate(Error **errp) case MIG_STATE_ACTIVE: info->has_status = true; info->status = g_strdup("active"); + info->total_time = qemu_get_clock_ms(rt_clock) + - s->total_time; info->has_ram = true; info->ram = g_malloc0(sizeof(*info->ram)); info->ram->transferred = ram_bytes_transferred(); info->ram->remaining = ram_bytes_remaining(); info->ram->total = ram_bytes_total(); - info->ram->total_time = qemu_get_clock_ms(rt_clock) - - s->total_time; info->ram->duplicate = dup_mig_pages_transferred(); info->ram->normal = norm_mig_pages_transferred(); info->ram->normal_bytes = norm_mig_bytes_transferred(); @@ -193,13 +193,13 @@ MigrationInfo *qmp_query_migrate(Error **errp) info->has_status = true; info->status = g_strdup("completed"); + info->total_time = s->total_time; info->has_ram = true; info->ram = g_malloc0(sizeof(*info->ram)); info->ram->transferred = ram_bytes_transferred(); info->ram->remaining = 0; info->ram->total = ram_bytes_total(); - info->ram->total_time = s->total_time; info->ram->duplicate = dup_mig_pages_transferred(); info->ram->normal = norm_mig_pages_transferred(); info->ram->normal_bytes = norm_mig_bytes_transferred(); diff --git a/qapi-schema.json b/qapi-schema.json index 3d2b2d1..f83cf22 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -290,10 +290,6 @@ # # @total: total amount of bytes involved in the migration process # -# @total-time: total amount of ms since migration started. If -# migration has ended, it returns the total migration -# time. (since 1.2) -# # @duplicate: number of duplicate pages (since 1.2) # # @normal : number of normal pages (since 1.2) @@ -304,8 +300,7 @@ ## { 'type': 'MigrationStats', 'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' , - 'total-time': 'int', 'duplicate': 'int', 'normal': 'int', - 'normal-bytes': 'int' } } + 'duplicate': 'int', 'normal': 'int', 'normal-bytes': 'int' } } ## # @XBZRLECacheStats @@ -350,12 +345,17 @@ # migration statistics, only returned if XBZRLE feature is on and # status is 'active' or 'completed' (since 1.2) # +# @total-time: total amount of milliseconds since migration started. +# If migration has ended, it returns the total migration +# time. (since 1.2) +# # Since: 0.14.0 ## { 'type': 'MigrationInfo', 'data': {'*status': 'str', '*ram': 'MigrationStats', '*disk': 'MigrationStats', - '*xbzrle-cache': 'XBZRLECacheStats'} } + '*xbzrle-cache': 'XBZRLECacheStats', + 'total-time': 'int'} } ## # @query-migrate diff --git a/qmp-commands.hx b/qmp-commands.hx index 2ce4ce6..8671bf3 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -2239,14 +2239,14 @@ The main json-object contains the following: - "status": migration status (json-string) - Possible values: "active", "completed", "failed", "cancelled" +- "total-time": total amount of ms since migration started. If + migration has ended, it returns the total migration + time (json-int) - "ram": only present if "status" is "active", it is a json-object with the following RAM information (in bytes): - "transferred": amount transferred (json-int) - "remaining": amount remaining (json-int) - "total": total (json-int) - - "total-time": total amount of ms since migration started. If - migration has ended, it returns the total migration time - (json-int) - "duplicate": number of duplicated pages (json-int) - "normal" : number of normal pages transferred (json-int) - "normal-bytes" : number of normal bytes transferred (json-int)
Signed-off-by: Juan Quintela <quintela@redhat.com> --- hmp.c | 4 ++-- migration.c | 6 +++--- qapi-schema.json | 14 +++++++------- qmp-commands.hx | 6 +++--- 4 files changed, 15 insertions(+), 15 deletions(-)