* Eric Blake (ebl...@redhat.com) wrote:
> On 02/26/2015 11:19 PM, zhanghailiang wrote:
> > The original 'status' is an open-coded 'str' type, convert it to use an
> > enum type.
> > This conversion is backwards compatible, better documented and
> > more convenient for future extensibility.
> >
> > In addition, Fix a typo for qapi-schema.json: comppleted -> completed
> >
> > Signed-off-by: zhanghailiang <zhang.zhanghaili...@huawei.com>
> > ---
> > v2:
> > - Remove '(since xyz)' strings. (Eric Blake)
> > ---
> > hmp.c | 7 ++++---
> > migration/migration.c | 37 +++++++++++++++----------------------
> > qapi-schema.json | 37 ++++++++++++++++++++++++++++++++-----
> > 3 files changed, 51 insertions(+), 30 deletions(-)
>
> > case MIG_STATE_ACTIVE:
> > case MIG_STATE_CANCELLING:
> > info->has_status = true;
> > - info->status = g_strdup("active");
> > + /* Note: when the real state of migration is 'cancelling',
> > + we still return 'active' status to user, it makes no difference
> > + for user. */
>
> Rather than pollute the user-exposed enum with a state that we will
> never report, can we come up with some internal-only method for tracking
> cancelling separate from the enum?
Well I guess we could just report it; but would that break any external tools?
IMHO this change is a sensible change, the legacy of 'cancelling' is already
there and it doesn't seem right to fix that at the same time as cleaning this
up.
Dave
>
>
> > +++ b/qapi-schema.json
> > @@ -411,18 +411,45 @@
> > 'overflow': 'int' } }
> >
> > ##
> > +# @MigState:
>
> Do we have to abbreviate? I guess leaving it like this makes the rest
> of the existing code base have less churn (since it matches the spelling
> of the enum that was previous interanl only), but it might look nicer as
> MigrationState. If you do decide to go with a longer name, it might be
> nice to split this into a series, one patch that does only renames to
> migration.c (but no code additions outside of that file), and the other
> that moves the (now-correctly-named) enum into the public qapi file.
>
> > +#
> > +# An enumeration of migration status.
> > +#
> > +# @failed: some error occurred during migration process.
> > +#
> > +# @none: no migration has ever happened.
> > +#
> > +# @setup: migration process has been initiated.
> > +#
> > +# @cancelling: in the process of cancelling migration.
> > +#
>
> If the user can never see this state, I'd rather we think about a
> solution that avoids advertising the state in the public api...
>
> > +# @cancelled: cancelling migration is finished.
> > +#
> > +# @active: in the process of doing migration.
> > +#
> > +# @completed: migration is finished.
> > +#
> > +# Since: 2.3
> > +#
> > +# Notes: @cancelling is only used internally, and return @active to user
> > +# instead of @cancelling, it make no difference for users.
>
> ...rather than needing this note to air our dirty laundry.
>
> --
> Eric Blake eblake redhat com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
