From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46596) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZVRbZ-0008Er-O1 for qemu-devel@nongnu.org; Fri, 28 Aug 2015 17:54:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZVRbW-0005Mh-FF for qemu-devel@nongnu.org; Fri, 28 Aug 2015 17:54:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54357) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZVRbW-0005La-8K for qemu-devel@nongnu.org; Fri, 28 Aug 2015 17:54:34 -0400 References: <1438159544-6224-1-git-send-email-zhang.zhanghailiang@huawei.com> <1438159544-6224-3-git-send-email-zhang.zhanghailiang@huawei.com> From: Eric Blake Message-ID: <55E0D894.4050408@redhat.com> Date: Fri, 28 Aug 2015 15:54:28 -0600 MIME-Version: 1.0 In-Reply-To: <1438159544-6224-3-git-send-email-zhang.zhanghailiang@huawei.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="q8Cb9hkoioj6BdV3flKX3IMTaTGcxqWjM" Subject: Re: [Qemu-devel] [PATCH COLO-Frame v8 02/34] migration: Introduce capability 'colo' to migration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: zhanghailiang , qemu-devel@nongnu.org Cc: lizhijian@cn.fujitsu.com, quintela@redhat.com, Markus Armbruster , yunhong.jiang@intel.com, eddie.dong@intel.com, peter.huangpeng@huawei.com, dgilbert@redhat.com, arei.gonglei@huawei.com, amit.shah@redhat.com, Yang Hongyang This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --q8Cb9hkoioj6BdV3flKX3IMTaTGcxqWjM Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 07/29/2015 02:45 AM, zhanghailiang wrote: > We add helper function colo_supported() to indicate whether > colo is supported or not, with which we use to control whether or not > showing 'colo' string to users, they can use qmp command > 'query-migrate-capabilities' or hmp command 'info migrate_capabilities'= > to learn if colo is supported. >=20 > Cc: Juan Quintela > Cc: Amit Shah > Cc: Eric Blake > Cc: Markus Armbruster > Signed-off-by: zhanghailiang > Signed-off-by: Yang Hongyang > Signed-off-by: Gonglei > --- Just focusing on the interface. > @@ -338,6 +339,9 @@ MigrationCapabilityStatusList *qmp_query_migrate_ca= pabilities(Error **errp) > =20 > caps =3D NULL; /* silence compiler warning */ > for (i =3D 0; i < MIGRATION_CAPABILITY_MAX; i++) { > + if (i =3D=3D MIGRATION_CAPABILITY_COLO && !colo_supported()) {= > + continue; > + } Interesting - you completely omit the capability if it can't be set to true; so the presence of the capability is therefore the witness of whether it works. Which means it is a true runtime probe, rather than a static introspection, and therefore more accurate :) > if (head =3D=3D NULL) { > head =3D g_malloc0(sizeof(*caps)); > caps =3D head; > @@ -478,6 +482,13 @@ void qmp_migrate_set_capabilities(MigrationCapabil= ityStatusList *params, > } > =20 > for (cap =3D params; cap; cap =3D cap->next) { > + if (cap->value->capability =3D=3D MIGRATION_CAPABILITY_COLO &&= > + cap->value->state && !colo_supported()) { > + error_setg(errp, "COLO is not currently supported, please"= > + " configure with --enable-colo option in = order to" > + " support COLO feature"); > + continue; > + } This says that you error out if colo:true is requested but not supported, but silently ignore colo:false even when it is unsupported. Isn't it better to error out that colo is unsupported, regardless of enabled true/false being requested, since you explicitly avoided advertising the feature? > +++ b/qapi-schema.json > @@ -529,11 +529,14 @@ > # @auto-converge: If enabled, QEMU will automatically throttle down th= e guest > # to speed up convergence of RAM migration. (since 1.6) > # > +# @colo: If enabled, migration will never end, and the state of VM in = primary side Long line. Please wrap to fit within 80 columns. > +# will be migrated continuously to VM in secondary side. (since= 2.4) You've missed 2.4; change this to 2.5. Grammar suggestion: s/of VM in primary/of the VM on the primary/ s/to VM in secondary/to the VM on the secondary/ > +++ b/qmp-commands.hx > @@ -3434,6 +3434,7 @@ Query current migration capabilities > - "rdma-pin-all" : RDMA Pin Page state (json-bool) > - "auto-converge" : Auto Converge state (json-bool) > - "zero-blocks" : Zero Blocks state (json-bool) > + - "colo" : COLO FT state (json-bool) Acronym soup. Might be nice to state more human-readable text about what 'colo' actually controls (stating COarse-grain LOcking fault tolerance would help). --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --q8Cb9hkoioj6BdV3flKX3IMTaTGcxqWjM 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/ iQEcBAEBCAAGBQJV4NiVAAoJEKeha0olJ0NqYugH+wXyHScFMkrg/aeEzhzd9Izl KcJbcuEey+neh8xSio/nN4CucOsIlXJII+FIXsakP7enMfAG/8cCcNyMhh+5dMfD 5iveMckHvnuJEdYTedzZOvajkxUAE4jQLSGf6FJs8dwhf7AvmRSSyH1mtRpJ6gJm LIb809KAnanJUzaPLO9spWJuqqccDhw+0KRUz5goaGRNhMgrVxOVWr1XgWhQMBOK ZJFMV3ROPieGGik5xWyfFqTzaY7NGZ83m/ACe+O3NNyczias9sUW1KPPfY57j+Kr oabmft3maV4TzXon7TG4o+Zf2GB6RJJw3kkBE2WYK5QScl3/c7PNmvCxydnft7Q= =vVnb -----END PGP SIGNATURE----- --q8Cb9hkoioj6BdV3flKX3IMTaTGcxqWjM--