[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v4 3/4] migration: Convert 'status' of Migration

From: zhanghailiang
Subject: Re: [Qemu-devel] [PATCH v4 3/4] migration: Convert 'status' of MigrationInfo to use an enum type
Date: Fri, 13 Mar 2015 14:23:08 +0800
User-agent: Mozilla/5.0 (Windows NT 6.1; rv:31.0) Gecko/20100101 Thunderbird/31.1.1

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 <address@hidden>
  hmp.c                 |  7 ++++---
  migration/migration.c | 20 +++++---------------
  qapi-schema.json      | 34 +++++++++++++++++++++++++++++-----
  3 files changed, 38 insertions(+), 23 deletions(-)

-enum {

Note that we are changing any 0-initialized struct from having

+{ '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 
'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.


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 <address@hidden>

reply via email to

[Prev in Thread] Current Thread [Next in Thread]