From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58731) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UsKIC-0002tH-7n for qemu-devel@nongnu.org; Thu, 27 Jun 2013 18:03:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UsKIB-00042H-0O for qemu-devel@nongnu.org; Thu, 27 Jun 2013 18:03:52 -0400 Received: from mx1.redhat.com ([209.132.183.28]:10151) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UsKIA-000429-P1 for qemu-devel@nongnu.org; Thu, 27 Jun 2013 18:03:50 -0400 Message-ID: <51CCB6C4.1020800@redhat.com> Date: Thu, 27 Jun 2013 16:03:48 -0600 From: Eric Blake MIME-Version: 1.0 References: <1372246684-12499-1-git-send-email-kraxel@redhat.com> <1372246684-12499-2-git-send-email-kraxel@redhat.com> <20130626115635.5271afb3@redhat.com> <51CC0E6D.1000808@redhat.com> <20130627092906.31f51f1e@redhat.com> In-Reply-To: <20130627092906.31f51f1e@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="----enig2EFGJRBQKGADPSHBBFBVI" Subject: Re: [Qemu-devel] [PATCH 2/2] fbdev: add monitor commands to enable/disable/query List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: Markus Armbruster , Anthony Liguori , Gerd Hoffmann , qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2EFGJRBQKGADPSHBBFBVI Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 06/27/2013 07:29 AM, Luiz Capitulino wrote: > On Thu, 27 Jun 2013 12:05:33 +0200 > Gerd Hoffmann wrote: >=20 >>>> +# @device: #optional specifies framebuffer device, default: /dev/fb= 0 >>> >>> Actually, it will try to get the device name from an env variable fir= st, >>> which sounds too automatic for a building block API like QMP. >> >> The env variable is icky indeed. But a fixed default (which will be t= he >> one you need in 99% of the cases) is fine IMO. >=20 > I prefer having no defaults, but I'm not strong about this. I also think that letting HMP leave the parameter optional, with first-level fallback to the environment variable and secondary fallback to the hardcoded /dev/fv0, while making the parameter mandatory in QMP, makes more sense. >=20 >> >>>> +# Since 1.6 >>>> +## >>>> +{ 'type': 'FramebufferInfo', >>>> + 'data': { 'enabled': 'bool', >>>> + '*scale' : 'bool', >>>> + '*device': 'str', >>> >>> Why is device optional? >> >> When the framebuffer is'nt active you'll get just >> { "enabled" : "false" }, thats why the other ones are optional. They >> all are filled in case the framebuffer is active. Would it be any simpler to just have the query command fail when the framebuffer is disabled, leave out the 'enabled' member from FramebufferInfo, and have all the other members mandatory? Also, would it ever make sense for a future enhancement to allow having more than one framebuffer in use by a single domain? Even if your initial implementation supports at most one /dev/fb* device per guest, it might be worth having the query-framebuffer command return an array of all enabled framebuffers (a 0-length array when none is enabled), rather than hard-coding it to a single FramebufferInfo return. Likewise, make sure that framebuffer-display can deal with extension if it makes sense (the suggestion to split into -enable and -disable commands might be better stated as splitting into framebuffer-display-add and framebuffer-display-delete, more like hotplugging of other devices). --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org ------enig2EFGJRBQKGADPSHBBFBVI 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/ iQEcBAEBCAAGBQJRzLbEAAoJEKeha0olJ0NqcFkH/RVF9nbkGMGP6ZCz2gTm18TR Paj9BVGPa+G1W16bftLmTt3PfwrIVwd5DE8hDZFPtkqaFy+vI8NE4WtkbjuuTq59 ztkHdv+DDerBENHN9hwzaaAU+A9JxSvlk+6BriS5JXubWZsttGlvZsj26Jehanj+ A3aI4Ituof1JXyUSpJ+b/ifi3T7rX2Wi4BKhcnhVkBMtX9dLeCbEMHeyw14+h/M8 Y2yC4OGr1+RrzaceYfvLiK/Dz7KO3jXU09G4rrrpA1yFEb+hakBl+At9YCsj3irK Gf2eg5b/ADSA85XXZt9fkcN7UHFP4nzqL3YJyhYys5XB8YIwsN30lfgVViaajAk= =n6IE -----END PGP SIGNATURE----- ------enig2EFGJRBQKGADPSHBBFBVI--