From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56489) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YWJ0S-0000d9-V4 for qemu-devel@nongnu.org; Fri, 13 Mar 2015 02:23:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YWJ0P-0004qc-OJ for qemu-devel@nongnu.org; Fri, 13 Mar 2015 02:23:36 -0400 Received: from szxga03-in.huawei.com ([119.145.14.66]:64087) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YWJ0O-0004oJ-VD for qemu-devel@nongnu.org; Fri, 13 Mar 2015 02:23:33 -0400 Message-ID: <5502824C.1000802@huawei.com> Date: Fri, 13 Mar 2015 14:23:08 +0800 From: zhanghailiang MIME-Version: 1.0 References: <1425883543-8852-1-git-send-email-zhang.zhanghailiang@huawei.com> <1425883543-8852-4-git-send-email-zhang.zhanghailiang@huawei.com> <5501EAFA.70103@redhat.com> In-Reply-To: <5501EAFA.70103@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 3/4] 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 , qemu-devel@nongnu.org Cc: hangaohuai@huawei.com, quintela@redhat.com, armbru@redhat.com, lcapitulino@redhat.com, amit.shah@redhat.com, peter.huangpeng@huawei.com, dgilbert@redhat.com On 2015/3/13 3:37, Eric Blake wrote: > On 03/09/2015 12:45 AM, 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 (just remove the typo) : >> s/'completed'. 'comppleted' (since 1.2)/'completed' (since 1.2) >> >> Signed-off-by: zhanghailiang >> --- >> hmp.c | 7 ++++--- >> migration/migration.c | 20 +++++--------------- >> qapi-schema.json | 34 +++++++++++++++++++++++++++++----- >> 3 files changed, 38 insertions(+), 23 deletions(-) > >> >> -enum { >> - MIGRATION_STATUS_FAILED = -1, >> - MIGRATION_STATUS_NONE, > > Note that we are changing any 0-initialized struct from having > STATUS_NONE... > > >> +## >> +{ 'enum': 'MigrationStatus', >> + 'data': [ 'failed', 'none', 'setup', 'cancelling', 'cancelled', >> + 'active', 'completed' ] } > > ...to now having status _FAILED. Fortunately, in migration/migration.c > (the only caller prior to this conversion), I didn't spot any > zero-initialization (migrate_get_current() returns a struct that is > explicitly initialized to _NONE). So it looks correct. > Hmm, however, i think it deserves a small modification, i will adjust the place of 'failed', which i will move it to the behind of 'completed' in v5. In this way, all of these enum type macros except 'failed' will be consistent with there original value. >> + >> ## >> # @MigrationInfo >> # >> # Information about current migration process. >> # >> -# @status: #optional string describing the current migration status. >> -# As of 0.14.0 this can be 'setup', 'active', 'completed', 'failed' or >> -# 'cancelled'. If this field is not returned, no migration process >> +# @status: #optional @MigState describing the current migration status. > > s/MigState/MigrationStatus/ > fix in v5. > >> + 'data': {'*status': 'MigrationStatus', '*ram': 'MigrationStats', >> '*disk': 'MigrationStats', > > Hmm. "Status" and "Stats" look awfully close to one another. But I can > live with the two names (it's not worth a respin just for the rename). > With the typo fix pointed out in the documentation, > I will keep it like that. Thanks. > Reviewed-by: Eric Blake >