Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2017-03-16' into staging

QAPI patches for 2017-03-16

# gpg: Signature made Thu 16 Mar 2017 06:18:38 GMT
# gpg:                using RSA key 0x3870B400EB918653
# gpg: Good signature from "Markus Armbruster <armbru@redhat.com>"
# gpg:                 aka "Markus Armbruster <armbru@pond.sub.org>"
# Primary key fingerprint: 354B C8B3 D7EB 2A6B 6867  4E5F 3870 B400 EB91 8653

* remotes/armbru/tags/pull-qapi-2017-03-16: (49 commits)
  qapi: Fix a misleading parser error message
  qapi: Make pylint a bit happier
  qapi: Drop unused .check_clash() parameter schema
  qapi: union_types is a list used like a dict, make it one
  qapi: struct_types is a list used like a dict, make it one
  qapi: enum_types is a list used like a dict, make it one
  qapi: Factor add_name() calls out of the meta conditional
  qapi: Simplify what gets stored in enum_types
  qapi: Drop unused variable events
  qapi: Eliminate check_docs() and drop QAPIDoc.expr
  qapi: Fix detection of bogus member documentation
  tests/qapi-schema: Improve coverage of bogus member docs
  tests/qapi-schema: Rename doc-bad-args to doc-bad-command-arg
  qapi: Move empty doc section checking to doc parser
  qapi: Improve error message on @NAME: in free-form doc
  qapi: Move detection of doc / expression name mismatch
  qapi: Fix detection of doc / expression mismatch
  tests/qapi-schema: Improve doc / expression mismatch coverage
  qapi2texi: Use category "Object" for all object types
  qapi2texi: Generate descriptions for simple union tags
  ...

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

8 files changed, 492 insertions, 484 deletions

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 0c05449cb6..1943de4852 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -13,7 +13,6 @@
 # See the COPYING file in the top-level directory.
 
 from qapi import *
-import re
 
 
 def gen_command_decl(name, arg_type, boxed, ret_type):
@@ -84,7 +83,8 @@ static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out,
 
 
 def gen_marshal_proto(name):
-    return 'void qmp_marshal_%s(QDict *args, QObject **ret, Error **errp)' % c_name(name)
+    return ('void qmp_marshal_%s(QDict *args, QObject **ret, Error **errp)'
+            % c_name(name))
 
 
 def gen_marshal_decl(name):
@@ -198,7 +198,7 @@ def gen_register_command(name, success_response):
         options = 'QCO_NO_SUCCESS_RESP'
 
     ret = mcgen('''
-    qmp_register_command(cmds, "%(name)s", 
+    qmp_register_command(cmds, "%(name)s",
                          qmp_marshal_%(c_name)s, %(opts)s);
 ''',
                 name=name, c_name=c_name(name),
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index f4eb7f85b1..0485e39145 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -223,7 +223,7 @@ fdecl.write(mcgen('''
 ''',
                   prefix=prefix))
 
-event_enum_name = c_name(prefix + "QAPIEvent", protect=False)
+event_enum_name = c_name(prefix + 'QAPIEvent', protect=False)
 
 schema = QAPISchema(input_file)
 gen = QAPISchemaGenEventVisitor()
diff --git a/scripts/qapi-introspect.py b/scripts/qapi-introspect.py
index fb72c61d02..032bcea491 100644
--- a/scripts/qapi-introspect.py
+++ b/scripts/qapi-introspect.py
@@ -170,10 +170,10 @@ const char %(c_name)s[] = %(c_string)s;
 opt_unmask = False
 
 (input_file, output_dir, do_c, do_h, prefix, opts) = \
-    parse_command_line("u", ["unmask-non-abi-names"])
+    parse_command_line('u', ['unmask-non-abi-names'])
 
 for o, a in opts:
-    if o in ("-u", "--unmask-non-abi-names"):
+    if o in ('-u', '--unmask-non-abi-names'):
         opt_unmask = True
 
 c_comment = '''
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index dabc42e047..b45e7b5634 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -244,10 +244,10 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
 do_builtins = False
 
 (input_file, output_dir, do_c, do_h, prefix, opts) = \
-    parse_command_line("b", ["builtins"])
+    parse_command_line('b', ['builtins'])
 
 for o, a in opts:
-    if o in ("-b", "--builtins"):
+    if o in ('-b', '--builtins'):
         do_builtins = True
 
 c_comment = '''
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 330b9f321b..5737aefa05 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -13,7 +13,6 @@
 # See the COPYING file in the top-level directory.
 
 from qapi import *
-import re
 
 
 def gen_visit_decl(name, scalar=False):
@@ -335,10 +334,10 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
 do_builtins = False
 
 (input_file, output_dir, do_c, do_h, prefix, opts) = \
-    parse_command_line("b", ["builtins"])
+    parse_command_line('b', ['builtins'])
 
 for o, a in opts:
-    if o in ("-b", "--builtins"):
+    if o in ('-b', '--builtins'):
         do_builtins = True
 
 c_comment = '''
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 53a44779d0..e88c047c2e 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -11,13 +11,13 @@
 # This work is licensed under the terms of the GNU GPL, version 2.
 # See the COPYING file in the top-level directory.
 
-import re
-from ordereddict import OrderedDict
 import errno
 import getopt
 import os
-import sys
+import re
 import string
+import sys
+from ordereddict import OrderedDict
 
 builtin_types = {
     'str':      'QTYPE_QSTRING',
@@ -37,44 +37,18 @@ builtin_types = {
     'QType':    'QTYPE_QSTRING',
 }
 
+# Are documentation comments required?
+doc_required = False
+
 # Whitelist of commands allowed to return a non-dictionary
-returns_whitelist = [
-    # From QMP:
-    'human-monitor-command',
-    'qom-get',
-    'query-migrate-cache-size',
-    'query-tpm-models',
-    'query-tpm-types',
-    'ringbuf-read',
-
-    # From QGA:
-    'guest-file-open',
-    'guest-fsfreeze-freeze',
-    'guest-fsfreeze-freeze-list',
-    'guest-fsfreeze-status',
-    'guest-fsfreeze-thaw',
-    'guest-get-time',
-    'guest-set-vcpus',
-    'guest-sync',
-    'guest-sync-delimited',
-]
+returns_whitelist = []
 
 # Whitelist of entities allowed to violate case conventions
-case_whitelist = [
-    # From QMP:
-    'ACPISlotType',         # DIMM, visible through query-acpi-ospm-status
-    'CpuInfoMIPS',          # PC, visible through query-cpu
-    'CpuInfoTricore',       # PC, visible through query-cpu
-    'QapiErrorClass',       # all members, visible through errors
-    'UuidInfo',             # UUID, visible through query-uuid
-    'X86CPURegister32',     # all members, visible indirectly through qom-get
-    'q_obj_CpuInfo-base',   # CPU, visible through query-cpu
-]
-
-enum_types = []
-struct_types = []
-union_types = []
-events = []
+name_case_whitelist = []
+
+enum_types = {}
+struct_types = {}
+union_types = {}
 all_names = {}
 
 #
@@ -83,9 +57,9 @@ all_names = {}
 
 
 def error_path(parent):
-    res = ""
+    res = ''
     while parent:
-        res = ("In file included from %s:%d:\n" % (parent['file'],
+        res = ('In file included from %s:%d:\n' % (parent['file'],
                                                    parent['line'])) + res
         parent = parent['parent']
     return res
@@ -101,10 +75,10 @@ class QAPIError(Exception):
         self.msg = msg
 
     def __str__(self):
-        loc = "%s:%d" % (self.fname, self.line)
+        loc = '%s:%d' % (self.fname, self.line)
         if self.col is not None:
-            loc += ":%s" % self.col
-        return error_path(self.info) + "%s: %s" % (loc, self.msg)
+            loc += ':%s' % self.col
+        return error_path(self.info) + '%s: %s' % (loc, self.msg)
 
 
 class QAPIParseError(QAPIError):
@@ -132,15 +106,21 @@ class QAPIDoc(object):
             self.name = name
             # the list of lines for this section
             self.content = []
+            self.optional = False
 
         def append(self, line):
             self.content.append(line)
 
         def __repr__(self):
-            return "\n".join(self.content).strip()
+            return '\n'.join(self.content).strip()
 
     class ArgSection(Section):
-        pass
+        def __init__(self, name):
+            QAPIDoc.Section.__init__(self, name)
+            self.member = None
+
+        def connect(self, member):
+            self.member = member
 
     def __init__(self, parser, info):
         # self.parser is used to report errors with QAPIParseError.  The
@@ -157,8 +137,6 @@ class QAPIDoc(object):
         self.sections = []
         # the current section
         self.section = self.body
-        # associated expression (to be set by expression parser)
-        self.expr = None
 
     def has_section(self, name):
         """Return True if we have a section with this name."""
@@ -182,8 +160,8 @@ class QAPIDoc(object):
         # recognized, and get silently treated as ordinary text
         if self.symbol:
             self._append_symbol_line(line)
-        elif not self.body.content and line.startswith("@"):
-            if not line.endswith(":"):
+        elif not self.body.content and line.startswith('@'):
+            if not line.endswith(':'):
                 raise QAPIParseError(self.parser, "Line should end with :")
             self.symbol = line[1:-1]
             # FIXME invalid names other than the empty string aren't flagged
@@ -192,17 +170,20 @@ class QAPIDoc(object):
         else:
             self._append_freeform(line)
 
+    def end_comment(self):
+        self._end_section()
+
     def _append_symbol_line(self, line):
         name = line.split(' ', 1)[0]
 
-        if name.startswith("@") and name.endswith(":"):
+        if name.startswith('@') and name.endswith(':'):
             line = line[len(name)+1:]
             self._start_args_section(name[1:-1])
-        elif name in ("Returns:", "Since:",
+        elif name in ('Returns:', 'Since:',
                       # those are often singular or plural
-                      "Note:", "Notes:",
-                      "Example:", "Examples:",
-                      "TODO:"):
+                      'Note:', 'Notes:',
+                      'Example:', 'Examples:',
+                      'TODO:'):
             line = line[len(name)+1:]
             self._start_section(name[:-1])
 
@@ -219,16 +200,26 @@ class QAPIDoc(object):
             raise QAPIParseError(self.parser,
                                  "'@%s:' can't follow '%s' section"
                                  % (name, self.sections[0].name))
+        self._end_section()
         self.section = QAPIDoc.ArgSection(name)
         self.args[name] = self.section
 
-    def _start_section(self, name=""):
-        if name in ("Returns", "Since") and self.has_section(name):
+    def _start_section(self, name=''):
+        if name in ('Returns', 'Since') and self.has_section(name):
             raise QAPIParseError(self.parser,
                                  "Duplicated '%s' section" % name)
+        self._end_section()
         self.section = QAPIDoc.Section(name)
         self.sections.append(self.section)
 
+    def _end_section(self):
+        if self.section:
+            contents = str(self.section)
+            if self.section.name and (not contents or contents.isspace()):
+                raise QAPIParseError(self.parser, "Empty doc section '%s'"
+                                     % self.section.name)
+            self.section = None
+
     def _append_freeform(self, line):
         in_arg = isinstance(self.section, QAPIDoc.ArgSection)
         if (in_arg and self.section.content
@@ -236,10 +227,39 @@ class QAPIDoc(object):
                 and line and not line[0].isspace()):
             self._start_section()
         if (in_arg or not self.section.name
-                or not self.section.name.startswith("Example")):
+                or not self.section.name.startswith('Example')):
             line = line.strip()
+        match = re.match(r'(@\S+:)', line)
+        if match:
+            raise QAPIParseError(self.parser,
+                                 "'%s' not allowed in free-form documentation"
+                                 % match.group(1))
+        # TODO Drop this once the dust has settled
+        if (isinstance(self.section, QAPIDoc.ArgSection)
+                and '#optional' in line):
+            raise QAPISemError(self.info, "Please drop the #optional tag")
         self.section.append(line)
 
+    def connect_member(self, member):
+        if member.name not in self.args:
+            # Undocumented TODO outlaw
+            self.args[member.name] = QAPIDoc.ArgSection(member.name)
+        self.args[member.name].connect(member)
+
+    def check_expr(self, expr):
+        if self.has_section('Returns') and 'command' not in expr:
+            raise QAPISemError(self.info,
+                               "'Returns:' is only valid for commands")
+
+    def check(self):
+        bogus = [name for name, section in self.args.iteritems()
+                 if not section.member]
+        if bogus:
+            raise QAPISemError(
+                self.info,
+                "The following documented members are not in "
+                "the declaration: %s" % ", ".join(bogus))
+
 
 class QAPISchemaParser(object):
 
@@ -257,55 +277,102 @@ class QAPISchemaParser(object):
         self.line_pos = 0
         self.exprs = []
         self.docs = []
+        self.cur_doc = None
         self.accept()
 
         while self.tok is not None:
             info = {'file': fname, 'line': self.line,
                     'parent': self.incl_info}
             if self.tok == '#':
-                doc = self.get_doc(info)
-                self.docs.append(doc)
+                self.reject_expr_doc()
+                self.cur_doc = self.get_doc(info)
+                self.docs.append(self.cur_doc)
                 continue
 
             expr = self.get_expr(False)
-            if isinstance(expr, dict) and "include" in expr:
+            if 'include' in expr:
+                self.reject_expr_doc()
                 if len(expr) != 1:
                     raise QAPISemError(info, "Invalid 'include' directive")
-                include = expr["include"]
+                include = expr['include']
                 if not isinstance(include, str):
                     raise QAPISemError(info,
                                        "Value of 'include' must be a string")
-                incl_abs_fname = os.path.join(os.path.dirname(abs_fname),
-                                              include)
-                # catch inclusion cycle
-                inf = info
-                while inf:
-                    if incl_abs_fname == os.path.abspath(inf['file']):
-                        raise QAPISemError(info, "Inclusion loop for %s"
-                                           % include)
-                    inf = inf['parent']
-
-                # skip multiple include of the same file
-                if incl_abs_fname in previously_included:
-                    continue
-                try:
-                    fobj = open(incl_abs_fname, 'r')
-                except IOError as e:
-                    raise QAPISemError(info, '%s: %s' % (e.strerror, include))
-                exprs_include = QAPISchemaParser(fobj, previously_included,
-                                                 info)
-                self.exprs.extend(exprs_include.exprs)
-                self.docs.extend(exprs_include.docs)
+                self._include(include, info, os.path.dirname(abs_fname),
+                              previously_included)
+            elif "pragma" in expr:
+                self.reject_expr_doc()
+                if len(expr) != 1:
+                    raise QAPISemError(info, "Invalid 'pragma' directive")
+                pragma = expr['pragma']
+                if not isinstance(pragma, dict):
+                    raise QAPISemError(
+                        info, "Value of 'pragma' must be a dictionary")
+                for name, value in pragma.iteritems():
+                    self._pragma(name, value, info)
             else:
                 expr_elem = {'expr': expr,
                              'info': info}
-                if (self.docs
-                        and self.docs[-1].info['file'] == fname
-                        and not self.docs[-1].expr):
-                    self.docs[-1].expr = expr
-                    expr_elem['doc'] = self.docs[-1]
-
+                if self.cur_doc:
+                    if not self.cur_doc.symbol:
+                        raise QAPISemError(
+                            self.cur_doc.info,
+                            "Expression documentation required")
+                    expr_elem['doc'] = self.cur_doc
                 self.exprs.append(expr_elem)
+            self.cur_doc = None
+        self.reject_expr_doc()
+
+    def reject_expr_doc(self):
+        if self.cur_doc and self.cur_doc.symbol:
+            raise QAPISemError(
+                self.cur_doc.info,
+                "Documentation for '%s' is not followed by the definition"
+                % self.cur_doc.symbol)
+
+    def _include(self, include, info, base_dir, previously_included):
+        incl_abs_fname = os.path.join(base_dir, include)
+        # catch inclusion cycle
+        inf = info
+        while inf:
+            if incl_abs_fname == os.path.abspath(inf['file']):
+                raise QAPISemError(info, "Inclusion loop for %s" % include)
+            inf = inf['parent']
+
+        # skip multiple include of the same file
+        if incl_abs_fname in previously_included:
+            return
+        try:
+            fobj = open(incl_abs_fname, 'r')
+        except IOError as e:
+            raise QAPISemError(info, '%s: %s' % (e.strerror, include))
+        exprs_include = QAPISchemaParser(fobj, previously_included, info)
+        self.exprs.extend(exprs_include.exprs)
+        self.docs.extend(exprs_include.docs)
+
+    def _pragma(self, name, value, info):
+        global doc_required, returns_whitelist, name_case_whitelist
+        if name == 'doc-required':
+            if not isinstance(value, bool):
+                raise QAPISemError(info,
+                                   "Pragma 'doc-required' must be boolean")
+            doc_required = value
+        elif name == 'returns-whitelist':
+            if (not isinstance(value, list)
+                    or any([not isinstance(elt, str) for elt in value])):
+                raise QAPISemError(info,
+                                   "Pragma returns-whitelist must be"
+                                   " a list of strings")
+            returns_whitelist = value
+        elif name == 'name-case-whitelist':
+            if (not isinstance(value, list)
+                    or any([not isinstance(elt, str) for elt in value])):
+                raise QAPISemError(info,
+                                   "Pragma name-case-whitelist must be"
+                                   " a list of strings")
+            name_case_whitelist = value
+        else:
+            raise QAPISemError(info, "Unknown pragma '%s'" % name)
 
     def accept(self, skip_comment=True):
         while True:
@@ -322,7 +389,7 @@ class QAPISchemaParser(object):
                 if not skip_comment:
                     self.val = self.src[self.pos:self.cursor]
                     return
-            elif self.tok in "{}:,[]":
+            elif self.tok in '{}:,[]':
                 return
             elif self.tok == "'":
                 string = ''
@@ -348,7 +415,7 @@ class QAPISchemaParser(object):
                             for _ in range(0, 4):
                                 ch = self.src[self.cursor]
                                 self.cursor += 1
-                                if ch not in "0123456789abcdefABCDEF":
+                                if ch not in '0123456789abcdefABCDEF':
                                     raise QAPIParseError(self,
                                                          '\\u escape needs 4 '
                                                          'hex digits')
@@ -363,28 +430,28 @@ class QAPISchemaParser(object):
                                                      'only supports non-zero '
                                                      'values up to \\u007f')
                             string += chr(value)
-                        elif ch in "\\/'\"":
+                        elif ch in '\\/\'"':
                             string += ch
                         else:
                             raise QAPIParseError(self,
                                                  "Unknown escape \\%s" % ch)
                         esc = False
-                    elif ch == "\\":
+                    elif ch == '\\':
                         esc = True
                     elif ch == "'":
                         self.val = string
                         return
                     else:
                         string += ch
-            elif self.src.startswith("true", self.pos):
+            elif self.src.startswith('true', self.pos):
                 self.val = True
                 self.cursor += 3
                 return
-            elif self.src.startswith("false", self.pos):
+            elif self.src.startswith('false', self.pos):
                 self.val = False
                 self.cursor += 4
                 return
-            elif self.src.startswith("null", self.pos):
+            elif self.src.startswith('null', self.pos):
                 self.val = None
                 self.cursor += 3
                 return
@@ -452,7 +519,8 @@ class QAPISchemaParser(object):
             expr = self.val
             self.accept()
         else:
-            raise QAPIParseError(self, 'Expected "{", "[" or string')
+            raise QAPIParseError(self, 'Expected "{", "[", string, '
+                                 'boolean or "null"')
         return expr
 
     def get_doc(self, info):
@@ -468,6 +536,7 @@ class QAPISchemaParser(object):
                 if self.val != '##':
                     raise QAPIParseError(self, "Junk after '##' at end of "
                                          "documentation comment")
+                doc.end_comment()
                 self.accept()
                 return doc
             else:
@@ -487,7 +556,7 @@ class QAPISchemaParser(object):
 def find_base_members(base):
     if isinstance(base, dict):
         return base
-    base_struct_define = find_struct(base)
+    base_struct_define = struct_types.get(base)
     if not base_struct_define:
         return None
     return base_struct_define['data']
@@ -497,12 +566,12 @@ def find_base_members(base):
 def find_alternate_member_qtype(qapi_type):
     if qapi_type in builtin_types:
         return builtin_types[qapi_type]
-    elif find_struct(qapi_type):
-        return "QTYPE_QDICT"
-    elif find_enum(qapi_type):
-        return "QTYPE_QSTRING"
-    elif find_union(qapi_type):
-        return "QTYPE_QDICT"
+    elif qapi_type in struct_types:
+        return 'QTYPE_QDICT'
+    elif qapi_type in enum_types:
+        return 'QTYPE_QSTRING'
+    elif qapi_type in union_types:
+        return 'QTYPE_QDICT'
     return None
 
 
@@ -523,13 +592,13 @@ def discriminator_find_enum_define(expr):
     if not discriminator_type:
         return None
 
-    return find_enum(discriminator_type)
+    return enum_types.get(discriminator_type)
 
 
 # Names must be letters, numbers, -, and _.  They must start with letter,
 # except for downstream extensions which must start with __RFQDN_.
 # Dots are only valid in the downstream extension prefix.
-valid_name = re.compile('^(__[a-zA-Z0-9.-]+_)?'
+valid_name = re.compile(r'^(__[a-zA-Z0-9.-]+_)?'
                         '[a-zA-Z][a-zA-Z0-9_-]*$')
 
 
@@ -570,54 +639,6 @@ def add_name(name, info, meta, implicit=False):
     all_names[name] = meta
 
 
-def add_struct(definition, info):
-    global struct_types
-    name = definition['struct']
-    add_name(name, info, 'struct')
-    struct_types.append(definition)
-
-
-def find_struct(name):
-    global struct_types
-    for struct in struct_types:
-        if struct['struct'] == name:
-            return struct
-    return None
-
-
-def add_union(definition, info):
-    global union_types
-    name = definition['union']
-    add_name(name, info, 'union')
-    union_types.append(definition)
-
-
-def find_union(name):
-    global union_types
-    for union in union_types:
-        if union['union'] == name:
-            return union
-    return None
-
-
-def add_enum(name, info, enum_values=None, implicit=False):
-    global enum_types
-    add_name(name, info, 'enum', implicit)
-    enum_types.append({"enum_name": name, "enum_values": enum_values})
-
-
-def find_enum(name):
-    global enum_types
-    for enum in enum_types:
-        if enum['enum_name'] == name:
-            return enum
-    return None
-
-
-def is_enum(name):
-    return find_enum(name) is not None
-
-
 def check_type(info, source, value, allow_array=False,
                allow_dict=False, allow_optional=False,
                allow_metas=[]):
@@ -687,14 +708,12 @@ def check_command(expr, info):
 
 
 def check_event(expr, info):
-    global events
     name = expr['event']
     boxed = expr.get('boxed', False)
 
     meta = ['struct']
     if boxed:
         meta += ['union', 'alternate']
-    events.append(name)
     check_type(info, "'data' for event '%s'" % name,
                expr.get('data'), allow_dict=not boxed, allow_optional=True,
                allow_metas=meta)
@@ -726,7 +745,7 @@ def check_union(expr, info):
             raise QAPISemError(info, "Flat union '%s' must have a base"
                                % name)
         base_members = find_base_members(base)
-        assert base_members
+        assert base_members is not None
 
         # The value of member 'discriminator' must name a non-optional
         # member of the base struct.
@@ -738,7 +757,7 @@ def check_union(expr, info):
                                "Discriminator '%s' is not a member of base "
                                "struct '%s'"
                                % (discriminator, base))
-        enum_define = find_enum(discriminator_type)
+        enum_define = enum_types.get(discriminator_type)
         allow_metas = ['struct']
         # Do not allow string discriminator
         if not enum_define:
@@ -759,15 +778,15 @@ def check_union(expr, info):
         # If the discriminator names an enum type, then all members
         # of 'data' must also be members of the enum type.
         if enum_define:
-            if key not in enum_define['enum_values']:
+            if key not in enum_define['data']:
                 raise QAPISemError(info,
                                    "Discriminator value '%s' is not found in "
                                    "enum '%s'"
-                                   % (key, enum_define["enum_name"]))
+                                   % (key, enum_define['enum']))
 
     # If discriminator is user-defined, ensure all values are covered
     if enum_define:
-        for value in enum_define['enum_values']:
+        for value in enum_define['data']:
             if value not in members.keys():
                 raise QAPISemError(info, "Union '%s' data missing '%s' branch"
                                    % (name, value))
@@ -855,56 +874,69 @@ def check_keys(expr_elem, meta, required, optional=[]):
 def check_exprs(exprs):
     global all_names
 
-    # Learn the types and check for valid expression keys
+    # Populate name table with names of built-in types
     for builtin in builtin_types.keys():
         all_names[builtin] = 'built-in'
+
+    # Learn the types and check for valid expression keys
     for expr_elem in exprs:
         expr = expr_elem['expr']
         info = expr_elem['info']
+        doc = expr_elem.get('doc')
 
-        if 'doc' not in expr_elem:
+        if not doc and doc_required:
             raise QAPISemError(info,
                                "Expression missing documentation comment")
 
         if 'enum' in expr:
+            meta = 'enum'
             check_keys(expr_elem, 'enum', ['data'], ['prefix'])
-            add_enum(expr['enum'], info, expr['data'])
+            enum_types[expr[meta]] = expr
         elif 'union' in expr:
+            meta = 'union'
             check_keys(expr_elem, 'union', ['data'],
                        ['base', 'discriminator'])
-            add_union(expr, info)
+            union_types[expr[meta]] = expr
         elif 'alternate' in expr:
+            meta = 'alternate'
             check_keys(expr_elem, 'alternate', ['data'])
-            add_name(expr['alternate'], info, 'alternate')
         elif 'struct' in expr:
+            meta = 'struct'
             check_keys(expr_elem, 'struct', ['data'], ['base'])
-            add_struct(expr, info)
+            struct_types[expr[meta]] = expr
         elif 'command' in expr:
+            meta = 'command'
             check_keys(expr_elem, 'command', [],
                        ['data', 'returns', 'gen', 'success-response', 'boxed'])
-            add_name(expr['command'], info, 'command')
         elif 'event' in expr:
+            meta = 'event'
             check_keys(expr_elem, 'event', [], ['data', 'boxed'])
-            add_name(expr['event'], info, 'event')
         else:
             raise QAPISemError(expr_elem['info'],
                                "Expression is missing metatype")
+        name = expr[meta]
+        add_name(name, info, meta)
+        if doc and doc.symbol != name:
+            raise QAPISemError(info, "Definition of '%s' follows documentation"
+                               " for '%s'" % (name, doc.symbol))
 
     # Try again for hidden UnionKind enum
     for expr_elem in exprs:
         expr = expr_elem['expr']
-        if 'union' in expr:
-            if not discriminator_find_enum_define(expr):
-                add_enum('%sKind' % expr['union'], expr_elem['info'],
-                         implicit=True)
+        if 'union' in expr and not discriminator_find_enum_define(expr):
+            name = '%sKind' % expr['union']
         elif 'alternate' in expr:
-            add_enum('%sKind' % expr['alternate'], expr_elem['info'],
-                     implicit=True)
+            name = '%sKind' % expr['alternate']
+        else:
+            continue
+        enum_types[name] = {'enum': name}
+        add_name(name, info, 'enum', implicit=True)
 
     # Validate that exprs make sense
     for expr_elem in exprs:
         expr = expr_elem['expr']
         info = expr_elem['info']
+        doc = expr_elem.get('doc')
 
         if 'enum' in expr:
             check_enum(expr, info)
@@ -921,89 +953,10 @@ def check_exprs(exprs):
         else:
             assert False, 'unexpected meta type'
 
-    return exprs
-
-
-def check_freeform_doc(doc):
-    if doc.symbol:
-        raise QAPISemError(doc.info,
-                           "Documention for '%s' is not followed"
-                           " by the definition" % doc.symbol)
-
-    body = str(doc.body)
-    if re.search(r'@\S+:', body, re.MULTILINE):
-        raise QAPISemError(doc.info,
-                           "Free-form documentation block must not contain"
-                           " @NAME: sections")
-
+        if doc:
+            doc.check_expr(expr)
 
-def check_definition_doc(doc, expr, info):
-    for i in ('enum', 'union', 'alternate', 'struct', 'command', 'event'):
-        if i in expr:
-            meta = i
-            break
-
-    name = expr[meta]
-    if doc.symbol != name:
-        raise QAPISemError(info, "Definition of '%s' follows documentation"
-                           " for '%s'" % (name, doc.symbol))
-    if doc.has_section('Returns') and 'command' not in expr:
-        raise QAPISemError(info, "'Returns:' is only valid for commands")
-
-    if meta == 'union':
-        args = expr.get('base', [])
-    else:
-        args = expr.get('data', [])
-    if isinstance(args, str):
-        return
-    if isinstance(args, dict):
-        args = args.keys()
-    assert isinstance(args, list)
-
-    if (meta == 'alternate'
-            or (meta == 'union' and not expr.get('discriminator'))):
-        args.append('type')
-
-    for arg in args:
-        if arg[0] == '*':
-            opt = True
-            desc = doc.args.get(arg[1:])
-        else:
-            opt = False
-            desc = doc.args.get(arg)
-        if not desc:
-            continue
-        desc_opt = "#optional" in str(desc)
-        if desc_opt and not opt:
-            raise QAPISemError(info, "Description has #optional, "
-                               "but the declaration doesn't")
-        if not desc_opt and opt:
-            # silently fix the doc
-            # TODO either fix the schema and make this an error,
-            # or drop #optional entirely
-            desc.append("#optional")
-
-    doc_args = set(doc.args.keys())
-    args = set([name.strip('*') for name in args])
-    if not doc_args.issubset(args):
-        raise QAPISemError(info, "The following documented members are not in "
-                           "the declaration: %s" % ", ".join(doc_args - args))
-
-
-def check_docs(docs):
-    for doc in docs:
-        for section in doc.args.values() + doc.sections:
-            content = str(section)
-            if not content or content.isspace():
-                raise QAPISemError(doc.info,
-                                   "Empty doc section '%s'" % section.name)
-
-        if not doc.expr:
-            check_freeform_doc(doc)
-        else:
-            check_definition_doc(doc, doc.expr, doc.info)
-
-    return docs
+    return exprs
 
 
 #
@@ -1011,7 +964,7 @@ def check_docs(docs):
 #
 
 class QAPISchemaEntity(object):
-    def __init__(self, name, info):
+    def __init__(self, name, info, doc):
         assert isinstance(name, str)
         self.name = name
         # For explicitly defined entities, info points to the (explicit)
@@ -1020,6 +973,7 @@ class QAPISchemaEntity(object):
         # triggered the implicit definition (there may be more than one
         # such place).
         self.info = info
+        self.doc = doc
 
     def c_name(self):
         return c_name(self.name)
@@ -1098,10 +1052,15 @@ class QAPISchemaType(QAPISchemaEntity):
         }
         return json2qtype.get(self.json_type())
 
+    def doc_type(self):
+        if self.is_implicit():
+            return None
+        return self.name
+
 
 class QAPISchemaBuiltinType(QAPISchemaType):
     def __init__(self, name, json_type, c_type):
-        QAPISchemaType.__init__(self, name, None)
+        QAPISchemaType.__init__(self, name, None, None)
         assert not c_type or isinstance(c_type, str)
         assert json_type in ('string', 'number', 'int', 'boolean', 'null',
                              'value')
@@ -1122,13 +1081,16 @@ class QAPISchemaBuiltinType(QAPISchemaType):
     def json_type(self):
         return self._json_type_name
 
+    def doc_type(self):
+        return self.json_type()
+
     def visit(self, visitor):
         visitor.visit_builtin_type(self.name, self.info, self.json_type())
 
 
 class QAPISchemaEnumType(QAPISchemaType):
-    def __init__(self, name, info, values, prefix):
-        QAPISchemaType.__init__(self, name, info)
+    def __init__(self, name, info, doc, values, prefix):
+        QAPISchemaType.__init__(self, name, info, doc)
         for v in values:
             assert isinstance(v, QAPISchemaMember)
             v.set_owner(name)
@@ -1140,10 +1102,12 @@ class QAPISchemaEnumType(QAPISchemaType):
         seen = {}
         for v in self.values:
             v.check_clash(self.info, seen)
+            if self.doc:
+                self.doc.connect_member(v)
 
     def is_implicit(self):
-        # See QAPISchema._make_implicit_enum_type()
-        return self.name.endswith('Kind')
+        # See QAPISchema._make_implicit_enum_type() and ._def_predefineds()
+        return self.name.endswith('Kind') or self.name == 'QType'
 
     def c_type(self):
         return c_name(self.name)
@@ -1161,7 +1125,7 @@ class QAPISchemaEnumType(QAPISchemaType):
 
 class QAPISchemaArrayType(QAPISchemaType):
     def __init__(self, name, info, element_type):
-        QAPISchemaType.__init__(self, name, info)
+        QAPISchemaType.__init__(self, name, info, None)
         assert isinstance(element_type, str)
         self._element_type_name = element_type
         self.element_type = None
@@ -1179,16 +1143,22 @@ class QAPISchemaArrayType(QAPISchemaType):
     def json_type(self):
         return 'array'
 
+    def doc_type(self):
+        elt_doc_type = self.element_type.doc_type()
+        if not elt_doc_type:
+            return None
+        return 'array of ' + elt_doc_type
+
     def visit(self, visitor):
         visitor.visit_array_type(self.name, self.info, self.element_type)
 
 
 class QAPISchemaObjectType(QAPISchemaType):
-    def __init__(self, name, info, base, local_members, variants):
+    def __init__(self, name, info, doc, base, local_members, variants):
         # struct has local_members, optional base, and no variants
         # flat union has base, variants, and no local_members
         # simple union has local_members, variants, and no base
-        QAPISchemaType.__init__(self, name, info)
+        QAPISchemaType.__init__(self, name, info, doc)
         assert base is None or isinstance(base, str)
         for m in local_members:
             assert isinstance(m, QAPISchemaObjectTypeMember)
@@ -1214,20 +1184,24 @@ class QAPISchemaObjectType(QAPISchemaType):
             self.base = schema.lookup_type(self._base_name)
             assert isinstance(self.base, QAPISchemaObjectType)
             self.base.check(schema)
-            self.base.check_clash(schema, self.info, seen)
+            self.base.check_clash(self.info, seen)
         for m in self.local_members:
             m.check(schema)
             m.check_clash(self.info, seen)
+            if self.doc:
+                self.doc.connect_member(m)
         self.members = seen.values()
         if self.variants:
             self.variants.check(schema, seen)
             assert self.variants.tag_member in self.members
-            self.variants.check_clash(schema, self.info, seen)
+            self.variants.check_clash(self.info, seen)
+        if self.doc:
+            self.doc.check()
 
     # Check that the members of this type do not cause duplicate JSON members,
     # and update seen to track the members seen so far. Report any errors
     # on behalf of info, which is not necessarily self.info
-    def check_clash(self, schema, info, seen):
+    def check_clash(self, info, seen):
         assert not self.variants       # not implemented
         for m in self.members:
             m.check_clash(info, seen)
@@ -1276,7 +1250,7 @@ class QAPISchemaMember(object):
 
     def check_clash(self, info, seen):
         cname = c_name(self.name)
-        if cname.lower() != cname and self.owner not in case_whitelist:
+        if cname.lower() != cname and self.owner not in name_case_whitelist:
             raise QAPISemError(info,
                                "%s should not use uppercase" % self.describe())
         if cname in seen:
@@ -1356,12 +1330,12 @@ class QAPISchemaObjectTypeVariants(object):
                 assert isinstance(v.type, QAPISchemaObjectType)
                 v.type.check(schema)
 
-    def check_clash(self, schema, info, seen):
+    def check_clash(self, info, seen):
         for v in self.variants:
             # Reset seen map for each variant, since qapi names from one
             # branch do not affect another branch
             assert isinstance(v.type, QAPISchemaObjectType)
-            v.type.check_clash(schema, info, dict(seen))
+            v.type.check_clash(info, dict(seen))
 
 
 class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
@@ -1372,8 +1346,8 @@ class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
 
 
 class QAPISchemaAlternateType(QAPISchemaType):
-    def __init__(self, name, info, variants):
-        QAPISchemaType.__init__(self, name, info)
+    def __init__(self, name, info, doc, variants):
+        QAPISchemaType.__init__(self, name, info, doc)
         assert isinstance(variants, QAPISchemaObjectTypeVariants)
         assert variants.tag_member
         variants.set_owner(name)
@@ -1390,6 +1364,10 @@ class QAPISchemaAlternateType(QAPISchemaType):
         seen = {}
         for v in self.variants.variants:
             v.check_clash(self.info, seen)
+            if self.doc:
+                self.doc.connect_member(v)
+        if self.doc:
+            self.doc.check()
 
     def c_type(self):
         return c_name(self.name) + pointer_suffix
@@ -1405,9 +1383,9 @@ class QAPISchemaAlternateType(QAPISchemaType):
 
 
 class QAPISchemaCommand(QAPISchemaEntity):
-    def __init__(self, name, info, arg_type, ret_type, gen, success_response,
-                 boxed):
-        QAPISchemaEntity.__init__(self, name, info)
+    def __init__(self, name, info, doc, arg_type, ret_type,
+                 gen, success_response, boxed):
+        QAPISchemaEntity.__init__(self, name, info, doc)
         assert not arg_type or isinstance(arg_type, str)
         assert not ret_type or isinstance(ret_type, str)
         self._arg_type_name = arg_type
@@ -1444,8 +1422,8 @@ class QAPISchemaCommand(QAPISchemaEntity):
 
 
 class QAPISchemaEvent(QAPISchemaEntity):
-    def __init__(self, name, info, arg_type, boxed):
-        QAPISchemaEntity.__init__(self, name, info)
+    def __init__(self, name, info, doc, arg_type, boxed):
+        QAPISchemaEntity.__init__(self, name, info, doc)
         assert not arg_type or isinstance(arg_type, str)
         self._arg_type_name = arg_type
         self.arg_type = None
@@ -1474,9 +1452,9 @@ class QAPISchemaEvent(QAPISchemaEntity):
 class QAPISchema(object):
     def __init__(self, fname):
         try:
-            parser = QAPISchemaParser(open(fname, "r"))
+            parser = QAPISchemaParser(open(fname, 'r'))
             self.exprs = check_exprs(parser.exprs)
-            self.docs = check_docs(parser.docs)
+            self.docs = parser.docs
             self._entity_dict = {}
             self._predefining = True
             self._def_predefineds()
@@ -1527,14 +1505,14 @@ class QAPISchema(object):
                   ('bool',   'boolean', 'bool'),
                   ('any',    'value',   'QObject' + pointer_suffix)]:
             self._def_builtin_type(*t)
-        self.the_empty_object_type = QAPISchemaObjectType('q_empty', None,
-                                                          None, [], None)
+        self.the_empty_object_type = QAPISchemaObjectType(
+            'q_empty', None, None, None, [], None)
         self._def_entity(self.the_empty_object_type)
         qtype_values = self._make_enum_members(['none', 'qnull', 'qint',
                                                 'qstring', 'qdict', 'qlist',
                                                 'qfloat', 'qbool'])
-        self._def_entity(QAPISchemaEnumType('QType', None, qtype_values,
-                                            'QTYPE'))
+        self._def_entity(QAPISchemaEnumType('QType', None, None,
+                                            qtype_values, 'QTYPE'))
 
     def _make_enum_members(self, values):
         return [QAPISchemaMember(v) for v in values]
@@ -1543,7 +1521,7 @@ class QAPISchema(object):
         # See also QAPISchemaObjectTypeMember._pretty_owner()
         name = name + 'Kind'   # Use namespace reserved by add_name()
         self._def_entity(QAPISchemaEnumType(
-            name, info, self._make_enum_members(values), None))
+            name, info, None, self._make_enum_members(values), None))
         return name
 
     def _make_array_type(self, element_type, info):
@@ -1552,22 +1530,22 @@ class QAPISchema(object):
             self._def_entity(QAPISchemaArrayType(name, info, element_type))
         return name
 
-    def _make_implicit_object_type(self, name, info, role, members):
+    def _make_implicit_object_type(self, name, info, doc, role, members):
         if not members:
             return None
         # See also QAPISchemaObjectTypeMember._pretty_owner()
         name = 'q_obj_%s-%s' % (name, role)
         if not self.lookup_entity(name, QAPISchemaObjectType):
-            self._def_entity(QAPISchemaObjectType(name, info, None,
+            self._def_entity(QAPISchemaObjectType(name, info, doc, None,
                                                   members, None))
         return name
 
-    def _def_enum_type(self, expr, info):
+    def _def_enum_type(self, expr, info, doc):
         name = expr['enum']
         data = expr['data']
         prefix = expr.get('prefix')
         self._def_entity(QAPISchemaEnumType(
-            name, info, self._make_enum_members(data), prefix))
+            name, info, doc, self._make_enum_members(data), prefix))
 
     def _make_member(self, name, typ, info):
         optional = False
@@ -1583,11 +1561,11 @@ class QAPISchema(object):
         return [self._make_member(key, value, info)
                 for (key, value) in data.iteritems()]
 
-    def _def_struct_type(self, expr, info):
+    def _def_struct_type(self, expr, info, doc):
         name = expr['struct']
         base = expr.get('base')
         data = expr['data']
-        self._def_entity(QAPISchemaObjectType(name, info, base,
+        self._def_entity(QAPISchemaObjectType(name, info, doc, base,
                                               self._make_members(data, info),
                                               None))
 
@@ -1599,10 +1577,10 @@ class QAPISchema(object):
             assert len(typ) == 1
             typ = self._make_array_type(typ[0], info)
         typ = self._make_implicit_object_type(
-            typ, info, 'wrapper', [self._make_member('data', typ, info)])
+            typ, info, None, 'wrapper', [self._make_member('data', typ, info)])
         return QAPISchemaObjectTypeVariant(case, typ)
 
-    def _def_union_type(self, expr, info):
+    def _def_union_type(self, expr, info, doc):
         name = expr['union']
         data = expr['data']
         base = expr.get('base')
@@ -1610,7 +1588,7 @@ class QAPISchema(object):
         tag_member = None
         if isinstance(base, dict):
             base = (self._make_implicit_object_type(
-                    name, info, 'base', self._make_members(base, info)))
+                name, info, doc, 'base', self._make_members(base, info)))
         if tag_name:
             variants = [self._make_variant(key, value)
                         for (key, value) in data.iteritems()]
@@ -1623,24 +1601,24 @@ class QAPISchema(object):
             tag_member = QAPISchemaObjectTypeMember('type', typ, False)
             members = [tag_member]
         self._def_entity(
-            QAPISchemaObjectType(name, info, base, members,
+            QAPISchemaObjectType(name, info, doc, base, members,
                                  QAPISchemaObjectTypeVariants(tag_name,
                                                               tag_member,
                                                               variants)))
 
-    def _def_alternate_type(self, expr, info):
+    def _def_alternate_type(self, expr, info, doc):
         name = expr['alternate']
         data = expr['data']
         variants = [self._make_variant(key, value)
                     for (key, value) in data.iteritems()]
         tag_member = QAPISchemaObjectTypeMember('type', 'QType', False)
         self._def_entity(
-            QAPISchemaAlternateType(name, info,
+            QAPISchemaAlternateType(name, info, doc,
                                     QAPISchemaObjectTypeVariants(None,
                                                                  tag_member,
                                                                  variants)))
 
-    def _def_command(self, expr, info):
+    def _def_command(self, expr, info, doc):
         name = expr['command']
         data = expr.get('data')
         rets = expr.get('returns')
@@ -1649,38 +1627,39 @@ class QAPISchema(object):
         boxed = expr.get('boxed', False)
         if isinstance(data, OrderedDict):
             data = self._make_implicit_object_type(
-                name, info, 'arg', self._make_members(data, info))
+                name, info, doc, 'arg', self._make_members(data, info))
         if isinstance(rets, list):
             assert len(rets) == 1
             rets = self._make_array_type(rets[0], info)
-        self._def_entity(QAPISchemaCommand(name, info, data, rets, gen,
-                                           success_response, boxed))
+        self._def_entity(QAPISchemaCommand(name, info, doc, data, rets,
+                                           gen, success_response, boxed))
 
-    def _def_event(self, expr, info):
+    def _def_event(self, expr, info, doc):
         name = expr['event']
         data = expr.get('data')
         boxed = expr.get('boxed', False)
         if isinstance(data, OrderedDict):
             data = self._make_implicit_object_type(
-                name, info, 'arg', self._make_members(data, info))
-        self._def_entity(QAPISchemaEvent(name, info, data, boxed))
+                name, info, doc, 'arg', self._make_members(data, info))
+        self._def_entity(QAPISchemaEvent(name, info, doc, data, boxed))
 
     def _def_exprs(self):
         for expr_elem in self.exprs:
             expr = expr_elem['expr']
             info = expr_elem['info']
+            doc = expr_elem.get('doc')
             if 'enum' in expr:
-                self._def_enum_type(expr, info)
+                self._def_enum_type(expr, info, doc)
             elif 'struct' in expr:
-                self._def_struct_type(expr, info)
+                self._def_struct_type(expr, info, doc)
             elif 'union' in expr:
-                self._def_union_type(expr, info)
+                self._def_union_type(expr, info, doc)
             elif 'alternate' in expr:
-                self._def_alternate_type(expr, info)
+                self._def_alternate_type(expr, info, doc)
             elif 'command' in expr:
-                self._def_command(expr, info)
+                self._def_command(expr, info, doc)
             elif 'event' in expr:
-                self._def_event(expr, info)
+                self._def_event(expr, info, doc)
             else:
                 assert False
 
@@ -1726,8 +1705,8 @@ def camel_to_upper(value):
     l = len(c_fun_str)
     for i in range(l):
         c = c_fun_str[i]
-        # When c is upper and no "_" appears before, do more checks
-        if c.isupper() and (i > 0) and c_fun_str[i - 1] != "_":
+        # When c is upper and no '_' appears before, do more checks
+        if c.isupper() and (i > 0) and c_fun_str[i - 1] != '_':
             if i < l - 1 and c_fun_str[i + 1].islower():
                 new_name += '_'
             elif c_fun_str[i - 1].isdigit():
@@ -1746,7 +1725,7 @@ c_name_trans = string.maketrans('.-', '__')
 
 # Map @name to a valid C identifier.
 # If @protect, avoid returning certain ticklish identifiers (like
-# C keywords) by prepending "q_".
+# C keywords) by prepending 'q_'.
 #
 # Used for converting 'name' from a 'name':'type' qapi definition
 # into a generated struct member, as well as converting type names
@@ -1784,7 +1763,7 @@ def c_name(name, protect=True):
     name = name.translate(c_name_trans)
     if protect and (name in c89_words | c99_words | c11_words | gcc_words
                     | cpp_words | polluted_words):
-        return "q_" + name
+        return 'q_' + name
     return name
 
 eatspace = '\033EATSPACE.'
@@ -1792,9 +1771,9 @@ pointer_suffix = ' *' + eatspace
 
 
 def genindent(count):
-    ret = ""
+    ret = ''
     for _ in range(count):
-        ret += " "
+        ret += ' '
     return ret
 
 indent_level = 0
@@ -1817,10 +1796,10 @@ def cgen(code, **kwds):
     if indent_level:
         indent = genindent(indent_level)
         # re.subn() lacks flags support before Python 2.7, use re.compile()
-        raw = re.subn(re.compile("^.", re.MULTILINE),
+        raw = re.subn(re.compile(r'^.', re.MULTILINE),
                       indent + r'\g<0>', raw)
         raw = raw[0]
-    return re.sub(re.escape(eatspace) + ' *', '', raw)
+    return re.sub(re.escape(eatspace) + r' *', '', raw)
 
 
 def mcgen(code, **kwds):
@@ -1934,38 +1913,38 @@ def gen_params(arg_type, boxed, extra):
 #
 
 
-def parse_command_line(extra_options="", extra_long_options=[]):
+def parse_command_line(extra_options='', extra_long_options=[]):
 
     try:
         opts, args = getopt.gnu_getopt(sys.argv[1:],
-                                       "chp:o:" + extra_options,
-                                       ["source", "header", "prefix=",
-                                        "output-dir="] + extra_long_options)
+                                       'chp:o:' + extra_options,
+                                       ['source', 'header', 'prefix=',
+                                        'output-dir='] + extra_long_options)
     except getopt.GetoptError as err:
         print >>sys.stderr, "%s: %s" % (sys.argv[0], str(err))
         sys.exit(1)
 
-    output_dir = ""
-    prefix = ""
+    output_dir = ''
+    prefix = ''
     do_c = False
     do_h = False
     extra_opts = []
 
     for oa in opts:
         o, a = oa
-        if o in ("-p", "--prefix"):
-            match = re.match('([A-Za-z_.-][A-Za-z0-9_.-]*)?', a)
+        if o in ('-p', '--prefix'):
+            match = re.match(r'([A-Za-z_.-][A-Za-z0-9_.-]*)?', a)
             if match.end() != len(a):
                 print >>sys.stderr, \
                     "%s: 'funny character '%s' in argument of --prefix" \
                     % (sys.argv[0], a[match.end()])
                 sys.exit(1)
             prefix = a
-        elif o in ("-o", "--output-dir"):
-            output_dir = a + "/"
-        elif o in ("-c", "--source"):
+        elif o in ('-o', '--output-dir'):
+            output_dir = a + '/'
+        elif o in ('-c', '--source'):
             do_c = True
-        elif o in ("-h", "--header"):
+        elif o in ('-h', '--header'):
             do_h = True
         else:
             extra_opts.append(oa)
diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index c1071c62c6..8eed11a60c 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -9,7 +9,7 @@ import sys
 
 import qapi
 
-COMMAND_FMT = """
+MSG_FMT = """
 @deftypefn {type} {{}} {name}
 
 {body}
@@ -18,16 +18,7 @@ COMMAND_FMT = """
 
 """.format
 
-ENUM_FMT = """
-@deftp Enum {name}
-
-{body}
-
-@end deftp
-
-""".format
-
-STRUCT_FMT = """
+TYPE_FMT = """
 @deftp {{{type}}} {name}
 
 {body}
@@ -59,7 +50,7 @@ def subst_vars(doc):
 
 def subst_braces(doc):
     """Replaces {} with @{ @}"""
-    return doc.replace("{", "@{").replace("}", "@}")
+    return doc.replace('{', '@{').replace('}', '@}')
 
 
 def texi_example(doc):
@@ -88,10 +79,10 @@ def texi_format(doc):
     doc = subst_vars(doc)
     doc = subst_emph(doc)
     doc = subst_strong(doc)
-    inlist = ""
+    inlist = ''
     lastempty = False
     for line in doc.split('\n'):
-        empty = line == ""
+        empty = line == ''
 
         # FIXME: Doing this in a single if / elif chain is
         # problematic.  For instance, a line without markup terminates
@@ -101,158 +92,194 @@ def texi_format(doc):
         #
         # Make sure to update section "Documentation markup" in
         # docs/qapi-code-gen.txt when fixing this.
-        if line.startswith("| "):
+        if line.startswith('| '):
             line = EXAMPLE_FMT(code=line[2:])
-        elif line.startswith("= "):
-            line = "@section " + line[2:]
-        elif line.startswith("== "):
-            line = "@subsection " + line[3:]
+        elif line.startswith('= '):
+            line = '@section ' + line[2:]
+        elif line.startswith('== '):
+            line = '@subsection ' + line[3:]
         elif re.match(r'^([0-9]*\.) ', line):
             if not inlist:
-                lines.append("@enumerate")
-                inlist = "enumerate"
-            line = line[line.find(" ")+1:]
-            lines.append("@item")
+                lines.append('@enumerate')
+                inlist = 'enumerate'
+            line = line[line.find(' ')+1:]
+            lines.append('@item')
         elif re.match(r'^[*-] ', line):
             if not inlist:
-                lines.append("@itemize %s" % {'*': "@bullet",
-                                              '-': "@minus"}[line[0]])
-                inlist = "itemize"
-            lines.append("@item")
+                lines.append('@itemize %s' % {'*': '@bullet',
+                                              '-': '@minus'}[line[0]])
+                inlist = 'itemize'
+            lines.append('@item')
             line = line[2:]
         elif lastempty and inlist:
-            lines.append("@end %s\n" % inlist)
-            inlist = ""
+            lines.append('@end %s\n' % inlist)
+            inlist = ''
 
         lastempty = empty
         lines.append(line)
 
     if inlist:
-        lines.append("@end %s\n" % inlist)
-    return "\n".join(lines)
+        lines.append('@end %s\n' % inlist)
+    return '\n'.join(lines)
 
 
 def texi_body(doc):
-    """
-    Format the body of a symbol documentation:
-    - main body
-    - table of arguments
-    - followed by "Returns/Notes/Since/Example" sections
-    """
-    body = texi_format(str(doc.body)) + "\n"
-    if doc.args:
-        body += "@table @asis\n"
-        for arg, section in doc.args.iteritems():
-            desc = str(section)
-            opt = ''
-            if "#optional" in desc:
-                desc = desc.replace("#optional", "")
-                opt = ' (optional)'
-            body += "@item @code{'%s'}%s\n%s\n" % (arg, opt,
-                                                   texi_format(desc))
-        body += "@end table\n"
-
+    """Format the main documentation body"""
+    return texi_format(str(doc.body)) + '\n'
+
+
+def texi_enum_value(value):
+    """Format a table of members item for an enumeration value"""
+    return '@item @code{%s}\n' % value.name
+
+
+def texi_member(member, suffix=''):
+    """Format a table of members item for an object type member"""
+    typ = member.type.doc_type()
+    return '@item @code{%s%s%s}%s%s\n' % (
+        member.name,
+        ': ' if typ else '',
+        typ if typ else '',
+        ' (optional)' if member.optional else '',
+        suffix)
+
+
+def texi_members(doc, what, base, variants, member_func):
+    """Format the table of members"""
+    items = ''
+    for section in doc.args.itervalues():
+        # TODO Drop fallbacks when undocumented members are outlawed
+        if section.content:
+            desc = texi_format(str(section))
+        elif (variants and variants.tag_member == section.member
+              and not section.member.type.doc_type()):
+            values = section.member.type.member_names()
+            desc = 'One of ' + ', '.join(['@t{"%s"}' % v for v in values])
+        else:
+            desc = 'Not documented'
+        items += member_func(section.member) + desc + '\n'
+    if base:
+        items += '@item The members of @code{%s}\n' % base.doc_type()
+    if variants:
+        for v in variants.variants:
+            when = ' when @code{%s} is @t{"%s"}' % (
+                variants.tag_member.name, v.name)
+            if v.type.is_implicit():
+                assert not v.type.base and not v.type.variants
+                for m in v.type.local_members:
+                    items += member_func(m, when)
+            else:
+                items += '@item The members of @code{%s}%s\n' % (
+                    v.type.doc_type(), when)
+    if not items:
+        return ''
+    return '\n@b{%s:}\n@table @asis\n%s@end table\n' % (what, items)
+
+
+def texi_sections(doc):
+    """Format additional sections following arguments"""
+    body = ''
     for section in doc.sections:
         name, doc = (section.name, str(section))
         func = texi_format
-        if name.startswith("Example"):
+        if name.startswith('Example'):
             func = texi_example
 
         if name:
             # prefer @b over @strong, so txt doesn't translate it to *Foo:*
-            body += "\n\n@b{%s:}\n" % name
+            body += '\n\n@b{%s:}\n' % name
 
         body += func(doc)
-
     return body
 
 
-def texi_alternate(expr, doc):
-    """Format an alternate to texi"""
-    body = texi_body(doc)
-    return STRUCT_FMT(type="Alternate",
-                      name=doc.symbol,
-                      body=body)
-
-
-def texi_union(expr, doc):
-    """Format a union to texi"""
-    discriminator = expr.get("discriminator")
-    if discriminator:
-        union = "Flat Union"
-    else:
-        union = "Simple Union"
-
-    body = texi_body(doc)
-    return STRUCT_FMT(type=union,
-                      name=doc.symbol,
-                      body=body)
-
-
-def texi_enum(expr, doc):
-    """Format an enum to texi"""
-    for i in expr['data']:
-        if i not in doc.args:
-            doc.args[i] = ''
-    body = texi_body(doc)
-    return ENUM_FMT(name=doc.symbol,
-                    body=body)
-
-
-def texi_struct(expr, doc):
-    """Format a struct to texi"""
-    body = texi_body(doc)
-    return STRUCT_FMT(type="Struct",
-                      name=doc.symbol,
-                      body=body)
-
-
-def texi_command(expr, doc):
-    """Format a command to texi"""
-    body = texi_body(doc)
-    return COMMAND_FMT(type="Command",
-                       name=doc.symbol,
-                       body=body)
-
-
-def texi_event(expr, doc):
-    """Format an event to texi"""
-    body = texi_body(doc)
-    return COMMAND_FMT(type="Event",
-                       name=doc.symbol,
-                       body=body)
-
-
-def texi_expr(expr, doc):
-    """Format an expr to texi"""
-    (kind, _) = expr.items()[0]
-
-    fmt = {"command": texi_command,
-           "struct": texi_struct,
-           "enum": texi_enum,
-           "union": texi_union,
-           "alternate": texi_alternate,
-           "event": texi_event}[kind]
-
-    return fmt(expr, doc)
-
-
-def texi(docs):
-    """Convert QAPI schema expressions to texi documentation"""
-    res = []
-    for doc in docs:
-        expr = doc.expr
-        if not expr:
-            res.append(texi_body(doc))
-            continue
-        try:
-            doc = texi_expr(expr, doc)
-            res.append(doc)
-        except:
-            print >>sys.stderr, "error at @%s" % doc.info
-            raise
-
-    return '\n'.join(res)
+def texi_entity(doc, what, base=None, variants=None,
+                member_func=texi_member):
+    return (texi_body(doc)
+            + texi_members(doc, what, base, variants, member_func)
+            + texi_sections(doc))
+
+
+class QAPISchemaGenDocVisitor(qapi.QAPISchemaVisitor):
+    def __init__(self):
+        self.out = None
+        self.cur_doc = None
+
+    def visit_begin(self, schema):
+        self.out = ''
+
+    def visit_enum_type(self, name, info, values, prefix):
+        doc = self.cur_doc
+        if self.out:
+            self.out += '\n'
+        self.out += TYPE_FMT(type='Enum',
+                             name=doc.symbol,
+                             body=texi_entity(doc, 'Values',
+                                              member_func=texi_enum_value))
+
+    def visit_object_type(self, name, info, base, members, variants):
+        doc = self.cur_doc
+        if base and base.is_implicit():
+            base = None
+        if self.out:
+            self.out += '\n'
+        self.out += TYPE_FMT(type='Object',
+                             name=doc.symbol,
+                             body=texi_entity(doc, 'Members', base, variants))
+
+    def visit_alternate_type(self, name, info, variants):
+        doc = self.cur_doc
+        if self.out:
+            self.out += '\n'
+        self.out += TYPE_FMT(type='Alternate',
+                             name=doc.symbol,
+                             body=texi_entity(doc, 'Members'))
+
+    def visit_command(self, name, info, arg_type, ret_type,
+                      gen, success_response, boxed):
+        doc = self.cur_doc
+        if self.out:
+            self.out += '\n'
+        if boxed:
+            body = texi_body(doc)
+            body += '\n@b{Arguments:} the members of @code{%s}' % arg_type.name
+            body += texi_sections(doc)
+        else:
+            body = texi_entity(doc, 'Arguments')
+        self.out += MSG_FMT(type='Command',
+                            name=doc.symbol,
+                            body=body)
+
+    def visit_event(self, name, info, arg_type, boxed):
+        doc = self.cur_doc
+        if self.out:
+            self.out += '\n'
+        self.out += MSG_FMT(type='Event',
+                            name=doc.symbol,
+                            body=texi_entity(doc, 'Arguments'))
+
+    def symbol(self, doc, entity):
+        self.cur_doc = doc
+        entity.visit(self)
+        self.cur_doc = None
+
+    def freeform(self, doc):
+        assert not doc.args
+        if self.out:
+            self.out += '\n'
+        self.out += texi_body(doc) + texi_sections(doc)
+
+
+def texi_schema(schema):
+    """Convert QAPI schema documentation to Texinfo"""
+    gen = QAPISchemaGenDocVisitor()
+    gen.visit_begin(schema)
+    for doc in schema.docs:
+        if doc.symbol:
+            gen.symbol(doc, schema.lookup_entity(doc.symbol))
+        else:
+            gen.freeform(doc)
+    return gen.out
 
 
 def main(argv):
@@ -262,8 +289,11 @@ def main(argv):
         sys.exit(1)
 
     schema = qapi.QAPISchema(argv[1])
-    print texi(schema.docs)
+    if not qapi.doc_required:
+        print >>sys.stderr, ("%s: need pragma 'doc-required' "
+                             "to generate documentation" % argv[0])
+    print texi_schema(schema)
 
 
-if __name__ == "__main__":
+if __name__ == '__main__':
     main(sys.argv)
diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 0373b24b20..eccb88a4e8 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -166,8 +166,8 @@ class QMPShell(qmp.QEMUMonitorProtocol):
 
     def __cli_expr(self, tokens, parent):
         for arg in tokens:
-            (key, _, val) = arg.partition('=')
-            if not val:
+            (key, sep, val) = arg.partition('=')
+            if sep != '=':
                 raise QMPShellError("Expected a key=value pair, got '%s'" % arg)
 
             value = self.__parse_value(val)