From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35805) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YvuGs-0004aH-4e for qemu-devel@nongnu.org; Fri, 22 May 2015 17:14:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YvuGo-0005XS-Mc for qemu-devel@nongnu.org; Fri, 22 May 2015 17:14:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45850) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YvuGo-0005XG-EZ for qemu-devel@nongnu.org; Fri, 22 May 2015 17:14:18 -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 (8.14.4/8.14.4) with ESMTP id t4MLEH39016748 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Fri, 22 May 2015 17:14:17 -0400 Message-ID: <555F9C28.4040205@redhat.com> Date: Fri, 22 May 2015 15:14:16 -0600 From: Eric Blake MIME-Version: 1.0 References: <1432294585-5984-1-git-send-email-armbru@redhat.com> <1432294585-5984-2-git-send-email-armbru@redhat.com> In-Reply-To: <1432294585-5984-2-git-send-email-armbru@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="MIhosMhjH3swfRUOmB0afCpaLnS1dwlxC" Subject: Re: [Qemu-devel] [PATCH 01/20] monitor: Drop broken, unused asynchronous command interface List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , qemu-devel@nongnu.org Cc: lcapitulino@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --MIhosMhjH3swfRUOmB0afCpaLnS1dwlxC Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 05/22/2015 05:36 AM, Markus Armbruster wrote: > The asynchronous monitor command interface goes back to commit 940cc30 > (Jan 2010). Added a third case to command execution. The hope back > then according to the commit message was that all commands get > converted to the asynchronous interface, killing off the other two > cases. Didn't happen. >=20 > The initial asynchronous commands balloon and info balloon were > converted back to synchronous long ago (commit 96637bc and d72f32), > with commit messages calling the asynchronous interface "not fully > working" and "deprecated". The only other user went away in commit > 3b5704b. The history is helpful; thanks. >=20 > New code generally uses synchronous commands and asynchronous events. >=20 > What exactly is still "not fully working" with asynchronous commands? > Well, here's a bug that defeats actual asynchronous use pretty > reliably: the reply's ID is wrong (and has always been wrong) unless > you use the command synchronously! To reproduce, we need an > asynchronous command, so we have to go back before the commit 3b5704b. > Run QEMU with spice: >=20 > $ qemu-system-x86_64 -nodefaults -S -spice port=3D5900,disable-tick= eting -qmp stdio > {"QMP": {"version": {"qemu": {"micro": 50, "minor": 2, "major": > 2}, "package": ""}, "capabilities": []}} >=20 > Connect a spice client in another terminal: >=20 > $ remote-viewer spice://localhost:5900 >=20 > Set up a migration destination dummy in a third terminal: >=20 > $ socat TCP-LISTEN:12345 STDIO >=20 > Now paste the following into the QMP monitor: >=20 > { "execute": "qmp_capabilities" } If you stick and "id":"i0" here... > { "execute": "client_migrate_info", "id": "i1", "arguments": { "pro= tocol": "spice", "hostname": "localhost", "port": 12345 } } > { "execute": "query-kvm", "id": "i2" } >=20 > Produces two replies immediately, one to qmp_capabilities, and one to > query-kvm: >=20 > {"return": {}} =2E..it should show up here, for even less ambiguity about the problems with the async response. > {"return": {"enabled": false, "present": true}, "id": "i2"} >=20 > Both are correct. Two lines of debug output from libspice-server not > shown. >=20 > Now EOF socat's standard input to make it close the connection. This > makes the asynchronous client_migrate_info complete. It replies: >=20 > {"return": {}} >=20 > Bug: "id": "i1" is missing. Two lines of debug output from > libspice-server not shown. Cherry on top: storage for the missing ID > is leaked. >=20 > Get rid of this stuff before somebody hurts himself with it. >=20 > Signed-off-by: Markus Armbruster > --- > include/monitor/monitor.h | 5 ---- > monitor.c | 68 ++-------------------------------------= -------- > 2 files changed, 2 insertions(+), 71 deletions(-) Certainly takes an axe to it! Reviewed-by: Eric Blake --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --MIhosMhjH3swfRUOmB0afCpaLnS1dwlxC 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/ iQEcBAEBCAAGBQJVX5woAAoJEKeha0olJ0NqWZ4H/ij1fyM0Kqewx+sMx6alG8kP hEWO7AOmtmQ+YJfGegCzdg2Bv2pSYOkkzu2gJufur9fDAUGKXJxbiqJQOkqQSYnI gq6zfsLoscHo5IsOd3WPnXS4+ySwVoin1lU5WWbG3d8zCnv8DtlGIq/VcFOB2rFi XXacFxW9M9xP+DFhGchOzsFHKWTA+YAiqRqhe4vB4VDpQfQcnd1h38G9OSahCKTL ZkBWN1Xsplvx0z3/H4ck++ZtJYoacXrI0Eq9AcHxQtxWpvN8ioZgjD1Bm/ubie2+ K5tAgbohaG1EB0coG0nIHeVeiZ5AqRbSvU7mpui+P396+RrSkR+9WUomnEQ7xD8= =3ZLM -----END PGP SIGNATURE----- --MIhosMhjH3swfRUOmB0afCpaLnS1dwlxC--