From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41516) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1USqrR-0000Bp-Qc for qemu-devel@nongnu.org; Thu, 18 Apr 2013 11:35:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1USqrN-0007GK-0o for qemu-devel@nongnu.org; Thu, 18 Apr 2013 11:34:57 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50131) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1USqrM-0007GE-OY for qemu-devel@nongnu.org; Thu, 18 Apr 2013 11:34:52 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r3IFYpJt013599 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 18 Apr 2013 11:34:52 -0400 Message-ID: <5170129A.8010807@redhat.com> Date: Thu, 18 Apr 2013 09:34:50 -0600 From: Eric Blake MIME-Version: 1.0 References: <1366275680-15416-1-git-send-email-kraxel@redhat.com> <516FE2AB.5060304@redhat.com> <20130418112106.36ca6c51@redhat.com> In-Reply-To: <20130418112106.36ca6c51@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="----enig2MQTBMMXKDBTKEDWDGMJF" Subject: Re: [Qemu-devel] [RfC PATCH 0/5] console: qom-ify & extent screendump monitor command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: Gerd Hoffmann , qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2MQTBMMXKDBTKEDWDGMJF Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 04/18/2013 09:21 AM, Luiz Capitulino wrote: >>> Question for the libvirt guys: Is it ok for libvirt to just extend t= he >>> existing screendump command? Can libvirt figure there is a new >>> (optional) parameter? See patch #5. >> >> So yes, I think libvirt will be able to drive the new command by knowi= ng >> how many heads appear per device, then passing in the appropriate name= d >> device to the QMP command. And yes, I'll review patch 5 regarding >> interface design. >=20 > We can extend screendump on HMP, but the general rule for QMP is to add= a > new command instead so that clients don't have to play tricks like Eric= is > suggesting :) The problem at hand is that your proposal in patch 5: -{ 'command': 'screendump', 'data': {'filename': 'str'} } +{ 'command': 'screendump', 'data': {'filename': 'str', + '*device' : 'str'} } still doesn't support the case of dumping just one head of a multi-head device. Libvirt's API is already adequately flexible to cover an arbitrary head of an arbitrary device, once mapped down to QMP, so the question at hand now is whether to map it down to QMP by adding optional parameters to the existing command or adding a new command. >=20 > Is there any big issue with adding a new command? I haven't yet mentioned any tricks, but now that you bring it up, there are two options: Option 1: reuse the same QMP command, but add optional parameters (ability to specify device in this patch, but libvirt would also want the ability to specify which head of a multi-head device). Libvirt always calls screendump, and omits the optional parameters if the user asked libvirt for screen 0. If user asks libvirt for screen 1, libvirt passes the optional parameter, and if qemu is too old, qemu gives an error about invalid argument, which libvirt then feeds back to the user as a 'screen 1 not supported'. Option 2: existing 'screendump' command can ONLY dump the primary head of the primary device, and a new QMP command is added that supports head and device selection. Libvirt uses query-commands to determine whether new command has been backported; if so, it uses the new command, if not, it gives the user a nice error unless screen 0 was selected. Option 2 is probably slightly nicer for guaranteeing a sane error message back to the user, but option 1 (the approach of this series) still seems workable. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org ------enig2MQTBMMXKDBTKEDWDGMJF 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.13 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJRcBKaAAoJEKeha0olJ0NqlAsH/Aj7VGfqzOJ0mkxkxTfj55n5 BjEDLG7Ru3ybo5PnyOugShvxMZ6q7VU+kMwAM+AbSH/BVX9qIq2jdL+THsEVrTRs ydjFp38GaJqSpPBsJcvG4WZOpqVDcSAdl6ZRBrWDjSr0DYXOOVnWLcvNbJF3WMim VFqfAhuPizfvaxST9tovkfOZpZEzIlCkeL53fSgVb5fyUubUBZlhMPZHisdB3fsi oK20KA4DTs0E9U8BEZzFn4S93n1L2T93uZWv2GWzWzz1Ns7PZ1y7zUnGEPFDYTxG V1NPAu2Qj4DwQdQr8zXs8hJS7X/8yMMDod0PERIls0zURL35AQrnNPvCTOZbZ4Y= =Nqqg -----END PGP SIGNATURE----- ------enig2MQTBMMXKDBTKEDWDGMJF--