qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: zhanghailiang <zhang.zhanghailiang@huawei.com>
To: Eric Blake <eblake@redhat.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: hangaohuai@huawei.com, quintela@redhat.com,
	qemu-devel@nongnu.org, armbru@redhat.com,
	peter.huangpeng@huawei.com, lcapitulino@redhat.com,
	amit.shah@redhat.com
Subject: Re: [Qemu-devel] [PATCH v2] migration: Convert 'status' of MigrationInfo to use an enum type
Date: Sat, 28 Feb 2015 11:09:11 +0800	[thread overview]
Message-ID: <54F13157.5000709@huawei.com> (raw)
In-Reply-To: <54F0ACA3.5080105@redhat.com>

On 2015/2/28 1:42, Eric Blake wrote:
> On 02/27/2015 10:07 AM, Dr. David Alan Gilbert wrote:
>>>
>>> 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?
>
> It might - I seem to recall in the past that when we added a new state
> string, that at least libvirt choked when encountering the unknown
> string (but I don't recall if it was migration or something else).  At
> least libvirt already has an enum tracking:
>
> enum {
>      QEMU_MONITOR_MIGRATION_STATUS_INACTIVE,
>      QEMU_MONITOR_MIGRATION_STATUS_ACTIVE,
>      QEMU_MONITOR_MIGRATION_STATUS_COMPLETED,
>      QEMU_MONITOR_MIGRATION_STATUS_ERROR,
>      QEMU_MONITOR_MIGRATION_STATUS_CANCELLED,
>      QEMU_MONITOR_MIGRATION_STATUS_SETUP,
>
>      QEMU_MONITOR_MIGRATION_STATUS_LAST
> };
>
> and a string mapping of just the following states:
>
> VIR_ENUM_IMPL(qemuMonitorMigrationStatus,
>                QEMU_MONITOR_MIGRATION_STATUS_LAST,
>                "inactive", "active", "completed", "failed", "cancelled",
> "setup")
>
> Furthermore, the function qemuMigrationUpdateJobStatus() is doing
> various things depending on the observed state, where the current trick
> of treating 'cancelling' like 'active' would mean that changing
> 'cancelling' to be an independent state WOULD have observable behavior
> change in libvirt.  But I don't know if the change would break things,

Er, i have tested with returning 'cancelling' to users, and
only when we try to cancel a migration, libvirt sometimes will report :
Migration: [ 69 %]^Cerror: internal error: unexpected migration status in cancelling.
But the cancelling process is still completed.

And, yes, it is very rare, depend on the time window. (In my test, i add a sleep of 5s
to extend the time between cancelling and cancelled.)

> or if it would still end up resolving nicely (after all, cancelling only
> occurs for a short window before the migration aborts anyway, so it
> might just sort itself out when it finally gets to cancelled).
>

> On the other hand, we can argue that clients that are unprepared to
> handle new enum states gracefully are broken, and we also have the
> argument that it is okay for a new qemu to require a new libvirt release
> (the other direction is not okay - a new libvirt must not require

Agreed, upgrade libvirt is reasonable.
So, should i send v3 with exposing 'cancelling'to user, and CC libvirt ?

Thanks.

> upgrading to a new qemu).  So exposing 'cancelling' may make this patch
> easier.
>

  reply	other threads:[~2015-02-28  3:11 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-27  6:19 [Qemu-devel] [PATCH v2] migration: Convert 'status' of MigrationInfo to use an enum type zhanghailiang
2015-02-27 16:48 ` Eric Blake
2015-02-27 17:07   ` Dr. David Alan Gilbert
2015-02-27 17:42     ` Eric Blake
2015-02-28  3:09       ` zhanghailiang [this message]
2015-03-02 15:57         ` Eric Blake
2015-02-28  2:54   ` zhanghailiang
2015-03-02 15:56     ` Eric Blake
2015-03-03  7:15       ` zhanghailiang
2015-03-03  8:59         ` Dr. David Alan Gilbert
2015-03-04 12:37           ` zhanghailiang
2015-03-04 12:50             ` Dr. David Alan Gilbert
2015-03-04 13:01               ` zhanghailiang
2015-03-03 17:21         ` Eric Blake
2015-03-04  7:48           ` Markus Armbruster
2015-03-04  8:55             ` zhanghailiang
2015-03-04  8:52           ` zhanghailiang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54F13157.5000709@huawei.com \
    --to=zhang.zhanghailiang@huawei.com \
    --cc=amit.shah@redhat.com \
    --cc=armbru@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=eblake@redhat.com \
    --cc=hangaohuai@huawei.com \
    --cc=lcapitulino@redhat.com \
    --cc=peter.huangpeng@huawei.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).