Skip to content
Snippets Groups Projects
  1. May 20, 2021
    • John Snow's avatar
      qapi/parser: Fix token membership tests when token can be None · c256263f
      John Snow authored
      
      When the token can be None (EOF), we can't use 'x in "abc"' style
      membership tests to group types of tokens together, because 'None in
      "abc"' is a TypeError.
      
      Easy enough to fix. (Use a tuple: It's neither a static typing error nor
      a runtime error to check for None in Tuple[str, ...])
      
      Add tests to prevent a regression. (Note: they cannot be added prior to
      this fix, as the unhandled stack trace will not match test output in the
      CI system.)
      
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      Message-Id: <20210519183951.3946870-11-jsnow@redhat.com>
      Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      c256263f
    • John Snow's avatar
      qapi: add must_match helper · e0e8a0ac
      John Snow authored
      
      Mypy cannot generally understand that these regex functions cannot
      possibly fail. Add a "must_match" helper that makes this clear for
      mypy.
      
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      Message-Id: <20210519183951.3946870-10-jsnow@redhat.com>
      Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      e0e8a0ac
    • John Snow's avatar
      qapi/parser: Use @staticmethod where appropriate · 43b1be65
      John Snow authored
      
      No self, no thank you!
      
      (Quiets pylint warnings.)
      
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      Message-Id: <20210519183951.3946870-9-jsnow@redhat.com>
      Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      43b1be65
    • John Snow's avatar
      qapi/parser: assert object keys are strings · 234dce2c
      John Snow authored
      
      The single quote token implies the value is a string. Assert this to be
      the case, to allow us to write an accurate return type for get_members.
      
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      Message-Id: <20210519183951.3946870-8-jsnow@redhat.com>
      Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      234dce2c
    • John Snow's avatar
      qapi/parser: enforce all top-level expressions must be dict in _parse() · 9cd0205d
      John Snow authored
      
      Instead of using get_expr nested=False, allow get_expr to always return
      any expression. In exchange, add a new error message to the top-level
      parser that explains the semantic error: Top-level expressions must
      always be JSON objects.
      
      This helps mypy understand the rest of this function which assumes that
      get_expr did indeed return a dict.
      
      The exception type changes from QAPIParseError to QAPISemError as a
      result, and the error message in two tests now changes.
      
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      
      Message-Id: <20210519183951.3946870-7-jsnow@redhat.com>
      Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      9cd0205d
    • John Snow's avatar
      qapi/parser: Assert lexer value is a string · 7c610ce6
      John Snow authored
      
      The type checker can't narrow the type of the token value to string,
      because it's only loosely correlated with the return token.
      
      We know that a token of '#' should always have a "str" value.
      Add an assertion.
      
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      Message-Id: <20210519183951.3946870-6-jsnow@redhat.com>
      Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      7c610ce6
    • John Snow's avatar
      qapi/parser: factor parsing routine into method · 16ff40ac
      John Snow authored
      
      For the sake of keeping __init__ smaller (and treating it more like a
      gallery of what state variables we can expect to see), put the actual
      parsing action into a parse method. It remains invoked from the init
      method to reduce churn.
      
      To accomplish this, @previously_included becomes the private data
      member ._included, and the filename is stashed as ._fname.
      
      Add any missing declarations to the init method, and group them by
      function so they can be understood quickly at a glance.
      
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      Message-Id: <20210519183951.3946870-5-jsnow@redhat.com>
      Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      16ff40ac
    • John Snow's avatar
      qapi/source: Remove line number from QAPISourceInfo initializer · b2b31fdf
      John Snow authored
      
      With the QAPISourceInfo(None, None, None) construct gone, there's no
      longer any reason to have to specify that a file starts on the first
      line. Remove it from the initializer and default it to 1.
      
      Remove the last vestiges where we check for 'line' being unset, that
      can't happen, now.
      
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      Message-Id: <20210519183951.3946870-4-jsnow@redhat.com>
      Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      b2b31fdf
    • John Snow's avatar
      qapi: Add test for nonexistent schema file · 334c3cd5
      John Snow authored
      
      This tests the error-return pathway introduced in the previous commit.
      (Thanks to Paolo for the help with the Meson magic.)
      
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      
      Message-Id: <20210519183951.3946870-3-jsnow@redhat.com>
      Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      334c3cd5
    • John Snow's avatar
      qapi/parser: Don't try to handle file errors · 3404e574
      John Snow authored
      
      Fixes: f5d4361c
      Fixes: 52a47418
      Fixes: 46f49468
      
      Remove the try/except block that handles file-opening errors in
      QAPISchemaParser.__init__() and add one each to
      QAPISchemaParser._include() and QAPISchema.__init__() respectively.
      
      This simultaneously fixes the typing of info.fname (f5d4361c), A
      static typing violation in test-qapi (46f49468), and a regression of
      an error message (52a47418).
      
      The short-ish version of what motivates this patch is:
      
      - It's hard to write a good error message in the init method,
        because we need to determine the context of our caller to do so.
        It's easier to just let the caller write the message.
      - We don't want to allow QAPISourceInfo(None, None, None) to exist. The
        typing introduced by commit f5d4361c types the 'fname' field as
        (non-optional) str, which was premature until the removal of this
        construct.
      - Errors made using such an object are currently incorrect (since
        52a47418)
      - It's not technically a semantic error if we cannot open the schema.
      - There are various typing constraints that make mixing these two cases
        undesirable for a single special case.
      - test-qapi's code handling an fname of 'None' is now dead, drop it.
        Additionally, Not all QAPIError objects have an 'info' field (since
        46f49468), so deleting this stanza corrects a typing oversight in
        test-qapi introduced by that commit.
      
      Other considerations:
      
      - open() is moved to a 'with' block to ensure file pointers are
        cleaned up deterministically.
      - Python 3.3 deprecated IOError and made it a synonym for OSError.
        Avoid the misleading perception these exception handlers are
        narrower than they really are.
      
      The long version:
      
      The error message here is incorrect (since commit 52a47418):
      
      > python3 qapi-gen.py 'fake.json'
      qapi-gen.py: qapi-gen.py: can't read schema file 'fake.json': No such file or directory
      
      In pursuing it, we find that QAPISourceInfo has a special accommodation
      for when there's no filename. Meanwhile, the intent when QAPISourceInfo
      was typed (f5d4361c) was non-optional 'str'. This usage was
      overlooked.
      
      To remove this, I'd want to avoid having a "fake" QAPISourceInfo
      object. I also don't want to explicitly begin accommodating
      QAPISourceInfo itself being None, because we actually want to eventually
      prove that this can never happen -- We don't want to confuse "The file
      isn't open yet" with "This error stems from a definition that wasn't
      defined in any file".
      
      (An earlier series tried to create a dummy info object, but it was tough
      to prove in review that it worked correctly without creating new
      regressions. This patch avoids that distraction. We would like to first
      prove that we never raise QAPISemError for any built-in object before we
      add "special" info objects. We aren't ready to do that yet.)
      
      So, which way out of the labyrinth?
      
      Here's one way: Don't try to handle errors at a level with "mixed"
      semantic contexts; i.e. don't mix inclusion errors (should report a
      source line where the include was triggered) and command line errors
      (where we specified a file we couldn't read).
      
      Remove the error handling from the initializer of the parser. Pythonic!
      Now it's the caller's job to figure out what to do about it. Handle the
      error in QAPISchemaParser._include() instead, where we can write a
      targeted error message where we are guaranteed to have an 'info' context
      to report with.
      
      The root level error can similarly move to QAPISchema.__init__(), where
      we know we'll never have an info context to report with, so we use a
      more abstract error type.
      
      Now the error looks sensible again:
      
      > python3 qapi-gen.py 'fake.json'
      qapi-gen.py: can't read schema file 'fake.json': No such file or directory
      
      With these error cases separated, QAPISourceInfo can be solidified as
      never having placeholder arguments that violate our desired types. Clean
      up test-qapi along similar lines.
      
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      Message-Id: <20210519183951.3946870-2-jsnow@redhat.com>
      Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      3404e574
  2. May 19, 2021
    • Peter Maydell's avatar
      Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging · d874bc08
      Peter Maydell authored
      
      Block layer patches
      
      - vhost-user-blk: Fix error handling during initialisation
      - Add test cases for the vhost-user-blk export
      - Fix leaked Transaction objects
      - qcow2: Expose dirty bit in 'qemu-img info'
      
      # gpg: Signature made Tue 18 May 2021 11:57:46 BST
      # gpg:                using RSA key DC3DEB159A9AF95D3D7456FE7F09B272C88F2FD6
      # gpg:                issuer "kwolf@redhat.com"
      # gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>" [full]
      # Primary key fingerprint: DC3D EB15 9A9A F95D 3D74  56FE 7F09 B272 C88F 2FD6
      
      * remotes/kevin/tags/for-upstream:
        vhost-user-blk: Check that num-queues is supported by backend
        virtio: Fail if iommu_platform is requested, but unsupported
        vhost-user-blk: Get more feature flags from vhost device
        vhost-user-blk: Improve error reporting in realize
        vhost-user-blk: Don't reconnect during initialisation
        vhost-user-blk: Make sure to set Error on realize failure
        vhost-user-blk-test: test discard/write zeroes invalid inputs
        tests/qtest: add multi-queue test case to vhost-user-blk-test
        test: new qTest case to test the vhost-user-blk-server
        block/export: improve vu_blk_sect_range_ok()
        block: Fix Transaction leak in bdrv_reopen_multiple()
        block: Fix Transaction leak in bdrv_root_attach_child()
        qcow2: set bdi->is_dirty
      
      Signed-off-by: default avatarPeter Maydell <peter.maydell@linaro.org>
      d874bc08
  3. May 18, 2021
Loading