Skip to content
Snippets Groups Projects
  1. Feb 23, 2023
  2. Feb 01, 2023
    • Hanna Reitz's avatar
      qemu-img: Change info key names for protocol nodes · d570177b
      Hanna Reitz authored
      
      Currently, when querying a qcow2 image, qemu-img info reports something
      like this:
      
      image: test.qcow2
      file format: qcow2
      virtual size: 64 MiB (67108864 bytes)
      disk size: 196 KiB
      cluster_size: 65536
      Format specific information:
          compat: 1.1
          compression type: zlib
          lazy refcounts: false
          refcount bits: 16
          corrupt: false
          extended l2: false
      Child node '/file':
          image: test.qcow2
          file format: file
          virtual size: 192 KiB (197120 bytes)
          disk size: 196 KiB
          Format specific information:
              extent size hint: 1048576
      
      Notably, the way the keys are named is specific for image files: The
      filename is shown under "image", the BDS driver under "file format", and
      the BDS length under "virtual size".  This does not make much sense for
      nodes that are not actually supposed to be guest images, like the /file
      child node shown above.
      
      Give bdrv_node_info_dump() a @protocol parameter that gives a hint that
      the respective node is probably just used for data storage and does not
      necessarily present the data for a VM guest disk.  This renames the keys
      so that with this patch, the output becomes:
      
      image: test.qcow2
      [...]
      Child node '/file':
          filename: test.qcow2
          protocol type: file
          file length: 192 KiB (197120 bytes)
          disk size: 196 KiB
          Format specific information:
              extent size hint: 1048576
      
      (Perhaps we should also rename "Format specific information", but I
      could not come up with anything better that will not become problematic
      if we guess wrong with the protocol "heuristic".)
      
      This change affects iotest 302, which has protocol node information in
      its reference output.
      
      Signed-off-by: default avatarHanna Reitz <hreitz@redhat.com>
      Message-Id: <20220620162704.80987-13-hreitz@redhat.com>
      Reviewed-by: default avatarKevin Wolf <kwolf@redhat.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      d570177b
    • Hanna Reitz's avatar
      qemu-img: Let info print block graph · c04d0ab0
      Hanna Reitz authored
      
      For every node in the backing chain, collect its BlockGraphInfo struct
      using bdrv_query_block_graph_info().  Print all nodes' information,
      indenting child nodes and labelling them with a path constructed from
      the child names leading to the node from the root (e.g. /file/file).
      
      Note that we open each image with BDRV_O_NO_BACKING, so its backing
      child is omitted from this graph, and thus presented in the previous
      manner: By simply concatenating all images' information, separated with
      blank lines.
      
      This affects two iotests:
      - 065: Here we try to get the format node's format specific information.
        The pre-patch code does so by taking all lines from "Format specific
        information:" until an empty line.  This format specific information
        is no longer followed by an empty line, though, but by child node
        information, so limit the range by "Child node '/file':".
      - 302: Calls qemu_img() for qemu-img info directly, which does not
        filter the output, so the child node information ends up in the
        output.
      
      Signed-off-by: default avatarHanna Reitz <hreitz@redhat.com>
      Message-Id: <20220620162704.80987-12-hreitz@redhat.com>
      Reviewed-by: default avatarKevin Wolf <kwolf@redhat.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      c04d0ab0
    • Hanna Reitz's avatar
      block/qapi: Add indentation to bdrv_node_info_dump() · 76c9e975
      Hanna Reitz authored
      
      In order to let qemu-img info present a block graph, add a parameter to
      bdrv_node_info_dump() and bdrv_image_info_specific_dump() so that the
      information of nodes below the root level can be given an indentation.
      
      Signed-off-by: default avatarHanna Reitz <hreitz@redhat.com>
      Message-Id: <20220620162704.80987-9-hreitz@redhat.com>
      Reviewed-by: default avatarKevin Wolf <kwolf@redhat.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      76c9e975
    • Hanna Reitz's avatar
      qemu-img: Use BlockNodeInfo · b1f4cd15
      Hanna Reitz authored
      
      qemu-img info never uses ImageInfo's backing-image field, because it
      opens the backing chain one by one with BDRV_O_NO_BACKING, and prints
      all backing chain nodes' information consecutively.  Use BlockNodeInfo
      to make it clear that we only print information about a single node, and
      that we are not using the backing-image field.
      
      Notably, bdrv_image_info_dump() does not evaluate the backing-image
      field, so we can easily make it take a BlockNodeInfo pointer (and
      consequentially rename it to bdrv_node_info_dump()).  It makes more
      sense this way, because again, the interface now makes it syntactically
      clear that backing-image is ignored by this function.
      
      Signed-off-by: default avatarHanna Reitz <hreitz@redhat.com>
      Message-Id: <20220620162704.80987-6-hreitz@redhat.com>
      Reviewed-by: default avatarKevin Wolf <kwolf@redhat.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      b1f4cd15
  3. Jan 24, 2023
  4. Jan 20, 2023
    • Markus Armbruster's avatar
      include/block: Untangle inclusion loops · e2c1c34f
      Markus Armbruster authored
      
      We have two inclusion loops:
      
             block/block.h
          -> block/block-global-state.h
          -> block/block-common.h
          -> block/blockjob.h
          -> block/block.h
      
             block/block.h
          -> block/block-io.h
          -> block/block-common.h
          -> block/blockjob.h
          -> block/block.h
      
      I believe these go back to Emanuele's reorganization of the block API,
      merged a few months ago in commit d7e2fe4a.
      
      Fortunately, breaking them is merely a matter of deleting unnecessary
      includes from headers, and adding them back in places where they are
      now missing.
      
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Message-Id: <20221221133551.3967339-2-armbru@redhat.com>
      e2c1c34f
  5. Dec 14, 2022
    • Markus Armbruster's avatar
      qapi block: Elide redundant has_FOO in generated C · 54fde4ff
      Markus Armbruster authored
      
      The has_FOO for pointer-valued FOO are redundant, except for arrays.
      They are also a nuisance to work with.  Recent commit "qapi: Start to
      elide redundant has_FOO in generated C" provided the means to elide
      them step by step.  This is the step for qapi/block*.json.
      
      Said commit explains the transformation in more detail.
      
      There is one instance of the invariant violation mentioned there:
      qcow2_signal_corruption() passes false, "" when node_name is an empty
      string.  Take care to pass NULL then.
      
      The previous two commits cleaned up two more.
      
      Additionally, helper bdrv_latency_histogram_stats() loses its output
      parameters and returns a value instead.
      
      Cc: Kevin Wolf <kwolf@redhat.com>
      Cc: Hanna Reitz <hreitz@redhat.com>
      Cc: qemu-block@nongnu.org
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Message-Id: <20221104160712.3005652-11-armbru@redhat.com>
      [Fixes for #ifndef LIBRBD_SUPPORTS_ENCRYPTION and MacOS squashed in]
      54fde4ff
  6. Nov 11, 2022
  7. Oct 26, 2022
    • Stefan Hajnoczi's avatar
      block: return errors from bdrv_register_buf() · f4ec04ba
      Stefan Hajnoczi authored
      
      Registering an I/O buffer is only a performance optimization hint but it
      is still necessary to return errors when it fails.
      
      Later patches will need to detect errors when registering buffers but an
      immediate advantage is that error_report() calls are no longer needed in
      block driver .bdrv_register_buf() functions.
      
      Signed-off-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
      Message-id: 20221013185908.1297568-8-stefanha@redhat.com
      Signed-off-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
      f4ec04ba
    • Stefan Hajnoczi's avatar
      block: pass size to bdrv_unregister_buf() · 4f384011
      Stefan Hajnoczi authored
      
      The only implementor of bdrv_register_buf() is block/nvme.c, where the
      size is not needed when unregistering a buffer. This is because
      util/vfio-helpers.c can look up mappings by address.
      
      Future block drivers that implement bdrv_register_buf() may not be able
      to do their job given only the buffer address. Add a size argument to
      bdrv_unregister_buf().
      
      Also document the assumptions about
      bdrv_register_buf()/bdrv_unregister_buf() calls. The same <host, size>
      values that were given to bdrv_register_buf() must be given to
      bdrv_unregister_buf().
      
      gcc 11.2.1 emits a spurious warning that img_bench()'s buf_size local
      variable might be uninitialized, so it's necessary to silence the
      compiler.
      
      Signed-off-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
      Reviewed-by: default avatarStefano Garzarella <sgarzare@redhat.com>
      Message-id: 20221013185908.1297568-5-stefanha@redhat.com
      Signed-off-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
      4f384011
  8. Oct 07, 2022
    • Emanuele Giuseppe Esposito's avatar
      job.c: enable job lock/unlock and remove Aiocontext locks · 6f592e5a
      Emanuele Giuseppe Esposito authored
      
      Change the job_{lock/unlock} and macros to use job_mutex.
      
      Now that they are not nop anymore, remove the aiocontext
      to avoid deadlocks.
      
      Therefore:
      - when possible, remove completely the aiocontext lock/unlock pair
      - if it is used by some other function too, reduce the locking
        section as much as possible, leaving the job API outside.
      - change AIO_WAIT_WHILE in AIO_WAIT_WHILE_UNLOCKED, since we
        are not using the aiocontext lock anymore
      
      The only functions that still need the aiocontext lock are:
      - the JobDriver callbacks, already documented in job.h
      - job_cancel_sync() in replication.c is called with aio_context_lock
        taken, but now job is using AIO_WAIT_WHILE_UNLOCKED so we need to
        release the lock.
      
      Reduce the locking section to only cover the callback invocation
      and document the functions that take the AioContext lock,
      to avoid taking it twice.
      
      Also remove real_job_{lock/unlock}, as they are replaced by the
      public functions.
      
      Signed-off-by: default avatarEmanuele Giuseppe Esposito <eesposit@redhat.com>
      Message-Id: <20220926093214.506243-19-eesposit@redhat.com>
      Reviewed-by: default avatarKevin Wolf <kwolf@redhat.com>
      Reviewed-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      6f592e5a
    • Emanuele Giuseppe Esposito's avatar
      jobs: group together API calls under the same job lock · 880eeec6
      Emanuele Giuseppe Esposito authored
      
      Now that the API offers also _locked() functions, take advantage
      of it and give also the caller control to take the lock and call
      _locked functions.
      
      This makes sense especially when we have for loops, because it
      makes no sense to have:
      
      for(job = job_next(); ...)
      
      where each job_next() takes the lock internally.
      Instead we want
      
      JOB_LOCK_GUARD();
      for(job = job_next_locked(); ...)
      
      In addition, protect also direct field accesses, by either creating a
      new critical section or widening the existing ones.
      
      Note: at this stage, job_{lock/unlock} and job lock guard macros
      are *nop*.
      
      Signed-off-by: default avatarEmanuele Giuseppe Esposito <eesposit@redhat.com>
      Reviewed-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
      Message-Id: <20220926093214.506243-12-eesposit@redhat.com>
      Reviewed-by: default avatarKevin Wolf <kwolf@redhat.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      880eeec6
  9. Sep 30, 2022
  10. Jul 12, 2022
  11. Apr 26, 2022
  12. Apr 21, 2022
  13. Apr 20, 2022
  14. Apr 06, 2022
  15. Mar 22, 2022
  16. Mar 07, 2022
  17. Jan 14, 2022
  18. Dec 28, 2021
  19. Sep 15, 2021
    • Eric Blake's avatar
      qemu-img: Add -F shorthand to convert · 1899bf47
      Eric Blake authored
      
      Although we have long supported 'qemu-img convert -o
      backing_file=foo,backing_fmt=bar', the fact that we have a shortcut -B
      for backing_file but none for backing_fmt has made it more likely that
      users accidentally run into:
      
      qemu-img: warning: Deprecated use of backing file without explicit backing format
      
      when using -B instead of -o.  For similarity with other qemu-img
      commands, such as create and compare, add '-F $fmt' as the shorthand
      for '-o backing_fmt=$fmt'.  Update iotest 122 for coverage of both
      spellings.
      
      Signed-off-by: default avatarEric Blake <eblake@redhat.com>
      Message-Id: <20210913131735.1948339-1-eblake@redhat.com>
      Reviewed-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
      Reviewed-by: default avatarMaxim Levitsky <mlevitsk@redhat.com>
      Signed-off-by: default avatarHanna Reitz <hreitz@redhat.com>
      1899bf47
    • Hanna Reitz's avatar
      qemu-img: Allow target be aligned to sector size · a1c62436
      Hanna Reitz authored
      We cannot write to images opened with O_DIRECT unless we allow them to
      be resized so they are aligned to the sector size: Since 9c60a5d1,
      bdrv_node_refresh_perm() ensures that for nodes whose length is not
      aligned to the request alignment and where someone has taken a WRITE
      permission, the RESIZE permission is taken, too).
      
      Let qemu-img convert pass the BDRV_O_RESIZE flag (which causes
      blk_new_open() to take the RESIZE permission) when using cache=none for
      the target, so that when writing to it, it can be aligned to the target
      sector size.
      
      Without this patch, an error is returned:
      
      $ qemu-img convert -f raw -O raw -t none foo.img /mnt/tmp/foo.img
      qemu-img: Could not open '/mnt/tmp/foo.img': Cannot get 'write'
      permission without 'resize': Image size is not a multiple of request
      alignment
      
      Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1994266
      
      
      Signed-off-by: default avatarHanna Reitz <hreitz@redhat.com>
      Message-Id: <20210819101200.64235-1-hreitz@redhat.com>
      Reviewed-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
      a1c62436
  20. Aug 26, 2021
  21. Jul 21, 2021
    • Eric Blake's avatar
      qemu-img: Add --skip-broken-bitmaps for 'convert --bitmaps' · 955171e4
      Eric Blake authored
      The point of 'qemu-img convert --bitmaps' is to be a convenience for
      actions that are already possible through a string of smaller
      'qemu-img bitmap' sub-commands.  One situation not accounted for
      already is that if a source image contains an inconsistent bitmap (for
      example, because a qemu process died abruptly before flushing bitmap
      state), the user MUST delete those inconsistent bitmaps before
      anything else useful can be done with the image.
      
      We don't want to delete inconsistent bitmaps by default: although a
      corrupt bitmap is only a loss of optimization rather than a corruption
      of user-visible data, it is still nice to require the user to opt in
      to the fact that they are aware of the loss of the bitmap.  Still,
      requiring the user to check 'qemu-img info' to see whether bitmaps are
      consistent, then use 'qemu-img bitmap --remove' to remove offenders,
      all before using 'qemu-img convert', is a lot more work than just
      adding a knob 'qemu-img convert --bitmaps --skip-broken-bitmaps' which
      opts in to skipping the broken bitmaps.
      
      After testing the new option, also demonstrate the way to manually fix
      things (either deleting bad bitmaps, or re-creating them as empty) so
      that it is possible to convert without the option.
      
      Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1946084
      
      
      Signed-off-by: default avatarEric Blake <eblake@redhat.com>
      Message-Id: <20210709153951.2801666-4-eblake@redhat.com>
      [eblake: warning message tweak, test enhancements]
      Reviewed-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
      955171e4
    • Eric Blake's avatar
      qemu-img: Fail fast on convert --bitmaps with inconsistent bitmap · 74a4320f
      Eric Blake authored
      
      Waiting until the end of the convert operation (a potentially
      time-consuming task) to finally detect that we can't copy a bitmap is
      bad, comparing to failing fast up front.  Furthermore, this prevents
      us from leaving a file behind with a bitmap that is not marked as
      inconsistent even though it does not have sane contents.
      
      This fixes the problems exposed in the previous patch to the iotest:
      it adds a fast failure up front, and even if we don't fail early, it
      ensures that any bitmap we add but do not properly populate is removed
      again rather than left behind incomplete.
      
      Signed-off-by: default avatarEric Blake <eblake@redhat.com>
      Message-Id: <20210709153951.2801666-3-eblake@redhat.com>
      [eblake: add a hint to the warning message, simplify name computation]
      Reviewed-by: default avatarNir Soffer <nsoffer@redhat.com>
      Reviewed-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
      74a4320f
  22. Jul 12, 2021
    • Eric Blake's avatar
      qemu-img: Make unallocated part of backing chain obvious in map · 8417e137
      Eric Blake authored
      
      The recently-added NBD context qemu:allocation-depth is able to
      distinguish between locally-present data (even when that data is
      sparse) [shown as depth 1 over NBD], and data that could not be found
      anywhere in the backing chain [shown as depth 0]; and the libnbd
      project was recently patched to give the human-readable name "absent"
      to an allocation-depth of 0.  But qemu-img map --output=json predates
      that addition, and has the unfortunate behavior that all portions of
      the backing chain that resolve without finding a hit in any backing
      layer report the same depth as the final backing layer.  This makes it
      harder to reconstruct a qcow2 backing chain using just 'qemu-img map'
      output, especially when using "backing":null to artificially limit a
      backing chain, because it is impossible to distinguish between a
      QCOW2_CLUSTER_UNALLOCATED (which defers to a [missing] backing file)
      and a QCOW2_CLUSTER_ZERO_PLAIN cluster (which would override any
      backing file), since both types of clusters otherwise show as
      "data":false,"zero":true" (but note that we can distinguish a
      QCOW2_CLUSTER_ZERO_ALLOCATED, which would also have an "offset":
      listing).
      
      The task of reconstructing a qcow2 chain was made harder in commit
      0da98568 (nbd: server: Report holes for raw images), because prior
      to that point, it was possible to abuse NBD's block status command to
      see which portions of a qcow2 file resulted in BDRV_BLOCK_ALLOCATED
      (showing up as NBD_STATE_ZERO in isolation) vs. missing from the chain
      (showing up as NBD_STATE_ZERO|NBD_STATE_HOLE); but now qemu reports
      more accurate sparseness information over NBD.
      
      An obvious solution is to make 'qemu-img map --output=json' add an
      additional "present":false designation to any cluster lacking an
      allocation anywhere in the chain, without any change to the "depth"
      parameter to avoid breaking existing clients.  The iotests have
      several examples where this distinction demonstrates the additional
      accuracy.
      
      Signed-off-by: default avatarEric Blake <eblake@redhat.com>
      Message-Id: <20210701190655.2131223-3-eblake@redhat.com>
      Reviewed-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
      [eblake: fix more iotest fallout]
      Signed-off-by: default avatarEric Blake <eblake@redhat.com>
      8417e137
  23. Jul 09, 2021
  24. Jun 25, 2021
  25. Apr 30, 2021
    • Kevin Wolf's avatar
      qemu-img convert: Unshare write permission for source · 0b8fb55c
      Kevin Wolf authored
      
      For a successful conversion of an image, we must make sure that its
      content doesn't change during the conversion.
      
      A special case of this is using the same image file both as the source
      and as the destination. If both input and output format are raw, the
      operation would just be useless work, with other formats it is a sure
      way to destroy the image. This will now fail because the image file
      can't be opened a second time for the output when opening it for the
      input has already acquired file locks to unshare BLK_PERM_WRITE.
      
      Nevertheless, if there is some reason in a special case why it is
      actually okay to allow writes to the image while it is being converted,
      -U can still be used to force sharing all permissions.
      
      Note that for most image formats, BLK_PERM_WRITE would already be
      unshared by the format driver, so this only really makes a difference
      for raw source images (but any output format).
      
      Reported-by: default avatarXueqiang Wei <xuwei@redhat.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      Reviewed-by: default avatarEric Blake <eblake@redhat.com>
      Message-Id: <20210422164344.283389-3-kwolf@redhat.com>
      Reviewed-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      0b8fb55c
Loading