1. 19 5月, 2020 2 次提交
  2. 08 5月, 2020 1 次提交
    • E
      block: Drop unused .bdrv_has_zero_init_truncate · 47e0b38a
      Eric Blake 提交于
      Now that there are no clients of bdrv_has_zero_init_truncate, none of
      the drivers need to worry about providing it.
      
      What's more, this eliminates a source of some confusion: a literal
      reading of the documentation as written in ceaca56f and implemented in
      commit 1dcaf527 claims that a driver which returns 0 for
      bdrv_has_zero_init_truncate() must not return 1 for
      bdrv_has_zero_init(); this condition was violated for parallels, qcow,
      and sometimes for vdi, although in practice it did not matter since
      those drivers also lacked .bdrv_co_truncate.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <20200428202905.770727-10-eblake@redhat.com>
      Acked-by: NRichard W.M. Jones <rjones@redhat.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      47e0b38a
  3. 05 5月, 2020 1 次提交
  4. 30 4月, 2020 2 次提交
  5. 26 3月, 2020 1 次提交
  6. 28 10月, 2019 2 次提交
    • M
      block: Pass truncate exact=true where reasonable · e8d04f92
      Max Reitz 提交于
      This is a change in behavior, so all instances need a good
      justification.  The comments added here should explain my reasoning.
      
      qed already had a comment that suggests it always expected
      bdrv_truncate()/blk_truncate() to behave as if exact=true were passed
      (c743849b came eight months before 55b949c8), so it was simply
      broken until now.
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      Message-id: 20190918095144.955-8-mreitz@redhat.com
      Reviewed-by: NMaxim Levitsky <mlevitsk@redhat.com>
      [mreitz: Changed comment in qed.c to explain why a new QED file must be
               empty, as requested and suggested by Maxim]
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      e8d04f92
    • M
      block: Add @exact parameter to bdrv_co_truncate() · c80d8b06
      Max Reitz 提交于
      We have two drivers (iscsi and file-posix) that (in some cases) return
      success from their .bdrv_co_truncate() implementation if the block
      device is larger than the requested offset, but cannot be shrunk.  Some
      callers do not want that behavior, so this patch adds a new parameter
      that they can use to turn off that behavior.
      
      This patch just adds the parameter and lets the block/io.c and
      block/block-backend.c functions pass it around.  All other callers
      always pass false and none of the implementations evaluate it, so that
      this patch does not change existing behavior.  Future patches take care
      of that.
      Suggested-by: NMaxim Levitsky <mlevitsk@redhat.com>
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      Message-id: 20190918095144.955-5-mreitz@redhat.com
      Reviewed-by: NMaxim Levitsky <mlevitsk@redhat.com>
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      c80d8b06
  7. 19 8月, 2019 1 次提交
  8. 16 8月, 2019 1 次提交
  9. 12 6月, 2019 1 次提交
  10. 04 6月, 2019 1 次提交
    • K
      block: Add BlockBackend.ctx · d861ab3a
      Kevin Wolf 提交于
      This adds a new parameter to blk_new() which requires its callers to
      declare from which AioContext this BlockBackend is going to be used (or
      the locks of which AioContext need to be taken anyway).
      
      The given context is only stored and kept up to date when changing
      AioContexts. Actually applying the stored AioContext to the root node
      is saved for another commit.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      d861ab3a
  11. 30 4月, 2019 2 次提交
  12. 25 2月, 2019 1 次提交
    • M
      block: Add BDS.auto_backing_file · 998c2019
      Max Reitz 提交于
      If the backing file is overridden, this most probably does change the
      guest-visible data of a BDS.  Therefore, we will need to consider this
      in bdrv_refresh_filename().
      
      To see whether it has been overridden, we might want to compare
      bs->backing_file and bs->backing->bs->filename.  However,
      bs->backing_file is changed by bdrv_set_backing_hd() (which is just used
      to change the backing child at runtime, without modifying the image
      header), so bs->backing_file most of the time simply contains a copy of
      bs->backing->bs->filename anyway, so it is useless for such a
      comparison.
      
      This patch adds an auto_backing_file BDS field which contains the
      backing file path as indicated by the image header, which is not changed
      by bdrv_set_backing_hd().
      
      Because of bdrv_refresh_filename() magic, however, a BDS's filename may
      differ from what has been specified during bdrv_open().  Then, the
      comparison between bs->auto_backing_file and bs->backing->bs->filename
      may fail even though bs->backing was opened from bs->auto_backing_file.
      To mitigate this, we can copy the real BDS's filename (after the whole
      bdrv_open() and bdrv_refresh_filename() process) into
      bs->auto_backing_file, if we know the former has been opened based on
      the latter.  This is only possible if no options modifying the backing
      file's behavior have been specified, though.  To simplify things, this
      patch only copies the filename from the backing file if no options have
      been specified for it at all.
      
      Furthermore, there are cases where an overlay is created by qemu which
      already contains a BDS's filename (e.g. in blockdev-snapshot-sync).  We
      do not need to worry about updating the overlay's bs->auto_backing_file
      there, because we actually wrote a post-bdrv_refresh_filename() filename
      into the image header.
      
      So all in all, there will be false negatives where (as of a future
      patch) bdrv_refresh_filename() will assume that the backing file differs
      from what was specified in the image header, even though it really does
      not.  However, these cases should be limited to where (1) the user
      actually did override something in the backing chain (e.g. by specifying
      options for the backing file), or (2) the user executed a QMP command to
      change some node's backing file (e.g. change-backing-file or
      block-commit with @backing-file given) where the given filename does not
      happen to coincide with qemu's idea of the backing BDS's filename.
      
      Then again, (1) really is limited to -drive.  With -blockdev or
      blockdev-add, you have to adhere to the schema, so a user cannot give
      partial "unimportant" options (e.g. by just setting backing.node-name
      and leaving the rest to the image header).  Therefore, trying to fix
      this would mean trying to fix something for -drive only.
      
      To improve on (2), we would need a full infrastructure to "canonicalize"
      an arbitrary filename (+ options), so it can be compared against
      another.  That seems a bit over the top, considering that filenames
      nowadays are there mostly for the user's entertainment.
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NAlberto Garcia <berto@igalia.com>
      Message-id: 20190201192935.18394-5-mreitz@redhat.com
      Signed-off-by: NMax Reitz <mreitz@redhat.com>
      998c2019
  13. 22 2月, 2019 1 次提交
  14. 01 2月, 2019 1 次提交
    • K
      block: Fix hangs in synchronous APIs with iothreads · 4720cbee
      Kevin Wolf 提交于
      In the block layer, synchronous APIs are often implemented by creating a
      coroutine that calls the asynchronous coroutine-based implementation and
      then waiting for completion with BDRV_POLL_WHILE().
      
      For this to work with iothreads (more specifically, when the synchronous
      API is called in a thread that is not the home thread of the block
      device, so that the coroutine will run in a different thread), we must
      make sure to call aio_wait_kick() at the end of the operation. Many
      places are missing this, so that BDRV_POLL_WHILE() keeps hanging even if
      the condition has long become false.
      
      Note that bdrv_dec_in_flight() involves an aio_wait_kick() call. This
      corresponds to the BDRV_POLL_WHILE() in the drain functions, but it is
      generally not enough for most other operations because they haven't set
      the return value in the coroutine entry stub yet. To avoid race
      conditions there, we need to kick after setting the return value.
      
      The race window is small enough that the problem doesn't usually surface
      in the common path. However, it does surface and causes easily
      reproducible hangs if the operation can return early before even calling
      bdrv_inc/dec_in_flight, which many of them do (trivial error or no-op
      success paths).
      
      The bug in bdrv_truncate(), bdrv_check() and bdrv_invalidate_cache() is
      slightly different: These functions even neglected to schedule the
      coroutine in the home thread of the node. This avoids the hang, but is
      obviously wrong, too. Fix those to schedule the coroutine in the right
      AioContext in addition to adding aio_wait_kick() calls.
      
      Cc: qemu-stable@nongnu.org
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      4720cbee
  15. 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
  16. 29 6月, 2018 1 次提交
    • K
      block: Convert .bdrv_truncate callback to coroutine_fn · 061ca8a3
      Kevin Wolf 提交于
      bdrv_truncate() is an operation that can block (even for a quite long
      time, depending on the PreallocMode) in I/O paths that shouldn't block.
      Convert it to a coroutine_fn so that we have the infrastructure for
      drivers to make their .bdrv_co_truncate implementation asynchronous.
      
      This change could potentially introduce new race conditions because
      bdrv_truncate() isn't necessarily executed atomically any more. Whether
      this is a problem needs to be evaluated for each block driver that
      supports truncate:
      
      * file-posix/win32, gluster, iscsi, nfs, rbd, ssh, sheepdog: The
        protocol drivers are trivially safe because they don't actually yield
        yet, so there is no change in behaviour.
      
      * copy-on-read, crypto, raw-format: Essentially just filter drivers that
        pass the request to a child node, no problem.
      
      * qcow2: The implementation modifies metadata, so it needs to hold
        s->lock to be safe with concurrent I/O requests. In order to avoid
        double locking, this requires pulling the locking out into
        preallocate_co() and using qcow2_write_caches() instead of
        bdrv_flush().
      
      * qed: Does a single header update, this is fine without locking.
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      061ca8a3
  17. 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
  18. 15 5月, 2018 1 次提交
    • E
      block: Merge .bdrv_co_writev{,_flags} in drivers · e18a58b4
      Eric Blake 提交于
      We have too many driver callback interfaces; simplify the mess
      somewhat by merging the flags parameter of .bdrv_co_writev_flags()
      into .bdrv_co_writev().  Note that as long as a driver doesn't set
      .supported_write_flags, the flags argument will be 0 and behavior is
      identical.  Also note that the public function bdrv_co_writev() still
      lacks a flags argument; so the driver signature is thus intentionally
      slightly different.  But that's not the end of the world, nor the first
      time that the driver interface differs slightly from the public
      interface.
      
      Ideally, we should be rewriting all of these drivers to use modern
      byte-based interfaces.  But that's a more invasive patch to write
      and audit, compared to the simplification done here.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>
      Signed-off-by: NKevin Wolf <kwolf@redhat.com>
      e18a58b4
  19. 04 5月, 2018 1 次提交
  20. 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
  21. 19 3月, 2018 1 次提交
  22. 09 3月, 2018 3 次提交
  23. 03 3月, 2018 2 次提交
  24. 10 2月, 2018 1 次提交
    • E
      block: Simplify bdrv_can_write_zeroes_with_unmap() · e24d813b
      Eric Blake 提交于
      We don't need the can_write_zeroes_with_unmap field in
      BlockDriverInfo, because it is redundant information with
      supported_zero_flags & BDRV_REQ_MAY_UNMAP.  Note that
      BlockDriverInfo and supported_zero_flags are both per-device
      settings, rather than global state about the driver as a
      whole, which means one or both of these bits of information
      can already be conditional.  Let's audit how they were set:
      
      crypto: always setting can_write_ to false is pointless (the
      struct starts life zero-initialized), no use of supported_
      
      nbd: just recently fixed to set can_write_ if supported_
      includes MAY_UNMAP (thus this commit effectively reverts
      bca80059e and solves the problem mentioned there in a more
      global way)
      
      file-posix, iscsi, qcow2: can_write_ is conditional, while
      supported_ was unconditional; but passing MAY_UNMAP would
      fail with ENOTSUP if the condition wasn't met
      
      qed: can_write_ is unconditional, but pwrite_zeroes lacks
      support for MAY_UNMAP and supported_ is not set. Perhaps
      support can be added later (since it would be similar to
      qcow2), but for now claiming false is no real loss
      
      all other drivers: can_write_ is not set, and supported_ is
      either unset or a passthrough
      
      Simplify the code by moving the conditional into
      supported_zero_flags for all drivers, then dropping the
      now-unused BDI field.  For callers that relied on
      bdrv_can_write_zeroes_with_unmap(), we return the same
      per-device settings for drivers that had conditions (no
      observable change in behavior there); and can now return
      true (instead of false) for drivers that support passthrough
      (for example, the commit driver) which gives those drivers
      the same fix as nbd just got in bca80059e.  For callers that
      relied on supported_zero_flags, we now have a few more places
      that can avoid a wasted call to pwrite_zeroes() that will
      just fail with ENOTSUP.
      Suggested-by: NPaolo Bonzini <pbonzini@redhat.com>
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <20180126193439.20219-1-eblake@redhat.com>
      Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
      e24d813b
  25. 09 2月, 2018 2 次提交
  26. 13 10月, 2017 1 次提交
  27. 04 9月, 2017 1 次提交
  28. 17 7月, 2017 2 次提交