From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51892) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WVwiS-0001zG-U0 for qemu-devel@nongnu.org; Fri, 04 Apr 2014 01:31:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WVwiJ-0008KL-RC for qemu-devel@nongnu.org; Fri, 04 Apr 2014 01:31:00 -0400 Received: from e8.ny.us.ibm.com ([32.97.182.138]:58254) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WVwiJ-0008KH-Mk for qemu-devel@nongnu.org; Fri, 04 Apr 2014 01:30:51 -0400 Received: from /spool/local by e8.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 4 Apr 2014 01:30:51 -0400 Received: from b01cxnp23034.gho.pok.ibm.com (b01cxnp23034.gho.pok.ibm.com [9.57.198.29]) by d01dlp03.pok.ibm.com (Postfix) with ESMTP id 24C00C90041 for ; Fri, 4 Apr 2014 01:30:44 -0400 (EDT) Received: from d01av01.pok.ibm.com (d01av01.pok.ibm.com [9.56.224.215]) by b01cxnp23034.gho.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s345Umm457409568 for ; Fri, 4 Apr 2014 05:30:48 GMT Received: from d01av01.pok.ibm.com (localhost [127.0.0.1]) by d01av01.pok.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s345UkuN009888 for ; Fri, 4 Apr 2014 01:30:47 -0400 Message-ID: <533E4331.2020202@linux.vnet.ibm.com> Date: Fri, 04 Apr 2014 13:29:21 +0800 From: "Michael R. Hines" MIME-Version: 1.0 References: <1392713429-18201-1-git-send-email-mrhines@linux.vnet.ibm.com> <1392713429-18201-11-git-send-email-mrhines@linux.vnet.ibm.com> <531F84FD.7010608@redhat.com> <87mwgwz8ja.fsf@elfo.mitica> <531F9300.50304@redhat.com> In-Reply-To: <531F9300.50304@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH v2 10/12] mc: expose tunable parameter for checkpointing frequency List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , quintela@redhat.com Cc: GILR@il.ibm.com, SADEKJ@il.ibm.com, pbonzini@redhat.com, abali@us.ibm.com, qemu-devel@nongnu.org, EREZH@il.ibm.com, Luiz Capitulino , owasserm@redhat.com, onom@us.ibm.com, hinesmr@cn.ibm.com, isaku.yamahata@gmail.com, gokul@us.ibm.com, dbulkow@gmail.com, junqing.wang@cs2c.com.cn, BIRAN@il.ibm.com, lig.fnst@cn.fujitsu.com, "Michael R. Hines" On 03/12/2014 06:49 AM, Eric Blake wrote: > On 03/11/2014 04:15 PM, Juan Quintela wrote: >> Eric Blake wrote: >>> On 02/18/2014 01:50 AM, mrhines@linux.vnet.ibm.com wrote: >>>> From: "Michael R. Hines" >>> We're building up a LOT of migrate- tunable commands. Maybe it's time >>> to think about building a more generic migrate-set-parameter, which >>> takes both the name of the parameter to set and its value, so that a >>> single command serves all parameters, instead of needing a proliferation >>> of commands. Of course, for that to be useful, we also need a way to >>> introspect which parameters can be tuned; whereas with the current >>> approach of one command per parameter (well, 2 for set vs. get) the >>> introspection is based on whether the command exists. >> I asked to have that. My suggestion was that >> >> migrate_set_capability auto-throotle on >> >> So we could add it to new variables without extra change. >> >> And I agree that having a way to read them, and ask what values they >> have is a good idea. >> >> Luiz, any good idea about how to do it through QMP? > I'm trying to thing of a back-compat method, which exploits the fact > that we now have flat unions (something we didn't have when > migrate-set-capabilities was first added). Maybe something like: > > { 'type': 'MigrationCapabilityBase', > 'data': { 'capability': 'MigrationCapability' } } > { 'type': 'MigrationCapabilityBool', > 'data': { 'state': 'bool' } } > { 'type': 'Migration CapabilityInt', > 'data': { 'value': 'int' } } > { 'union': 'MigrationCapabilityStatus', > 'base': 'MigrationCapabilityBase', > 'discriminator': 'capability', > 'data': { > 'xbzrle': 'MigrationCapabilityBool', > 'auto-converge': 'MigrationCapabilityBool', > ... > 'mc-delay': 'MigrationCapabilityInt' > } } > > along with a tweak to query-migrate-capabilities for full back-compat: > > # @query-migrate-capabilities > # @extended: #optional defaults to false; set to true to see non-boolean > capabilities (since 2.1) > { 'command: 'query-migrate-capabilities', > 'data': { '*extended': 'bool' }, > 'returns': ['MigrationCapabilityStatus'] } > > Now, observe what happens. If an old client calls { "execute": > "query-migrate-capabilities" }, they get a return that lists ONLY the > boolean members of the MigrationCapabilityStatus array (good, because if > we returned a non-boolean, we would confuse the consumer when they are > expecting a 'state' variable that is not present) - what's more, this > representation is identical on the wire to the format used in earlier > qemu. But new clients can call { "execute": > "query-migrate-capabilities", "arguments": { "extended": true } }, and > get back: > > { "capabilities": [ > { "capability": "xbzrle", "state": true }, > { "capability": "auto-converge", "state": false }, > ... > { "capability": "mc-delay", "value": 100 } > ] } > > Also, once a new client has learned of non-boolean extended > capabilities, they can also set them via the existing command: > { "execute": "migrate-set-capabilities", > "arguments": [ > { "capability": "xbzrle", "state", false }, > { "capability": "mc-delay", "value", 200 } > ] } > > So, what do you think? My slick type manipulation means that we need > zero new commands, just a new option the the query command, and a new > flat union type that replaces the current struct type. The existence > (but not the type) of non-boolean parameters is already introspectible > to a client new enough to request an 'extended' query, and down the > road, if we ever gain full QAPI introspection, then a client also would > gain the ability to learn the type of any non-boolean parameter as well. > Stability wise, as long as we never change the type of a capability > once first exposed, then if a client plans on using a particular > parameter when available, it can already hard-code what type that > parameter should have without even needing full QAPI introspection (that > is, if libvirt is taught to manipulate mc-delay, libvirt will already > know to expect mc-delay as an int, and not any other type, and merely > needs to probe if qemu supports the 'mc-delay' extended capability). > And of course, this new schema idea can retroactively cover all existing > migration tunables, such as migrate_set_downtime, migrate_set_speed, > migrate-set-cache-size, and so on. I like this a lot - it's very complicated, but it is clean, I think. Maybe you should add some "reserved" fields in there as well to the union, in case you want to expand the number of members of the union in the future? - Michael