1. 05 3月, 2016 1 次提交
    • E
      qapi: Update docs to match recent generator changes · 9ee86b85
      Eric Blake 提交于
      Several commits have been changing the generator, but not updating
      the docs to match:
      - The implicit tag member is named "type", not "kind".  Screwed up in
      commit 39a18158.
      - Commit 9f08c8ec made list types lazy, and thereby dropped
      UserDefOneList if nothing explicitly uses the list type.
      - Commit 51e72bc1 switched the parameter order with 'name' occurring
      earlier.
      - Commit e65d89bf changed the layout of UserDefOneList.
      - Prefer the term 'member' over 'field'.
      - We now expose visit_type_FOO_members() for objects.
      - etc.
      
      Rework the examples to show slightly more output (we don't want to
      show too much; that's what the testsuite is for), and regenerate the
      output to match all recent changes.  Also, rearrange output to show
      .h files before .c (understanding the interface first often makes
      the implementation easier to follow).
      Reported-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Message-Id: <1457021813-10704-5-git-send-email-eblake@redhat.com>
      9ee86b85
  2. 01 3月, 2016 2 次提交
  3. 23 2月, 2016 1 次提交
  4. 22 2月, 2016 2 次提交
  5. 19 2月, 2016 1 次提交
    • E
      qapi: Forbid empty unions and useless alternates · 02a57ae3
      Eric Blake 提交于
      Empty unions serve no purpose, and while we compile with gcc
      which permits them, strict C99 forbids them.  We happen to inject
      a dummy 'void *data' member into the C unions that represent QAPI
      unions and alternates, but we want to get rid of that member (it
      pollutes the namespace for no good reason), which would leave us
      with an empty union if the user didn't provide any branches.  While
      empty structs make sense in QAPI, empty unions don't add any
      expressiveness to the QMP language.  So prohibit them at parse
      time.  Update the documentation and testsuite to match.
      
      Note that the documentation already mentioned that alternates
      should have "two or more JSON data types"; so this also fixes
      the code to enforce that.  However, we have existing uses of a
      union type with only one branch, so the 2-or-more strictness
      is intentionally limited to alternates.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1455778109-6278-3-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      02a57ae3
  6. 11 2月, 2016 1 次提交
  7. 09 2月, 2016 1 次提交
  8. 13 1月, 2016 1 次提交
  9. 23 12月, 2015 1 次提交
  10. 22 12月, 2015 1 次提交
  11. 17 12月, 2015 5 次提交
    • E
      qapi: Simplify visiting of alternate types · 0426d53c
      Eric Blake 提交于
      Previously, working with alternates required two lookup arrays
      and some indirection: for type Foo, we created Foo_qtypes[]
      which maps each qtype to a value of the generated FooKind enum,
      then look up that value in FooKind_lookup[] like we do for other
      union types.
      
      This has a couple of subtle bugs.  First, the generator was
      creating a call with a parameter '(int *) &(*obj)->type' where
      type is an enum type; this is unsafe if the compiler chooses
      to store the enum type in a different size than int, where
      assigning through the wrong size pointer can corrupt data or
      cause a SIGBUS.
      
      Related bug, not not fixed in this patch: qapi-visit.py's
      gen_visit_enum() generates a cast of its enum * argument to
      int *. Marked FIXME.
      
      Second, since the values of the FooKind enum start at zero, all
      entries of the Foo_qtypes[] array that were not explicitly
      initialized will map to the same branch of the union as the
      first member of the alternate, rather than triggering a desired
      failure in visit_get_next_type().  Fortunately, the bug seldom
      bites; the very next thing the input visitor does is try to
      parse the incoming JSON with the wrong parser, which normally
      fails; the output visitor is not used with a C struct in that
      state, and the dealloc visitor has nothing to clean up (so
      there is no leak).
      
      However, the second bug IS observable in one case: parsing an
      integer causes unusual behavior in an alternate that contains
      at least a 'number' member but no 'int' member, because the
      'number' parser accepts QTYPE_QINT in addition to the expected
      QTYPE_QFLOAT (that is, since 'int' is not a member, the type
      QTYPE_QINT accidentally maps to FooKind 0; if this enum value
      is the 'number' branch the integer parses successfully, but if
      the 'number' branch is not first, some other branch tries to
      parse the integer and rejects it).  A later patch will worry
      about fixing alternates to always parse all inputs that a
      non-alternate 'number' would accept, for now this is still
      marked FIXME in the updated test-qmp-input-visitor.c, to
      merely point out that new undesired behavior of 'ans' matches
      the existing undesired behavior of 'asn'.
      
      This patch fixes the default-initialization bug by deleting the
      indirection, and modifying get_next_type() to directly assign a
      QTypeCode parameter.  This in turn fixes the type-casting bug,
      as we are no longer casting a pointer to enum to a questionable
      size. There is no longer a need to generate an implicit FooKind
      enum associated with the alternate type (since the QMP wire
      format never uses the stringized counterparts of the C union
      member names).  Since the updated visit_get_next_type() does not
      know which qtypes are expected, the generated visitor is
      modified to generate an error statement if an unexpected type is
      encountered.
      
      Callers now have to know the QTYPE_* mapping when looking at the
      discriminator; but so far, only the testsuite was even using the
      C struct of an alternate types.  I considered the possibility of
      keeping the internal enum FooKind, but initialized differently
      than most generated arrays, as in:
        typedef enum FooKind {
            FOO_KIND_A = QTYPE_QDICT,
            FOO_KIND_B = QTYPE_QINT,
        } FooKind;
      to create nicer aliases for knowing when to use foo->a or foo->b
      when inspecting foo->type; but it turned out to add too much
      complexity, especially without a client.
      
      There is a user-visible side effect to this change, but I
      consider it to be an improvement. Previously,
      the invalid QMP command:
        {"execute":"blockdev-add", "arguments":{"options":
          {"driver":"raw", "id":"a", "file":true}}}
      failed with:
        {"error": {"class": "GenericError",
          "desc": "Invalid parameter type for 'file', expected: QDict"}}
      (visit_get_next_type() succeeded, and the error comes from the
      visit_type_BlockdevOptions() expecting {}; there is no mention of
      the fact that a string would also work).  Now it fails with:
        {"error": {"class": "GenericError",
          "desc": "Invalid parameter type for 'file', expected: BlockdevRef"}}
      (the error when the next type doesn't match any expected types for
      the overall alternate).
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1449033659-25497-5-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      0426d53c
    • E
      qapi: Convert QType into QAPI built-in enum type · 7264f5c5
      Eric Blake 提交于
      What's more meta than using qapi to define qapi? :)
      
      Convert QType into a full-fledged[*] builtin qapi enum type, so
      that a subsequent patch can then use it as the discriminator
      type of qapi alternate types.  Fortunately, the judicious use of
      'prefix' in the qapi definition avoids churn to the spelling of
      the enum constants.
      
      To avoid circular definitions, we have to flip the order of
      inclusion between "qobject.h" vs. "qapi-types.h".  Back in commit
      28770e05, we had the latter include the former, so that we could
      use 'QObject *' for our implementation of 'any'.  But that usage
      also works with only a forward declaration, whereas the
      definition of QObject requires QType to be a complete type.
      
      [*] The type has to be builtin, rather than declared in
      qapi/common.json, because we want to use it for alternates even
      when common.json is not included. But since it is the first
      builtin enum type, we have to add special cases to qapi-types
      and qapi-visit to only emit definitions once, even when two
      qapi files are being compiled into the same binary (the way we
      already handled builtin list types like 'intList').  We may
      need to revisit how multiple qapi files share common types,
      but that's a project for another day.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1449033659-25497-4-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      7264f5c5
    • E
      qapi: Don't let implicit enum MAX member collide · 7fb1cf16
      Eric Blake 提交于
      Now that we guarantee the user doesn't have any enum values
      beginning with a single underscore, we can use that for our
      own purposes.  Renaming ENUM_MAX to ENUM__MAX makes it obvious
      that the sentinel is generated.
      
      This patch was mostly generated by applying a temporary patch:
      
      |diff --git a/scripts/qapi.py b/scripts/qapi.py
      |index e6d014b..b862ec9 100644
      |--- a/scripts/qapi.py
      |+++ b/scripts/qapi.py
      |@@ -1570,6 +1570,7 @@ const char *const %(c_name)s_lookup[] = {
      |     max_index = c_enum_const(name, 'MAX', prefix)
      |     ret += mcgen('''
      |     [%(max_index)s] = NULL,
      |+// %(max_index)s
      | };
      | ''',
      |                max_index=max_index)
      
      then running:
      
      $ cat qapi-{types,event}.c tests/test-qapi-types.c |
          sed -n 's,^// \(.*\)MAX,s|\1MAX|\1_MAX|g,p' > list
      $ git grep -l _MAX | xargs sed -i -f list
      
      The only things not generated are the changes in scripts/qapi.py.
      
      Rejecting enum members named 'MAX' is now useless, and will be dropped
      in the next patch.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1447836791-369-23-git-send-email-eblake@redhat.com>
      Reviewed-by: NJuan Quintela <quintela@redhat.com>
      [Rebased to current master, commit message tweaked]
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      7fb1cf16
    • E
      qapi: Tighten the regex on valid names · 59a92fee
      Eric Blake 提交于
      We already documented that qapi names should match specific
      patterns (such as starting with a letter unless it was an enum
      value or a downstream extension).  Tighten that from a suggestion
      into a hard requirement, which frees up names beginning with a
      single underscore for qapi internal usage.
      
      The tighter regex doesn't forbid everything insane that a user
      could provide (for example, a user could name a type 'Foo-lookup'
      to collide with the generated 'Foo_lookup[]' for an enum 'Foo'),
      but does a good job at protecting the most obvious uses, and
      also happens to reserve single leading underscore for later use.
      
      The handling of enum values starting with a digit is tricky:
      commit 9fb081e0 introduced a subtle bug by using c_name() on
      a munged value, which would allow an enum to include the
      member 'q-int' in spite of our reservation.  Furthermore,
      munging with a leading '_' would fail our tighter regex.  So
      fix it by only munging for leading digits (which are never
      ticklish in c_name()) and by using a different prefix (I
      picked 'D', although any letter should do).
      
      Add new tests, reserved-member-underscore and reserved-enum-q,
      to demonstrate the tighter checking.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1447836791-369-22-git-send-email-eblake@redhat.com>
      Message-Id: <1447883135-18020-1-git-send-email-eblake@redhat.com>
      [Eric's fixup squashed in]
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      59a92fee
    • E
      blkdebug: Merge hand-rolled and qapi BlkdebugEvent enum · a31939e6
      Eric Blake 提交于
      No need to keep two separate enums, where editing one is likely
      to forget the other.  Now that we can specify a qapi enum prefix,
      we don't even have to change the bulk of the uses.
      
      get_event_by_name() could perhaps be replaced by qapi_enum_parse(),
      but I left that for another day.
      
      CC: Kevin Wolf <kwolf@redhat.com>
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1447836791-369-20-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      a31939e6
  12. 15 12月, 2015 1 次提交
  13. 25 11月, 2015 1 次提交
    • M
      vhost-user: clarify start and enable · c61f09ed
      Michael S. Tsirkin 提交于
      It seems that we currently have some duplication between
      started and enabled states.
      
      The actual reason is that enable is not documented correctly:
      what it does is connecting ring to the backend.
      
      This is important for MQ, because a Linux guest expects TX
      packets to be completed even if it disables some queues
      temporarily.
      
      Cc: Yuanhan Liu <yuanhan.liu@linux.intel.com>
      Cc: Victor Kaplansky <victork@redhat.com>
      Signed-off-by: NMichael S. Tsirkin <mst@redhat.com>
      c61f09ed
  14. 17 11月, 2015 3 次提交
  15. 16 11月, 2015 1 次提交
  16. 12 11月, 2015 4 次提交
  17. 10 11月, 2015 3 次提交
    • D
      2bfdd1c8
    • E
      qapi-introspect: Document lack of sorting · f5455044
      Eric Blake 提交于
      qapi-code-gen.txt already claims that types, commands, and
      events share a common namespace; set this in stone by further
      documenting that our introspection output will never have
      collisions with the same name tied to more than one meta-type.
      
      Our largest QMP enum currently has 125 values, our largest
      object type has 27 members, and the mean for each is less than
      10.  These sizes are small enough that the per-element overhead
      of O(log n) binary searching probably outweighs the speed
      possible with direct O(n) linear searching (a better algorithm
      with more overhead will only beat a leaner naive algorithm only
      as you scale to larger input sizes).
      
      Arguably, the overall SchemaInfo array could be sorted by name;
      there, we currently have 531 entities, large enough for a binary
      search to be faster than linear.  However, remember that we have
      mutually-recursive types, which means there is no topological
      ordering that will allow clients to learn all information about
      that type in a single linear pass; thus clients will want to do
      random access over the data, and they will probably read the
      introspection output into a hashtable for O(1) lookup rather
      than O(log n) binary searching, at which point, pre-sorting our
      introspection output doesn't help the client.
      
      It doesn't help that sorting can be subjective if you introduce
      locales into the mix (I'm not experienced enough with Python
      to know for sure, but at least it looks like it defaults to
      sorting in the C locale even when run under a different locale).
      And while our current introspection output is deterministic
      (because we visit entities in a sorted order), we may want
      to change that order in the future (such as using OrderedDict
      to stick to .json declaration order).
      
      For these reasons, we simply document that clients should not
      rely on any particular order of items in introspection output.
      And since it is now a documented part of the contract, we have
      the freedom to later rearrange output if needed, without
      worrying about breaking well-written clients.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1446791754-23823-13-git-send-email-eblake@redhat.com>
      [Commit message tweaked]
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      f5455044
    • E
      qapi: Provide nicer array names in introspection · ce5fcb47
      Eric Blake 提交于
      For the sake of humans reading introspection output, it is nice
      to have the name of implicit array types be recognizable as
      arrays of the underlying type.  However, while this patch allows
      humans to skip from a command with return type "[123]" straight
      to the definition of type "123" without having to first inspect
      type "[123]", document that this shortcut should not be taken by
      client apps.
      
      This makes the resulting introspection string slightly larger by
      default (just over 200 bytes), but it's in the noise (less than
      0.3% of the overall 70k size of 'query-qmp-capabilities').
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1446791754-23823-12-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      ce5fcb47
  18. 05 11月, 2015 1 次提交
  19. 02 11月, 2015 2 次提交
    • E
      qapi: Reserve 'q_*' and 'has_*' member names · 9fb081e0
      Eric Blake 提交于
      c_name() produces names starting with 'q_' when protecting a
      dictionary member name that would fail to directly compile, but
      in doing so can cause clashes with any member name already
      beginning with 'q-' or 'q_'.  Likewise, we create a C name 'has_'
      for any optional member that can clash with any member name
      beginning with 'has-' or 'has_'.
      
      Technically, rather than blindly reserving the namespace,
      we could try to complain about user names only when an actual
      collision occurs, or even teach c_name() how to munge names
      to avoid collisions.  But it is not trivial, especially when
      collisions can occur across multiple types (such as via
      inheritance or flat unions).  Besides, no existing .json
      files are trying to use these names.  So it's easier to just
      outright forbid the potential for collision.  We can always
      relax things in the future if a real need arises for QMP to
      express member names that have been forbidden here.
      
      'has_' only has to be reserved for struct/union member names,
      while 'q_' is reserved everywhere (matching the fact that
      only members can be optional, while we use c_name() for munging
      both members and entities).  Note that we could relax 'q_'
      restrictions on entities independently from member names; for
      example, c_name('qmp_' + 'unix') would result in a different
      function name than our current 'qmp_' + c_name('unix').
      
      Update and add tests to cover the new error messages.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1445898903-12082-6-git-send-email-eblake@redhat.com>
      [Consistently pass protect=False to c_name(); commit message tweaked
      slightly]
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      9fb081e0
    • E
      qapi: Reserve '*List' type names for list types · 255960dd
      Eric Blake 提交于
      Type names ending in 'List' can clash with qapi list types in
      generated C.  We don't currently use such names. It is easier to
      outlaw them now than to worry about how to resolve such a clash
      in the future. For precedence, see commit 4dc2e690, which did the
      same for names ending in 'Kind' versus implicit enum types for
      qapi unions.
      
      Update the testsuite to match.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1445898903-12082-5-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      255960dd
  20. 30 10月, 2015 1 次提交
  21. 26 10月, 2015 1 次提交
  22. 25 10月, 2015 2 次提交
  23. 22 10月, 2015 3 次提交