1. 17 12月, 2015 12 次提交
    • E
      qapi: Enforce (or whitelist) case conventions on qapi members · 893e1f2c
      Eric Blake 提交于
      We document that members of enums and objects should be
      'lower-case', although we were not enforcing it.  We have to
      whitelist a few pre-existing entities that violate the norms.
      Add three new tests to expose the new error message, each of
      which first uses the whitelisted name 'UuidInfo' to prove the
      whitelist works, then triggers the failure (this is the same
      pattern used in the existing returns-whitelist.json test).
      
      Note that by adding this check, we have effectively forbidden
      an entity with a case-insensitive clash of member names, for
      any entity that is not on the whitelist (although there is
      still the possibility to clash via '-' vs. '_').
      
      Not done here: a future patch should also add naming convention
      support and whitelist exceptions for command, event, and type
      names.
      
      The additions to QAPISchemaMember.check_clash() check whether
      info['name'] is in the whitelist (the top-most entity name at
      the point 'info' tracks), rather than self.owner (the type,
      possibly implicit, that directly owns the member), because it
      is easier to maintain the whitelist by the names actually in
      the user's .json file, rather than worrying about the names
      of implicit types.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1449033659-25497-14-git-send-email-eblake@redhat.com>
      [Simplified a bit as per discussion with Eric]
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      893e1f2c
    • E
      qapi: Fix alternates that accept 'number' but not 'int' · d00341af
      Eric Blake 提交于
      The QMP input visitor allows integral values to be assigned by
      promotion to a QTYPE_QFLOAT.  However, when parsing an alternate,
      we did not take this into account, such that an alternate that
      accepts 'number' and some other type, but not 'int', would reject
      integral values.
      
      With this patch, we now have the following desirable table:
      
          alternate has      case selected for
          'int'  'number'    QTYPE_QINT  QTYPE_QFLOAT
            no        no     error       error
            no       yes     'number'    'number'
           yes        no     'int'       error
           yes       yes     'int'       'number'
      
      While it is unlikely that we will ever use 'number' in an
      alternate other than in the testsuite, it never hurts to be
      more precise in what we allow.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1449033659-25497-8-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      d00341af
    • 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: Remove obsolete tests for MAX collision · 04e0639d
      Eric Blake 提交于
      Now that we no longer collide with an implicit _MAX enum member,
      we no longer need to reject it in the ad hoc parser, and can
      remove several tests that are no longer needed.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1447836791-369-24-git-send-email-eblake@redhat.com>
      [Commit message tweaked]
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      04e0639d
    • 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: Avoid '.' in enum values · 5be5b776
      Eric Blake 提交于
      Our qapi conventions document that '.' should only be used in
      the prefix of downstream names.  BlkdebugEvent was a lone
      exception to this.  Changing this is not backwards compatible
      to the 'blockdev-add' QMP command; however, that command is
      not yet fully stable.  It can also be argued that the testsuite
      is the biggest user of blkdebug, and that any other user can
      be taught to deal with the change by paying attention to
      introspection results.
      
      Done with:
      
      $ for str in \
           l1_grow.{alloc,write,activate}_table \
           l2_alloc.{cow_read,write} \
           refblock_alloc.{hookup,write,write_blocks,write_table,switch_table} \
           pwritev_rmw.{head,after_head,tail,after_tail}; do
         str1=$(echo "$str" | sed 's/\./\\./')
         str2=$(echo "$str" | sed 's/\./_/')
         git grep -l "$str1" | xargs -r sed -i "s/$str1/$str2/g"
       done
      
      followed by a manual touchup to test 77 to keep the test working.
      Reported-by: NMarkus Armbruster <armbru@redhat.com>
      CC: Kevin Wolf <kwolf@redhat.com>
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1447836791-369-21-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      5be5b776
    • E
      qapi: Fix c_name() munging · c43567c1
      Eric Blake 提交于
      The method c_name() is supposed to do two different actions: munge
      '-' into '_', and add a 'q_' prefix to ticklish names.  But it did
      these steps out of order, making it possible to submit input that
      is not ticklish until after munging, where the output then lacked
      the desired prefix.
      
      The failure is exposed easily if you have a compiler that recognizes
      C11 keywords, and try to name a member '_Thread-local', as it would
      result in trying to compile the declaration 'uint64_t _Thread_local;'
      which is not valid.  However, this name violates our conventions
      (ultimately, want to enforce that no qapi names start with single
      underscore), so the test is slightly weaker by instead testing
      'wchar-t'; the declaration 'uint64_t wchar_t;' is valid in C (where
      wchar_t is only a typedef) but would fail with a C++ compiler (where
      it is a keyword).
      
      Fix things by reversing the order of actions within c_name().
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1447836791-369-18-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      c43567c1
    • E
      qapi: Detect collisions in C member names · 27b60ab9
      Eric Blake 提交于
      Detect attempts to declare two object members that would result
      in the same C member name, by keying the 'seen' dictionary off
      of the C name rather than the qapi name.  It also requires passing
      info through the check_clash() methods.
      
      This addresses a TODO and fixes the previously-broken
      args-name-clash test.  The resulting error message demonstrates
      the utility of the .describe() method added previously.  No change
      to generated code.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1447836791-369-17-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      27b60ab9
    • E
      qapi: Remove outdated tests related to QMP/branch collisions · 61a94661
      Eric Blake 提交于
      Now that branches are in a separate C namespace, we can remove
      the restrictions in the parser that claim a branch name would
      collide with QMP, and delete the negative tests that are no
      longer problematic.  A separate patch can then add positive
      tests to qapi-schema-test to test that any corner cases will
      compile correctly.
      
      This reverts the scripts/qapi.py portion of commit 7b2a5c2f,
      now that the assertions that it plugged are no longer possible.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1447836791-369-15-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      61a94661
    • E
      qapi: Track simple union tag in object.local_members · da34a9bd
      Eric Blake 提交于
      We were previously creating all unions with an empty list for
      local_members.  However, it will make it easier to unify struct
      and union generation if we include the generated tag member in
      local_members.  That way, we can have a common code pattern:
      visit the base (if any), visit the local members (if any), visit
      the variants (if any).  The local_members of a flat union
      remains empty (because the discriminator is already visited as
      part of the base).  Then, by visiting tag_member.check() during
      AlternateType.check(), we no longer need to call it during
      Variants.check().
      
      The various front end entities now exist as follows:
      struct: optional base, optional local_members, no variants
      simple union: no base, one-element local_members, variants with tag_member
        from local_members
      flat union: base, no local_members, variants with tag_member from base
      alternate: no base, no local_members, variants
      
      With the new local members, we require a bit of finesse to
      avoid assertions in the clients.
      
      No change to generated code.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1447836791-369-2-git-send-email-eblake@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      da34a9bd
  2. 11 12月, 2015 1 次提交
  3. 05 12月, 2015 2 次提交
  4. 04 12月, 2015 1 次提交
  5. 03 12月, 2015 3 次提交
  6. 02 12月, 2015 3 次提交
  7. 27 11月, 2015 1 次提交
  8. 26 11月, 2015 5 次提交
  9. 25 11月, 2015 8 次提交
  10. 21 11月, 2015 1 次提交
  11. 19 11月, 2015 3 次提交
    • M
      tests: re-enable vhost-user-test · 421f4448
      Marc-André Lureau 提交于
      Commit 7fe34ca9 actually disabled vhost-user-test altogether,
      since CONFIG_VHOST_NET is a per-target config variable.
      
      tests/vhost-user-test is already x86/x64 softmmu specific test, in order
      to enable it correctly, kvm & vhost-net are also conditions. To check
      that, set CONFIG_VHOST_NET_TEST_$target when kvm is also enabled.
      
      Since "check-qtest-x86_64-y = $(check-qtest-i386-y)", avoid duplication
      when both x86 & x64 are enabled.
      
      Other targets than x86 aren't enabled yet, and is intentionally left as
      a future improvement, since I can't easily test those.
      Signed-off-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
      Reviewed-by: NMichael S. Tsirkin <mst@redhat.com>
      Signed-off-by: NMichael S. Tsirkin <mst@redhat.com>
      421f4448
    • D
      qom: Add a test case for complex property finalization · 8c4d156c
      Daniel P. Berrange 提交于
      Devices have some quite complex object child/link relationships
      which place some requirements on the object_property_del_all()
      function to consider that properties can be modified while
      being iterated over.
      
      This extends the QOM property test case to replicate the
      device like structure and expose any potential bugs in the
      object_property_del_all() function.
      Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
      Signed-off-by: NAndreas Färber <afaerber@suse.de>
      8c4d156c
    • D
      qom: Introduce ObjectPropertyIterator struct for iteration · a00c9482
      Daniel P. Berrange 提交于
      Some users of QOM need to be able to iterate over properties
      defined against an object instance. Currently they are just
      directly using the QTAIL macros against the object properties
      data structure.
      
      This is bad because it exposes them to changes in the data
      structure used to store properties, as well as changes in
      functionality such as ability to register properties against
      the class.
      
      This provides an ObjectPropertyIterator struct which will
      insulate the callers from the particular data structure
      used to store properties. It can be used thus
      
        ObjectProperty *prop;
        ObjectPropertyIterator *iter;
      
        iter = object_property_iter_init(obj);
        while ((prop = object_property_iter_next(iter))) {
            ... do something with prop ...
        }
        object_property_iter_free(iter);
      Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
      Tested-by: NPavel Fedin <p.fedin@samsung.com>
      [AF: Fixed examples, style cleanups]
      Signed-off-by: NAndreas Färber <afaerber@suse.de>
      a00c9482