1. 30 10月, 2018 1 次提交
  2. 19 10月, 2018 1 次提交
    • M
      error: Fix use of error_prepend() with &error_fatal, &error_abort · 4b576648
      Markus Armbruster 提交于
      From include/qapi/error.h:
      
        * Pass an existing error to the caller with the message modified:
        *     error_propagate(errp, err);
        *     error_prepend(errp, "Could not frobnicate '%s': ", name);
      
      Fei Li pointed out that doing error_propagate() first doesn't work
      well when @errp is &error_fatal or &error_abort: the error_prepend()
      is never reached.
      
      Since I doubt fixing the documentation will stop people from getting
      it wrong, introduce error_propagate_prepend(), in the hope that it
      lures people away from using its constituents in the wrong order.
      Update the instructions in error.h accordingly.
      
      Convert existing error_prepend() next to error_propagate to
      error_propagate_prepend().  If any of these get reached with
      &error_fatal or &error_abort, the error messages improve.  I didn't
      check whether that's the case anywhere.
      
      Cc: Fei Li <fli@suse.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NPhilippe Mathieu-Daudé <philmd@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Message-Id: <20181017082702.5581-2-armbru@redhat.com>
      4b576648
  3. 01 10月, 2018 6 次提交
  4. 29 8月, 2018 1 次提交
  5. 30 7月, 2018 1 次提交
  6. 10 7月, 2018 4 次提交
  7. 05 7月, 2018 2 次提交
  8. 29 6月, 2018 7 次提交
  9. 22 6月, 2018 1 次提交
  10. 15 6月, 2018 4 次提交
    • M
    • M
      block: Clean up a misuse of qobject_to() in .bdrv_co_create_opts() · 92adf9db
      Markus Armbruster 提交于
      The following pattern occurs in the .bdrv_co_create_opts() methods of
      parallels, qcow, qcow2, qed, vhdx and vpc:
      
          qobj = qdict_crumple_for_keyval_qiv(qdict, errp);
          qobject_unref(qdict);
          qdict = qobject_to(QDict, qobj);
          if (qdict == NULL) {
               ret = -EINVAL;
               goto done;
          }
      
          v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
          [...]
          ret = 0;
      done:
          qobject_unref(qdict);
          [...]
          return ret;
      
      If qobject_to() fails, we return failure without setting errp.  That's
      wrong.  As far as I can tell, it cannot fail here.  Clean it up
      anyway, by removing the useless conversion.
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NKevin Wolf <kwolf@redhat.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      92adf9db
    • M
      block: Fix -blockdev for certain non-string scalars · e5af0da1
      Markus Armbruster 提交于
      Configuration flows through the block subsystem in a rather peculiar
      way.  Configuration made with -drive enters it as QemuOpts.
      Configuration made with -blockdev / blockdev-add enters it as QAPI
      type BlockdevOptions.  The block subsystem uses QDict, QemuOpts and
      QAPI types internally.  The precise flow is next to impossible to
      explain (I tried for this commit message, but gave up after wasting
      several hours).  What I can explain is a flaw in the BlockDriver
      interface that leads to this bug:
      
          $ qemu-system-x86_64 -blockdev node-name=n1,driver=nfs,server.type=inet,server.host=localhost,path=/foo/bar,user=1234
          qemu-system-x86_64: -blockdev node-name=n1,driver=nfs,server.type=inet,server.host=localhost,path=/foo/bar,user=1234: Internal error: parameter user invalid
      
      QMP blockdev-add is broken the same way.
      
      Here's what happens.  The block layer passes configuration represented
      as flat QDict (with dotted keys) to BlockDriver methods
      .bdrv_file_open().  The QDict's members are typed according to the
      QAPI schema.
      
      nfs_file_open() converts it to QAPI type BlockdevOptionsNfs, with
      qdict_crumple() and a qobject input visitor.
      
      This visitor comes in two flavors.  The plain flavor requires scalars
      to be typed according to the QAPI schema.  That's the case here.  The
      keyval flavor requires string scalars.  That's not the case here.
      nfs_file_open() uses the latter, and promptly falls apart for members
      @user, @group, @tcp-syn-count, @readahead-size, @page-cache-size,
      @debug.
      
      Switching to the plain flavor would fix -blockdev, but break -drive,
      because there the scalars arrive in nfs_file_open() as strings.
      
      The proper fix would be to replace the QDict by QAPI type
      BlockdevOptions in the BlockDriver interface.  Sadly, that's beyond my
      reach right now.
      
      Next best would be to fix the block layer to always pass correctly
      typed QDicts to the BlockDriver methods.  Also beyond my reach.
      
      What I can do is throw another hack onto the pile: have
      nfs_file_open() convert all members to string, so use of the keyval
      flavor actually works, by replacing qdict_crumple() by new function
      qdict_crumple_for_keyval_qiv().
      
      The pattern "pass result of qdict_crumple() to
      qobject_input_visitor_new_keyval()" occurs several times more:
      
      * qemu_rbd_open()
      
        Same issue as nfs_file_open(), but since BlockdevOptionsRbd has only
        string members, its only a latent bug.  Fix it anyway.
      
      * parallels_co_create_opts(), qcow_co_create_opts(),
        qcow2_co_create_opts(), bdrv_qed_co_create_opts(),
        sd_co_create_opts(), vhdx_co_create_opts(), vpc_co_create_opts()
      
        These work, because they create the QDict with
        qemu_opts_to_qdict_filtered(), which creates only string scalars.
        The function sports a TODO comment asking for better typing; that's
        going to be fun.  Use qdict_crumple_for_keyval_qiv() to be safe.
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NKevin Wolf <kwolf@redhat.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      e5af0da1
    • M
      block: Add block-specific QDict header · 609f45ea
      Max Reitz 提交于
      There are numerous QDict functions that have been introduced for and are
      used only by the block layer.  Move their declarations into an own
      header file to reflect that.
      
      While qdict_extract_subqdict() is in fact used outside of the block
      layer (in util/qemu-config.c), it is still a function related very
      closely to how the block layer works with nested QDicts, namely by
      sometimes flattening them.  Therefore, its declaration is put into this
      header as well and util/qemu-config.c includes it with a comment stating
      exactly which function it needs.
      Suggested-by: NMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      Message-Id: <20180509165530.29561-7-mreitz@redhat.com>
      [Copyright note tweaked, superfluous includes dropped]
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NKevin Wolf <kwolf@redhat.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      609f45ea
  11. 11 6月, 2018 2 次提交
    • M
      qcow2: Do not mark inactive images corrupt · ddf3b47e
      Max Reitz 提交于
      When signaling a corruption on a read-only image, qcow2 already makes
      fatal events non-fatal (i.e., they will not result in the image being
      closed, and the image header's corrupt flag will not be set).  This is
      necessary because we cannot set the corrupt flag on read-only images,
      and it is possible because further corruption of read-only images is
      impossible.
      
      Inactive images are effectively read-only, too, so we should do the same
      for them.  bdrv_is_writable() can tell us whether an image can actually
      be written to, so use its result instead of !bs->read_only.
      
      (Otherwise, the assert(!(bs->open_flags & BDRV_O_INACTIVE)) in
      bdrv_co_pwritev() will fail, crashing qemu.)
      
      Cc: qemu-stable@nongnu.org
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      Message-id: 20180606193702.7113-3-mreitz@redhat.com
      Reviewed-by: NJohn Snow <jsnow@redhat.com>
      Reviewed-by: NJeff Cody <jcody@redhat.com>
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      ddf3b47e
    • M
      block: Add Error parameter to bdrv_amend_options · d1402b50
      Max Reitz 提交于
      Looking at the qcow2 code that is riddled with error_report() calls,
      this is really how it should have been from the start.
      
      Along the way, turn the target_version/current_version comparisons at
      the beginning of qcow2_downgrade() into assertions (the caller has to
      make sure these conditions are met), and rephrase the error message on
      using compat=1.1 to get refcount widths other than 16 bits.
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      Message-id: 20180509210023.20283-3-mreitz@redhat.com
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NJohn Snow <jsnow@redhat.com>
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      d1402b50
  12. 01 6月, 2018 1 次提交
  13. 31 5月, 2018 1 次提交
  14. 30 5月, 2018 1 次提交
  15. 15 5月, 2018 1 次提交
    • A
      qcow2: Give the refcount cache the minimum possible size by default · 52253998
      Alberto Garcia 提交于
      The L2 and refcount caches have default sizes that can be overridden
      using the l2-cache-size and refcount-cache-size (an additional
      parameter named cache-size sets the combined size of both caches).
      
      Unless forced by one of the aforementioned parameters, QEMU will set
      the unspecified sizes so that the L2 cache is 4 times larger than the
      refcount cache.
      
      This is based on the premise that the refcount metadata needs to be
      only a fourth of the L2 metadata to cover the same amount of disk
      space. This is incorrect for two reasons:
      
       a) The amount of disk covered by an L2 table depends solely on the
          cluster size, but in the case of a refcount block it depends on
          the cluster size *and* the width of each refcount entry.
          The 4/1 ratio is only valid with 16-bit entries (the default).
      
       b) When we talk about disk space and L2 tables we are talking about
          guest space (L2 tables map guest clusters to host clusters),
          whereas refcount blocks are used for host clusters (including
          L1/L2 tables and the refcount blocks themselves). On a fully
          populated (and uncompressed) qcow2 file, image size > virtual size
          so there are more refcount entries than L2 entries.
      
      Problem (a) could be fixed by adjusting the algorithm to take into
      account the refcount entry width. Problem (b) could be fixed by
      increasing a bit the refcount cache size to account for the clusters
      used for qcow2 metadata.
      
      However this patch takes a completely different approach and instead
      of keeping a ratio between both cache sizes it assigns as much as
      possible to the L2 cache and the remainder to the refcount cache.
      
      The reason is that L2 tables are used for every single I/O request
      from the guest and the effect of increasing the cache is significant
      and clearly measurable. Refcount blocks are however only used for
      cluster allocation and internal snapshots and in practice are accessed
      sequentially in most cases, so the effect of increasing the cache is
      negligible (even when doing random writes from the guest).
      
      So, make the refcount cache as small as possible unless the user
      explicitly asks for a larger one.
      Signed-off-by: NAlberto Garcia <berto@igalia.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NMax Reitz <mreitz@redhat.com>
      Message-id: 9695182c2eb11b77cb319689a1ebaa4e7c9d6591.1523968389.git.berto@igalia.com
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      52253998
  16. 04 5月, 2018 1 次提交
  17. 16 4月, 2018 1 次提交
  18. 27 3月, 2018 1 次提交
  19. 20 3月, 2018 1 次提交
    • M
      qapi: Replace qobject_to_X(o) by qobject_to(X, o) · 7dc847eb
      Max Reitz 提交于
      This patch was generated using the following Coccinelle script:
      
      @@
      expression Obj;
      @@
      (
      - qobject_to_qnum(Obj)
      + qobject_to(QNum, Obj)
      |
      - qobject_to_qstring(Obj)
      + qobject_to(QString, Obj)
      |
      - qobject_to_qdict(Obj)
      + qobject_to(QDict, Obj)
      |
      - qobject_to_qlist(Obj)
      + qobject_to(QList, Obj)
      |
      - qobject_to_qbool(Obj)
      + qobject_to(QBool, Obj)
      )
      
      and a bit of manual fix-up for overly long lines and three places in
      tests/check-qjson.c that Coccinelle did not find.
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      Reviewed-by: NAlberto Garcia <berto@igalia.com>
      Message-Id: <20180224154033.29559-4-mreitz@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      [eblake: swap order from qobject_to(o, X), rebase to master, also a fix
      to latent false-positive compiler complaint about hw/i386/acpi-build.c]
      Signed-off-by: NEric Blake <eblake@redhat.com>
      7dc847eb
  20. 09 3月, 2018 2 次提交
    • K
      block: x-blockdev-create QMP command · b0292b85
      Kevin Wolf 提交于
      This adds a synchronous x-blockdev-create QMP command that can create
      qcow2 images on a given node name.
      
      We don't want to block while creating an image, so this is not the final
      interface in all aspects, but BlockdevCreateOptionsQcow2 and
      .bdrv_co_create() are what they actually might look like in the end. In
      any case, this should be good enough to test whether we interpret
      BlockdevCreateOptions as we should.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NMax Reitz <mreitz@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      b0292b85
    • K
      qcow2: Use visitor for options in qcow2_create() · b76b4f60
      Kevin Wolf 提交于
      Instead of manually creating the BlockdevCreateOptions object, use a
      visitor to parse the given options into the QAPI object.
      
      This involves translation from the old command line syntax to the syntax
      mandated by the QAPI schema. Option names are still checked against
      qcow2_create_opts, so only the old option names are allowed on the
      command line, even if they are translated in qcow2_create().
      
      In contrast, new option values are optionally recognised besides the old
      values: 'compat' accepts 'v2'/'v3' as an alias for '0.10'/'1.1', and
      'encrypt.format' accepts 'qcow' as an alias for 'aes' now.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NMax Reitz <mreitz@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      b76b4f60