[V6,11/17] qapi: Add new command to query colo status

Libvirt or other high level software can use this command query colo status.
You can test this command like that:
{'execute':'query-colo-status'}

Signed-off-by: Zhang Chen <zhangckid@gmail.com>
---
 migration/colo.c    | 31 +++++++++++++++++++++++++++++++
 qapi/migration.json | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+)

Zhang Chen <zhangckid@gmail.com> writes:

> Libvirt or other high level software can use this command query colo status.
> You can test this command like that:
> {'execute':'query-colo-status'}
>
> Signed-off-by: Zhang Chen <zhangckid@gmail.com>
> ---
>  migration/colo.c    | 31 +++++++++++++++++++++++++++++++
>  qapi/migration.json | 33 +++++++++++++++++++++++++++++++++
>  2 files changed, 64 insertions(+)
>
> diff --git a/migration/colo.c b/migration/colo.c
> index 8ca6381..127db3c 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -237,6 +237,37 @@ void qmp_xen_colo_do_checkpoint(Error **errp)
>  #endif
>  }
>  
> +COLOStatus *qmp_query_colo_status(Error **errp)
> +{
> +    MigrationState *m;
> +    MigrationIncomingState *mis;
> +    COLOStatus *s = g_new0(COLOStatus, 1);
> +
> +    if (get_colo_mode() == COLO_MODE_PRIMARY) {
> +        m = migrate_get_current();
> +
> +        if (m->state == MIGRATION_STATUS_COLO) {
> +            s->colo_running = true;
> +        } else {
> +            s->colo_running = false;
> +        }
> +        s->mode = COLO_MODE_PRIMARY;
> +    } else {

Can get_colo_mode() == COLO_MODE_UNKNOWN happen here?  If not, why not?

> +        mis = migration_incoming_get_current();
> +
> +        if (mis->state == MIGRATION_STATUS_COLO) {
> +            s->colo_running = true;
> +        } else {
> +            s->colo_running = false;
> +        }
> +        s->mode = COLO_MODE_SECONDARY;
> +    }

Simpler:

       if (get_colo_mode() == COLO_MODE_PRIMARY) {
           state = migrate_get_current()->state;
       } else {
           state = migration_incoming_get_current()->state;
       }
       s->colo_running = state == MIGRATION_STATUS_COLO;
       s->mode = get_colo_mode();

> +
> +    s->reason = failover_get_state();

COLOStatus member @reason is COLOExitReason, but failover_get_state()
returns FailoverStatus, a different enum.  Am I confused?

> +
> +    return s;
> +}
> +
>  static void colo_send_message(QEMUFile *f, COLOMessage msg,
>                                Error **errp)
>  {
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 6c6c50e..8ccdd21 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1201,3 +1201,36 @@
>  # Since: 2.9
>  ##
>  { 'command': 'xen-colo-do-checkpoint' }
> +
> +##
> +# @COLOStatus:
> +#
> +# The result format for 'query-colo-status'.
> +#
> +# @mode: which COLO mode the VM was in when it exited.

This documents the meaning of @mode "when it exited".  Two questions:

1. What's "it"?  The VM?  COLO?

2. What's the meaning of @mode when "it" hasn't exited, yet?

> +#
> +# @colo-running: if true means COLO running well, otherwise COLO have done.

Suggest

   # @colo-running: true if COLO is running

> +#
> +# @reason: describes the reason for the COLO exit.

If there has been no COLO exit, there can't be a reason for one.  What's
the value of @reason then?  Hmm, peeking at FailoverStatus makes me
guess its @none.  Correct?

> +#
> +# Since: 2.12
> +##
> +{ 'struct': 'COLOStatus',
> +  'data': { 'mode': 'COLOMode', 'colo-running': 'bool', 'reason': 'COLOExitReason' } }
> +
> +##
> +# @query-colo-status:
> +#
> +# Query COLO status while the vm is running.
> +#
> +# Returns: A @COLOStatus object showing the status.
> +#
> +# Example:
> +#
> +# -> { "execute": "query-colo-status" }
> +# <- { "return": { "mode": "primary", "colo-running": true, "reason": "request" } }
> +#
> +# Since: 2.12
> +##
> +{ 'command': 'query-colo-status',
> +  'returns': 'COLOStatus' }

On Tue, Apr 17, 2018 at 8:46 PM, Markus Armbruster <armbru@redhat.com>
wrote:

> Zhang Chen <zhangckid@gmail.com> writes:
>
> > Libvirt or other high level software can use this command query colo
> status.
> > You can test this command like that:
> > {'execute':'query-colo-status'}
> >
> > Signed-off-by: Zhang Chen <zhangckid@gmail.com>
> > ---
> >  migration/colo.c    | 31 +++++++++++++++++++++++++++++++
> >  qapi/migration.json | 33 +++++++++++++++++++++++++++++++++
> >  2 files changed, 64 insertions(+)
> >
> > diff --git a/migration/colo.c b/migration/colo.c
> > index 8ca6381..127db3c 100644
> > --- a/migration/colo.c
> > +++ b/migration/colo.c
> > @@ -237,6 +237,37 @@ void qmp_xen_colo_do_checkpoint(Error **errp)
> >  #endif
> >  }
> >
> > +COLOStatus *qmp_query_colo_status(Error **errp)
> > +{
> > +    MigrationState *m;
> > +    MigrationIncomingState *mis;
> > +    COLOStatus *s = g_new0(COLOStatus, 1);
> > +
> > +    if (get_colo_mode() == COLO_MODE_PRIMARY) {
> > +        m = migrate_get_current();
> > +
> > +        if (m->state == MIGRATION_STATUS_COLO) {
> > +            s->colo_running = true;
> > +        } else {
> > +            s->colo_running = false;
> > +        }
> > +        s->mode = COLO_MODE_PRIMARY;
> > +    } else {
>
> Can get_colo_mode() == COLO_MODE_UNKNOWN happen here?  If not, why not?
>

Good catch, I will check it in next version.


>
> > +        mis = migration_incoming_get_current();
> > +
> > +        if (mis->state == MIGRATION_STATUS_COLO) {
> > +            s->colo_running = true;
> > +        } else {
> > +            s->colo_running = false;
> > +        }
> > +        s->mode = COLO_MODE_SECONDARY;
> > +    }
>
> Simpler:
>
>        if (get_colo_mode() == COLO_MODE_PRIMARY) {
>            state = migrate_get_current()->state;
>        } else {
>            state = migration_incoming_get_current()->state;
>        }
>        s->colo_running = state == MIGRATION_STATUS_COLO;
>        s->mode = get_colo_mode();
>

Good idea. I will use this codes in next version.
By the way, do I need add your SOB?


>
> > +
> > +    s->reason = failover_get_state();
>
> COLOStatus member @reason is COLOExitReason, but failover_get_state()
> returns FailoverStatus, a different enum.  Am I confused?
>

No, here I want to reused the FailoverStatus, but missed something in
rebase progress.
I will make FailoverStatus's @none = COLOExitReason's @none,
                  FailoverStatus's @requre = COLOExitReason's @request
                  FailoverStatus's other status = COLOExitReason's @error.
in next version.



>
> > +
> > +    return s;
> > +}
> > +
> >  static void colo_send_message(QEMUFile *f, COLOMessage msg,
> >                                Error **errp)
> >  {
> > diff --git a/qapi/migration.json b/qapi/migration.json
> > index 6c6c50e..8ccdd21 100644
> > --- a/qapi/migration.json
> > +++ b/qapi/migration.json
> > @@ -1201,3 +1201,36 @@
> >  # Since: 2.9
> >  ##
> >  { 'command': 'xen-colo-do-checkpoint' }
> > +
> > +##
> > +# @COLOStatus:
> > +#
> > +# The result format for 'query-colo-status'.
> > +#
> > +# @mode: which COLO mode the VM was in when it exited.
>
> This documents the meaning of @mode "when it exited".  Two questions:
>
> 1. What's "it"?  The VM?  COLO?
>

COLO.


>
> 2. What's the meaning of @mode when "it" hasn't exited, yet?
>

Just return the COLO running mode(primary, secondary or unknown) when COLO
hasn't exited.


>
> > +#
> > +# @colo-running: if true means COLO running well, otherwise COLO have
> done.
>
> Suggest
>
>    # @colo-running: true if COLO is running
>
> > +#
> > +# @reason: describes the reason for the COLO exit.
>
> If there has been no COLO exit, there can't be a reason for one.  What's
> the value of @reason then?  Hmm, peeking at FailoverStatus makes me
> guess its @none.  Correct?
>

Yes, you are right. I will fix it in next version.

Thanks your comments.
Zhang Chen



>
> > +#
> > +# Since: 2.12
> > +##
> > +{ 'struct': 'COLOStatus',
> > +  'data': { 'mode': 'COLOMode', 'colo-running': 'bool', 'reason':
> 'COLOExitReason' } }
> > +
> > +##
> > +# @query-colo-status:
> > +#
> > +# Query COLO status while the vm is running.
> > +#
> > +# Returns: A @COLOStatus object showing the status.
> > +#
> > +# Example:
> > +#
> > +# -> { "execute": "query-colo-status" }
> > +# <- { "return": { "mode": "primary", "colo-running": true, "reason":
> "request" } }
> > +#
> > +# Since: 2.12
> > +##
> > +{ 'command': 'query-colo-status',
> > +  'returns': 'COLOStatus' }
>