From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:46413)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <lcapitulino@redhat.com>) id 1THadI-0008V4-HD
	for qemu-devel@nongnu.org; Fri, 28 Sep 2012 09:29:38 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <lcapitulino@redhat.com>) id 1THadD-0000q4-HA
	for qemu-devel@nongnu.org; Fri, 28 Sep 2012 09:29:32 -0400
Received: from mx1.redhat.com ([209.132.183.28]:50811)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <lcapitulino@redhat.com>) id 1THadD-0000pn-8r
	for qemu-devel@nongnu.org; Fri, 28 Sep 2012 09:29:27 -0400
Date: Fri, 28 Sep 2012 10:30:17 -0300
From: Luiz Capitulino <lcapitulino@redhat.com>
Message-ID: <20120928103017.1d1cda37@doriath.home>
In-Reply-To: <50659FAB.7020909@redhat.com>
References: <1348752509-1142-1-git-send-email-lcapitulino@redhat.com>
	<1348752509-1142-14-git-send-email-lcapitulino@redhat.com>
	<50657DC5.9000205@redhat.com> <50659FAB.7020909@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PULL 13/15] qmp: qmp_send_key(): accept key codes
 in hex
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Eric Blake <eblake@redhat.com>
Cc: aliguori@us.ibm.com, Amos Kong <akong@redhat.com>, qemu-devel@nongnu.org

On Fri, 28 Sep 2012 07:01:31 -0600
Eric Blake <eblake@redhat.com> wrote:

> On 09/28/2012 04:36 AM, Amos Kong wrote:
> > On 27/09/12 21:28, Luiz Capitulino wrote:
> >>
> > 
> > Sorry the delay review.
> > 
> 
> > 
> > hex isn't support when using qmp monitor.
> > 
> 
> >> +{ 'union': 'KeyValue',
> >> +  'data': {
> >> +    'number': 'int',
> > 
> > It's 'int' not hex format.
> 
> Indeed - I just re-read the JSON overview at http://www.json.org/, which
> is explicit that numbers are decimal only (no octal or hexadecimal
> support, and no support for a leading 0 except when the number is
> exactly 0).  [Serves me right for not realizing this aspect of JSON when
> I did my review earlier.]

Sorry if I wasn't clear or if my use of "hex" caused confusion, but
it's known that the only way of having an hex value on the wire is
to make it a string. As the base number we use on the wire is pretty
irrelevant (at least for this case) I don't see any issue here (actually,
for json this is pretty natural).

> I don't think this invalidates the QMP (libvirt already sends decimal,
> and wasn't planning on sending hex), but you DO have a point that:
> 
> > 
> > qmp-commands.hx also needs to be updated with latest examples:
> 
> Since this is already in the PULL request, I think the docs touchup
> could be a separate patch.

Yes.