1. 15 10月, 2015 2 次提交
    • E
      qapi: Use predicate callback to determine visit filtering · 25a0d9c9
      Eric Blake 提交于
      Previously, qapi-types and qapi-visit filtered out implicit
      objects during visit_object_type() by using 'info' (works since
      implicit objects do not [yet] have associated info); meanwhile
      qapi-introspect filtered out all schema types on the first pass
      by returning a python type from visit_begin(), which was then
      used at a distance in QAPISchema.visit() to do the filtering.
      
      Rather than keeping these ad hoc approaches, add a new visitor
      callback visit_needed() which returns False to skip a given
      entity, and which defaults to True unless overridden.  Use the
      new mechanism to simplify all three filtering visitors.
      
      No change to the generated code.
      Suggested-by: NMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1444710158-8723-2-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      25a0d9c9
    • E
      qapi: Fix regression with '-netdev help' · d08ac81a
      Eric Blake 提交于
      Commit e36c714e causes 'qemu -netdev help' to dump core, because the
      call to visit_end_union() is no longer conditional on whether *obj was
      allocated.
      
      Reported by Marc-André Lureau <marcandre.lureau@gmail.com>
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1444861825-19256-1-git-send-email-eblake@redhat.com>
      [Commit message tweaked to say 'help' instead of '?']
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      d08ac81a
  2. 13 10月, 2015 21 次提交
    • P
      Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2015-10-12' into staging · c49d3411
      Peter Maydell 提交于
      QAPI patches
      
      # gpg: Signature made Mon 12 Oct 2015 18:56:35 BST using RSA key ID EB918653
      # gpg: Good signature from "Markus Armbruster <armbru@redhat.com>"
      # gpg:                 aka "Markus Armbruster <armbru@pond.sub.org>"
      
      * remotes/armbru/tags/pull-qapi-2015-10-12:
        qapi: Simplify gen_visit_fields() error handling
        qapi: Share gen_visit_fields()
        qapi: Share gen_err_check()
        qapi: Consistent generated code: minimize push_indent() usage
        qapi: Consistent generated code: prefer common indentation
        qapi: Consistent generated code: prefer common labels
        qapi: Consistent generated code: prefer visitor 'v'
        qapi: Consistent generated code: prefer error 'err'
        qapi: Reuse code for flat union base validation
        qapi: Test use of 'number' within alternates
        qapi: Add tests for empty unions
        qapi: Avoid assertion failure on union 'type' collision
        qapi: Test for various name collisions
        qapi: Clean up qapi.py per pep8
        qapi: Invoke exception superclass initializer
        qapi: Improve 'include' error message
        qapi: Sort qapi-schema tests
        MAINTAINERS: Specify QAPI include and test files
        MAINTAINERS: Specify QObject include and test files
        docs: Move files from docs/qmp/ to docs/
      Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>
      c49d3411
    • E
      qapi: Simplify gen_visit_fields() error handling · 18bdbc3a
      Eric Blake 提交于
      Since we have consolidated all generated code to use 'err' as
      the name of the local variable for error detection, we can
      simplify the decision on whether to skip error detection (useful
      for deallocation paths) to be a boolean.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1443565276-4535-18-git-send-email-eblake@redhat.com>
      [Change to gen_visit_fields() simplified]
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      18bdbc3a
    • E
      qapi: Share gen_visit_fields() · 82ca8e46
      Eric Blake 提交于
      Consolidate the code between visit, command marshalling, and
      event generation that iterates over the members of a struct.
      It reduces code duplication in the generator, so that a future
      patch can reduce the size of generated code while touching only
      one instead of three locations.
      
      There are no changes to the generated marshal code.
      
      The visitor code becomes slightly more verbose, but remains
      semantically equivalent, and is actually easier to read as
      it follows a more common idiom:
      
      |     visit_optional(v, &(*obj)->has_device, "device", &err);
      |-    if (!err && (*obj)->has_device) {
      |-        visit_type_str(v, &(*obj)->device, "device", &err);
      |-    }
      |     if (err) {
      |         goto out;
      |     }
      |+    if ((*obj)->has_device) {
      |+        visit_type_str(v, &(*obj)->device, "device", &err);
      |+        if (err) {
      |+            goto out;
      |+        }
      |+    }
      
      The event code becomes slightly more verbose, but this is
      arguably a bug fix: although the visitors are not well
      documented, use of an optional member should not be attempted
      unless guarded by a prior call to visit_optional().  Works only
      because the output qmp visitor has a no-op visit_optional():
      
      |+    visit_optional(v, &has_offset, "offset", &err);
      |+    if (err) {
      |+        goto out;
      |+    }
      |     if (has_offset) {
      |         visit_type_int(v, &offset, "offset", &err);
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1443565276-4535-17-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      82ca8e46
    • E
      qapi: Share gen_err_check() · 1f353344
      Eric Blake 提交于
      qapi-commands has a nice helper gen_err_check(), but did not
      use it everywhere. In fact, using it in more places makes it
      easier to reduce the lines of code used for generating error
      checks.  This in turn will make it easier for later patches
      to consolidate another common pattern among the generators.
      
      The generated code has fewer blank lines in qapi-event.c functions,
      but has no semantic difference.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1443565276-4535-16-git-send-email-eblake@redhat.com>
      [Drop another blank line for symmetry]
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      1f353344
    • E
      qapi: Consistent generated code: minimize push_indent() usage · 05372f70
      Eric Blake 提交于
      We had some pointless differences in the generated code for visit,
      command marshalling, and events; unifying them makes it easier for
      future patches to consolidate to common helper functions.
      This is one patch of a series to clean up these differences.
      
      This patch reduces the number of push_indent()/pop_indent() pairs
      so that generated code is typically already at its natural output
      indentation in the python files.  It is easier to reason about
      generated code if the reader does not have to track how much
      spacing will be inserted alongside the code, and moreso when all
      of the generators use the same patterns (qapi-type and qapi-event
      were already using in-place indentation).
      
      Arguably, the resulting python may be a bit harder to read with C
      code at the same indentation as python; on the other hand, not
      having to think about push_indent() is a win, and most decent
      editors provide syntax highlighting that makes it easier to
      visually distinguish python code from string literals that will
      become C code.
      
      There is no change to the generated output.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1443565276-4535-15-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      05372f70
    • E
      qapi: Consistent generated code: prefer common indentation · e36c714e
      Eric Blake 提交于
      We had some pointless differences in the generated code for visit,
      command marshalling, and events; unifying them makes it easier for
      future patches to consolidate to common helper functions.
      This is one patch of a series to clean up these differences.
      
      This patch adjusts gen_visit_union() to use the same indentation
      as other functions, namely, by jumping early to the error label
      if the object was not set rather than placing the rest of the
      body inside an if for when it is set.
      
      No change in semantics to the generated code.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1443565276-4535-14-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      e36c714e
    • E
      qapi: Consistent generated code: prefer common labels · f782399c
      Eric Blake 提交于
      We had some pointless differences in the generated code for visit,
      command marshalling, and events; unifying them makes it easier for
      future patches to consolidate to common helper functions.
      This is one patch of a series to clean up these differences.
      
      This patch names the goto labels 'out' (not 'clean') and 'out_obj'
      (not 'out_end').  Additionally, the generator was inconsistent on
      whether labels had a leading space [our HACKING is silent; while
      emacs 'gnu' style adds the space to avoid littering column 1].
      For minimal churn, prefer no leading space; this also matches
      the style that is more prevalent in current qemu.git.
      
      No change in semantics to the generated code.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1443565276-4535-13-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      f782399c
    • E
      qapi: Consistent generated code: prefer visitor 'v' · f8b7f1a8
      Eric Blake 提交于
      We had some pointless differences in the generated code for visit,
      command marshalling, and events; unifying them makes it easier for
      future patches to consolidate to common helper functions.
      This is one patch of a series to clean up these differences.
      
      This patch names the local visitor variable 'v' rather than 'm'.
      Related objects, such as 'QapiDeallocVisitor', are also named by
      their initials instead of an unrelated leading m.
      
      No change in semantics to the generated code.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1443565276-4535-12-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      f8b7f1a8
    • E
      qapi: Consistent generated code: prefer error 'err' · 2a0f50e8
      Eric Blake 提交于
      We had some pointless differences in the generated code for visit,
      command marshalling, and events; unifying them makes it easier for
      future patches to consolidate to common helper functions.
      This is one patch of a series to clean up these differences.
      
      This patch consistently names the local error variable 'err' rather
      than 'local_err'.
      
      No change in semantics to the generated code.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1443565276-4535-11-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      2a0f50e8
    • E
      qapi: Reuse code for flat union base validation · 376863ef
      Eric Blake 提交于
      Rather than open-code the check for a valid base type, we
      should reuse the common functionality. This allows for
      consistent error messages, and also makes it easier for a
      later patch to turn on support for inline anonymous base
      structures.
      
      Test flat-union-inline is updated to test only one feature
      (anonymous branch dictionaries), which can be implemented
      independently (test flat-union-bad-base already covers the
      idea of an anonymous base dictionary).
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1443565276-4535-10-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      376863ef
    • E
      qapi: Test use of 'number' within alternates · 9c51b441
      Eric Blake 提交于
      Add some testsuite exposure for use of a 'number' as part of
      an alternate.  The current state of the tree has a few bugs
      exposed by this: our input parser depends on the ordering of
      how the qapi schema declared the alternate, and the parser
      does not accept integers for a 'number' in an alternate even
      though it does for numbers outside of an alternate.
      
      Mixing 'int' and 'number' in the same alternate is unusual,
      since both are supplied by json-numbers, but there does not
      seem to be a technical reason to forbid it given that our
      json lexer distinguishes between json-numbers that can be
      represented as an int vs. those that cannot.
      
      Improve the existing test_visitor_in_alternate() to match the
      style of the new test_visitor_in_alternate_number(), and to
      ensure full coverage of all possible qtype parsing.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1443565276-4535-9-git-send-email-eblake@redhat.com>
      [Eric's follow-up fixes squashed in]
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      9c51b441
    • E
      qapi: Add tests for empty unions · 8d25dd10
      Eric Blake 提交于
      The documentation claims that alternates are useful for
      allowing two or more types, although nothing enforces this.
      Meanwhile, it is silent on whether empty unions are allowed.
      In practice, the generated code will compile, in part because
      we have a 'void *data' branch; but attempting to visit such a
      type will cause an abort().  While there's no technical reason
      that a degenerate union could not be made to work, it's harder
      to justify the time spent in chasing known (the current
      abort() during visit) and unknown corner cases, than it would
      be to just outlaw them.  A future patch will probably take the
      approach of forbidding them; in the meantime, we can at least
      add testsuite coverage to make it obvious where things stand.
      
      In addition to adding tests to expose the problems, we also
      need to adjust existing tests that are meant to test something
      else, but which could fail for the wrong reason if we reject
      degenerate alternates/unions.
      
      Note that empty structs are explicitly supported (for example,
      right now they are the only way to specify that one branch of a
      flat union adds no additional members), and empty enums are
      covered by the testsuite as working (even if they do not seem
      to have much use).
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1443565276-4535-8-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      8d25dd10
    • E
      qapi: Avoid assertion failure on union 'type' collision · 7b2a5c2f
      Eric Blake 提交于
      The previous commit added two tests that triggered an assertion
      failure. It's fairly straightforward to avoid the failure by
      just outright forbidding the collision between a union's tag
      values and its discriminator name (including the implicit name
      'kind' supplied for simple unions [*]).  Ultimately, we'd like
      to move the collision detection into QAPISchema*.check(), but
      for now it is easier just to enhance the existing checks.
      
      [*] Of course, down the road, we have plans to rename the simple
      union tag name to 'type' to match the QMP wire name, but the
      idea of the collision will still be present even then.
      
      Technically, we could avoid the collision by naming the C union
      members representing each enum value as '_case_value' rather
      than 'value'; but until we have an actual qapi client (and not
      just our testsuite) that has a legitimate reason to match a
      case label to the name of a QMP key and needs the name munging
      to satisfy the compiler, it's easier to just reject the qapi
      as invalid.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1443565276-4535-7-git-send-email-eblake@redhat.com>
      [Polished a few comments]
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      7b2a5c2f
    • E
      qapi: Test for various name collisions · d220fbcd
      Eric Blake 提交于
      Expose some weaknesses in the generator: we don't always forbid
      the generation of structs that contain multiple members that map
      to the same C or QMP name.  This has already been marked FIXME in
      qapi.py in commit d90675fa, but having more tests will make sure
      future patches produce desired behavior; and updating existing
      patches to better document things doesn't hurt, either.  Some of
      these collisions are already caught in the old-style parser
      checks, but ultimately we want all collisions to be caught in the
      new-style QAPISchema*.check() methods.
      
      This patch focuses on C struct members, and does not consider
      collisions between commands and events (affecting C function
      names), or even collisions between generated C type names with
      user type names (for things like automatic FOOList struct
      representing array types or FOOKind for an implicit enum).
      
      There are two types of struct collisions we want to catch:
       1) Collision between two keys in a JSON object. qapi.py prevents
          that within a single struct (see test duplicate-key), but it is
          possible to have collisions between a type's members and its
          base type's members (existing tests struct-base-clash,
          struct-base-clash-deep), and its flat union variant members
          (renamed test flat-union-clash-member).
       2) Collision between two members of the C struct that is generated
          for a given QAPI type:
          a) Multiple QAPI names map to the same C name (new test
             args-name-clash)
          b) A QAPI name maps to a C name that is used for another purpose
             (new tests flat-union-clash-branch, struct-base-clash-base,
             union-clash-data). We already fixed some such cases in commit
             0f61af3e and 1e6c1616, but more remain.
          c) Two C names generated for other purposes clash
             (updated test alternate-clash, new test union-clash-branches,
             union-clash-type, flat-union-clash-type)
      
      Ultimately, if we need to have a flat union where a tag value
      clashes with a base member name, we could change the generator to
      name the union (using 'foo.u.value' rather than 'foo.value') or
      otherwise munge the C name corresponding to tag values.  But
      unless such a need arises, it will probably be easier to just
      forbid these collisions.
      
      Some of these negative tests will be deleted later, and positive
      tests added to qapi-schema-test.json in their place, when the
      generator code is reworked to avoid particular code generation
      collisions in class 2).
      
      [Note that viewing this patch with git rename detection enabled
      may see some confusion due to renaming some tests while adding
      others, but where the content is similar enough that git picks
      the wrong pre- and post-patch files to associate]
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1443565276-4535-6-git-send-email-eblake@redhat.com>
      [Improve commit message and comments a bit, drop an unrelated test]
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      d220fbcd
    • E
      qapi: Clean up qapi.py per pep8 · 437db254
      Eric Blake 提交于
      Silence pep8, and make pylint a bit happier.  Just style cleanups,
      plus killing a useless comment in camel_to_upper(); no semantic
      changes.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1443565276-4535-5-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      437db254
    • E
      qapi: Invoke exception superclass initializer · 59b00542
      Eric Blake 提交于
      pylint recommends that every exception class should explicitly
      invoke the superclass __init__, even though things seem to work
      fine without it.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1443565276-4535-4-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      59b00542
    • E
      qapi: Improve 'include' error message · 7408fb67
      Eric Blake 提交于
      Use of '"...%s" % include' to print non-strings can lead to
      ugly messages, such as this (if the .json change is applied
      without the qapi.py change):
       Expected a file name (string), got: OrderedDict()
      
      Better is to just omit the actual non-string value in the
      message.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1443565276-4535-3-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      7408fb67
    • E
      qapi: Sort qapi-schema tests · 1ffe818a
      Eric Blake 提交于
      Recent changes to qapi have provided quite a bit of churn in
      the makefile, because we are inconsistent on what order test
      names appear in, and on whether to re-wrap the list of tests or
      just add arbitrary line lengths.  Writing the list in a sorted
      fashion, one test per line, will make future patches easier
      to see what tests are being added or removed by a patch.
      
      Although it is tempting to use $(wildcard qapi-schema/*.json)
      for a more compact listing, such an approach would risk picking
      up leftover garbage .json files in the directory; so keeping
      the list explicit is safer for ensuring reproducible tarballs
      and test results.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1443565276-4535-2-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      1ffe818a
    • M
      MAINTAINERS: Specify QAPI include and test files · ac4abb9a
      Markus Armbruster 提交于
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Message-Id: <1443111117-29831-4-git-send-email-armbru@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NLuiz Capitulino <lcapitulino@redhat.com>
      ac4abb9a
    • M
      MAINTAINERS: Specify QObject include and test files · 7735d2b5
      Markus Armbruster 提交于
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Message-Id: <1443111117-29831-3-git-send-email-armbru@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NLuiz Capitulino <lcapitulino@redhat.com>
      7735d2b5
    • M
      docs: Move files from docs/qmp/ to docs/ · 9b89b6a2
      Markus Armbruster 提交于
      Giving QMP its own subdirectory in docs/ is hardly worthwhile when we
      have just four files, and one of them isn't even in the subdirectory.
      Move the files from docs/qmp/ to docs/, renaming docs/qmp/README to
      docs/qmp-intro.
      
      Update MAINTAINERS.  The new pattern also captures the fourth file
      docs/writing-qmp-commands.txt.
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Message-Id: <1443111117-29831-2-git-send-email-armbru@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NLuiz Capitulino <lcapitulino@redhat.com>
      9b89b6a2
  3. 12 10月, 2015 17 次提交