qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 4/4] qapi: Rename ChardevBackend member "memory"


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH 4/4] qapi: Rename ChardevBackend member "memory" to "ringbuf"
Date: Mon, 01 Jul 2013 11:28:58 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux)

Amos Kong <address@hidden> writes:

> On Mon, Jul 01, 2013 at 10:10:40AM +0200, Markus Armbruster wrote:
>> Amos Kong <address@hidden> writes:
>> 
>> > On Fri, Jun 28, 2013 at 07:19:39PM +0200, Markus Armbruster wrote:
>> >> Eric Blake <address@hidden> writes:
>> >> 
>> >> > On 06/27/2013 08:22 AM, Markus Armbruster wrote:
>> >> >> Commit 1da48c6 called the new member "memory" after commit 3949e59
>> >> >> standardized "ringbuf".  Rename for consistency.
>> >> >> 
>> >> >> However, member name "memory" is visible in QMP since 1.5.  It's
>> >> >> undocumented just like the driver name.  Keep it working anyway.
>> >> >> 
>> >> >> Cc: address@hidden
>> >> >> Signed-off-by: Markus Armbruster <address@hidden>
>> >> >> ---
>> >> >>  qapi-schema.json |  6 ++++--
>> >> >>  qemu-char.c      | 11 ++++++-----
>> >> >>  2 files changed, 10 insertions(+), 7 deletions(-)
>> >> >> 
>> >> >> diff --git a/qapi-schema.json b/qapi-schema.json
>> >> >> index 6445da6..b3df8a5 100644
>> >> >> --- a/qapi-schema.json
>> >> >> +++ b/qapi-schema.json
>> >> >> @@ -3277,9 +3277,9 @@
>> >> >>  ##
>> >> >>  # @ChardevRingbuf:
>> >> >>  #
>> >> >> -# Configuration info for memory chardevs
>> >> >> +# Configuration info for ring buffer chardevs.
>> >> >>  #
>> >> >> -# @size: #optional Ringbuffer size, must be power of two, default is 
>> >> >> 65536
>> >> >> +# @size: #optional ring buffer size, must be power of two, default is 
>> >> >> 65536
>> >> >>  #
>> >> >>  # Since: 1.5
>> >> >>  ##
>> >> >> @@ -3310,6 +3310,8 @@
>> >> >>                                         'spicevmc' : 
>> >> >> 'ChardevSpiceChannel',
>> >> >>                                         'spiceport' : 
>> >> >> 'ChardevSpicePort',
>> >> >>                                         'vc'     : 'ChardevVC',
>> >> >> +                                       'ringbuf': 'ChardevRingbuf',
>> >> >> +                                       # next one is just for 
>> >> >> compatibility
>> >> >>                                         'memory' : 'ChardevRingbuf' } }
>> >> >
>> >> > Does JSON allow comments in the middle of content?  Is this going to
>> >> > screw up Amos' work on introspection?  You may need to instead have a
>> >> > comment before the open '{' stating that 'memory' is an alias within the
>> >> > union for back-compat reasons.
>> >
>> > I didn't parse the json file by myself. I just used the parsed
>> > dictionary. So it only needs to make qapi.py happy.
>> >
>> >> RFC 4627 doesn't do comments at all.
>> >> 
>> >> This file is parsed by scripts/qapi.py, which as far as I can tell
>> >> ignores lines starting with '#' anywhere in the input.
>> >
>> > Not anywhere, only start with '#'
>> 
>> Isn't that what I said?
>> 
>> > | def parse_schema(fp):
>> > |     exprs = []
>> > |     raw_exprs = []
>> > |     expr = ''
>> > |     expr_eval = None
>> > | 
>> > |     for line in fp:
>> > |         if line.startswith('#') or line == '\n':
>> >
>> >       # ignores lines starting with '#' anywhere
>> >           if line.strip().startswith('#')   
>> >
>> > |             continue
>> > | 
>> >
>> >
>> > So we should not add this kind of comment for back-compat.
>> 
>> Now I'm confused.  My patch adds a line that starts with '#'.
>> parse_schema() ignores it.  Works as designed.  Why do you think we
>> shouldn't do that?
>
> The comment line in your patch doen't start with '#', it starts with
> blank-space. If we want qapi.py to process it, we need to do strip()
> first.

Aha.  I agree with your reading of parse_schema().  However, I get
exactly identical generated files with and without this comment.

Michael, Anthony, can you explain why?

Any particular reason for requiring comments to start in column 0?



reply via email to

[Prev in Thread] Current Thread [Next in Thread]