From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47879) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z0qle-0002pH-RN for qemu-devel@nongnu.org; Fri, 05 Jun 2015 08:30:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z0qla-0007X4-OF for qemu-devel@nongnu.org; Fri, 05 Jun 2015 08:30:34 -0400 Received: from mx1.redhat.com ([209.132.183.28]:5405) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z0qla-0007Wt-Gz for qemu-devel@nongnu.org; Fri, 05 Jun 2015 08:30:30 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id C94B03702EB for ; Fri, 5 Jun 2015 12:30:29 +0000 (UTC) Message-ID: <5571965F.7040502@redhat.com> Date: Fri, 05 Jun 2015 06:30:23 -0600 From: Eric Blake MIME-Version: 1.0 References: <20150605095004.GB2139@work-vm> In-Reply-To: <20150605095004.GB2139@work-vm> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="3TKMm6pV20vKNA2D2H42JmNwg72LvrdKR" 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, armbru@redhat.com Cc: amit.shah@redhat.com, quintela@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --3TKMm6pV20vKNA2D2H42JmNwg72LvrdKR Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 06/05/2015 03:50 AM, 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: >=20 > 1) qapi-schema.json > a) Add to 'MigrationParameter' enum, include comment > b) Add to migrate-set-parameters > c) Add to MigrationParameters Perhaps putting a "see XXX for use" could reduce documentation duplication, but it doesn't help the need to add to the enum and to multiple clients. > 2) Define the 'default' macro at the top of migration.c > 3) Add the initialisation to migrate_get_current to set the default If we get to the point where qapi can define default values for variables, then the defaulting moves out of migration.c into the .json fi= le. > 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-param= eters! Also, moving defaults into qapi will eliminate the need for the has_ counterpart on optional variables (the C code will be passed the defaulted value, if the user omitted the variable at the QMP layer). > b) Add a bounds check on the value Once we have the qapi syntax for defaults, it would not be that much more work to move bounds checking into qapi. For example: 'data': { 'value': { 'type': 'uint', 'default': 1, 'max': 10 } } would be a reasonable way to document an option that can range from 0 to 10 but defaults to 1. > 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 ri= ght Is there a way to pass a QDict instead of individual parameters to make this part easier? Back when we started adding blockdev-add, a lot of the magic was related to adding code for passing dictionaries around (keeping things in name/value pairs through more of the call stack) rather than adding parameters right and left at all points. > 7) Fixup hmp_info_migrate_parameters >=20 > oh, and don't forget to: > 8) add the entries to qmp_query_migrate_parameters >=20 > (I forgot). Yeah, that's a lot to do. I'm not sure if there is anything else that can be done to make it more automatic in some of those places, but even having a list of things to touch helps future additions. Maybe worth something in docs/? >=20 >=20 > 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 night= mare > 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).= >=20 > 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 Not sure I can simplify a) or b); but c) and d) seem doable at the qapi level. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --3TKMm6pV20vKNA2D2H42JmNwg72LvrdKR Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJVcZZfAAoJEKeha0olJ0NqkTkIAJGRTeacmTnFKV84Whbc4L3W Nf4s7Adkzofg56FmSyFtfv3uDvrXDSHEYc8NwCV3Xvc/clc4qnmPbxoeC6xaBWKz 9goOaSA1RxHIc5QaxA3A7kqIaVZzgAKPELHqHoB01D/tEC3ImPvYe2EV1XNPcvma 2zEIwfMvF0sNeR/O0lDwYvxoHGafLjZLMX2Rryjr95C52UFNieDCaxLmaMhq9qZ5 IK0LMRGfHsQbcKnXR9FGg7ZJ3UbynV6JSilEKQqndbL/sFUc4IiiNTxtgpOZZPwf v89ntGwPjIaZjIBeigRI54vlfLfbNxwPek0sjRzbEwx4/JudyfNvq2hck4mmHDQ= =Big8 -----END PGP SIGNATURE----- --3TKMm6pV20vKNA2D2H42JmNwg72LvrdKR--