From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57013) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z5AsP-0003LO-V4 for qemu-devel@nongnu.org; Wed, 17 Jun 2015 06:47:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z5AsK-0007Or-S9 for qemu-devel@nongnu.org; Wed, 17 Jun 2015 06:47:25 -0400 Received: from szxga02-in.huawei.com ([119.145.14.65]:20397) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z5AsJ-0007Ls-Q4 for qemu-devel@nongnu.org; Wed, 17 Jun 2015 06:47:20 -0400 Message-ID: <55815000.8060300@huawei.com> Date: Wed, 17 Jun 2015 18:46:24 +0800 From: zhanghailiang MIME-Version: 1.0 References: <20150605095004.GB2139@work-vm> In-Reply-To: <20150605095004.GB2139@work-vm> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] Adding new migration-parameters - any easier way? List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert" , qemu-devel@nongnu.org, eblake@redhat.com, armbru@redhat.com Cc: amit.shah@redhat.com, peter.huangpeng@huawei.com, quintela@redhat.com Hi, Is there any news about this discussion? Is anyone working on it? ;) Since the 'hard feature freeze' time is closer, we'd better to fix it in 2.4 before libvirt uses it. I have sent a RFC patch "[RFC] migration: Re-implement 'migrate-set-parameters' to make it easily for extension" http://patchwork.ozlabs.org/patch/482125/ Which just changes qmp command to '{ "execute": "migrate-set-parameters" , "arguments": { "compression": { "compress-level": 1 } } }', Compared with its original way, it seems to be more easy for reading and extension, but it can't address most problems discussed here... Thanks, zhanghailiang On 2015/6/5 17:50, Dr. David Alan Gilbert wrote: > Hi, > Is there any way that we could make it easier to add new migration > parameters? The current way is complicated and error prone; > as far as I can tell, to add a new parameter we need to: > > 1) qapi-schema.json > a) Add to 'MigrationParameter' enum, include comment > b) Add to migrate-set-parameters > c) Add to MigrationParameters > 2) Define the 'default' macro at the top of migration.c > 3) Add the initialisation to migrate_get_current to set the default > 4) qmp_migrate_set_parameters: > a) Add the 'has' and value arguments to qmp_migrate_set_parameters > *** Make really sure this matches the order in migrate-set-parameters! > b) Add a bounds check on the value > c) Set the value in the array if the has_ is true > 5) Fixup migrate_init to preserve the parameter around the init > 6) Add a bool and case entry to hmp_migrate_set_parameter and > pass to qmp_migrate_set_parameters > *** Make sure you get the order to qmp_migrate_set_parameters right > 7) Fixup hmp_info_migrate_parameters > > > The three separate changes needed in the qapi-schema.json seem odd, > and the 'has'/value pairs on qmp_migrate_set_parameters is just a nightmare > because there's nothing to check the ordering, and it's just getting > a silly number of arguments to the function now (I've got 10 > parameters in one of my dev worlds, so that function has 21 arguments). > > In my ideal world there would be: > a) One thing to add to qapi-schema.json > b) qmp_migrate_set_parameters would take an array pointer indexed > by the enum > c) A way to define the bounds so that we didn't have to manually > add the bound checking. > d) Something where I defined the default value > > (I'm fairly sure earlier versions of migrate parameters patches > managed (a) and possibly (b)). > > Dave > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > > >