From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38974) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YRXoY-0006R9-KE for qemu-devel@nongnu.org; Fri, 27 Feb 2015 22:11:39 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YRXoV-0001tp-CW for qemu-devel@nongnu.org; Fri, 27 Feb 2015 22:11:38 -0500 Received: from szxga03-in.huawei.com ([119.145.14.66]:63483) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YRXoU-0001rh-Ib for qemu-devel@nongnu.org; Fri, 27 Feb 2015 22:11:35 -0500 Message-ID: <54F13157.5000709@huawei.com> Date: Sat, 28 Feb 2015 11:09:11 +0800 From: zhanghailiang MIME-Version: 1.0 References: <1425017996-6748-1-git-send-email-zhang.zhanghailiang@huawei.com> <54F09FFB.2070008@redhat.com> <20150227170727.GB2570@work-vm> <54F0ACA3.5080105@redhat.com> In-Reply-To: <54F0ACA3.5080105@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] migration: Convert 'status' of MigrationInfo to use an enum type List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , "Dr. David Alan Gilbert" 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 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. >