From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:56441) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S9jOc-0005bI-92 for qemu-devel@nongnu.org; Mon, 19 Mar 2012 16:41:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S9jOa-0000B5-90 for qemu-devel@nongnu.org; Mon, 19 Mar 2012 16:41:37 -0400 Received: from mail-gx0-f173.google.com ([209.85.161.173]:58863) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S9jOa-0000Av-4t for qemu-devel@nongnu.org; Mon, 19 Mar 2012 16:41:36 -0400 Received: by ggnj2 with SMTP id j2so6786088ggn.4 for ; Mon, 19 Mar 2012 13:41:34 -0700 (PDT) Message-ID: <4F6799FB.9030606@codemonkey.ws> Date: Mon, 19 Mar 2012 15:41:31 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1332169763-30665-1-git-send-email-aliguori@us.ibm.com> <1332169763-30665-8-git-send-email-aliguori@us.ibm.com> <4F6792B6.9080508@redhat.com> <4F6794CF.9070409@codemonkey.ws> <4F6797B3.7030405@redhat.com> In-Reply-To: <4F6797B3.7030405@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 7/9] qmp: expose a command to query capabilities of config parser List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-devel@nongnu.org, Anthony Liguori , Gerd Hoffman , Eduardo Habkost On 03/19/2012 03:31 PM, Eric Blake wrote: > On 03/19/2012 02:19 PM, Anthony Liguori wrote: >>>> +# @size: an integer followed by either 'K', 'M', 'G', or 'T'. >>> >>> Not quite - I found three different parsers, but none of them match this >>> description. >> >> Heh, I was going by the documentation in the code: >> >> QEMU_OPT_SIZE, /* size, accepts (K)ilo, (M)ega, (G)iga, >> (T)era postfix */ >> >> >>> That is, qemu-option.c:parse_option_size() supports 'k' >>> and 'b', as well as omitting a suffix, but does not support 'm'. >> >> TBH, I'd prefer to not document 'k' and 'b' and explicitly document that >> the suffix is optional and bytes is implied (which I thought was implied >> in my documentation, but I can see that it may not be). >> >> I'd rather we support officially support one way of doing things and if >> we support more, do it under the mantra of being liberal in what we accept. > > Okay, I can live with documenting less than we accept, but we definitely > need to mention that the suffix is optional, and that when omitted, the > unit is bytes (as originally written, I read it that the suffix was > mandatory, and that there was no way to request bytes). I'm not sure > whether documenting an explicit 'b' for bytes helps or hurts, and I'm > not sure whether adding support for 'p' and 'e' as documented suffixes > makes sense. I'm not really concerned about 'p' and 'e' for now. 'T' still seems novel enough to me :-) >>> Then >>> there's cmd.c:cvtnum(), which is case-insensitive, and adds 'p' and 'e', >>> but omits 'b'. Then there's the 'o' type in monitor.c that defers to >>> cutils.c:strtosz(), which defaults to bytes but is case-insensitive and >>> supports 'b' but not 'p'. Why are we using three different parsers, >>> anyway? >> >> Yes, I am aware of that, and yes, I'm going to fix it. One bit at a >> time though. > > Fair enough :) It's just that I recently fixed a bug in libvirt where > it was using a human interface with no suffix and getting 'M' behavior, > where an explicit 'B' behavior fixed things to use bytes, and I was > quite put off by the inconsistencies and lack of documentation when I > finally discovered that a 'B' suffix did what I wanted. Yup, and hopefully qapi-schema.json is fixing the documentation problem here. For what it's worth, my aim is to refactor the QemuOpts definitions to be part of qapi-schema.json too such that they carry the same level of documentation (and self consistency). This is the advantage of having a single centralized schema. I think it has really helped make sure everything is consistently documented and specified. Having disjoint definitions and descriptions has led to a lot of inconsistency over time. > I'm glad that > QMP defaults to 'B', not 'M'. (Libvirt had its own fair share of > inconsistent scaling routines, where some interfaces default to 'k' > instead of bytes, and I recently consolidated libvirt to use a single > scaling routing for much the same reasons.) You can thank Xen for the kb default :-) That goes back to the libvir days. Regards, Anthony Liguori