From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59300) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VU6Np-0006fp-Ks for qemu-devel@nongnu.org; Wed, 09 Oct 2013 22:53:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VU6No-0002SK-5x for qemu-devel@nongnu.org; Wed, 09 Oct 2013 22:53:49 -0400 Received: from [2001:250:208:1181:6e92:bfff:fe00:bcdb] (port=41337 helo=mail.cs2c.com.cn) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VU6Nn-0002RI-Jx for qemu-devel@nongnu.org; Wed, 09 Oct 2013 22:53:48 -0400 Message-ID: <1381373572.1741.16.camel@localhost> From: Jules Date: Thu, 10 Oct 2013 10:52:52 +0800 In-Reply-To: <525545DE.2010505@redhat.com> References: <1380485683-4626-1-git-send-email-junqing.wang@cs2c.com.cn> <1380485683-4626-3-git-send-email-junqing.wang@cs2c.com.cn> <5249F842.8050405@redhat.com> <2c17177e.92cb.1419bfa46c2.Coremail.junqing.wang@cs2c.com.cn> <525545DE.2010505@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 2/4] Curling: cmdline interface. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: pbonzini@redhat.com, quintela@redhat.com, qemu-devel@nongnu.org, owasserm@redhat.com 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, junqing.wang@cs2c.com.cn 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?