Re: [Qemu-devel] [PATCH v2 2/4] Curling: cmdline interface.

[Jules]

On Wed, 2013-10-09 at 06:02 -0600, Eric Blake wrote:
> [your emailer munged the reply, making it a bit hard to read.  Are you
> set for plain-text-only mail to the list?]

Thanks VERY much for remind me that, I'm using another client now.

> On 10/09/2013 12:49 AM, address@hidden wrote:
> 
> >  >> +++ b/hmp.c
> >>> @@ -1213,10 +1213,11 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
> >>>      int detach = qdict_get_try_bool(qdict, "detach", 0);
> >>>      int blk = qdict_get_try_bool(qdict, "blk", 0);
> >>>      int inc = qdict_get_try_bool(qdict, "inc", 0);
> >>> +   int ft   = qdict_get_try_bool(qdict, "ft", 0);
> >>
> >> Why two spaces?
> > 
> > To align the '=',  I will remove them if you like. 
> 
> It's not a problem with me either way, other than we have a lot of code
> that doesn't care about alignment and consistently uses one space, and a
> fair amount of code where everything in a block of code is consistently
> aligned.  But your patch was neither, in the context of the block it
> lives within - if you're going to align, then line up everything with
> the longest line 'int detach' (including blk and inc).
> 

oh, I got it, you are right, I missed the longest line 'int detach ...'
I'm not going to align them.

> > 
> >  >
> >>> +++ b/qapi-schema.json
> >>> @@ -2420,7 +2420,8 @@
> >>>  # Since: 0.14.0
> >>>  ##
> >>>  { 'command': 'migrate',
> >>> -  'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool', '*detach': 
> >>> 'bool' } }
> >>> +  'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool', '*detach': 
> >>> 'bool',
> >>> +           '*ft': 'bool' } }
> >>
> >> Missing documentation, including mention that the new option was only
> >> made available in 1.7.  We still don't have introspection; is there some
> >> other means by which libvirt and other management apps can tell whether
> >> this feature is available? 
> > 
> > I'm not clear about how to do that, could you pls give me some hints, where 
> > to 
> > add code and documentation. 
> 
> As for the documentation, qapi-schema.json has plenty of examples (look
> for a field with "(since 1.7)" as a hint for how to document an optional
> field added in a later release than the main struct).

I see. Thanks.
> 
> As for the introspection, Amos Kong was most recently working on trying
> to add that (but missed the 1.6 deadline, and I haven't seen work on it
> since).  Introspection is not a hard requirement, but it makes it harder
> for libvirt to know if it can use 'ft':true if there is no other
> 'query-*' command that it can call first that would give it a hint that
> this is a new enough qemu to support 'ft' during migration.  Maybe even
> having something listed under query-migrate-capabilities would be
> sufficient (ie. modify the 'MigrationCapability' enum to advertise a new
> capability).

Adding a new migration capability is a work-around method. we turn on ft
by using the -f option instead of setting fault-tolerant-capability to
true. I hesitate to add it.

What about adding a query for the options of migration similar to
@query-command-line-options?