From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:56834) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sm7xl-0002KS-Uo for qemu-devel@nongnu.org; Tue, 03 Jul 2012 14:36:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Sm7xg-0005y2-VT for qemu-devel@nongnu.org; Tue, 03 Jul 2012 14:36:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42182) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sm7xg-0005xm-ME for qemu-devel@nongnu.org; Tue, 03 Jul 2012 14:36:32 -0400 Message-ID: <4FF33BA5.4070900@redhat.com> Date: Tue, 03 Jul 2012 12:36:21 -0600 From: Eric Blake MIME-Version: 1.0 References: <1341323574-23206-1-git-send-email-owasserm@redhat.com> <1341323574-23206-3-git-send-email-owasserm@redhat.com> In-Reply-To: <1341323574-23206-3-git-send-email-owasserm@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="------------enig090A3F442FB29BE96C47F3B8" Subject: Re: [Qemu-devel] [PATCH v14 02/13] Add migration capabilities List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Orit Wasserman Cc: peter.maydell@linaro.org, aliguori@us.ibm.com, quintela@redhat.com, stefanha@gmail.com, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com, blauwirbel@gmail.com, avi@redhat.com, pbonzini@redhat.com, chegu_vinod@hp.com This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig090A3F442FB29BE96C47F3B8 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 07/03/2012 07:52 AM, Orit Wasserman wrote: > Add migration capabilities that can be queried by the management. > The management can query the source QEMU and the destination QEMU in or= der to > verify both support some migration capability (currently only XBZRLE). > The management can enable a capability for the next migration by using > migrate_set_parameter command. Couple of comment nits below. Also, if you don't mind a bit of bike-shedding... [roughly translated - feel free to ignore the bulk of this email; the patch as-is works, if you don't think my alternate design makes sense to implement] > +++ b/hmp-commands.hx > @@ -861,6 +861,20 @@ Set maximum tolerated downtime (in seconds) for mi= gration. > ETEXI > =20 > { > + .name =3D "migrate_set_parameter", > + .args_type =3D "capability:s,state:b", > + .params =3D "", > + .help =3D "Enable the usage of a capability for migratio= n", > + .mhandler.cmd =3D hmp_migrate_set_parameter, This requires the 'state' parameter to be passed. But wouldn't it be easier to default things to state=3Dtrue if no parameter was present, since the most common usage of this command will be to enable, rather than disable, a migration parameter? > - /* no migration has happened ever */ > + /* no migration has happened ever show enabled capabilities */= Missing some punctuation, so this was hard to read. Maybe: /* no migration has ever happened; show enabled capabilities */ > +++ b/qapi-schema.json > +## > +# @MigrationCapabilityInfo > +# > +# Migration capability information > +# > +# @capability: capability enum > +# > +# Since: 1.2 > +## > +{ 'type': 'MigrationCapabilityInfo', > + 'data': { 'capability' : 'MigrationCapability', 'state' : 'bool' } }= Back to the bike-shedding - if you would mark this '*state':'bool', then the enabled parameter becomes optional on input (it should still always be present on query-migration-capabilities output, though). > +migrate_set_parameters > +------- > + > +Enable/Disable migration capabilities > + > +- "xbzrle": xbzrle support > + > +Arguments: > + > +Example: > + > +-> { "execute": "migrate_set_parameters" , "arguments": > + { "parameters": { "capability": "xbzrle", "state": true } ] } } Missing a '[' after "parameters": --=20 Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --------------enig090A3F442FB29BE96C47F3B8 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQEcBAEBCAAGBQJP8zulAAoJEKeha0olJ0Nqf5IH/0F6Eg1dTxUhxwTDJAiGunPt QXD5ZttRE52F9ENPdvnnbMcZc9kmgWSeE+RYqYP4Kb2PIaVjmbVoMb3H64VjZ83V d1nNoV9pvZuefsVY+MMBIIqeQEWDI8EkDAgn5DnukC06UpwuBB875Rs/POGw0K9M tkYBHF7XuN0+6g+E60lco6orMNwHiLwpxk3ifm2Ih+0bRwNMyVVZwaaJAHISDmwP bGprbwJugPTXtbZ0tWloEX66sKb6UvqhWll8wtzbrDH0N95I7SalFVkMSYsmIZ0k wokWlQ6NQTUPpHD5onXTiRI8k14HftJ5cwOVYb6ze7RR3dvGxsUPWiIWepQpM9A= =fKLM -----END PGP SIGNATURE----- --------------enig090A3F442FB29BE96C47F3B8--