Series comparison

-[Qemu-devel] [PULL 00/15] Block layer patches
+[Qemu-devel] [PULL 00/13] Block layer patches
-The following changes since commit 9964e96dc9999cf7f7c936ee854a795415d19b60:
+The following changes since commit 6d9dd5fb9d0e9f4a174f53a0e20a39fbe809c71e:
-  Merge remote-tracking branch 'jasowang/tags/net-pull-request' into staging (2017-05-23 15:01:31 +0100)
+  Merge remote-tracking branch 'remotes/armbru/tags/pull-qobject-2018-07-27-v2' into staging (2018-07-30 09:55:47 +0100)
 are available in the git repository at:
   git://repo.or.cz/qemu/kevin.git tags/for-upstream
-for you to fetch changes up to 42a48128417b3bfade93d1a4721348cc480e9e50:
+for you to fetch changes up to 1239ac241fe170bb9fcf0be74bfff04f6f1c2560:
-  Merge remote-tracking branch 'mreitz/tags/pull-block-2017-05-29-v3' into queue-block (2017-05-29 16:34:27 +0200)
+  qemu-iotests: Test query-blockstats with -drive and -blockdev (2018-07-30 15:35:37 +0200)
 ----------------------------------------------------------------
+Block layer patches:
-Block layer patches
+- qemu-img convert -C is now required to enable copy offloading
 - file-posix: Fix write_zeroes with unmap on block devices (would fall
   back to explicit writes on recent kernels)
 - Fix query-blockstats interface for use with -blockdev
 - Minor fixes and documentation updates
 ----------------------------------------------------------------
-Alberto Garcia (2):
+Fam Zheng (6):
-      stream: fix crash in stream_start() when block_job_create() fails
+      file-posix: Handle EINTR in preallocation=full write
-      qcow2: remove extra local_error variable
+      docs: Describe using images in writing iotests
       iotests: Don't lock /dev/null in 226
       Revert "qemu-img: Document copy offloading implications with -S and -c"
       qemu-img: Add -C option for convert with copy offloading
       iotests: Add test for 'qemu-img convert -C' compatibility
-Daniel P. Berrange (4):
+KONRAD Frederic (1):
-      qemu-img: add support for --object with 'dd' command
+      qcow: fix a reference leak
       qemu-img: fix --image-opts usage with dd command
       qemu-img: introduce --target-image-opts for 'convert' command
       qemu-img: copy *key-secret opts when opening newly created files
-Eric Blake (1):
+Kevin Wolf (5):
-      block: Tweak error message related to qemu-img amend
+      block: Fix documentation for BDRV_REQ_MAY_UNMAP
       file-posix: Fix write_zeroes with unmap on block devices
       block/qapi: Add 'qdev' field to query-blockstats result
       block/qapi: Include anonymous BBs in query-blockstats
       qemu-iotests: Test query-blockstats with -drive and -blockdev
-Fam Zheng (3):
+Leonid Bloch (1):
-      iotests: 147: Don't test inet6 if not available
+      qcow2: A grammar fix in conflicting cache sizing error message
       qemu-img: Fix documentation of convert
       qemu-img: Fix leakage of options on error
-Kevin Wolf (3):
+ qapi/block-core.json       |  14 +++-
-      qemu-iotests: Test streaming with missing job ID
+ include/block/block.h      |  11 +--
-      mirror: Drop permissions on s->target on completion
+ block/file-posix.c         |  62 ++++++++++----
-      Merge remote-tracking branch 'mreitz/tags/pull-block-2017-05-29-v3' into queue-block
+ block/qapi.c               |  16 +++-
  block/qcow.c               |   1 +
  block/qcow2.c              |   2 +-
  qemu-img.c                 |  21 ++++-
  docs/devel/testing.rst     |  11 +++
  qemu-img-cmds.hx           |   2 +-
  qemu-img.texi              |  14 ++--
  tests/qemu-iotests/082     |   8 ++
  tests/qemu-iotests/082.out |  11 +++
  tests/qemu-iotests/103.out |   4 +-
  tests/qemu-iotests/137.out |   2 +-
  tests/qemu-iotests/226     |   4 +-
  tests/qemu-iotests/227     | 101 ++++++++++++++++++++++
  tests/qemu-iotests/227.out | 205 +++++++++++++++++++++++++++++++++++++++++++++
  tests/qemu-iotests/group   |   1 +
 files changed, 449 insertions(+), 41 deletions(-)
  create mode 100755 tests/qemu-iotests/227
  create mode 100644 tests/qemu-iotests/227.out
-Max Reitz (2):
-      block: Fix backing paths for filenames with colons
-      block/file-*: *_parse_filename() and colons
-Stephen Bates (1):
-      nvme: Add support for Controller Memory Buffers
- block.c                    |  50 +++++++++++++--
- block/file-posix.c         |  17 +-----
- block/file-win32.c         |  12 +---
- block/mirror.c             |   7 ++-
- block/qcow2-cluster.c      |   3 +-
- block/qcow2.c              |   5 +-
- block/stream.c             |   2 +-
- hw/block/nvme.c            |  75 +++++++++++++++++++++--
- hw/block/nvme.h            |  73 ++++++++++++++++++++++
- include/block/block_int.h  |   3 +
- qemu-img-cmds.hx           |   4 +-
- qemu-img.c                 | 148 +++++++++++++++++++++++++++++++++++----------
- qemu-img.texi              |  12 +++-
- tests/qemu-iotests/030     |   4 ++
- tests/qemu-iotests/030.out |   4 +-
- tests/qemu-iotests/060.out |   2 +-
- tests/qemu-iotests/147     |   7 +++
-files changed, 351 insertions(+), 77 deletions(-)

-[Qemu-devel] [PULL 14/15] block: Fix backing paths for filenames with colons
+[Qemu-devel] [PULL 01/13] qcow: fix a reference leak
-From: Max Reitz <mreitz@redhat.com>
+From: KONRAD Frederic <frederic.konrad@adacore.com>
-path_combine() naturally tries to preserve a protocol prefix. However,
+Since 42a3e1ab367cdf38cce093de24eb406b99a4ef96 qemu asserts when using the
-it recognizes such a prefix by scanning for the first colon; which is
+vvfat driver:
 different from what path_has_protocol() does: There only is a protocol
 prefix if there is a colon before the first slash.
-A protocol prefix that is not recognized by path_has_protocol() is none,
+git clone git://qemu.org/qemu.git
-and should thus not be taken as one.
+cd qemu
 ./configure --target-list=ppc-softmmu --enable-debug
 make -j8
 mkdir foo
 touch foo/hello
 ./ppc-softmmu/qemu-system-ppc -M prep --nographic --monitor null             \
                               -hda fat:rw:./foo
-Case in point, before this patch:
+"Ctrl-C"
 $ ./qemu-img create -f qcow2 -b backing.qcow2 ./top:image.qcow2
 qemu-img: ./top:image.qcow2: Could not open './top:backing.qcow2':
     No such file or directory
-Afterwards:
+qemu-system-ppc: block.c:3368: bdrv_close_all: Assertion                     \
-$ ./qemu-img create -f qcow2 -b backing.qcow2 ./top:image.qcow2
+   `((&all_bdrv_states)->tqh_first == ((void *)0))' failed.
 qemu-img: ./top:image.qcow2: Could not open './backing.qcow2':
     No such file or directory
-Reported-by: yangyang <yangyang@redhat.com>
+This is because we reference bs twice in qcow_co_create(..) one time in
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+bdrv_open_blockdev_ref(..) and in blk_insert_bs(..) but we unref it only once
-Reviewed-by: Eric Blake <eblake@redhat.com>
+in blk_unref which leads to the reference leak.
-Message-id: 20170522195217.12991-2-mreitz@redhat.com
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+Note that I didn't tested much QCOW after this change as I don't use it much.
 Signed-off-by: KONRAD Frederic <frederic.konrad@adacore.com>
 Signed-off-by: Kevin Wolf <kwolf@redhat.com>
 ---
- block.c | 15 ++++++++++-----
+ block/qcow.c | 1 +
-file changed, 10 insertions(+), 5 deletions(-)
+file changed, 1 insertion(+)
-diff --git a/block.c b/block.c
+diff --git a/block/qcow.c b/block/qcow.c
 index XXXXXXX..XXXXXXX 100644
---- a/block.c
+--- a/block/qcow.c
-+++ b/block.c
++++ b/block/qcow.c
-@@ -XXX,XX +XXX,XX @@ void path_combine(char *dest, int dest_size,
+@@ -XXX,XX +XXX,XX @@ static int coroutine_fn qcow_co_create(BlockdevCreateOptions *opts,
-     if (path_is_absolute(filename)) {
+     ret = 0;
-         pstrcpy(dest, dest_size, filename);
+ exit:
-     } else {
+     blk_unref(qcow_blk);
--        p = strchr(base_path, ':');
++    bdrv_unref(bs);
--        if (p)
+     qcrypto_block_free(crypto);
--            p++;
+     return ret;
--        else
+ }
 -            p = base_path;
 +        const char *protocol_stripped = NULL;
 +
 +        if (path_has_protocol(base_path)) {
 +            protocol_stripped = strchr(base_path, ':');
 +            if (protocol_stripped) {
 +                protocol_stripped++;
 +            }
 +        }
 +        p = protocol_stripped ?: base_path;
 +
          p1 = strrchr(base_path, '/');
  #ifdef _WIN32
          {
 --
-.8.3.1
+.13.6

-[Qemu-devel] [PULL 06/15] qcow2: remove extra local_error variable
+[Qemu-devel] [PULL 02/13] qcow2: A grammar fix in conflicting cache sizing error message
-From: Alberto Garcia <berto@igalia.com>
+From: Leonid Bloch <lbloch@janustech.com>
-Commit d7086422b1c1e75e320519cfe26176db6ec97a37 added a local_err
+Signed-off-by: Leonid Bloch <lbloch@janustech.com>
-variable global to the qcow2_amend_options() function, so there's no
+Reviewed-by: John Snow <jsnow@redhat.com>
-need to have this other one.
+Signed-off-by: Kevin Wolf <kwolf@redhat.com>
 Signed-off-by: Alberto Garcia <berto@igalia.com>
 Message-id: 20170511150337.21470-1-berto@igalia.com
 Reviewed-by: Eric Blake <eblake@redhat.com>
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
- block/qcow2.c | 5 ++---
+ block/qcow2.c              | 2 +-
-file changed, 2 insertions(+), 3 deletions(-)
+ tests/qemu-iotests/103.out | 4 ++--
  tests/qemu-iotests/137.out | 2 +-
 files changed, 4 insertions(+), 4 deletions(-)
 diff --git a/block/qcow2.c b/block/qcow2.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/qcow2.c
 +++ b/block/qcow2.c
-@@ -XXX,XX +XXX,XX @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
+@@ -XXX,XX +XXX,XX @@ static void read_cache_sizes(BlockDriverState *bs, QemuOpts *opts,
+         if (l2_cache_size_set && refcount_cache_size_set) {
-     if (s->refcount_bits != refcount_bits) {
+             error_setg(errp, QCOW2_OPT_CACHE_SIZE ", " QCOW2_OPT_L2_CACHE_SIZE
-         int refcount_order = ctz32(refcount_bits);
+                        " and " QCOW2_OPT_REFCOUNT_CACHE_SIZE " may not be set "
--        Error *local_error = NULL;
+-                       "the same time");
++                       "at the same time");
-         if (new_version < 3 && refcount_bits != 16) {
+             return;
-             error_report("Different refcount widths than 16 bits require "
+         } else if (*l2_cache_size > combined_cache_size) {
-@@ -XXX,XX +XXX,XX @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
+             error_setg(errp, QCOW2_OPT_L2_CACHE_SIZE " may not exceed "
-         helper_cb_info.current_operation = QCOW2_CHANGING_REFCOUNT_ORDER;
+diff --git a/tests/qemu-iotests/103.out b/tests/qemu-iotests/103.out
-         ret = qcow2_change_refcount_order(bs, refcount_order,
+index XXXXXXX..XXXXXXX 100644
-                                           &qcow2_amend_helper_cb,
+--- a/tests/qemu-iotests/103.out
--                                          &helper_cb_info, &local_error);
++++ b/tests/qemu-iotests/103.out
-+                                          &helper_cb_info, &local_err);
+@@ -XXX,XX +XXX,XX @@ wrote 65536/65536 bytes at offset 0
-         if (ret < 0) {
--            error_report_err(local_error);
+ === Testing invalid option combinations ===
-+            error_report_err(local_err);
-             return ret;
+-can't open device TEST_DIR/t.IMGFMT: cache-size, l2-cache-size and refcount-cache-size may not be set the same time
-         }
++can't open device TEST_DIR/t.IMGFMT: cache-size, l2-cache-size and refcount-cache-size may not be set at the same time
-     }
+ can't open device TEST_DIR/t.IMGFMT: l2-cache-size may not exceed cache-size
  can't open device TEST_DIR/t.IMGFMT: refcount-cache-size may not exceed cache-size
 -can't open device TEST_DIR/t.IMGFMT: cache-size, l2-cache-size and refcount-cache-size may not be set the same time
 +can't open device TEST_DIR/t.IMGFMT: cache-size, l2-cache-size and refcount-cache-size may not be set at the same time
  can't open device TEST_DIR/t.IMGFMT: L2 cache entry size must be a power of two between 512 and the cluster size (65536)
  can't open device TEST_DIR/t.IMGFMT: L2 cache entry size must be a power of two between 512 and the cluster size (65536)
  can't open device TEST_DIR/t.IMGFMT: L2 cache entry size must be a power of two between 512 and the cluster size (65536)
 diff --git a/tests/qemu-iotests/137.out b/tests/qemu-iotests/137.out
 index XXXXXXX..XXXXXXX 100644
 --- a/tests/qemu-iotests/137.out
 +++ b/tests/qemu-iotests/137.out
@@ -XXX,XX +XXX,XX @@ read 33554432/33554432 bytes at offset 0
  === Try setting some invalid values ===
  Parameter 'lazy-refcounts' expects 'on' or 'off'
 -cache-size, l2-cache-size and refcount-cache-size may not be set the same time
 +cache-size, l2-cache-size and refcount-cache-size may not be set at the same time
  l2-cache-size may not exceed cache-size
  refcount-cache-size may not exceed cache-size
  L2 cache size too big
 --
-.8.3.1
+.13.6

-[Qemu-devel] [PULL 01/15] stream: fix crash in stream_start() when block_job_create() fails
+[Qemu-devel] [PULL 03/13] file-posix: Handle EINTR in preallocation=full write
-From: Alberto Garcia <berto@igalia.com>
+From: Fam Zheng <famz@redhat.com>
 The code that tries to reopen a BlockDriverState in stream_start()
 when the creation of a new block job fails crashes because it attempts
 to dereference a pointer that is known to be NULL.
 This is a regression introduced in a170a91fd3eab6155da39e740381867e,
 likely because the code was copied from stream_complete().
 Cc: qemu-stable@nongnu.org
-Reported-by: Kashyap Chamarthy <kchamart@redhat.com>
+Signed-off-by: Fam Zheng <famz@redhat.com>
 Signed-off-by: Alberto Garcia <berto@igalia.com>
 Tested-by: Kashyap Chamarthy <kchamart@redhat.com>
 Signed-off-by: Kevin Wolf <kwolf@redhat.com>
 ---
- block/stream.c | 2 +-
+ block/file-posix.c | 3 +++
-file changed, 1 insertion(+), 1 deletion(-)
+file changed, 3 insertions(+)
-diff --git a/block/stream.c b/block/stream.c
+diff --git a/block/file-posix.c b/block/file-posix.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/stream.c
+--- a/block/file-posix.c
-+++ b/block/stream.c
++++ b/block/file-posix.c
-@@ -XXX,XX +XXX,XX @@ void stream_start(const char *job_id, BlockDriverState *bs,
+@@ -XXX,XX +XXX,XX @@ static int handle_aiocb_truncate(RawPosixAIOData *aiocb)
+             num = MIN(left, 65536);
- fail:
+             result = write(fd, buf, num);
-     if (orig_bs_flags != bdrv_get_flags(bs)) {
+             if (result < 0) {
--        bdrv_reopen(bs, s->bs_flags, NULL);
++                if (errno == EINTR) {
-+        bdrv_reopen(bs, orig_bs_flags, NULL);
++                    continue;
-     }
++                }
- }
+                 result = -errno;
                  error_setg_errno(errp, -result,
                                   "Could not write zeros for preallocation");
 --
-.8.3.1
+.13.6

-[Qemu-devel] [PULL 13/15] block: Tweak error message related to qemu-img amend
+[Qemu-devel] [PULL 04/13] docs: Describe using images in writing iotests
-From: Eric Blake <eblake@redhat.com>
+From: Fam Zheng <famz@redhat.com>
-When converting a 1.1 image down to 0.10, qemu-iotests 060 forces
+Signed-off-by: Fam Zheng <famz@redhat.com>
-a contrived failure where allocating a cluster used to replace a
+Reviewed-by: Eric Blake <eblake@redhat.com>
-zero cluster reads unaligned data.  Since it is a zero cluster
+Reviewed-by: John Snow <jsnow@redhat.com>
-rather than a data cluster being converted, changing the error
+Signed-off-by: Kevin Wolf <kwolf@redhat.com>
-message to match our earlier change in 'qcow2: Make distinction
+---
-between zero cluster types obvious' is worthwhile.
+ docs/devel/testing.rst | 11 +++++++++++
 file changed, 11 insertions(+)
-Suggested-by: Max Reitz <mreitz@redhat.com>
+diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
 Signed-off-by: Eric Blake <eblake@redhat.com>
 Message-id: 20170508171302.17805-1-eblake@redhat.com
 [mreitz: Commit message fixes]
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  block/qcow2-cluster.c      | 3 ++-
  tests/qemu-iotests/060.out | 2 +-
 files changed, 3 insertions(+), 2 deletions(-)
 diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2-cluster.c
+--- a/docs/devel/testing.rst
-+++ b/block/qcow2-cluster.c
++++ b/docs/devel/testing.rst
-@@ -XXX,XX +XXX,XX @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
+@@ -XXX,XX +XXX,XX @@ comparable library support for invoking and interacting with QEMU programs. If
-             }
+ you opt for Python, it is strongly recommended to write Python 3 compatible
+ code.
-             if (offset_into_cluster(s, offset)) {
--                qcow2_signal_corruption(bs, true, -1, -1, "Data cluster offset "
++Both Python and Bash frameworks in iotests provide helpers to manage test
-+                qcow2_signal_corruption(bs, true, -1, -1,
++images. They can be used to create and clean up images under the test
-+                                        "Cluster allocation offset "
++directory. If no I/O or any protocol specific feature is needed, it is often
-                                         "%#" PRIx64 " unaligned (L2 offset: %#"
++more convenient to use the pseudo block driver, ``null-co://``, as the test
-                                         PRIx64 ", L2 index: %#x)", offset,
++image, which doesn't require image creation or cleaning up. Avoid system-wide
-                                         l2_offset, j);
++devices or files whenever possible, such as ``/dev/null`` or ``/dev/zero``.
-diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
++Otherwise, image locking implications have to be considered.  For example,
-index XXXXXXX..XXXXXXX 100644
++another application on the host may have locked the file, possibly leading to a
---- a/tests/qemu-iotests/060.out
++test failure.  If using such devices are explicitly desired, consider adding
-+++ b/tests/qemu-iotests/060.out
++``locking=off`` option to disable image locking.
-@@ -XXX,XX +XXX,XX @@ read failed: Input/output error
++
- Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+ Docker based tests
- wrote 65536/65536 bytes at offset 0
+ ==================
-KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 -qcow2: Marking image as corrupt: Data cluster offset 0x52a00 unaligned (L2 offset: 0x40000, L2 index: 0); further corruption events will be suppressed
 +qcow2: Marking image as corrupt: Cluster allocation offset 0x52a00 unaligned (L2 offset: 0x40000, L2 index: 0); further corruption events will be suppressed
  qemu-img: Error while amending options: Input/output error
  === Testing unaligned reftable entry ===
 --
-.8.3.1
+.13.6

-[Qemu-devel] [PULL 12/15] qemu-img: Fix leakage of options on error
+[Qemu-devel] [PULL 05/13] iotests: Don't lock /dev/null in 226
 From: Fam Zheng <famz@redhat.com>
-Reported by Coverity.
+On my system (Fedora 28), this script reports a 'failed to get
 "consistent read" lock' error. Following docs/devel/testing.rst, it's
 better to add locking=off here.
 Signed-off-by: Fam Zheng <famz@redhat.com>
-Message-id: 20170515141014.25793-1-famz@redhat.com
 Reviewed-by: Eric Blake <eblake@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+Reviewed-by: John Snow <jsnow@redhat.com>
 Signed-off-by: Kevin Wolf <kwolf@redhat.com>
 ---
- qemu-img.c | 1 +
+ tests/qemu-iotests/226 | 4 ++--
-file changed, 1 insertion(+)
+file changed, 2 insertions(+), 2 deletions(-)
-diff --git a/qemu-img.c b/qemu-img.c
+diff --git a/tests/qemu-iotests/226 b/tests/qemu-iotests/226
-index XXXXXXX..XXXXXXX 100644
+index XXXXXXX..XXXXXXX 100755
---- a/qemu-img.c
+--- a/tests/qemu-iotests/226
-+++ b/qemu-img.c
++++ b/tests/qemu-iotests/226
-@@ -XXX,XX +XXX,XX @@ static BlockBackend *img_open_opts(const char *optstr,
+@@ -XXX,XX +XXX,XX @@ for PROTO in "file" "host_device" "host_cdrom"; do
-         if (qdict_haskey(options, BDRV_OPT_FORCE_SHARE)
+     echo
-             && !qdict_get_bool(options, BDRV_OPT_FORCE_SHARE)) {
+     echo "== Testing RO =="
-             error_report("--force-share/-U conflicts with image options");
+     $QEMU_IO -c "open -r -o driver=$PROTO,filename=$TEST_IMG" 2>&1 | _filter_testdir | _filter_imgfmt
-+            QDECREF(options);
+-    $QEMU_IO -c "open -r -o driver=$PROTO,filename=/dev/null" 2>&1 | _filter_imgfmt
-             return NULL;
++    $QEMU_IO -c "open -r -o driver=$PROTO,filename=/dev/null,locking=off" 2>&1 | _filter_imgfmt
-         }
+     echo "== Testing RW =="
-         qdict_put(options, BDRV_OPT_FORCE_SHARE, qbool_from_bool(true));
+     $QEMU_IO -c "open -o driver=$PROTO,filename=$TEST_IMG" 2>&1 | _filter_testdir | _filter_imgfmt
 -    $QEMU_IO -c "open -o driver=$PROTO,filename=/dev/null" 2>&1 | _filter_imgfmt
 +    $QEMU_IO -c "open -o driver=$PROTO,filename=/dev/null,locking=off" 2>&1 | _filter_imgfmt
  done
  # success, all done
 --
-.8.3.1
+.13.6

-[Qemu-devel] [PULL 03/15] iotests: 147: Don't test inet6 if not available
+[Qemu-devel] [PULL 06/13] Revert "qemu-img: Document copy offloading implications with -S and -c"
 From: Fam Zheng <famz@redhat.com>
-This is the case in our docker tests, as we use --net=none there. Skip
+This reverts commit eb461485f4558e362fab905735b50987505bca44.
-this method.
 Now that we introduce an explicit option, these implicit rules are not
 used.
 Signed-off-by: Fam Zheng <famz@redhat.com>
 Signed-off-by: Kevin Wolf <kwolf@redhat.com>
 ---
- tests/qemu-iotests/147 | 7 +++++++
+ qemu-img.texi | 6 ++----
-file changed, 7 insertions(+)
+file changed, 2 insertions(+), 4 deletions(-)
-diff --git a/tests/qemu-iotests/147 b/tests/qemu-iotests/147
+diff --git a/qemu-img.texi b/qemu-img.texi
-index XXXXXXX..XXXXXXX 100755
+index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/147
+--- a/qemu-img.texi
-+++ b/tests/qemu-iotests/147
++++ b/qemu-img.texi
-@@ -XXX,XX +XXX,XX @@ class BuiltinNBD(NBDBlockdevAddBase):
+@@ -XXX,XX +XXX,XX @@ will enumerate information about backing files in a disk image chain. Refer
-         self._server_down()
+ below for further description.
-     def test_inet6(self):
+ @item -c
-+        try:
+-indicates that target image must be compressed (qcow format only). If this
-+            socket.getaddrinfo("::0", "0", socket.AF_INET6,
+-option is used, copy offloading will not be attempted.
-+                               socket.SOCK_STREAM, socket.IPPROTO_TCP,
++indicates that target image must be compressed (qcow format only)
-+                               socket.AI_ADDRCONFIG | socket.AI_CANONNAME)
-+        except socket.gaierror:
+ @item -h
-+            # IPv6 not available, skip
+ with or without a command shows help and lists the supported formats
-+            return
+@@ -XXX,XX +XXX,XX @@ in case both @var{-q} and @var{-p} options are used.
-         address = { 'type': 'inet',
+ indicates the consecutive number of bytes that must contain only zeros
-                     'data': {
+ for qemu-img to create a sparse image during conversion. This value is rounded
-                         'host': '::1',
+ down to the nearest 512 bytes. You may use the common size suffixes like
 -@code{k} for kilobytes. If this option is used, copy offloading will not be
 -attempted.
 +@code{k} for kilobytes.
  @item -t @var{cache}
  specifies the cache mode that should be used with the (destination) file. See
 --
-.8.3.1
+.13.6

-[Qemu-devel] [PULL 10/15] qemu-img: introduce --target-image-opts for 'convert' command
+[Qemu-devel] [PULL 07/13] qemu-img: Add -C option for convert with copy offloading
-From: "Daniel P. Berrange" <berrange@redhat.com>
+From: Fam Zheng <famz@redhat.com>
-The '--image-opts' flag indicates whether the source filename
+Signed-off-by: Fam Zheng <famz@redhat.com>
-includes options. The target filename has to remain in the
+Signed-off-by: Kevin Wolf <kwolf@redhat.com>
-plain filename format though, since it needs to be passed to
+---
-bdrv_create().  When using --skip-create though, it would be
+ qemu-img.c       | 21 +++++++++++++++++----
-possible to use image-opts syntax. This adds --target-image-opts
+ qemu-img-cmds.hx |  2 +-
-to indicate that the target filename includes options. Currently
+ qemu-img.texi    |  8 +++++++-
-this mandates use of the --skip-create flag too.
+files changed, 25 insertions(+), 6 deletions(-)
-Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
+diff --git a/qemu-img.c b/qemu-img.c
-Message-id: 20170515164712.6643-4-berrange@redhat.com
+index XXXXXXX..XXXXXXX 100644
-Reviewed-by: Max Reitz <mreitz@redhat.com>
+--- a/qemu-img.c
-Reviewed-by: Eric Blake <eblake@redhat.com>
++++ b/qemu-img.c
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
----
+          skip_create = false, progress = false, tgt_image_opts = false;
- qemu-img-cmds.hx |  4 +--
+     int64_t ret = -EINVAL;
- qemu-img.c       | 84 ++++++++++++++++++++++++++++++++++++++------------------
+     bool force_share = false;
- qemu-img.texi    | 12 ++++++--
++    bool explict_min_sparse = false;
-files changed, 69 insertions(+), 31 deletions(-)
+     ImgConvertState s = (ImgConvertState) {
          /* Need at least 4k of zeros for sparse detection */
          .min_sparse         = 8,
 -        .copy_range         = true,
 +        .copy_range         = false,
          .buf_sectors        = IO_BUF_SIZE / BDRV_SECTOR_SIZE,
          .wr_in_order        = true,
          .num_coroutines     = 8,
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
              {"target-image-opts", no_argument, 0, OPTION_TARGET_IMAGE_OPTS},
              {0, 0, 0, 0}
          };
 -        c = getopt_long(argc, argv, ":hf:O:B:co:l:S:pt:T:qnm:WU",
 +        c = getopt_long(argc, argv, ":hf:O:B:Cco:l:S:pt:T:qnm:WU",
                          long_options, NULL);
          if (c == -1) {
              break;
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
          case 'B':
              out_baseimg = optarg;
              break;
 +        case 'C':
 +            s.copy_range = true;
 +            break;
          case 'c':
              s.compressed = true;
 -            s.copy_range = false;
              break;
          case 'o':
              if (!is_valid_option_list(optarg)) {
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
              }
              s.min_sparse = sval / BDRV_SECTOR_SIZE;
 -            s.copy_range = false;
 +            explict_min_sparse = true;
              break;
          }
          case 'p':
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
          goto fail_getopt;
      }
 +    if (s.compressed && s.copy_range) {
 +        error_report("Cannot enable copy offloading when -c is used");
 +        goto fail_getopt;
 +    }
 +
 +    if (explict_min_sparse && s.copy_range) {
 +        error_report("Cannot enable copy offloading when -S is used");
 +        goto fail_getopt;
 +    }
 +
      if (tgt_image_opts && !skip_create) {
          error_report("--target-image-opts requires use of -n flag");
          goto fail_getopt;
 diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
 index XXXXXXX..XXXXXXX 100644
 --- a/qemu-img-cmds.hx
 +++ b/qemu-img-cmds.hx
 @@ -XXX,XX +XXX,XX @@ STEXI
  ETEXI
  DEF("convert", img_convert,
--    "convert [--object objectdef] [--image-opts] [-U] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
+-    "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
-+    "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
++    "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
  STEXI
--@item convert [--object @var{objectdef}] [--image-opts] [-U] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
+ @item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
 +@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
  ETEXI
- DEF("dd", img_dd,
-diff --git a/qemu-img.c b/qemu-img.c
-index XXXXXXX..XXXXXXX 100644
---- a/qemu-img.c
-+++ b/qemu-img.c
-@@ -XXX,XX +XXX,XX @@ enum {
-     OPTION_PATTERN = 260,
-     OPTION_FLUSH_INTERVAL = 261,
-     OPTION_NO_DRAIN = 262,
-+    OPTION_TARGET_IMAGE_OPTS = 263,
- };
- typedef enum OutputFormat {
-@@ -XXX,XX +XXX,XX @@ static int convert_do_copy(ImgConvertState *s)
- static int img_convert(int argc, char **argv)
- {
-     int c, bs_i, flags, src_flags = 0;
--    const char *fmt = NULL, *out_fmt = "raw", *cache = "unsafe",
-+    const char *fmt = NULL, *out_fmt = NULL, *cache = "unsafe",
-                *src_cache = BDRV_DEFAULT_CACHE, *out_baseimg = NULL,
-                *out_filename, *out_baseimg_param, *snapshot_name = NULL;
--    BlockDriver *drv, *proto_drv;
-+    BlockDriver *drv = NULL, *proto_drv = NULL;
-     BlockDriverInfo bdi;
-     BlockDriverState *out_bs;
-     QemuOpts *opts = NULL, *sn_opts = NULL;
-@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
-     char *options = NULL;
-     Error *local_err = NULL;
-     bool writethrough, src_writethrough, quiet = false, image_opts = false,
--         skip_create = false, progress = false;
-+         skip_create = false, progress = false, tgt_image_opts = false;
-     int64_t ret = -EINVAL;
-     bool force_share = false;
-@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
-             {"object", required_argument, 0, OPTION_OBJECT},
-             {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
-             {"force-share", no_argument, 0, 'U'},
-+            {"target-image-opts", no_argument, 0, OPTION_TARGET_IMAGE_OPTS},
-             {0, 0, 0, 0}
-         };
-         c = getopt_long(argc, argv, ":hf:O:B:ce6o:s:l:S:pt:T:qnm:WU",
-@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
-         case OPTION_IMAGE_OPTS:
-             image_opts = true;
-             break;
-+        case OPTION_TARGET_IMAGE_OPTS:
-+            tgt_image_opts = true;
-+            break;
-         }
-     }
-+    if (!out_fmt && !tgt_image_opts) {
-+        out_fmt = "raw";
-+    }
-+
-     if (qemu_opts_foreach(&qemu_object_opts,
-                           user_creatable_add_opts_foreach,
-                           NULL, NULL)) {
-@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
-         goto fail_getopt;
-     }
-+    if (tgt_image_opts && !skip_create) {
-+        error_report("--target-image-opts requires use of -n flag");
-+        goto fail_getopt;
-+    }
-+
-     s.src_num = argc - optind - 1;
-     out_filename = s.src_num >= 1 ? argv[argc - 1] : NULL;
-     if (options && has_help_option(options)) {
--        ret = print_block_option_help(out_filename, out_fmt);
--        goto fail_getopt;
-+        if (out_fmt) {
-+            ret = print_block_option_help(out_filename, out_fmt);
-+            goto fail_getopt;
-+        } else {
-+            error_report("Option help requires a format be specified");
-+            goto fail_getopt;
-+        }
-     }
-     if (s.src_num < 1) {
-@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
-         goto out;
-     }
--    /* Find driver and parse its options */
--    drv = bdrv_find_format(out_fmt);
--    if (!drv) {
--        error_report("Unknown file format '%s'", out_fmt);
--        ret = -1;
--        goto out;
--    }
-+    if (!skip_create) {
-+        /* Find driver and parse its options */
-+        drv = bdrv_find_format(out_fmt);
-+        if (!drv) {
-+            error_report("Unknown file format '%s'", out_fmt);
-+            ret = -1;
-+            goto out;
-+        }
--    proto_drv = bdrv_find_protocol(out_filename, true, &local_err);
--    if (!proto_drv) {
--        error_report_err(local_err);
--        ret = -1;
--        goto out;
--    }
-+        proto_drv = bdrv_find_protocol(out_filename, true, &local_err);
-+        if (!proto_drv) {
-+            error_report_err(local_err);
-+            ret = -1;
-+            goto out;
-+        }
--    if (!skip_create) {
-         if (!drv->create_opts) {
-             error_report("Format driver '%s' does not support image creation",
-                          drv->format_name);
-@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
-         const char *preallocation =
-             qemu_opt_get(opts, BLOCK_OPT_PREALLOC);
--        if (!drv->bdrv_co_pwritev_compressed) {
-+        if (drv && !drv->bdrv_co_pwritev_compressed) {
-             error_report("Compression not supported for this file format");
-             ret = -1;
-             goto out;
-@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
-         goto out;
-     }
--    /* XXX we should allow --image-opts to trigger use of
--     * img_open() here, but then we have trouble with
--     * the bdrv_create() call which takes different params.
--     * Not critical right now, so fix can wait...
--     */
--    s.target = img_open_file(out_filename, out_fmt, flags, writethrough, quiet,
--                             false);
-+    if (skip_create) {
-+        s.target = img_open(tgt_image_opts, out_filename, out_fmt,
-+                            flags, writethrough, quiet, false);
-+    } else {
-+        /* TODO ultimately we should allow --target-image-opts
-+         * to be used even when -n is not given.
-+         * That has to wait for bdrv_create to be improved
-+         * to allow filenames in option syntax
-+         */
-+        s.target = img_open_file(out_filename, out_fmt, flags,
-+                                 writethrough, quiet, false);
-+    }
-     if (!s.target) {
-         ret = -1;
-         goto out;
-     }
-     out_bs = blk_bs(s.target);
-+    if (s.compressed && !out_bs->drv->bdrv_co_pwritev_compressed) {
-+        error_report("Compression not supported for this file format");
-+        ret = -1;
-+        goto out;
-+    }
-+
-     /* increase bufsectors from the default 4096 (2M) if opt_transfer
-      * or discard_alignment of the out_bs is greater. Limit to 32768 (16MB)
-      * as maximum. */
 diff --git a/qemu-img.texi b/qemu-img.texi
 index XXXXXXX..XXXXXXX 100644
 --- a/qemu-img.texi
 +++ b/qemu-img.texi
-@@ -XXX,XX +XXX,XX @@ keys.
+@@ -XXX,XX +XXX,XX @@ Number of parallel coroutines for the convert process
+ Allow out-of-order writes to the destination. This option improves performance,
- @item --image-opts
+ but is only recommended for preallocated devices like host devices or other
+ raw block devices.
--Indicates that the @var{filename} parameter is to be interpreted as a
++@item -C
-+Indicates that the source @var{filename} parameter is to be interpreted as a
++Try to use copy offloading to move data from source image to target. This may
- full option string, not a plain filename. This parameter is mutually
++improve performance if the data is remote, such as with NFS or iSCSI backends,
--exclusive with the @var{-f} and @var{-F} parameters.
++but will not automatically sparsify zero sectors, and may result in a fully
-+exclusive with the @var{-f} parameter.
++allocated target image depending on the host support for getting allocation
-+
++information.
-+@item --target-image-opts
+ @end table
-+
-+Indicates that the @var{output_filename} parameter(s) are to be interpreted as
+ Parameters to dd subcommand:
-+a full option string, not a plain filename. This parameter is mutually
+@@ -XXX,XX +XXX,XX @@ Error on reading data
-+exclusive with the @var{-O} parameters. It is currently required to also use
-+the @var{-n} parameter to skip image creation. This restriction may be relaxed
+ @end table
-+in a future release.
+-@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
- @item fmt
++@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
- is the disk image format. It is guessed automatically in most cases. See below
  Convert the disk image @var{filename} or a snapshot @var{snapshot_param}
  to disk image @var{output_filename} using format @var{output_fmt}. It can be optionally compressed (@code{-c}
 --
-.8.3.1
+.13.6

-[Qemu-devel] [PULL 07/15] qemu-img: Fix documentation of convert
+[Qemu-devel] [PULL 08/13] iotests: Add test for 'qemu-img convert -C' compatibility
 From: Fam Zheng <famz@redhat.com>
-It got lost in commit a8d16f9ca "qemu-img: Update documentation for -U".
+Signed-off-by: Fam Zheng <famz@redhat.com>
 Signed-off-by: Kevin Wolf <kwolf@redhat.com>
 ---
  tests/qemu-iotests/082     |  8 ++++++++
  tests/qemu-iotests/082.out | 11 +++++++++++
 files changed, 19 insertions(+)
-Reported-by: Max Reitz <mreitz@redhat.com>
+diff --git a/tests/qemu-iotests/082 b/tests/qemu-iotests/082
-Signed-off-by: Fam Zheng <famz@redhat.com>
+index XXXXXXX..XXXXXXX 100755
-Message-id: 20170515103551.31313-1-famz@redhat.com
+--- a/tests/qemu-iotests/082
-Reviewed-by: Eric Blake <eblake@redhat.com>
++++ b/tests/qemu-iotests/082
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+@@ -XXX,XX +XXX,XX @@ run_qemu_img convert -o help
----
+ run_qemu_img convert -O bochs -o help
- qemu-img-cmds.hx | 4 ++--
-file changed, 2 insertions(+), 2 deletions(-)
+ echo
++echo === convert: -C and other options ===
-diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
++
 +# Adding the help option to a command without other -o options
 +run_qemu_img convert -C -S 4k -O $IMGFMT "$TEST_IMG" "$TEST_IMG".target
 +run_qemu_img convert -C -S 8k -O $IMGFMT "$TEST_IMG" "$TEST_IMG".target
 +run_qemu_img convert -C -c -O $IMGFMT "$TEST_IMG" "$TEST_IMG".target
 +
 +echo
  echo === amend: Options specified more than once ===
  # Last -f should win
 diff --git a/tests/qemu-iotests/082.out b/tests/qemu-iotests/082.out
 index XXXXXXX..XXXXXXX 100644
---- a/qemu-img-cmds.hx
+--- a/tests/qemu-iotests/082.out
-+++ b/qemu-img-cmds.hx
++++ b/tests/qemu-iotests/082.out
-@@ -XXX,XX +XXX,XX @@ STEXI
+@@ -XXX,XX +XXX,XX @@ size             Virtual disk size
- ETEXI
+ Testing: convert -O bochs -o help
+ qemu-img: Format driver 'bochs' does not support image creation
- DEF("convert", img_convert,
--    "convert [--object objectdef] [--image-opts] [-U] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
++=== convert: -C and other options ===
-+    "convert [--object objectdef] [--image-opts] [-U] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
++
- STEXI
++Testing: convert -C -S 4k -O qcow2 TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.target
--@item convert [--object @var{objectdef}] [--image-opts] [-U] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
++qemu-img: Cannot enable copy offloading when -S is used
-+@item convert [--object @var{objectdef}] [--image-opts] [-U] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
++
- ETEXI
++Testing: convert -C -S 8k -O qcow2 TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.target
++qemu-img: Cannot enable copy offloading when -S is used
- DEF("dd", img_dd,
++
 +Testing: convert -C -c -O qcow2 TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.target
 +qemu-img: Cannot enable copy offloading when -c is used
 +
  === amend: Options specified more than once ===
  Testing: amend -f foo -f qcow2 -o lazy_refcounts=on TEST_DIR/t.qcow2
 --
-.8.3.1
+.13.6

-[Qemu-devel] [PULL 08/15] qemu-img: add support for --object with 'dd' command
+[Qemu-devel] [PULL 09/13] block: Fix documentation for BDRV_REQ_MAY_UNMAP
-From: "Daniel P. Berrange" <berrange@redhat.com>
+BDRV_REQ_MAY_UNMAP in a write_zeroes request does not only allow the
 driver to unmap the blocks, but it actively requests that the blocks be
 unmapped afterwards if at all possible.
-The qemu-img dd command added --image-opts support, but missed
+Signed-off-by: Kevin Wolf <kwolf@redhat.com>
-the corresponding --object support. This prevented passing
+---
-secrets (eg auth passwords) needed by certain disk images.
+ include/block/block.h | 11 ++++++-----
 file changed, 6 insertions(+), 5 deletions(-)
-Reviewed-by: Fam Zheng <famz@redhat.com>
+diff --git a/include/block/block.h b/include/block/block.h
 Reviewed-by: Max Reitz <mreitz@redhat.com>
 Reviewed-by: Eric Blake <eblake@redhat.com>
 Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
 Message-id: 20170515164712.6643-2-berrange@redhat.com
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  qemu-img.c | 18 ++++++++++++++++++
 file changed, 18 insertions(+)
 diff --git a/qemu-img.c b/qemu-img.c
 index XXXXXXX..XXXXXXX 100644
---- a/qemu-img.c
+--- a/include/block/block.h
-+++ b/qemu-img.c
++++ b/include/block/block.h
-@@ -XXX,XX +XXX,XX @@ static int img_dd(int argc, char **argv)
+@@ -XXX,XX +XXX,XX @@ typedef struct BlockFragInfo {
-     };
+ typedef enum {
-     const struct option long_options[] = {
+     BDRV_REQ_COPY_ON_READ       = 0x1,
-         { "help", no_argument, 0, 'h'},
+     BDRV_REQ_ZERO_WRITE         = 0x2,
-+        { "object", required_argument, 0, OPTION_OBJECT},
+-    /* The BDRV_REQ_MAY_UNMAP flag is used to indicate that the block driver
-         { "image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+-     * is allowed to optimize a write zeroes request by unmapping (discarding)
-         { "force-share", no_argument, 0, 'U'},
+-     * blocks if it is guaranteed that the result will read back as
-         { 0, 0, 0, 0 }
+-     * zeroes. The flag is only passed to the driver if the block device is
-@@ -XXX,XX +XXX,XX @@ static int img_dd(int argc, char **argv)
+-     * opened with BDRV_O_UNMAP.
          case 'U':
              force_share = true;
              break;
 +        case OPTION_OBJECT: {
 +            QemuOpts *opts;
 +            opts = qemu_opts_parse_noisily(&qemu_object_opts,
 +                                           optarg, true);
 +            if (!opts) {
 +                ret = -1;
 +                goto out;
 +            }
 +        }   break;
          case OPTION_IMAGE_OPTS:
              image_opts = true;
              break;
@@ -XXX,XX +XXX,XX @@ static int img_dd(int argc, char **argv)
          ret = -1;
          goto out;
      }
 +
-+    if (qemu_opts_foreach(&qemu_object_opts,
++    /*
-+                          user_creatable_add_opts_foreach,
++     * The BDRV_REQ_MAY_UNMAP flag is used in write_zeroes requests to indicate
-+                          NULL, NULL)) {
++     * that the block driver should unmap (discard) blocks if it is guaranteed
-+        ret = -1;
++     * that the result will read back as zeroes. The flag is only passed to the
-+        goto out;
++     * driver if the block device is opened with BDRV_O_UNMAP.
-+    }
+      */
-+
+     BDRV_REQ_MAY_UNMAP          = 0x4,
      blk1 = img_open(image_opts, in.filename, fmt, 0, false, false,
                      force_share);
 --
-.8.3.1
+.13.6

-[Qemu-devel] [PULL 15/15] block/file-*: *_parse_filename() and colons
+[Qemu-devel] [PULL 10/13] file-posix: Fix write_zeroes with unmap on block devices
-From: Max Reitz <mreitz@redhat.com>
+The BLKDISCARD ioctl doesn't guarantee that the discarded blocks read as
 all-zero afterwards, so don't try to abuse it for zero writing. We try
 to only use this if BLKDISCARDZEROES tells us that it is safe, but this
 is unreliable on older kernels and a constant 0 in newer kernels. In
 other words, this code path is never actually used with newer kernels,
 so we don't even try to unmap while writing zeros.
-The file drivers' *_parse_filename() implementations just strip the
+This patch removes the abuse of discard for writing zeroes from
-optional protocol prefix off the filename. However, for e.g.
+file-posix and instead adds a new function that uses interfaces that are
-"file:foo:bar", this would lead to "foo:bar" being stored as the BDS's
+actually meant to deallocate and zero out at the same time. Only if
-filename which looks like it should be managed using the "foo" protocol.
+those fail, it falls back to zeroing out without unmap. We never fall
-This is especially troublesome if you then try to resolve a backing
+back to a discard operation any more that may or may not result in
-filename based on "foo:bar".
+zeros.
-This issue can only occur if the stripped part is a relative filename
+Signed-off-by: Kevin Wolf <kwolf@redhat.com>
-("file:/foo:bar" will be shortened to "/foo:bar" and having a slash
+---
-before the first colon means that "/foo" is not recognized as a protocol
+ block/file-posix.c | 59 ++++++++++++++++++++++++++++++++++++++++--------------
-part). Therefore, we can easily fix it by prepending "./" to such
+file changed, 44 insertions(+), 15 deletions(-)
 filenames.
-Before this patch:
-$ ./qemu-img create -f qcow2 backing.qcow2 64M
-Formatting 'backing.qcow2', fmt=qcow2 size=67108864 encryption=off
-    cluster_size=65536 lazy_refcounts=off refcount_bits=16
-$ ./qemu-img create -f qcow2 -b backing.qcow2 file:top:image.qcow2
-Formatting 'file:top:image.qcow2', fmt=qcow2 size=67108864
-    backing_file=backing.qcow2 encryption=off cluster_size=65536
-    lazy_refcounts=off refcount_bits=16
-$ ./qemu-io file:top:image.qcow2
-can't open device file:top:image.qcow2: Could not open backing file:
-    Unknown protocol 'top'
-After this patch:
-$ ./qemu-io file:top:image.qcow2
-[no error]
-Signed-off-by: Max Reitz <mreitz@redhat.com>
-Message-id: 20170522195217.12991-3-mreitz@redhat.com
-Reviewed-by: Eric Blake <eblake@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block.c                   | 35 +++++++++++++++++++++++++++++++++++
- block/file-posix.c        | 17 +++--------------
- block/file-win32.c        | 12 ++----------
- include/block/block_int.h |  3 +++
-files changed, 43 insertions(+), 24 deletions(-)
-diff --git a/block.c b/block.c
-index XXXXXXX..XXXXXXX 100644
---- a/block.c
-+++ b/block.c
-@@ -XXX,XX +XXX,XX @@ void path_combine(char *dest, int dest_size,
-     }
- }
-+/*
-+ * Helper function for bdrv_parse_filename() implementations to remove optional
-+ * protocol prefixes (especially "file:") from a filename and for putting the
-+ * stripped filename into the options QDict if there is such a prefix.
-+ */
-+void bdrv_parse_filename_strip_prefix(const char *filename, const char *prefix,
-+                                      QDict *options)
-+{
-+    if (strstart(filename, prefix, &filename)) {
-+        /* Stripping the explicit protocol prefix may result in a protocol
-+         * prefix being (wrongly) detected (if the filename contains a colon) */
-+        if (path_has_protocol(filename)) {
-+            QString *fat_filename;
-+
-+            /* This means there is some colon before the first slash; therefore,
-+             * this cannot be an absolute path */
-+            assert(!path_is_absolute(filename));
-+
-+            /* And we can thus fix the protocol detection issue by prefixing it
-+             * by "./" */
-+            fat_filename = qstring_from_str("./");
-+            qstring_append(fat_filename, filename);
-+
-+            assert(!path_has_protocol(qstring_get_str(fat_filename)));
-+
-+            qdict_put(options, "filename", fat_filename);
-+        } else {
-+            /* If no protocol prefix was detected, we can use the shortened
-+             * filename as-is */
-+            qdict_put_str(options, "filename", filename);
-+        }
-+    }
-+}
-+
-+
- /* Returns whether the image file is opened as read-only. Note that this can
-  * return false and writing to the image file is still not possible because the
-  * image is inactivated. */
 diff --git a/block/file-posix.c b/block/file-posix.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/file-posix.c
 +++ b/block/file-posix.c
-@@ -XXX,XX +XXX,XX @@ static void raw_parse_flags(int bdrv_flags, int *open_flags)
+@@ -XXX,XX +XXX,XX @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
- static void raw_parse_filename(const char *filename, QDict *options,
+     }
-                                Error **errp)
+ #endif
 -    bs->supported_zero_flags = s->discard_zeroes ? BDRV_REQ_MAY_UNMAP : 0;
 +    bs->supported_zero_flags = BDRV_REQ_MAY_UNMAP;
      ret = 0;
  fail:
      if (filename && (bdrv_flags & BDRV_O_TEMPORARY)) {
@@ -XXX,XX +XXX,XX @@ static ssize_t handle_aiocb_write_zeroes(RawPosixAIOData *aiocb)
      return -ENOTSUP;
  }
 +static ssize_t handle_aiocb_write_zeroes_unmap(RawPosixAIOData *aiocb)
 +{
 +    BDRVRawState *s G_GNUC_UNUSED = aiocb->bs->opaque;
 +    int ret;
 +
 +    /* First try to write zeros and unmap at the same time */
 +
 +#ifdef CONFIG_FALLOCATE_PUNCH_HOLE
 +    ret = do_fallocate(s->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
 +                       aiocb->aio_offset, aiocb->aio_nbytes);
 +    if (ret != -ENOTSUP) {
 +        return ret;
 +    }
 +#endif
 +
 +#ifdef CONFIG_XFS
 +    if (s->is_xfs) {
 +        /* xfs_discard() guarantees that the discarded area reads as all-zero
 +         * afterwards, so we can use it here. */
 +        return xfs_discard(s, aiocb->aio_offset, aiocb->aio_nbytes);
 +    }
 +#endif
 +
 +    /* If we couldn't manage to unmap while guaranteed that the area reads as
 +     * all-zero afterwards, just write zeroes without unmapping */
 +    ret = handle_aiocb_write_zeroes(aiocb);
 +    return ret;
 +}
 +
  #ifndef HAVE_COPY_FILE_RANGE
  static off_t copy_file_range(int in_fd, off_t *in_off, int out_fd,
                               off_t *out_off, size_t len, unsigned int flags)
@@ -XXX,XX +XXX,XX @@ static int aio_worker(void *arg)
      case QEMU_AIO_WRITE_ZEROES:
          ret = handle_aiocb_write_zeroes(aiocb);
          break;
 +    case QEMU_AIO_WRITE_ZEROES | QEMU_AIO_DISCARD:
 +        ret = handle_aiocb_write_zeroes_unmap(aiocb);
 +        break;
      case QEMU_AIO_COPY_RANGE:
          ret = handle_aiocb_copy_range(aiocb);
          break;
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_pwrite_zeroes(
      int bytes, BdrvRequestFlags flags)
  {
--    /* The filename does not have to be prefixed by the protocol name, since
+     BDRVRawState *s = bs->opaque;
--     * "file" is the default protocol; therefore, the return value of this
++    int operation = QEMU_AIO_WRITE_ZEROES;
--     * function call can be ignored. */
--    strstart(filename, "file:", &filename);
+-    if (!(flags & BDRV_REQ_MAY_UNMAP)) {
--
+-        return paio_submit_co(bs, s->fd, offset, NULL, bytes,
--    qdict_put_str(options, "filename", filename);
+-                              QEMU_AIO_WRITE_ZEROES);
-+    bdrv_parse_filename_strip_prefix(filename, "file:", options);
+-    } else if (s->discard_zeroes) {
 -        return paio_submit_co(bs, s->fd, offset, NULL, bytes,
 -                              QEMU_AIO_DISCARD);
 +    if (flags & BDRV_REQ_MAY_UNMAP) {
 +        operation |= QEMU_AIO_DISCARD;
      }
 -    return -ENOTSUP;
 +
 +    return paio_submit_co(bs, s->fd, offset, NULL, bytes, operation);
  }
- static QemuOptsList raw_runtime_opts = {
+ static int raw_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
-@@ -XXX,XX +XXX,XX @@ static int check_hdev_writable(BDRVRawState *s)
+@@ -XXX,XX +XXX,XX @@ static coroutine_fn int hdev_co_pwrite_zeroes(BlockDriverState *bs,
- static void hdev_parse_filename(const char *filename, QDict *options,
+     int64_t offset, int bytes, BdrvRequestFlags flags)
                                  Error **errp)
  {
--    /* The prefix is optional, just as for "file". */
+     BDRVRawState *s = bs->opaque;
--    strstart(filename, "host_device:", &filename);
++    int operation = QEMU_AIO_WRITE_ZEROES | QEMU_AIO_BLKDEV;
--
+     int rc;
--    qdict_put_str(options, "filename", filename);
-+    bdrv_parse_filename_strip_prefix(filename, "host_device:", options);
+     rc = fd_open(bs);
      if (rc < 0) {
          return rc;
      }
 -    if (!(flags & BDRV_REQ_MAY_UNMAP)) {
 -        return paio_submit_co(bs, s->fd, offset, NULL, bytes,
 -                              QEMU_AIO_WRITE_ZEROES|QEMU_AIO_BLKDEV);
 -    } else if (s->discard_zeroes) {
 -        return paio_submit_co(bs, s->fd, offset, NULL, bytes,
 -                              QEMU_AIO_DISCARD|QEMU_AIO_BLKDEV);
 +
 +    if (flags & BDRV_REQ_MAY_UNMAP) {
 +        operation |= QEMU_AIO_DISCARD;
      }
 -    return -ENOTSUP;
 +
 +    return paio_submit_co(bs, s->fd, offset, NULL, bytes, operation);
  }
- static bool hdev_is_sg(BlockDriverState *bs)
+ static int coroutine_fn hdev_co_create_opts(const char *filename, QemuOpts *opts,
@@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_host_device = {
  static void cdrom_parse_filename(const char *filename, QDict *options,
                                   Error **errp)
  {
 -    /* The prefix is optional, just as for "file". */
 -    strstart(filename, "host_cdrom:", &filename);
 -
 -    qdict_put_str(options, "filename", filename);
 +    bdrv_parse_filename_strip_prefix(filename, "host_cdrom:", options);
  }
  #endif
 diff --git a/block/file-win32.c b/block/file-win32.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/file-win32.c
 +++ b/block/file-win32.c
@@ -XXX,XX +XXX,XX @@ static void raw_parse_flags(int flags, bool use_aio, int *access_flags,
  static void raw_parse_filename(const char *filename, QDict *options,
                                 Error **errp)
  {
 -    /* The filename does not have to be prefixed by the protocol name, since
 -     * "file" is the default protocol; therefore, the return value of this
 -     * function call can be ignored. */
 -    strstart(filename, "file:", &filename);
 -
 -    qdict_put_str(options, "filename", filename);
 +    bdrv_parse_filename_strip_prefix(filename, "file:", options);
  }
  static QemuOptsList raw_runtime_opts = {
@@ -XXX,XX +XXX,XX @@ static int hdev_probe_device(const char *filename)
  static void hdev_parse_filename(const char *filename, QDict *options,
                                  Error **errp)
  {
 -    /* The prefix is optional, just as for "file". */
 -    strstart(filename, "host_device:", &filename);
 -
 -    qdict_put_str(options, "filename", filename);
 +    bdrv_parse_filename_strip_prefix(filename, "host_device:", options);
  }
  static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
 diff --git a/include/block/block_int.h b/include/block/block_int.h
 index XXXXXXX..XXXXXXX 100644
 --- a/include/block/block_int.h
 +++ b/include/block/block_int.h
@@ -XXX,XX +XXX,XX @@ int get_tmp_filename(char *filename, int size);
  BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
                              const char *filename);
 +void bdrv_parse_filename_strip_prefix(const char *filename, const char *prefix,
 +                                      QDict *options);
 +
  /**
   * bdrv_add_before_write_notifier:
 --
-.8.3.1
+.13.6

-[Qemu-devel] [PULL 05/15] mirror: Drop permissions on s->target on completion
+[Qemu-devel] [PULL 11/13] block/qapi: Add 'qdev' field to query-blockstats result
-This fixes an assertion failure that was triggered by qemu-iotests 129
+Like for query-block, the client needs to identify which BlockBackend
-on some CI host, while the same test case didn't seem to fail on other
+the returned data is for. Anonymous BlockBackends are identified by the
-hosts.
+device model they are attached to. Add a 'qdev' field that contains the
 qdev ID or QOM path of the attached device model.
-Essentially the problem is that the blk_unref(s->target) in
+Signed-off-by: Kevin Wolf <kwolf@redhat.com>
-mirror_exit() doesn't necessarily mean that the BlockBackend goes away
+Reviewed-by: Eric Blake <eblake@redhat.com>
-immediately. It is possible that the job completion was triggered nested
+---
-in mirror_drain(), which looks like this:
+ qapi/block-core.json | 14 ++++++++++----
  block/qapi.c         | 10 ++++++++++
 files changed, 20 insertions(+), 4 deletions(-)
-    BlockBackend *target = s->target;
+diff --git a/qapi/block-core.json b/qapi/block-core.json
     blk_ref(target);
     blk_drain(target);
     blk_unref(target);
 In this case, the write permissions for s->target are retained until
 after blk_drain(), which makes removing mirror_top_bs fail for the
 active commit case (can't have a writable backing file in the chain
 without the filter driver).
 Explicitly dropping the permissions first means that the additional
 reference doesn't hurt and the job can complete successfully even if
 called from the nested blk_drain().
 Cc: qemu-stable@nongnu.org
 Signed-off-by: Kevin Wolf <kwolf@redhat.com>
 Acked-by: Paolo Bonzini <pbonzini@redhat.com>
 Reviewed-by: Max Reitz <mreitz@redhat.com>
 ---
  block/mirror.c | 7 ++++++-
 file changed, 6 insertions(+), 1 deletion(-)
 diff --git a/block/mirror.c b/block/mirror.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/mirror.c
+--- a/qapi/block-core.json
-+++ b/block/mirror.c
++++ b/qapi/block-core.json
-@@ -XXX,XX +XXX,XX @@ static void mirror_exit(BlockJob *job, void *opaque)
+@@ -XXX,XX +XXX,XX @@
+ #
-     /* Remove target parent that still uses BLK_PERM_WRITE/RESIZE before
+ # @node-name: The node name of the device. (Since 2.3)
-      * inserting target_bs at s->to_replace, where we might not be able to get
+ #
--     * these permissions. */
++# @qdev: The qdev ID, or if no ID is assigned, the QOM path of the block
-+     * these permissions.
++#        device. (since 3.0)
-+     *
++#
-+     * Note that blk_unref() alone doesn't necessarily drop permissions because
+ # @stats:  A @BlockDeviceStats for the device.
-+     * we might be running nested inside mirror_drain(), which takes an extra
+ #
-+     * reference, so use an explicit blk_set_perm() first. */
+ # @parent: This describes the file block device if it has one.
-+    blk_set_perm(s->target, 0, BLK_PERM_ALL, &error_abort);
+@@ -XXX,XX +XXX,XX @@
-     blk_unref(s->target);
+ # Since: 0.14.0
-     s->target = NULL;
+ ##
  { 'struct': 'BlockStats',
 -  'data': {'*device': 'str', '*node-name': 'str',
 +  'data': {'*device': 'str', '*qdev': 'str', '*node-name': 'str',
             'stats': 'BlockDeviceStats',
             '*parent': 'BlockStats',
             '*backing': 'BlockStats'} }
@@ -XXX,XX +XXX,XX @@
  #                "idle_time_ns":2953431879,
  #                "account_invalid":true,
  #                "account_failed":false
 -#             }
 +#             },
 +#             "qdev": "/machine/unattached/device[23]"
  #          },
  #          {
  #             "device":"ide1-cd0",
@@ -XXX,XX +XXX,XX @@
  #                "wr_merged":0,
  #                "account_invalid":false,
  #                "account_failed":false
 -#             }
 +#             },
 +#             "qdev": "/machine/unattached/device[24]"
  #          },
  #          {
  #             "device":"floppy0",
@@ -XXX,XX +XXX,XX @@
  #                "wr_merged":0,
  #                "account_invalid":false,
  #                "account_failed":false
 -#             }
 +#             },
 +#             "qdev": "/machine/unattached/device[16]"
  #          },
  #          {
  #             "device":"sd0",
 diff --git a/block/qapi.c b/block/qapi.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/qapi.c
 +++ b/block/qapi.c
@@ -XXX,XX +XXX,XX @@ BlockStatsList *qmp_query_blockstats(bool has_query_nodes,
              BlockStatsList *info = g_malloc0(sizeof(*info));
              AioContext *ctx = blk_get_aio_context(blk);
              BlockStats *s;
 +            char *qdev;
              aio_context_acquire(ctx);
              s = bdrv_query_bds_stats(blk_bs(blk), true);
              s->has_device = true;
              s->device = g_strdup(blk_name(blk));
 +
 +            qdev = blk_get_attached_dev_id(blk);
 +            if (qdev && *qdev) {
 +                s->has_qdev = true;
 +                s->qdev = qdev;
 +            } else {
 +                g_free(qdev);
 +            }
 +
              bdrv_query_blk_stats(s->stats, blk);
              aio_context_release(ctx);
 --
-.8.3.1
+.13.6

-[Qemu-devel] [PULL 02/15] qemu-iotests: Test streaming with missing job ID
+[Qemu-devel] [PULL 12/13] block/qapi: Include anonymous BBs in query-blockstats
-This adds a small test for the image streaming error path for failing
+Consistent with query-block, query-blockstats should not only include
-block_job_create(), which would have found the null pointer dereference
+named BlockBackends, but also those that are anonymous, but belong to a
-in commit a170a91f.
+device model.
 Signed-off-by: Kevin Wolf <kwolf@redhat.com>
-Reviewed-by: Alberto Garcia <berto@igalia.com>
+Reviewed-by: Eric Blake <eblake@redhat.com>
 Reviewed-by: Kashyap Chamarthy <kchamart@redhat.com>
 Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
 Reviewed-by: Jeff Cody <jcody@redhat.com>
 ---
- tests/qemu-iotests/030     | 4 ++++
+ block/qapi.c | 6 +++++-
- tests/qemu-iotests/030.out | 4 ++--
+file changed, 5 insertions(+), 1 deletion(-)
 files changed, 6 insertions(+), 2 deletions(-)
-diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
+diff --git a/block/qapi.c b/block/qapi.c
-index XXXXXXX..XXXXXXX 100755
+index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/030
+--- a/block/qapi.c
-+++ b/tests/qemu-iotests/030
++++ b/block/qapi.c
-@@ -XXX,XX +XXX,XX @@ class TestSingleDrive(iotests.QMPTestCase):
+@@ -XXX,XX +XXX,XX @@ BlockStatsList *qmp_query_blockstats(bool has_query_nodes,
-         result = self.vm.qmp('block-stream', device='nonexistent')
+             p_next = &info->next;
-         self.assert_qmp(result, 'error/class', 'GenericError')
+         }
+     } else {
-+    def test_job_id_missing(self):
+-        for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
-+        result = self.vm.qmp('block-stream', device='mid')
++        for (blk = blk_all_next(NULL); blk; blk = blk_all_next(blk)) {
-+        self.assert_qmp(result, 'error/class', 'GenericError')
+             BlockStatsList *info = g_malloc0(sizeof(*info));
              AioContext *ctx = blk_get_aio_context(blk);
              BlockStats *s;
              char *qdev;
 +            if (!*blk_name(blk) && !blk_get_attached_dev(blk)) {
 +                continue;
 +            }
 +
+             aio_context_acquire(ctx);
- class TestParallelOps(iotests.QMPTestCase):
+             s = bdrv_query_bds_stats(blk_bs(blk), true);
-     num_ops = 4 # Number of parallel block-stream operations
+             s->has_device = true;
 diff --git a/tests/qemu-iotests/030.out b/tests/qemu-iotests/030.out
 index XXXXXXX..XXXXXXX 100644
 --- a/tests/qemu-iotests/030.out
 +++ b/tests/qemu-iotests/030.out
@@ -XXX,XX +XXX,XX @@
 -......................
 +.......................
  ----------------------------------------------------------------------
 -Ran 22 tests
 +Ran 23 tests
  OK
 --
-.8.3.1
+.13.6

-[Qemu-devel] [PULL 04/15] nvme: Add support for Controller Memory Buffers
+[Qemu-devel] [PULL 13/13] qemu-iotests: Test query-blockstats with -drive and -blockdev
-From: Stephen Bates <sbates@raithlin.com>
+Make sure that query-blockstats returns information for every
 BlockBackend that is named or attached to a device model (or both).
-Implement NVMe Controller Memory Buffers (CMBs) which were added in
+Signed-off-by: Kevin Wolf <kwolf@redhat.com>
-version 1.2 of the NVMe Specification. This patch adds an optional
+Reviewed-by: Eric Blake <eblake@redhat.com>
 argument (cmb_size_mb) which indicates the size of the CMB (in
 MB). Currently only the Submission Queue Support (SQS) is enabled
 which aligns with the current Linux driver for NVMe.
 Signed-off-by: Stephen Bates <sbates@raithlin.com>
 Acked-by: Keith Busch <keith.busch@intel.com>
 Signed-off-by: Kevin Wolf <kwolf@redhat.com>
 ---
- hw/block/nvme.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
+ tests/qemu-iotests/227     | 101 ++++++++++++++++++++++
- hw/block/nvme.h | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
+ tests/qemu-iotests/227.out | 205 +++++++++++++++++++++++++++++++++++++++++++++
-files changed, 144 insertions(+), 4 deletions(-)
+ tests/qemu-iotests/group   |   1 +
 files changed, 307 insertions(+)
  create mode 100755 tests/qemu-iotests/227
  create mode 100644 tests/qemu-iotests/227.out
-diff --git a/hw/block/nvme.c b/hw/block/nvme.c
+diff --git a/tests/qemu-iotests/227 b/tests/qemu-iotests/227
 new file mode 100755
 index XXXXXXX..XXXXXXX
 --- /dev/null
 +++ b/tests/qemu-iotests/227
@@ -XXX,XX +XXX,XX @@
 +#!/bin/bash
 +#
 +# Test query-blockstats with different ways to create a BB
 +#
 +# Copyright (C) 2018 Red Hat, Inc.
 +#
 +# This program is free software; you can redistribute it and/or modify
 +# it under the terms of the GNU General Public License as published by
 +# the Free Software Foundation; either version 2 of the License, or
 +# (at your option) any later version.
 +#
 +# This program is distributed in the hope that it will be useful,
 +# but WITHOUT ANY WARRANTY; without even the implied warranty of
 +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 +# GNU General Public License for more details.
 +#
 +# You should have received a copy of the GNU General Public License
 +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
 +#
 +
 +# creator
 +owner=kwolf@redhat.com
 +
 +seq=$(basename $0)
 +echo "QA output created by $seq"
 +
 +here=$PWD
 +status=1    # failure is the default!
 +
 +_cleanup()
 +{
 +    _cleanup_test_img
 +}
 +trap "_cleanup; exit \$status" 0 1 2 3 15
 +
 +# get standard environment, filters and checks
 +. ./common.rc
 +. ./common.filter
 +
 +_supported_fmt generic
 +_supported_proto file
 +_supported_os Linux
 +
 +function do_run_qemu()
 +{
 +    echo Testing: "$@"
 +    $QEMU -nographic -qmp-pretty stdio -serial none "$@"
 +    echo
 +}
 +
 +function run_qemu()
 +{
 +    do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qmp \
 +                          | _filter_qemu | _filter_imgfmt \
 +                          | _filter_generated_node_ids
 +}
 +
 +echo
 +echo '=== blockstats with -drive if=virtio ==='
 +echo
 +
 +run_qemu -drive driver=null-co,if=virtio <<EOF
 +{ "execute": "qmp_capabilities" }
 +{ "execute": "query-blockstats"}
 +{ "execute": "quit" }
 +EOF
 +
 +echo
 +echo '=== blockstats with -drive if=none ==='
 +echo
 +
 +run_qemu -drive driver=null-co,if=none <<EOF
 +{ "execute": "qmp_capabilities" }
 +{ "execute": "query-blockstats"}
 +{ "execute": "quit" }
 +EOF
 +
 +echo
 +echo '=== blockstats with -blockdev ==='
 +echo
 +
 +run_qemu -blockdev driver=null-co,node-name=null <<EOF
 +{ "execute": "qmp_capabilities" }
 +{ "execute": "query-blockstats"}
 +{ "execute": "quit" }
 +EOF
 +
 +echo
 +echo '=== blockstats with -blockdev and -device ==='
 +echo
 +
 +run_qemu -blockdev driver=null-co,node-name=null -device virtio-blk,drive=null,id=virtio0 <<EOF
 +{ "execute": "qmp_capabilities" }
 +{ "execute": "query-blockstats"}
 +{ "execute": "quit" }
 +EOF
 +
 +# success, all done
 +echo "*** done"
 +rm -f $seq.full
 +status=0
 diff --git a/tests/qemu-iotests/227.out b/tests/qemu-iotests/227.out
 new file mode 100644
 index XXXXXXX..XXXXXXX
 --- /dev/null
 +++ b/tests/qemu-iotests/227.out
@@ -XXX,XX +XXX,XX @@
 +QA output created by 227
 +
 +=== blockstats with -drive if=virtio ===
 +
 +Testing: -drive driver=null-co,if=virtio
 +{
 +    QMP_VERSION
 +}
 +{
 +    "return": {
 +    }
 +}
 +{
 +    "return": [
 +        {
 +            "device": "virtio0",
 +            "stats": {
 +                "flush_total_time_ns": 0,
 +                "wr_highest_offset": 0,
 +                "wr_total_time_ns": 0,
 +                "failed_wr_operations": 0,
 +                "failed_rd_operations": 0,
 +                "wr_merged": 0,
 +                "wr_bytes": 0,
 +                "timed_stats": [
 +                ],
 +                "failed_flush_operations": 0,
 +                "account_invalid": true,
 +                "rd_total_time_ns": 0,
 +                "flush_operations": 0,
 +                "wr_operations": 0,
 +                "rd_merged": 0,
 +                "rd_bytes": 0,
 +                "invalid_flush_operations": 0,
 +                "account_failed": true,
 +                "rd_operations": 0,
 +                "invalid_wr_operations": 0,
 +                "invalid_rd_operations": 0
 +            },
 +            "node-name": "NODE_NAME",
 +            "qdev": "/machine/peripheral-anon/device[0]/virtio-backend"
 +        }
 +    ]
 +}
 +{
 +    "return": {
 +    }
 +}
 +{
 +    "timestamp": {
 +        "seconds":  TIMESTAMP,
 +        "microseconds":  TIMESTAMP
 +    },
 +    "event": "SHUTDOWN",
 +    "data": {
 +        "guest": false
 +    }
 +}
 +
 +
 +=== blockstats with -drive if=none ===
 +
 +Testing: -drive driver=null-co,if=none
 +{
 +    QMP_VERSION
 +}
 +{
 +    "return": {
 +    }
 +}
 +{
 +    "return": [
 +        {
 +            "device": "none0",
 +            "stats": {
 +                "flush_total_time_ns": 0,
 +                "wr_highest_offset": 0,
 +                "wr_total_time_ns": 0,
 +                "failed_wr_operations": 0,
 +                "failed_rd_operations": 0,
 +                "wr_merged": 0,
 +                "wr_bytes": 0,
 +                "timed_stats": [
 +                ],
 +                "failed_flush_operations": 0,
 +                "account_invalid": true,
 +                "rd_total_time_ns": 0,
 +                "flush_operations": 0,
 +                "wr_operations": 0,
 +                "rd_merged": 0,
 +                "rd_bytes": 0,
 +                "invalid_flush_operations": 0,
 +                "account_failed": true,
 +                "rd_operations": 0,
 +                "invalid_wr_operations": 0,
 +                "invalid_rd_operations": 0
 +            },
 +            "node-name": "NODE_NAME"
 +        }
 +    ]
 +}
 +{
 +    "return": {
 +    }
 +}
 +{
 +    "timestamp": {
 +        "seconds":  TIMESTAMP,
 +        "microseconds":  TIMESTAMP
 +    },
 +    "event": "SHUTDOWN",
 +    "data": {
 +        "guest": false
 +    }
 +}
 +
 +
 +=== blockstats with -blockdev ===
 +
 +Testing: -blockdev driver=null-co,node-name=null
 +{
 +    QMP_VERSION
 +}
 +{
 +    "return": {
 +    }
 +}
 +{
 +    "return": [
 +    ]
 +}
 +{
 +    "return": {
 +    }
 +}
 +{
 +    "timestamp": {
 +        "seconds":  TIMESTAMP,
 +        "microseconds":  TIMESTAMP
 +    },
 +    "event": "SHUTDOWN",
 +    "data": {
 +        "guest": false
 +    }
 +}
 +
 +
 +=== blockstats with -blockdev and -device ===
 +
 +Testing: -blockdev driver=null-co,node-name=null -device virtio-blk,drive=null,id=virtio0
 +{
 +    QMP_VERSION
 +}
 +{
 +    "return": {
 +    }
 +}
 +{
 +    "return": [
 +        {
 +            "device": "",
 +            "stats": {
 +                "flush_total_time_ns": 0,
 +                "wr_highest_offset": 0,
 +                "wr_total_time_ns": 0,
 +                "failed_wr_operations": 0,
 +                "failed_rd_operations": 0,
 +                "wr_merged": 0,
 +                "wr_bytes": 0,
 +                "timed_stats": [
 +                ],
 +                "failed_flush_operations": 0,
 +                "account_invalid": false,
 +                "rd_total_time_ns": 0,
 +                "flush_operations": 0,
 +                "wr_operations": 0,
 +                "rd_merged": 0,
 +                "rd_bytes": 0,
 +                "invalid_flush_operations": 0,
 +                "account_failed": false,
 +                "rd_operations": 0,
 +                "invalid_wr_operations": 0,
 +                "invalid_rd_operations": 0
 +            },
 +            "node-name": "null",
 +            "qdev": "/machine/peripheral/virtio0/virtio-backend"
 +        }
 +    ]
 +}
 +{
 +    "return": {
 +    }
 +}
 +{
 +    "timestamp": {
 +        "seconds":  TIMESTAMP,
 +        "microseconds":  TIMESTAMP
 +    },
 +    "event": "SHUTDOWN",
 +    "data": {
 +        "guest": false
 +    }
 +}
 +
 +*** done
 diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
 index XXXXXXX..XXXXXXX 100644
---- a/hw/block/nvme.c
+--- a/tests/qemu-iotests/group
-+++ b/hw/block/nvme.c
++++ b/tests/qemu-iotests/group
 @@ -XXX,XX +XXX,XX @@
-  */
+rw auto quick
+rw auto quick
- /**
+auto quick
-- * Reference Specs: http://www.nvmexpress.org, 1.1, 1.0e
++227 auto quick
 + * Reference Specs: http://www.nvmexpress.org, 1.2, 1.1, 1.0e
   *
   *  http://www.nvmexpress.org/resources/
   */
@@ -XXX,XX +XXX,XX @@
  /**
   * Usage: add options:
   *      -drive file=<file>,if=none,id=<drive_id>
 - *      -device nvme,drive=<drive_id>,serial=<serial>,id=<id[optional]>
 + *      -device nvme,drive=<drive_id>,serial=<serial>,id=<id[optional]>, \
 + *              cmb_size_mb=<cmb_size_mb[optional]>
 + *
 + * Note cmb_size_mb denotes size of CMB in MB. CMB is assumed to be at
 + * offset 0 in BAR2 and supports SQS only for now.
   */
  #include "qemu/osdep.h"
@@ -XXX,XX +XXX,XX @@
  static void nvme_process_sq(void *opaque);
 +static void nvme_addr_read(NvmeCtrl *n, hwaddr addr, void *buf, int size)
 +{
 +    if (n->cmbsz && addr >= n->ctrl_mem.addr &&
 +                addr < (n->ctrl_mem.addr + int128_get64(n->ctrl_mem.size))) {
 +        memcpy(buf, (void *)&n->cmbuf[addr - n->ctrl_mem.addr], size);
 +    } else {
 +        pci_dma_read(&n->parent_obj, addr, buf, size);
 +    }
 +}
 +
  static int nvme_check_sqid(NvmeCtrl *n, uint16_t sqid)
  {
      return sqid < n->num_queues && n->sq[sqid] != NULL ? 0 : -1;
@@ -XXX,XX +XXX,XX @@ static void nvme_process_sq(void *opaque)
      while (!(nvme_sq_empty(sq) || QTAILQ_EMPTY(&sq->req_list))) {
          addr = sq->dma_addr + sq->head * n->sqe_size;
 -        pci_dma_read(&n->parent_obj, addr, (void *)&cmd, sizeof(cmd));
 +        nvme_addr_read(n, addr, (void *)&cmd, sizeof(cmd));
          nvme_inc_sq_head(sq);
          req = QTAILQ_FIRST(&sq->req_list);
@@ -XXX,XX +XXX,XX @@ static const MemoryRegionOps nvme_mmio_ops = {
      },
  };
 +static void nvme_cmb_write(void *opaque, hwaddr addr, uint64_t data,
 +    unsigned size)
 +{
 +    NvmeCtrl *n = (NvmeCtrl *)opaque;
 +    memcpy(&n->cmbuf[addr], &data, size);
 +}
 +
 +static uint64_t nvme_cmb_read(void *opaque, hwaddr addr, unsigned size)
 +{
 +    uint64_t val;
 +    NvmeCtrl *n = (NvmeCtrl *)opaque;
 +
 +    memcpy(&val, &n->cmbuf[addr], size);
 +    return val;
 +}
 +
 +static const MemoryRegionOps nvme_cmb_ops = {
 +    .read = nvme_cmb_read,
 +    .write = nvme_cmb_write,
 +    .endianness = DEVICE_LITTLE_ENDIAN,
 +    .impl = {
 +        .min_access_size = 2,
 +        .max_access_size = 8,
 +    },
 +};
 +
  static int nvme_init(PCIDevice *pci_dev)
  {
      NvmeCtrl *n = NVME(pci_dev);
@@ -XXX,XX +XXX,XX @@ static int nvme_init(PCIDevice *pci_dev)
      NVME_CAP_SET_CSS(n->bar.cap, 1);
      NVME_CAP_SET_MPSMAX(n->bar.cap, 4);
 -    n->bar.vs = 0x00010100;
 +    n->bar.vs = 0x00010200;
      n->bar.intmc = n->bar.intms = 0;
 +    if (n->cmb_size_mb) {
 +
 +        NVME_CMBLOC_SET_BIR(n->bar.cmbloc, 2);
 +        NVME_CMBLOC_SET_OFST(n->bar.cmbloc, 0);
 +
 +        NVME_CMBSZ_SET_SQS(n->bar.cmbsz, 1);
 +        NVME_CMBSZ_SET_CQS(n->bar.cmbsz, 0);
 +        NVME_CMBSZ_SET_LISTS(n->bar.cmbsz, 0);
 +        NVME_CMBSZ_SET_RDS(n->bar.cmbsz, 0);
 +        NVME_CMBSZ_SET_WDS(n->bar.cmbsz, 0);
 +        NVME_CMBSZ_SET_SZU(n->bar.cmbsz, 2); /* MBs */
 +        NVME_CMBSZ_SET_SZ(n->bar.cmbsz, n->cmb_size_mb);
 +
 +        n->cmbuf = g_malloc0(NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
 +        memory_region_init_io(&n->ctrl_mem, OBJECT(n), &nvme_cmb_ops, n,
 +                              "nvme-cmb", NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
 +        pci_register_bar(&n->parent_obj, NVME_CMBLOC_BIR(n->bar.cmbloc),
 +            PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64 |
 +            PCI_BASE_ADDRESS_MEM_PREFETCH, &n->ctrl_mem);
 +
 +    }
 +
      for (i = 0; i < n->num_namespaces; i++) {
          NvmeNamespace *ns = &n->namespaces[i];
          NvmeIdNs *id_ns = &ns->id_ns;
@@ -XXX,XX +XXX,XX @@ static void nvme_exit(PCIDevice *pci_dev)
      g_free(n->namespaces);
      g_free(n->cq);
      g_free(n->sq);
 +    if (n->cmbsz) {
 +        memory_region_unref(&n->ctrl_mem);
 +    }
 +
      msix_uninit_exclusive_bar(pci_dev);
  }
  static Property nvme_props[] = {
      DEFINE_BLOCK_PROPERTIES(NvmeCtrl, conf),
      DEFINE_PROP_STRING("serial", NvmeCtrl, serial),
 +    DEFINE_PROP_UINT32("cmb_size_mb", NvmeCtrl, cmb_size_mb, 0),
      DEFINE_PROP_END_OF_LIST(),
  };
 diff --git a/hw/block/nvme.h b/hw/block/nvme.h
 index XXXXXXX..XXXXXXX 100644
 --- a/hw/block/nvme.h
 +++ b/hw/block/nvme.h
@@ -XXX,XX +XXX,XX @@ typedef struct NvmeBar {
      uint32_t    aqa;
      uint64_t    asq;
      uint64_t    acq;
 +    uint32_t    cmbloc;
 +    uint32_t    cmbsz;
  } NvmeBar;
  enum NvmeCapShift {
@@ -XXX,XX +XXX,XX @@ enum NvmeAqaMask {
  #define NVME_AQA_ASQS(aqa) ((aqa >> AQA_ASQS_SHIFT) & AQA_ASQS_MASK)
  #define NVME_AQA_ACQS(aqa) ((aqa >> AQA_ACQS_SHIFT) & AQA_ACQS_MASK)
 +enum NvmeCmblocShift {
 +    CMBLOC_BIR_SHIFT  = 0,
 +    CMBLOC_OFST_SHIFT = 12,
 +};
 +
 +enum NvmeCmblocMask {
 +    CMBLOC_BIR_MASK  = 0x7,
 +    CMBLOC_OFST_MASK = 0xfffff,
 +};
 +
 +#define NVME_CMBLOC_BIR(cmbloc) ((cmbloc >> CMBLOC_BIR_SHIFT)  & \
 +                                 CMBLOC_BIR_MASK)
 +#define NVME_CMBLOC_OFST(cmbloc)((cmbloc >> CMBLOC_OFST_SHIFT) & \
 +                                 CMBLOC_OFST_MASK)
 +
 +#define NVME_CMBLOC_SET_BIR(cmbloc, val)  \
 +    (cmbloc |= (uint64_t)(val & CMBLOC_BIR_MASK) << CMBLOC_BIR_SHIFT)
 +#define NVME_CMBLOC_SET_OFST(cmbloc, val) \
 +    (cmbloc |= (uint64_t)(val & CMBLOC_OFST_MASK) << CMBLOC_OFST_SHIFT)
 +
 +enum NvmeCmbszShift {
 +    CMBSZ_SQS_SHIFT   = 0,
 +    CMBSZ_CQS_SHIFT   = 1,
 +    CMBSZ_LISTS_SHIFT = 2,
 +    CMBSZ_RDS_SHIFT   = 3,
 +    CMBSZ_WDS_SHIFT   = 4,
 +    CMBSZ_SZU_SHIFT   = 8,
 +    CMBSZ_SZ_SHIFT    = 12,
 +};
 +
 +enum NvmeCmbszMask {
 +    CMBSZ_SQS_MASK   = 0x1,
 +    CMBSZ_CQS_MASK   = 0x1,
 +    CMBSZ_LISTS_MASK = 0x1,
 +    CMBSZ_RDS_MASK   = 0x1,
 +    CMBSZ_WDS_MASK   = 0x1,
 +    CMBSZ_SZU_MASK   = 0xf,
 +    CMBSZ_SZ_MASK    = 0xfffff,
 +};
 +
 +#define NVME_CMBSZ_SQS(cmbsz)  ((cmbsz >> CMBSZ_SQS_SHIFT)   & CMBSZ_SQS_MASK)
 +#define NVME_CMBSZ_CQS(cmbsz)  ((cmbsz >> CMBSZ_CQS_SHIFT)   & CMBSZ_CQS_MASK)
 +#define NVME_CMBSZ_LISTS(cmbsz)((cmbsz >> CMBSZ_LISTS_SHIFT) & CMBSZ_LISTS_MASK)
 +#define NVME_CMBSZ_RDS(cmbsz)  ((cmbsz >> CMBSZ_RDS_SHIFT)   & CMBSZ_RDS_MASK)
 +#define NVME_CMBSZ_WDS(cmbsz)  ((cmbsz >> CMBSZ_WDS_SHIFT)   & CMBSZ_WDS_MASK)
 +#define NVME_CMBSZ_SZU(cmbsz)  ((cmbsz >> CMBSZ_SZU_SHIFT)   & CMBSZ_SZU_MASK)
 +#define NVME_CMBSZ_SZ(cmbsz)   ((cmbsz >> CMBSZ_SZ_SHIFT)    & CMBSZ_SZ_MASK)
 +
 +#define NVME_CMBSZ_SET_SQS(cmbsz, val)   \
 +    (cmbsz |= (uint64_t)(val &  CMBSZ_SQS_MASK)  << CMBSZ_SQS_SHIFT)
 +#define NVME_CMBSZ_SET_CQS(cmbsz, val)   \
 +    (cmbsz |= (uint64_t)(val & CMBSZ_CQS_MASK) << CMBSZ_CQS_SHIFT)
 +#define NVME_CMBSZ_SET_LISTS(cmbsz, val) \
 +    (cmbsz |= (uint64_t)(val & CMBSZ_LISTS_MASK) << CMBSZ_LISTS_SHIFT)
 +#define NVME_CMBSZ_SET_RDS(cmbsz, val)   \
 +    (cmbsz |= (uint64_t)(val & CMBSZ_RDS_MASK) << CMBSZ_RDS_SHIFT)
 +#define NVME_CMBSZ_SET_WDS(cmbsz, val)   \
 +    (cmbsz |= (uint64_t)(val & CMBSZ_WDS_MASK) << CMBSZ_WDS_SHIFT)
 +#define NVME_CMBSZ_SET_SZU(cmbsz, val)   \
 +    (cmbsz |= (uint64_t)(val & CMBSZ_SZU_MASK) << CMBSZ_SZU_SHIFT)
 +#define NVME_CMBSZ_SET_SZ(cmbsz, val)    \
 +    (cmbsz |= (uint64_t)(val & CMBSZ_SZ_MASK) << CMBSZ_SZ_SHIFT)
 +
 +#define NVME_CMBSZ_GETSIZE(cmbsz) \
 +    (NVME_CMBSZ_SZ(cmbsz) * (1 << (12 + 4 * NVME_CMBSZ_SZU(cmbsz))))
 +
  typedef struct NvmeCmd {
      uint8_t     opcode;
      uint8_t     fuse;
@@ -XXX,XX +XXX,XX @@ typedef struct NvmeNamespace {
  typedef struct NvmeCtrl {
      PCIDevice    parent_obj;
      MemoryRegion iomem;
 +    MemoryRegion ctrl_mem;
      NvmeBar      bar;
      BlockConf    conf;
@@ -XXX,XX +XXX,XX @@ typedef struct NvmeCtrl {
      uint32_t    num_queues;
      uint32_t    max_q_ents;
      uint64_t    ns_size;
 +    uint32_t    cmb_size_mb;
 +    uint32_t    cmbsz;
 +    uint32_t    cmbloc;
 +    uint8_t     *cmbuf;
      char            *serial;
      NvmeNamespace   *namespaces;
 --
-.8.3.1
+.13.6

-[Qemu-devel] [PULL 09/15] qemu-img: fix --image-opts usage with dd command
+Deleted patch
-From: "Daniel P. Berrange" <berrange@redhat.com>
-The --image-opts flag can only be used to affect the parsing
-of the source image. The target image has to be specified in
-the traditional style regardless, since it needs to be passed
-to the bdrv_create() API which does not support the new style
-opts.
-Reviewed-by: Fam Zheng <famz@redhat.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
-Message-id: 20170515164712.6643-3-berrange@redhat.com
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- qemu-img.c | 9 +++++++--
-file changed, 7 insertions(+), 2 deletions(-)
-diff --git a/qemu-img.c b/qemu-img.c
-index XXXXXXX..XXXXXXX 100644
---- a/qemu-img.c
-+++ b/qemu-img.c
-@@ -XXX,XX +XXX,XX @@ static int img_dd(int argc, char **argv)
-         goto out;
-     }
--    blk2 = img_open(image_opts, out.filename, out_fmt, BDRV_O_RDWR,
--                    false, false, false);
-+    /* TODO, we can't honour --image-opts for the target,
-+     * since it needs to be given in a format compatible
-+     * with the bdrv_create() call above which does not
-+     * support image-opts style.
-+     */
-+    blk2 = img_open_file(out.filename, out_fmt, BDRV_O_RDWR,
-+                         false, false, false);
-     if (!blk2) {
-         ret = -1;
---
-.8.3.1

-[Qemu-devel] [PULL 11/15] qemu-img: copy *key-secret opts when opening newly created files
+Deleted patch
-From: "Daniel P. Berrange" <berrange@redhat.com>
-The qemu-img dd/convert commands will create an image file and
-then try to open it. Historically it has been possible to open
-new files without passing any options. With encrypted files
-though, the *key-secret options are mandatory, so we need to
-provide those options when opening the newly created file.
-Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
-Message-id: 20170515164712.6643-5-berrange@redhat.com
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- qemu-img.c | 42 +++++++++++++++++++++++++++++++++++++-----
-file changed, 37 insertions(+), 5 deletions(-)
-diff --git a/qemu-img.c b/qemu-img.c
-index XXXXXXX..XXXXXXX 100644
---- a/qemu-img.c
-+++ b/qemu-img.c
-@@ -XXX,XX +XXX,XX @@ static BlockBackend *img_open_opts(const char *optstr,
- }
- static BlockBackend *img_open_file(const char *filename,
-+                                   QDict *options,
-                                    const char *fmt, int flags,
-                                    bool writethrough, bool quiet,
-                                    bool force_share)
- {
-     BlockBackend *blk;
-     Error *local_err = NULL;
--    QDict *options = qdict_new();
-+    if (!options) {
-+        options = qdict_new();
-+    }
-     if (fmt) {
-         qdict_put_str(options, "driver", fmt);
-     }
-@@ -XXX,XX +XXX,XX @@ static BlockBackend *img_open_file(const char *filename,
- }
-+static int img_add_key_secrets(void *opaque,
-+                               const char *name, const char *value,
-+                               Error **errp)
-+{
-+    QDict *options = opaque;
-+
-+    if (g_str_has_suffix(name, "key-secret")) {
-+        qdict_put(options, name, qstring_from_str(value));
-+    }
-+
-+    return 0;
-+}
-+
-+static BlockBackend *img_open_new_file(const char *filename,
-+                                       QemuOpts *create_opts,
-+                                       const char *fmt, int flags,
-+                                       bool writethrough, bool quiet,
-+                                       bool force_share)
-+{
-+    QDict *options = NULL;
-+
-+    options = qdict_new();
-+    qemu_opt_foreach(create_opts, img_add_key_secrets, options, &error_abort);
-+
-+    return img_open_file(filename, options, fmt, flags, writethrough, quiet,
-+                         force_share);
-+}
-+
-+
- static BlockBackend *img_open(bool image_opts,
-                               const char *filename,
-                               const char *fmt, int flags, bool writethrough,
-@@ -XXX,XX +XXX,XX @@ static BlockBackend *img_open(bool image_opts,
-         blk = img_open_opts(filename, opts, flags, writethrough, quiet,
-                             force_share);
-     } else {
--        blk = img_open_file(filename, fmt, flags, writethrough, quiet,
-+        blk = img_open_file(filename, NULL, fmt, flags, writethrough, quiet,
-                             force_share);
-     }
-     return blk;
-@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
-          * That has to wait for bdrv_create to be improved
-          * to allow filenames in option syntax
-          */
--        s.target = img_open_file(out_filename, out_fmt, flags,
--                                 writethrough, quiet, false);
-+        s.target = img_open_new_file(out_filename, opts, out_fmt,
-+                                     flags, writethrough, quiet, false);
-     }
-     if (!s.target) {
-         ret = -1;
-@@ -XXX,XX +XXX,XX @@ static int img_dd(int argc, char **argv)
-      * with the bdrv_create() call above which does not
-      * support image-opts style.
-      */
--    blk2 = img_open_file(out.filename, out_fmt, BDRV_O_RDWR,
-+    blk2 = img_open_file(out.filename, NULL, out_fmt, BDRV_O_RDWR,
-                          false, false, false);
-     if (!blk2) {
---
-.8.3.1

The following changes since commit 9964e96dc9999cf7f7c936ee854a795415d19b60:

Merge remote-tracking branch 'jasowang/tags/net-pull-request' into staging (2017-05-23 15:01:31 +0100)

are available in the git repository at:

git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to 42a48128417b3bfade93d1a4721348cc480e9e50:

Merge remote-tracking branch 'mreitz/tags/pull-block-2017-05-29-v3' into queue-block (2017-05-29 16:34:27 +0200)

----------------------------------------------------------------

Block layer patches

----------------------------------------------------------------
Alberto Garcia (2):
      stream: fix crash in stream_start() when block_job_create() fails
      qcow2: remove extra local_error variable

Daniel P. Berrange (4):
      qemu-img: add support for --object with 'dd' command
      qemu-img: fix --image-opts usage with dd command
      qemu-img: introduce --target-image-opts for 'convert' command
      qemu-img: copy *key-secret opts when opening newly created files

Eric Blake (1):
      block: Tweak error message related to qemu-img amend

Fam Zheng (3):
      iotests: 147: Don't test inet6 if not available
      qemu-img: Fix documentation of convert
      qemu-img: Fix leakage of options on error

Kevin Wolf (3):
      qemu-iotests: Test streaming with missing job ID
      mirror: Drop permissions on s->target on completion
      Merge remote-tracking branch 'mreitz/tags/pull-block-2017-05-29-v3' into queue-block

Max Reitz (2):
      block: Fix backing paths for filenames with colons
      block/file-*: *_parse_filename() and colons

Stephen Bates (1):
      nvme: Add support for Controller Memory Buffers

From: Alberto Garcia <berto@igalia.com>

The code that tries to reopen a BlockDriverState in stream_start()
when the creation of a new block job fails crashes because it attempts
to dereference a pointer that is known to be NULL.

This is a regression introduced in a170a91fd3eab6155da39e740381867e,
likely because the code was copied from stream_complete().

Cc: qemu-stable@nongnu.org
Reported-by: Kashyap Chamarthy <kchamart@redhat.com>
Signed-off-by: Alberto Garcia <berto@igalia.com>
Tested-by: Kashyap Chamarthy <kchamart@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/stream.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/stream.c b/block/stream.c
index XXXXXXX..XXXXXXX 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -XXX,XX +XXX,XX @@ void stream_start(const char *job_id, BlockDriverState *bs,
 
 fail:
     if (orig_bs_flags != bdrv_get_flags(bs)) {
-        bdrv_reopen(bs, s->bs_flags, NULL);
+        bdrv_reopen(bs, orig_bs_flags, NULL);
     }
 }
-- 
1.8.3.1

This adds a small test for the image streaming error path for failing
block_job_create(), which would have found the null pointer dereference
in commit a170a91f.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Kashyap Chamarthy <kchamart@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
---
 tests/qemu-iotests/030     | 4 ++++
 tests/qemu-iotests/030.out | 4 ++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -XXX,XX +XXX,XX @@ class TestSingleDrive(iotests.QMPTestCase):
         result = self.vm.qmp('block-stream', device='nonexistent')
         self.assert_qmp(result, 'error/class', 'GenericError')
 
+    def test_job_id_missing(self):
+        result = self.vm.qmp('block-stream', device='mid')
+        self.assert_qmp(result, 'error/class', 'GenericError')
+
 
 class TestParallelOps(iotests.QMPTestCase):
     num_ops = 4 # Number of parallel block-stream operations
diff --git a/tests/qemu-iotests/030.out b/tests/qemu-iotests/030.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/030.out
+++ b/tests/qemu-iotests/030.out
@@ -XXX,XX +XXX,XX @@
-......................
+.......................
 ----------------------------------------------------------------------
-Ran 22 tests
+Ran 23 tests
 
 OK
-- 
1.8.3.1

From: Stephen Bates <sbates@raithlin.com>

Implement NVMe Controller Memory Buffers (CMBs) which were added in
version 1.2 of the NVMe Specification. This patch adds an optional
argument (cmb_size_mb) which indicates the size of the CMB (in
MB). Currently only the Submission Queue Support (SQS) is enabled
which aligns with the current Linux driver for NVMe.

Signed-off-by: Stephen Bates <sbates@raithlin.com>
Acked-by: Keith Busch <keith.busch@intel.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/block/nvme.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 hw/block/nvme.h | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 144 insertions(+), 4 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -XXX,XX +XXX,XX @@
  */
 
 /**
- * Reference Specs: http://www.nvmexpress.org, 1.1, 1.0e
+ * Reference Specs: http://www.nvmexpress.org, 1.2, 1.1, 1.0e
  *
  *  http://www.nvmexpress.org/resources/
  */
@@ -XXX,XX +XXX,XX @@
 /**
  * Usage: add options:
  *      -drive file=<file>,if=none,id=<drive_id>
- *      -device nvme,drive=<drive_id>,serial=<serial>,id=<id[optional]>
+ *      -device nvme,drive=<drive_id>,serial=<serial>,id=<id[optional]>, \
+ *              cmb_size_mb=<cmb_size_mb[optional]>
+ *
+ * Note cmb_size_mb denotes size of CMB in MB. CMB is assumed to be at
+ * offset 0 in BAR2 and supports SQS only for now.
  */
 
 #include "qemu/osdep.h"
@@ -XXX,XX +XXX,XX @@
 
 static void nvme_process_sq(void *opaque);
 
+static void nvme_addr_read(NvmeCtrl *n, hwaddr addr, void *buf, int size)
+{
+    if (n->cmbsz && addr >= n->ctrl_mem.addr &&
+                addr < (n->ctrl_mem.addr + int128_get64(n->ctrl_mem.size))) {
+        memcpy(buf, (void *)&n->cmbuf[addr - n->ctrl_mem.addr], size);
+    } else {
+        pci_dma_read(&n->parent_obj, addr, buf, size);
+    }
+}
+
 static int nvme_check_sqid(NvmeCtrl *n, uint16_t sqid)
 {
     return sqid < n->num_queues && n->sq[sqid] != NULL ? 0 : -1;
@@ -XXX,XX +XXX,XX @@ static void nvme_process_sq(void *opaque)
 
     while (!(nvme_sq_empty(sq) || QTAILQ_EMPTY(&sq->req_list))) {
         addr = sq->dma_addr + sq->head * n->sqe_size;
-        pci_dma_read(&n->parent_obj, addr, (void *)&cmd, sizeof(cmd));
+        nvme_addr_read(n, addr, (void *)&cmd, sizeof(cmd));
         nvme_inc_sq_head(sq);
 
         req = QTAILQ_FIRST(&sq->req_list);
@@ -XXX,XX +XXX,XX @@ static const MemoryRegionOps nvme_mmio_ops = {
     },
 };
 
+static void nvme_cmb_write(void *opaque, hwaddr addr, uint64_t data,
+    unsigned size)
+{
+    NvmeCtrl *n = (NvmeCtrl *)opaque;
+    memcpy(&n->cmbuf[addr], &data, size);
+}
+
+static uint64_t nvme_cmb_read(void *opaque, hwaddr addr, unsigned size)
+{
+    uint64_t val;
+    NvmeCtrl *n = (NvmeCtrl *)opaque;
+
+    memcpy(&val, &n->cmbuf[addr], size);
+    return val;
+}
+
+static const MemoryRegionOps nvme_cmb_ops = {
+    .read = nvme_cmb_read,
+    .write = nvme_cmb_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .impl = {
+        .min_access_size = 2,
+        .max_access_size = 8,
+    },
+};
+
 static int nvme_init(PCIDevice *pci_dev)
 {
     NvmeCtrl *n = NVME(pci_dev);
@@ -XXX,XX +XXX,XX @@ static int nvme_init(PCIDevice *pci_dev)
     NVME_CAP_SET_CSS(n->bar.cap, 1);
     NVME_CAP_SET_MPSMAX(n->bar.cap, 4);
 
-    n->bar.vs = 0x00010100;
+    n->bar.vs = 0x00010200;
     n->bar.intmc = n->bar.intms = 0;
 
+    if (n->cmb_size_mb) {
+
+        NVME_CMBLOC_SET_BIR(n->bar.cmbloc, 2);
+        NVME_CMBLOC_SET_OFST(n->bar.cmbloc, 0);
+
+        NVME_CMBSZ_SET_SQS(n->bar.cmbsz, 1);
+        NVME_CMBSZ_SET_CQS(n->bar.cmbsz, 0);
+        NVME_CMBSZ_SET_LISTS(n->bar.cmbsz, 0);
+        NVME_CMBSZ_SET_RDS(n->bar.cmbsz, 0);
+        NVME_CMBSZ_SET_WDS(n->bar.cmbsz, 0);
+        NVME_CMBSZ_SET_SZU(n->bar.cmbsz, 2); /* MBs */
+        NVME_CMBSZ_SET_SZ(n->bar.cmbsz, n->cmb_size_mb);
+
+        n->cmbuf = g_malloc0(NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
+        memory_region_init_io(&n->ctrl_mem, OBJECT(n), &nvme_cmb_ops, n,
+                              "nvme-cmb", NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
+        pci_register_bar(&n->parent_obj, NVME_CMBLOC_BIR(n->bar.cmbloc),
+            PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64 |
+            PCI_BASE_ADDRESS_MEM_PREFETCH, &n->ctrl_mem);
+
+    }
+
     for (i = 0; i < n->num_namespaces; i++) {
         NvmeNamespace *ns = &n->namespaces[i];
         NvmeIdNs *id_ns = &ns->id_ns;
@@ -XXX,XX +XXX,XX @@ static void nvme_exit(PCIDevice *pci_dev)
     g_free(n->namespaces);
     g_free(n->cq);
     g_free(n->sq);
+    if (n->cmbsz) {
+        memory_region_unref(&n->ctrl_mem);
+    }
+
     msix_uninit_exclusive_bar(pci_dev);
 }
 
 static Property nvme_props[] = {
     DEFINE_BLOCK_PROPERTIES(NvmeCtrl, conf),
     DEFINE_PROP_STRING("serial", NvmeCtrl, serial),
+    DEFINE_PROP_UINT32("cmb_size_mb", NvmeCtrl, cmb_size_mb, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index XXXXXXX..XXXXXXX 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -XXX,XX +XXX,XX @@ typedef struct NvmeBar {
     uint32_t    aqa;
     uint64_t    asq;
     uint64_t    acq;
+    uint32_t    cmbloc;
+    uint32_t    cmbsz;
 } NvmeBar;
 
 enum NvmeCapShift {
@@ -XXX,XX +XXX,XX @@ enum NvmeAqaMask {
 #define NVME_AQA_ASQS(aqa) ((aqa >> AQA_ASQS_SHIFT) & AQA_ASQS_MASK)
 #define NVME_AQA_ACQS(aqa) ((aqa >> AQA_ACQS_SHIFT) & AQA_ACQS_MASK)
 
+enum NvmeCmblocShift {
+    CMBLOC_BIR_SHIFT  = 0,
+    CMBLOC_OFST_SHIFT = 12,
+};
+
+enum NvmeCmblocMask {
+    CMBLOC_BIR_MASK  = 0x7,
+    CMBLOC_OFST_MASK = 0xfffff,
+};
+
+#define NVME_CMBLOC_BIR(cmbloc) ((cmbloc >> CMBLOC_BIR_SHIFT)  & \
+                                 CMBLOC_BIR_MASK)
+#define NVME_CMBLOC_OFST(cmbloc)((cmbloc >> CMBLOC_OFST_SHIFT) & \
+                                 CMBLOC_OFST_MASK)
+
+#define NVME_CMBLOC_SET_BIR(cmbloc, val)  \
+    (cmbloc |= (uint64_t)(val & CMBLOC_BIR_MASK) << CMBLOC_BIR_SHIFT)
+#define NVME_CMBLOC_SET_OFST(cmbloc, val) \
+    (cmbloc |= (uint64_t)(val & CMBLOC_OFST_MASK) << CMBLOC_OFST_SHIFT)
+
+enum NvmeCmbszShift {
+    CMBSZ_SQS_SHIFT   = 0,
+    CMBSZ_CQS_SHIFT   = 1,
+    CMBSZ_LISTS_SHIFT = 2,
+    CMBSZ_RDS_SHIFT   = 3,
+    CMBSZ_WDS_SHIFT   = 4,
+    CMBSZ_SZU_SHIFT   = 8,
+    CMBSZ_SZ_SHIFT    = 12,
+};
+
+enum NvmeCmbszMask {
+    CMBSZ_SQS_MASK   = 0x1,
+    CMBSZ_CQS_MASK   = 0x1,
+    CMBSZ_LISTS_MASK = 0x1,
+    CMBSZ_RDS_MASK   = 0x1,
+    CMBSZ_WDS_MASK   = 0x1,
+    CMBSZ_SZU_MASK   = 0xf,
+    CMBSZ_SZ_MASK    = 0xfffff,
+};
+
+#define NVME_CMBSZ_SQS(cmbsz)  ((cmbsz >> CMBSZ_SQS_SHIFT)   & CMBSZ_SQS_MASK)
+#define NVME_CMBSZ_CQS(cmbsz)  ((cmbsz >> CMBSZ_CQS_SHIFT)   & CMBSZ_CQS_MASK)
+#define NVME_CMBSZ_LISTS(cmbsz)((cmbsz >> CMBSZ_LISTS_SHIFT) & CMBSZ_LISTS_MASK)
+#define NVME_CMBSZ_RDS(cmbsz)  ((cmbsz >> CMBSZ_RDS_SHIFT)   & CMBSZ_RDS_MASK)
+#define NVME_CMBSZ_WDS(cmbsz)  ((cmbsz >> CMBSZ_WDS_SHIFT)   & CMBSZ_WDS_MASK)
+#define NVME_CMBSZ_SZU(cmbsz)  ((cmbsz >> CMBSZ_SZU_SHIFT)   & CMBSZ_SZU_MASK)
+#define NVME_CMBSZ_SZ(cmbsz)   ((cmbsz >> CMBSZ_SZ_SHIFT)    & CMBSZ_SZ_MASK)
+
+#define NVME_CMBSZ_SET_SQS(cmbsz, val)   \
+    (cmbsz |= (uint64_t)(val &  CMBSZ_SQS_MASK)  << CMBSZ_SQS_SHIFT)
+#define NVME_CMBSZ_SET_CQS(cmbsz, val)   \
+    (cmbsz |= (uint64_t)(val & CMBSZ_CQS_MASK) << CMBSZ_CQS_SHIFT)
+#define NVME_CMBSZ_SET_LISTS(cmbsz, val) \
+    (cmbsz |= (uint64_t)(val & CMBSZ_LISTS_MASK) << CMBSZ_LISTS_SHIFT)
+#define NVME_CMBSZ_SET_RDS(cmbsz, val)   \
+    (cmbsz |= (uint64_t)(val & CMBSZ_RDS_MASK) << CMBSZ_RDS_SHIFT)
+#define NVME_CMBSZ_SET_WDS(cmbsz, val)   \
+    (cmbsz |= (uint64_t)(val & CMBSZ_WDS_MASK) << CMBSZ_WDS_SHIFT)
+#define NVME_CMBSZ_SET_SZU(cmbsz, val)   \
+    (cmbsz |= (uint64_t)(val & CMBSZ_SZU_MASK) << CMBSZ_SZU_SHIFT)
+#define NVME_CMBSZ_SET_SZ(cmbsz, val)    \
+    (cmbsz |= (uint64_t)(val & CMBSZ_SZ_MASK) << CMBSZ_SZ_SHIFT)
+
+#define NVME_CMBSZ_GETSIZE(cmbsz) \
+    (NVME_CMBSZ_SZ(cmbsz) * (1 << (12 + 4 * NVME_CMBSZ_SZU(cmbsz))))
+
 typedef struct NvmeCmd {
     uint8_t     opcode;
     uint8_t     fuse;
@@ -XXX,XX +XXX,XX @@ typedef struct NvmeNamespace {
 typedef struct NvmeCtrl {
     PCIDevice    parent_obj;
     MemoryRegion iomem;
+    MemoryRegion ctrl_mem;
     NvmeBar      bar;
     BlockConf    conf;
 
@@ -XXX,XX +XXX,XX @@ typedef struct NvmeCtrl {
     uint32_t    num_queues;
     uint32_t    max_q_ents;
     uint64_t    ns_size;
+    uint32_t    cmb_size_mb;
+    uint32_t    cmbsz;
+    uint32_t    cmbloc;
+    uint8_t     *cmbuf;
 
     char            *serial;
     NvmeNamespace   *namespaces;
-- 
1.8.3.1

This fixes an assertion failure that was triggered by qemu-iotests 129
on some CI host, while the same test case didn't seem to fail on other
hosts.

Essentially the problem is that the blk_unref(s->target) in
mirror_exit() doesn't necessarily mean that the BlockBackend goes away
immediately. It is possible that the job completion was triggered nested
in mirror_drain(), which looks like this:

BlockBackend *target = s->target;
    blk_ref(target);
    blk_drain(target);
    blk_unref(target);

In this case, the write permissions for s->target are retained until
after blk_drain(), which makes removing mirror_top_bs fail for the
active commit case (can't have a writable backing file in the chain
without the filter driver).

Explicitly dropping the permissions first means that the additional
reference doesn't hurt and the job can complete successfully even if
called from the nested blk_drain().

Cc: qemu-stable@nongnu.org
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/mirror.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/block/mirror.c b/block/mirror.c
index XXXXXXX..XXXXXXX 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -XXX,XX +XXX,XX @@ static void mirror_exit(BlockJob *job, void *opaque)
 
     /* Remove target parent that still uses BLK_PERM_WRITE/RESIZE before
      * inserting target_bs at s->to_replace, where we might not be able to get
-     * these permissions. */
+     * these permissions.
+     *
+     * Note that blk_unref() alone doesn't necessarily drop permissions because
+     * we might be running nested inside mirror_drain(), which takes an extra
+     * reference, so use an explicit blk_set_perm() first. */
+    blk_set_perm(s->target, 0, BLK_PERM_ALL, &error_abort);
     blk_unref(s->target);
     s->target = NULL;
 
-- 
1.8.3.1

From: Alberto Garcia <berto@igalia.com>

Commit d7086422b1c1e75e320519cfe26176db6ec97a37 added a local_err
variable global to the qcow2_amend_options() function, so there's no
need to have this other one.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-id: 20170511150337.21470-1-berto@igalia.com
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -XXX,XX +XXX,XX @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
 
     if (s->refcount_bits != refcount_bits) {
         int refcount_order = ctz32(refcount_bits);
-        Error *local_error = NULL;
 
         if (new_version < 3 && refcount_bits != 16) {
             error_report("Different refcount widths than 16 bits require "
@@ -XXX,XX +XXX,XX @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
         helper_cb_info.current_operation = QCOW2_CHANGING_REFCOUNT_ORDER;
         ret = qcow2_change_refcount_order(bs, refcount_order,
                                           &qcow2_amend_helper_cb,
-                                          &helper_cb_info, &local_error);
+                                          &helper_cb_info, &local_err);
         if (ret < 0) {
-            error_report_err(local_error);
+            error_report_err(local_err);
             return ret;
         }
     }
-- 
1.8.3.1

From: Fam Zheng <famz@redhat.com>

It got lost in commit a8d16f9ca "qemu-img: Update documentation for -U".

Reported-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-id: 20170515103551.31313-1-famz@redhat.com
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-img-cmds.hx | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

From: "Daniel P. Berrange" <berrange@redhat.com>

The qemu-img dd command added --image-opts support, but missed
the corresponding --object support. This prevented passing
secrets (eg auth passwords) needed by certain disk images.

Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 20170515164712.6643-2-berrange@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-img.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/qemu-img.c b/qemu-img.c
index XXXXXXX..XXXXXXX 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -XXX,XX +XXX,XX @@ static int img_dd(int argc, char **argv)
     };
     const struct option long_options[] = {
         { "help", no_argument, 0, 'h'},
+        { "object", required_argument, 0, OPTION_OBJECT},
         { "image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
         { "force-share", no_argument, 0, 'U'},
         { 0, 0, 0, 0 }
@@ -XXX,XX +XXX,XX @@ static int img_dd(int argc, char **argv)
         case 'U':
             force_share = true;
             break;
+        case OPTION_OBJECT: {
+            QemuOpts *opts;
+            opts = qemu_opts_parse_noisily(&qemu_object_opts,
+                                           optarg, true);
+            if (!opts) {
+                ret = -1;
+                goto out;
+            }
+        }   break;
         case OPTION_IMAGE_OPTS:
             image_opts = true;
             break;
@@ -XXX,XX +XXX,XX @@ static int img_dd(int argc, char **argv)
         ret = -1;
         goto out;
     }
+
+    if (qemu_opts_foreach(&qemu_object_opts,
+                          user_creatable_add_opts_foreach,
+                          NULL, NULL)) {
+        ret = -1;
+        goto out;
+    }
+
     blk1 = img_open(image_opts, in.filename, fmt, 0, false, false,
                     force_share);
 
-- 
1.8.3.1

From: "Daniel P. Berrange" <berrange@redhat.com>

The --image-opts flag can only be used to affect the parsing
of the source image. The target image has to be specified in
the traditional style regardless, since it needs to be passed
to the bdrv_create() API which does not support the new style
opts.

Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 20170515164712.6643-3-berrange@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-img.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index XXXXXXX..XXXXXXX 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -XXX,XX +XXX,XX @@ static int img_dd(int argc, char **argv)
         goto out;
     }
 
-    blk2 = img_open(image_opts, out.filename, out_fmt, BDRV_O_RDWR,
-                    false, false, false);
+    /* TODO, we can't honour --image-opts for the target,
+     * since it needs to be given in a format compatible
+     * with the bdrv_create() call above which does not
+     * support image-opts style.
+     */
+    blk2 = img_open_file(out.filename, out_fmt, BDRV_O_RDWR,
+                         false, false, false);
 
     if (!blk2) {
         ret = -1;
-- 
1.8.3.1

From: "Daniel P. Berrange" <berrange@redhat.com>

The '--image-opts' flag indicates whether the source filename
includes options. The target filename has to remain in the
plain filename format though, since it needs to be passed to
bdrv_create().  When using --skip-create though, it would be
possible to use image-opts syntax. This adds --target-image-opts
to indicate that the target filename includes options. Currently
this mandates use of the --skip-create flag too.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 20170515164712.6643-4-berrange@redhat.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-img-cmds.hx |  4 +--
 qemu-img.c       | 84 ++++++++++++++++++++++++++++++++++++++------------------
 qemu-img.texi    | 12 ++++++--
 3 files changed, 69 insertions(+), 31 deletions(-)

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index XXXXXXX..XXXXXXX 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -XXX,XX +XXX,XX @@ STEXI
 ETEXI
 
 DEF("convert", img_convert,
-    "convert [--object objectdef] [--image-opts] [-U] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
+    "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
 STEXI
-@item convert [--object @var{objectdef}] [--image-opts] [-U] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
+@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
 ETEXI
 
 DEF("dd", img_dd,
diff --git a/qemu-img.c b/qemu-img.c
index XXXXXXX..XXXXXXX 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -XXX,XX +XXX,XX @@ enum {
     OPTION_PATTERN = 260,
     OPTION_FLUSH_INTERVAL = 261,
     OPTION_NO_DRAIN = 262,
+    OPTION_TARGET_IMAGE_OPTS = 263,
 };
 
 typedef enum OutputFormat {
@@ -XXX,XX +XXX,XX @@ static int convert_do_copy(ImgConvertState *s)
 static int img_convert(int argc, char **argv)
 {
     int c, bs_i, flags, src_flags = 0;
-    const char *fmt = NULL, *out_fmt = "raw", *cache = "unsafe",
+    const char *fmt = NULL, *out_fmt = NULL, *cache = "unsafe",
                *src_cache = BDRV_DEFAULT_CACHE, *out_baseimg = NULL,
                *out_filename, *out_baseimg_param, *snapshot_name = NULL;
-    BlockDriver *drv, *proto_drv;
+    BlockDriver *drv = NULL, *proto_drv = NULL;
     BlockDriverInfo bdi;
     BlockDriverState *out_bs;
     QemuOpts *opts = NULL, *sn_opts = NULL;
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
     char *options = NULL;
     Error *local_err = NULL;
     bool writethrough, src_writethrough, quiet = false, image_opts = false,
-         skip_create = false, progress = false;
+         skip_create = false, progress = false, tgt_image_opts = false;
     int64_t ret = -EINVAL;
     bool force_share = false;
 
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
             {"object", required_argument, 0, OPTION_OBJECT},
             {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
             {"force-share", no_argument, 0, 'U'},
+            {"target-image-opts", no_argument, 0, OPTION_TARGET_IMAGE_OPTS},
             {0, 0, 0, 0}
         };
         c = getopt_long(argc, argv, ":hf:O:B:ce6o:s:l:S:pt:T:qnm:WU",
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
         case OPTION_IMAGE_OPTS:
             image_opts = true;
             break;
+        case OPTION_TARGET_IMAGE_OPTS:
+            tgt_image_opts = true;
+            break;
         }
     }
 
+    if (!out_fmt && !tgt_image_opts) {
+        out_fmt = "raw";
+    }
+
     if (qemu_opts_foreach(&qemu_object_opts,
                           user_creatable_add_opts_foreach,
                           NULL, NULL)) {
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
         goto fail_getopt;
     }
 
+    if (tgt_image_opts && !skip_create) {
+        error_report("--target-image-opts requires use of -n flag");
+        goto fail_getopt;
+    }
+
     s.src_num = argc - optind - 1;
     out_filename = s.src_num >= 1 ? argv[argc - 1] : NULL;
 
     if (options && has_help_option(options)) {
-        ret = print_block_option_help(out_filename, out_fmt);
-        goto fail_getopt;
+        if (out_fmt) {
+            ret = print_block_option_help(out_filename, out_fmt);
+            goto fail_getopt;
+        } else {
+            error_report("Option help requires a format be specified");
+            goto fail_getopt;
+        }
     }
 
     if (s.src_num < 1) {
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
         goto out;
     }
 
-    /* Find driver and parse its options */
-    drv = bdrv_find_format(out_fmt);
-    if (!drv) {
-        error_report("Unknown file format '%s'", out_fmt);
-        ret = -1;
-        goto out;
-    }
+    if (!skip_create) {
+        /* Find driver and parse its options */
+        drv = bdrv_find_format(out_fmt);
+        if (!drv) {
+            error_report("Unknown file format '%s'", out_fmt);
+            ret = -1;
+            goto out;
+        }
 
-    proto_drv = bdrv_find_protocol(out_filename, true, &local_err);
-    if (!proto_drv) {
-        error_report_err(local_err);
-        ret = -1;
-        goto out;
-    }
+        proto_drv = bdrv_find_protocol(out_filename, true, &local_err);
+        if (!proto_drv) {
+            error_report_err(local_err);
+            ret = -1;
+            goto out;
+        }
 
-    if (!skip_create) {
         if (!drv->create_opts) {
             error_report("Format driver '%s' does not support image creation",
                          drv->format_name);
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
         const char *preallocation =
             qemu_opt_get(opts, BLOCK_OPT_PREALLOC);
 
-        if (!drv->bdrv_co_pwritev_compressed) {
+        if (drv && !drv->bdrv_co_pwritev_compressed) {
             error_report("Compression not supported for this file format");
             ret = -1;
             goto out;
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
         goto out;
     }
 
-    /* XXX we should allow --image-opts to trigger use of
-     * img_open() here, but then we have trouble with
-     * the bdrv_create() call which takes different params.
-     * Not critical right now, so fix can wait...
-     */
-    s.target = img_open_file(out_filename, out_fmt, flags, writethrough, quiet,
-                             false);
+    if (skip_create) {
+        s.target = img_open(tgt_image_opts, out_filename, out_fmt,
+                            flags, writethrough, quiet, false);
+    } else {
+        /* TODO ultimately we should allow --target-image-opts
+         * to be used even when -n is not given.
+         * That has to wait for bdrv_create to be improved
+         * to allow filenames in option syntax
+         */
+        s.target = img_open_file(out_filename, out_fmt, flags,
+                                 writethrough, quiet, false);
+    }
     if (!s.target) {
         ret = -1;
         goto out;
     }
     out_bs = blk_bs(s.target);
 
+    if (s.compressed && !out_bs->drv->bdrv_co_pwritev_compressed) {
+        error_report("Compression not supported for this file format");
+        ret = -1;
+        goto out;
+    }
+
     /* increase bufsectors from the default 4096 (2M) if opt_transfer
      * or discard_alignment of the out_bs is greater. Limit to 32768 (16MB)
      * as maximum. */
diff --git a/qemu-img.texi b/qemu-img.texi
index XXXXXXX..XXXXXXX 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -XXX,XX +XXX,XX @@ keys.
 
 @item --image-opts
 
-Indicates that the @var{filename} parameter is to be interpreted as a
+Indicates that the source @var{filename} parameter is to be interpreted as a
 full option string, not a plain filename. This parameter is mutually
-exclusive with the @var{-f} and @var{-F} parameters.
+exclusive with the @var{-f} parameter.
+
+@item --target-image-opts
+
+Indicates that the @var{output_filename} parameter(s) are to be interpreted as
+a full option string, not a plain filename. This parameter is mutually
+exclusive with the @var{-O} parameters. It is currently required to also use
+the @var{-n} parameter to skip image creation. This restriction may be relaxed
+in a future release.
 
 @item fmt
 is the disk image format. It is guessed automatically in most cases. See below
-- 
1.8.3.1

From: "Daniel P. Berrange" <berrange@redhat.com>

The qemu-img dd/convert commands will create an image file and
then try to open it. Historically it has been possible to open
new files without passing any options. With encrypted files
though, the *key-secret options are mandatory, so we need to
provide those options when opening the newly created file.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 20170515164712.6643-5-berrange@redhat.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-img.c | 42 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 37 insertions(+), 5 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index XXXXXXX..XXXXXXX 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -XXX,XX +XXX,XX @@ static BlockBackend *img_open_opts(const char *optstr,
 }
 
 static BlockBackend *img_open_file(const char *filename,
+                                   QDict *options,
                                    const char *fmt, int flags,
                                    bool writethrough, bool quiet,
                                    bool force_share)
 {
     BlockBackend *blk;
     Error *local_err = NULL;
-    QDict *options = qdict_new();
 
+    if (!options) {
+        options = qdict_new();
+    }
     if (fmt) {
         qdict_put_str(options, "driver", fmt);
     }
@@ -XXX,XX +XXX,XX @@ static BlockBackend *img_open_file(const char *filename,
 }
 
 
+static int img_add_key_secrets(void *opaque,
+                               const char *name, const char *value,
+                               Error **errp)
+{
+    QDict *options = opaque;
+
+    if (g_str_has_suffix(name, "key-secret")) {
+        qdict_put(options, name, qstring_from_str(value));
+    }
+
+    return 0;
+}
+
+static BlockBackend *img_open_new_file(const char *filename,
+                                       QemuOpts *create_opts,
+                                       const char *fmt, int flags,
+                                       bool writethrough, bool quiet,
+                                       bool force_share)
+{
+    QDict *options = NULL;
+
+    options = qdict_new();
+    qemu_opt_foreach(create_opts, img_add_key_secrets, options, &error_abort);
+
+    return img_open_file(filename, options, fmt, flags, writethrough, quiet,
+                         force_share);
+}
+
+
 static BlockBackend *img_open(bool image_opts,
                               const char *filename,
                               const char *fmt, int flags, bool writethrough,
@@ -XXX,XX +XXX,XX @@ static BlockBackend *img_open(bool image_opts,
         blk = img_open_opts(filename, opts, flags, writethrough, quiet,
                             force_share);
     } else {
-        blk = img_open_file(filename, fmt, flags, writethrough, quiet,
+        blk = img_open_file(filename, NULL, fmt, flags, writethrough, quiet,
                             force_share);
     }
     return blk;
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
          * That has to wait for bdrv_create to be improved
          * to allow filenames in option syntax
          */
-        s.target = img_open_file(out_filename, out_fmt, flags,
-                                 writethrough, quiet, false);
+        s.target = img_open_new_file(out_filename, opts, out_fmt,
+                                     flags, writethrough, quiet, false);
     }
     if (!s.target) {
         ret = -1;
@@ -XXX,XX +XXX,XX @@ static int img_dd(int argc, char **argv)
      * with the bdrv_create() call above which does not
      * support image-opts style.
      */
-    blk2 = img_open_file(out.filename, out_fmt, BDRV_O_RDWR,
+    blk2 = img_open_file(out.filename, NULL, out_fmt, BDRV_O_RDWR,
                          false, false, false);
 
     if (!blk2) {
-- 
1.8.3.1

From: Eric Blake <eblake@redhat.com>

When converting a 1.1 image down to 0.10, qemu-iotests 060 forces
a contrived failure where allocating a cluster used to replace a
zero cluster reads unaligned data.  Since it is a zero cluster
rather than a data cluster being converted, changing the error
message to match our earlier change in 'qcow2: Make distinction
between zero cluster types obvious' is worthwhile.

Suggested-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-id: 20170508171302.17805-1-eblake@redhat.com
[mreitz: Commit message fixes]
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2-cluster.c      | 3 ++-
 tests/qemu-iotests/060.out | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -XXX,XX +XXX,XX @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
             }
 
             if (offset_into_cluster(s, offset)) {
-                qcow2_signal_corruption(bs, true, -1, -1, "Data cluster offset "
+                qcow2_signal_corruption(bs, true, -1, -1,
+                                        "Cluster allocation offset "
                                         "%#" PRIx64 " unaligned (L2 offset: %#"
                                         PRIx64 ", L2 index: %#x)", offset,
                                         l2_offset, j);
diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/060.out
+++ b/tests/qemu-iotests/060.out
@@ -XXX,XX +XXX,XX @@ read failed: Input/output error
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 wrote 65536/65536 bytes at offset 0
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-qcow2: Marking image as corrupt: Data cluster offset 0x52a00 unaligned (L2 offset: 0x40000, L2 index: 0); further corruption events will be suppressed
+qcow2: Marking image as corrupt: Cluster allocation offset 0x52a00 unaligned (L2 offset: 0x40000, L2 index: 0); further corruption events will be suppressed
 qemu-img: Error while amending options: Input/output error
 
 === Testing unaligned reftable entry ===
-- 
1.8.3.1

From: Max Reitz <mreitz@redhat.com>

path_combine() naturally tries to preserve a protocol prefix. However,
it recognizes such a prefix by scanning for the first colon; which is
different from what path_has_protocol() does: There only is a protocol
prefix if there is a colon before the first slash.

A protocol prefix that is not recognized by path_has_protocol() is none,
and should thus not be taken as one.

Case in point, before this patch:
$ ./qemu-img create -f qcow2 -b backing.qcow2 ./top:image.qcow2
qemu-img: ./top:image.qcow2: Could not open './top:backing.qcow2':
    No such file or directory

Afterwards:
$ ./qemu-img create -f qcow2 -b backing.qcow2 ./top:image.qcow2
qemu-img: ./top:image.qcow2: Could not open './backing.qcow2':
    No such file or directory

Reported-by: yangyang <yangyang@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20170522195217.12991-2-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index XXXXXXX..XXXXXXX 100644
--- a/block.c
+++ b/block.c
@@ -XXX,XX +XXX,XX @@ void path_combine(char *dest, int dest_size,
     if (path_is_absolute(filename)) {
         pstrcpy(dest, dest_size, filename);
     } else {
-        p = strchr(base_path, ':');
-        if (p)
-            p++;
-        else
-            p = base_path;
+        const char *protocol_stripped = NULL;
+
+        if (path_has_protocol(base_path)) {
+            protocol_stripped = strchr(base_path, ':');
+            if (protocol_stripped) {
+                protocol_stripped++;
+            }
+        }
+        p = protocol_stripped ?: base_path;
+
         p1 = strrchr(base_path, '/');
 #ifdef _WIN32
         {
-- 
1.8.3.1

From: Max Reitz <mreitz@redhat.com>

The file drivers' *_parse_filename() implementations just strip the
optional protocol prefix off the filename. However, for e.g.
"file:foo:bar", this would lead to "foo:bar" being stored as the BDS's
filename which looks like it should be managed using the "foo" protocol.
This is especially troublesome if you then try to resolve a backing
filename based on "foo:bar".

This issue can only occur if the stripped part is a relative filename
("file:/foo:bar" will be shortened to "/foo:bar" and having a slash
before the first colon means that "/foo" is not recognized as a protocol
part). Therefore, we can easily fix it by prepending "./" to such
filenames.

Before this patch:
$ ./qemu-img create -f qcow2 backing.qcow2 64M
Formatting 'backing.qcow2', fmt=qcow2 size=67108864 encryption=off
    cluster_size=65536 lazy_refcounts=off refcount_bits=16
$ ./qemu-img create -f qcow2 -b backing.qcow2 file:top:image.qcow2
Formatting 'file:top:image.qcow2', fmt=qcow2 size=67108864
    backing_file=backing.qcow2 encryption=off cluster_size=65536
    lazy_refcounts=off refcount_bits=16
$ ./qemu-io file:top:image.qcow2
can't open device file:top:image.qcow2: Could not open backing file:
    Unknown protocol 'top'

After this patch:
$ ./qemu-io file:top:image.qcow2
[no error]

Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20170522195217.12991-3-mreitz@redhat.com
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c                   | 35 +++++++++++++++++++++++++++++++++++
 block/file-posix.c        | 17 +++--------------
 block/file-win32.c        | 12 ++----------
 include/block/block_int.h |  3 +++
 4 files changed, 43 insertions(+), 24 deletions(-)

diff --git a/block.c b/block.c
index XXXXXXX..XXXXXXX 100644
--- a/block.c
+++ b/block.c
@@ -XXX,XX +XXX,XX @@ void path_combine(char *dest, int dest_size,
     }
 }
 
+/*
+ * Helper function for bdrv_parse_filename() implementations to remove optional
+ * protocol prefixes (especially "file:") from a filename and for putting the
+ * stripped filename into the options QDict if there is such a prefix.
+ */
+void bdrv_parse_filename_strip_prefix(const char *filename, const char *prefix,
+                                      QDict *options)
+{
+    if (strstart(filename, prefix, &filename)) {
+        /* Stripping the explicit protocol prefix may result in a protocol
+         * prefix being (wrongly) detected (if the filename contains a colon) */
+        if (path_has_protocol(filename)) {
+            QString *fat_filename;
+
+            /* This means there is some colon before the first slash; therefore,
+             * this cannot be an absolute path */
+            assert(!path_is_absolute(filename));
+
+            /* And we can thus fix the protocol detection issue by prefixing it
+             * by "./" */
+            fat_filename = qstring_from_str("./");
+            qstring_append(fat_filename, filename);
+
+            assert(!path_has_protocol(qstring_get_str(fat_filename)));
+
+            qdict_put(options, "filename", fat_filename);
+        } else {
+            /* If no protocol prefix was detected, we can use the shortened
+             * filename as-is */
+            qdict_put_str(options, "filename", filename);
+        }
+    }
+}
+
+
 /* Returns whether the image file is opened as read-only. Note that this can
  * return false and writing to the image file is still not possible because the
  * image is inactivated. */
diff --git a/block/file-posix.c b/block/file-posix.c
index XXXXXXX..XXXXXXX 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -XXX,XX +XXX,XX @@ static void raw_parse_flags(int bdrv_flags, int *open_flags)
 static void raw_parse_filename(const char *filename, QDict *options,
                                Error **errp)
 {
-    /* The filename does not have to be prefixed by the protocol name, since
-     * "file" is the default protocol; therefore, the return value of this
-     * function call can be ignored. */
-    strstart(filename, "file:", &filename);
-
-    qdict_put_str(options, "filename", filename);
+    bdrv_parse_filename_strip_prefix(filename, "file:", options);
 }
 
 static QemuOptsList raw_runtime_opts = {
@@ -XXX,XX +XXX,XX @@ static int check_hdev_writable(BDRVRawState *s)
 static void hdev_parse_filename(const char *filename, QDict *options,
                                 Error **errp)
 {
-    /* The prefix is optional, just as for "file". */
-    strstart(filename, "host_device:", &filename);
-
-    qdict_put_str(options, "filename", filename);
+    bdrv_parse_filename_strip_prefix(filename, "host_device:", options);
 }
 
 static bool hdev_is_sg(BlockDriverState *bs)
@@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_host_device = {
 static void cdrom_parse_filename(const char *filename, QDict *options,
                                  Error **errp)
 {
-    /* The prefix is optional, just as for "file". */
-    strstart(filename, "host_cdrom:", &filename);
-
-    qdict_put_str(options, "filename", filename);
+    bdrv_parse_filename_strip_prefix(filename, "host_cdrom:", options);
 }
 #endif
 
diff --git a/block/file-win32.c b/block/file-win32.c
index XXXXXXX..XXXXXXX 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -XXX,XX +XXX,XX @@ static void raw_parse_flags(int flags, bool use_aio, int *access_flags,
 static void raw_parse_filename(const char *filename, QDict *options,
                                Error **errp)
 {
-    /* The filename does not have to be prefixed by the protocol name, since
-     * "file" is the default protocol; therefore, the return value of this
-     * function call can be ignored. */
-    strstart(filename, "file:", &filename);
-
-    qdict_put_str(options, "filename", filename);
+    bdrv_parse_filename_strip_prefix(filename, "file:", options);
 }
 
 static QemuOptsList raw_runtime_opts = {
@@ -XXX,XX +XXX,XX @@ static int hdev_probe_device(const char *filename)
 static void hdev_parse_filename(const char *filename, QDict *options,
                                 Error **errp)
 {
-    /* The prefix is optional, just as for "file". */
-    strstart(filename, "host_device:", &filename);
-
-    qdict_put_str(options, "filename", filename);
+    bdrv_parse_filename_strip_prefix(filename, "host_device:", options);
 }
 
 static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -XXX,XX +XXX,XX @@ int get_tmp_filename(char *filename, int size);
 BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
                             const char *filename);
 
+void bdrv_parse_filename_strip_prefix(const char *filename, const char *prefix,
+                                      QDict *options);
+
 
 /**
  * bdrv_add_before_write_notifier:
-- 
1.8.3.1

The following changes since commit 6d9dd5fb9d0e9f4a174f53a0e20a39fbe809c71e:

Merge remote-tracking branch 'remotes/armbru/tags/pull-qobject-2018-07-27-v2' into staging (2018-07-30 09:55:47 +0100)

are available in the git repository at:

git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to 1239ac241fe170bb9fcf0be74bfff04f6f1c2560:

qemu-iotests: Test query-blockstats with -drive and -blockdev (2018-07-30 15:35:37 +0200)

----------------------------------------------------------------
Block layer patches:

- qemu-img convert -C is now required to enable copy offloading
- file-posix: Fix write_zeroes with unmap on block devices (would fall
  back to explicit writes on recent kernels)
- Fix query-blockstats interface for use with -blockdev
- Minor fixes and documentation updates

----------------------------------------------------------------
Fam Zheng (6):
      file-posix: Handle EINTR in preallocation=full write
      docs: Describe using images in writing iotests
      iotests: Don't lock /dev/null in 226
      Revert "qemu-img: Document copy offloading implications with -S and -c"
      qemu-img: Add -C option for convert with copy offloading
      iotests: Add test for 'qemu-img convert -C' compatibility

KONRAD Frederic (1):
      qcow: fix a reference leak

Kevin Wolf (5):
      block: Fix documentation for BDRV_REQ_MAY_UNMAP
      file-posix: Fix write_zeroes with unmap on block devices
      block/qapi: Add 'qdev' field to query-blockstats result
      block/qapi: Include anonymous BBs in query-blockstats
      qemu-iotests: Test query-blockstats with -drive and -blockdev

Leonid Bloch (1):
      qcow2: A grammar fix in conflicting cache sizing error message

From: KONRAD Frederic <frederic.konrad@adacore.com>

Since 42a3e1ab367cdf38cce093de24eb406b99a4ef96 qemu asserts when using the
vvfat driver:

git clone git://qemu.org/qemu.git
cd qemu
./configure --target-list=ppc-softmmu --enable-debug
make -j8
mkdir foo
touch foo/hello
./ppc-softmmu/qemu-system-ppc -M prep --nographic --monitor null             \
                              -hda fat:rw:./foo

"Ctrl-C"

qemu-system-ppc: block.c:3368: bdrv_close_all: Assertion                     \
   `((&all_bdrv_states)->tqh_first == ((void *)0))' failed.

This is because we reference bs twice in qcow_co_create(..) one time in
bdrv_open_blockdev_ref(..) and in blk_insert_bs(..) but we unref it only once
in blk_unref which leads to the reference leak.

Note that I didn't tested much QCOW after this change as I don't use it much.

Signed-off-by: KONRAD Frederic <frederic.konrad@adacore.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/qcow.c b/block/qcow.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn qcow_co_create(BlockdevCreateOptions *opts,
     ret = 0;
 exit:
     blk_unref(qcow_blk);
+    bdrv_unref(bs);
     qcrypto_block_free(crypto);
     return ret;
 }
-- 
2.13.6

From: Leonid Bloch <lbloch@janustech.com>

Signed-off-by: Leonid Bloch <lbloch@janustech.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2.c              | 2 +-
 tests/qemu-iotests/103.out | 4 ++--
 tests/qemu-iotests/137.out | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -XXX,XX +XXX,XX @@ static void read_cache_sizes(BlockDriverState *bs, QemuOpts *opts,
         if (l2_cache_size_set && refcount_cache_size_set) {
             error_setg(errp, QCOW2_OPT_CACHE_SIZE ", " QCOW2_OPT_L2_CACHE_SIZE
                        " and " QCOW2_OPT_REFCOUNT_CACHE_SIZE " may not be set "
-                       "the same time");
+                       "at the same time");
             return;
         } else if (*l2_cache_size > combined_cache_size) {
             error_setg(errp, QCOW2_OPT_L2_CACHE_SIZE " may not exceed "
diff --git a/tests/qemu-iotests/103.out b/tests/qemu-iotests/103.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/103.out
+++ b/tests/qemu-iotests/103.out
@@ -XXX,XX +XXX,XX @@ wrote 65536/65536 bytes at offset 0
 
 === Testing invalid option combinations ===
 
-can't open device TEST_DIR/t.IMGFMT: cache-size, l2-cache-size and refcount-cache-size may not be set the same time
+can't open device TEST_DIR/t.IMGFMT: cache-size, l2-cache-size and refcount-cache-size may not be set at the same time
 can't open device TEST_DIR/t.IMGFMT: l2-cache-size may not exceed cache-size
 can't open device TEST_DIR/t.IMGFMT: refcount-cache-size may not exceed cache-size
-can't open device TEST_DIR/t.IMGFMT: cache-size, l2-cache-size and refcount-cache-size may not be set the same time
+can't open device TEST_DIR/t.IMGFMT: cache-size, l2-cache-size and refcount-cache-size may not be set at the same time
 can't open device TEST_DIR/t.IMGFMT: L2 cache entry size must be a power of two between 512 and the cluster size (65536)
 can't open device TEST_DIR/t.IMGFMT: L2 cache entry size must be a power of two between 512 and the cluster size (65536)
 can't open device TEST_DIR/t.IMGFMT: L2 cache entry size must be a power of two between 512 and the cluster size (65536)
diff --git a/tests/qemu-iotests/137.out b/tests/qemu-iotests/137.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/137.out
+++ b/tests/qemu-iotests/137.out
@@ -XXX,XX +XXX,XX @@ read 33554432/33554432 bytes at offset 0
 === Try setting some invalid values ===
 
 Parameter 'lazy-refcounts' expects 'on' or 'off'
-cache-size, l2-cache-size and refcount-cache-size may not be set the same time
+cache-size, l2-cache-size and refcount-cache-size may not be set at the same time
 l2-cache-size may not exceed cache-size
 refcount-cache-size may not exceed cache-size
 L2 cache size too big
-- 
2.13.6

From: Fam Zheng <famz@redhat.com>

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 docs/devel/testing.rst | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index XXXXXXX..XXXXXXX 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -XXX,XX +XXX,XX @@ comparable library support for invoking and interacting with QEMU programs. If
 you opt for Python, it is strongly recommended to write Python 3 compatible
 code.
 
+Both Python and Bash frameworks in iotests provide helpers to manage test
+images. They can be used to create and clean up images under the test
+directory. If no I/O or any protocol specific feature is needed, it is often
+more convenient to use the pseudo block driver, ``null-co://``, as the test
+image, which doesn't require image creation or cleaning up. Avoid system-wide
+devices or files whenever possible, such as ``/dev/null`` or ``/dev/zero``.
+Otherwise, image locking implications have to be considered.  For example,
+another application on the host may have locked the file, possibly leading to a
+test failure.  If using such devices are explicitly desired, consider adding
+``locking=off`` option to disable image locking.
+
 Docker based tests
 ==================
 
-- 
2.13.6

From: Fam Zheng <famz@redhat.com>

On my system (Fedora 28), this script reports a 'failed to get
"consistent read" lock' error. Following docs/devel/testing.rst, it's
better to add locking=off here.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/226 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/226 b/tests/qemu-iotests/226
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/226
+++ b/tests/qemu-iotests/226
@@ -XXX,XX +XXX,XX @@ for PROTO in "file" "host_device" "host_cdrom"; do
     echo
     echo "== Testing RO =="
     $QEMU_IO -c "open -r -o driver=$PROTO,filename=$TEST_IMG" 2>&1 | _filter_testdir | _filter_imgfmt
-    $QEMU_IO -c "open -r -o driver=$PROTO,filename=/dev/null" 2>&1 | _filter_imgfmt
+    $QEMU_IO -c "open -r -o driver=$PROTO,filename=/dev/null,locking=off" 2>&1 | _filter_imgfmt
     echo "== Testing RW =="
     $QEMU_IO -c "open -o driver=$PROTO,filename=$TEST_IMG" 2>&1 | _filter_testdir | _filter_imgfmt
-    $QEMU_IO -c "open -o driver=$PROTO,filename=/dev/null" 2>&1 | _filter_imgfmt
+    $QEMU_IO -c "open -o driver=$PROTO,filename=/dev/null,locking=off" 2>&1 | _filter_imgfmt
 done
 
 # success, all done
-- 
2.13.6

From: Fam Zheng <famz@redhat.com>

This reverts commit eb461485f4558e362fab905735b50987505bca44.

Now that we introduce an explicit option, these implicit rules are not
used.

Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-img.texi | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/qemu-img.texi b/qemu-img.texi
index XXXXXXX..XXXXXXX 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -XXX,XX +XXX,XX @@ will enumerate information about backing files in a disk image chain. Refer
 below for further description.
 
 @item -c
-indicates that target image must be compressed (qcow format only). If this
-option is used, copy offloading will not be attempted.
+indicates that target image must be compressed (qcow format only)
 
 @item -h
 with or without a command shows help and lists the supported formats
@@ -XXX,XX +XXX,XX @@ in case both @var{-q} and @var{-p} options are used.
 indicates the consecutive number of bytes that must contain only zeros
 for qemu-img to create a sparse image during conversion. This value is rounded
 down to the nearest 512 bytes. You may use the common size suffixes like
-@code{k} for kilobytes. If this option is used, copy offloading will not be
-attempted.
+@code{k} for kilobytes.
 
 @item -t @var{cache}
 specifies the cache mode that should be used with the (destination) file. See
-- 
2.13.6

From: Fam Zheng <famz@redhat.com>

Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-img.c       | 21 +++++++++++++++++----
 qemu-img-cmds.hx |  2 +-
 qemu-img.texi    |  8 +++++++-
 3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index XXXXXXX..XXXXXXX 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
          skip_create = false, progress = false, tgt_image_opts = false;
     int64_t ret = -EINVAL;
     bool force_share = false;
+    bool explict_min_sparse = false;
 
     ImgConvertState s = (ImgConvertState) {
         /* Need at least 4k of zeros for sparse detection */
         .min_sparse         = 8,
-        .copy_range         = true,
+        .copy_range         = false,
         .buf_sectors        = IO_BUF_SIZE / BDRV_SECTOR_SIZE,
         .wr_in_order        = true,
         .num_coroutines     = 8,
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
             {"target-image-opts", no_argument, 0, OPTION_TARGET_IMAGE_OPTS},
             {0, 0, 0, 0}
         };
-        c = getopt_long(argc, argv, ":hf:O:B:co:l:S:pt:T:qnm:WU",
+        c = getopt_long(argc, argv, ":hf:O:B:Cco:l:S:pt:T:qnm:WU",
                         long_options, NULL);
         if (c == -1) {
             break;
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
         case 'B':
             out_baseimg = optarg;
             break;
+        case 'C':
+            s.copy_range = true;
+            break;
         case 'c':
             s.compressed = true;
-            s.copy_range = false;
             break;
         case 'o':
             if (!is_valid_option_list(optarg)) {
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
             }
 
             s.min_sparse = sval / BDRV_SECTOR_SIZE;
-            s.copy_range = false;
+            explict_min_sparse = true;
             break;
         }
         case 'p':
@@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv)
         goto fail_getopt;
     }
 
+    if (s.compressed && s.copy_range) {
+        error_report("Cannot enable copy offloading when -c is used");
+        goto fail_getopt;
+    }
+
+    if (explict_min_sparse && s.copy_range) {
+        error_report("Cannot enable copy offloading when -S is used");
+        goto fail_getopt;
+    }
+
     if (tgt_image_opts && !skip_create) {
         error_report("--target-image-opts requires use of -n flag");
         goto fail_getopt;
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index XXXXXXX..XXXXXXX 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -XXX,XX +XXX,XX @@ STEXI
 ETEXI
 
 DEF("convert", img_convert,
-    "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
+    "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
 STEXI
 @item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
 ETEXI
diff --git a/qemu-img.texi b/qemu-img.texi
index XXXXXXX..XXXXXXX 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -XXX,XX +XXX,XX @@ Number of parallel coroutines for the convert process
 Allow out-of-order writes to the destination. This option improves performance,
 but is only recommended for preallocated devices like host devices or other
 raw block devices.
+@item -C
+Try to use copy offloading to move data from source image to target. This may
+improve performance if the data is remote, such as with NFS or iSCSI backends,
+but will not automatically sparsify zero sectors, and may result in a fully
+allocated target image depending on the host support for getting allocation
+information.
 @end table
 
 Parameters to dd subcommand:
@@ -XXX,XX +XXX,XX @@ Error on reading data
 
 @end table
 
-@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
+@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
 
 Convert the disk image @var{filename} or a snapshot @var{snapshot_param}
 to disk image @var{output_filename} using format @var{output_fmt}. It can be optionally compressed (@code{-c}
-- 
2.13.6

From: Fam Zheng <famz@redhat.com>

Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/082     |  8 ++++++++
 tests/qemu-iotests/082.out | 11 +++++++++++
 2 files changed, 19 insertions(+)

diff --git a/tests/qemu-iotests/082 b/tests/qemu-iotests/082
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/082
+++ b/tests/qemu-iotests/082
@@ -XXX,XX +XXX,XX @@ run_qemu_img convert -o help
 run_qemu_img convert -O bochs -o help
 
 echo
+echo === convert: -C and other options ===
+
+# Adding the help option to a command without other -o options
+run_qemu_img convert -C -S 4k -O $IMGFMT "$TEST_IMG" "$TEST_IMG".target
+run_qemu_img convert -C -S 8k -O $IMGFMT "$TEST_IMG" "$TEST_IMG".target
+run_qemu_img convert -C -c -O $IMGFMT "$TEST_IMG" "$TEST_IMG".target
+
+echo
 echo === amend: Options specified more than once ===
 
 # Last -f should win
diff --git a/tests/qemu-iotests/082.out b/tests/qemu-iotests/082.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/082.out
+++ b/tests/qemu-iotests/082.out
@@ -XXX,XX +XXX,XX @@ size             Virtual disk size
 Testing: convert -O bochs -o help
 qemu-img: Format driver 'bochs' does not support image creation
 
+=== convert: -C and other options ===
+
+Testing: convert -C -S 4k -O qcow2 TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.target
+qemu-img: Cannot enable copy offloading when -S is used
+
+Testing: convert -C -S 8k -O qcow2 TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.target
+qemu-img: Cannot enable copy offloading when -S is used
+
+Testing: convert -C -c -O qcow2 TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.target
+qemu-img: Cannot enable copy offloading when -c is used
+
 === amend: Options specified more than once ===
 
 Testing: amend -f foo -f qcow2 -o lazy_refcounts=on TEST_DIR/t.qcow2
-- 
2.13.6

BDRV_REQ_MAY_UNMAP in a write_zeroes request does not only allow the
driver to unmap the blocks, but it actively requests that the blocks be
unmapped afterwards if at all possible.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 include/block/block.h | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/block/block.h b/include/block/block.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -XXX,XX +XXX,XX @@ typedef struct BlockFragInfo {
 typedef enum {
     BDRV_REQ_COPY_ON_READ       = 0x1,
     BDRV_REQ_ZERO_WRITE         = 0x2,
-    /* The BDRV_REQ_MAY_UNMAP flag is used to indicate that the block driver
-     * is allowed to optimize a write zeroes request by unmapping (discarding)
-     * blocks if it is guaranteed that the result will read back as
-     * zeroes. The flag is only passed to the driver if the block device is
-     * opened with BDRV_O_UNMAP.
+
+    /*
+     * The BDRV_REQ_MAY_UNMAP flag is used in write_zeroes requests to indicate
+     * that the block driver should unmap (discard) blocks if it is guaranteed
+     * that the result will read back as zeroes. The flag is only passed to the
+     * driver if the block device is opened with BDRV_O_UNMAP.
      */
     BDRV_REQ_MAY_UNMAP          = 0x4,
 
-- 
2.13.6

The BLKDISCARD ioctl doesn't guarantee that the discarded blocks read as
all-zero afterwards, so don't try to abuse it for zero writing. We try
to only use this if BLKDISCARDZEROES tells us that it is safe, but this
is unreliable on older kernels and a constant 0 in newer kernels. In
other words, this code path is never actually used with newer kernels,
so we don't even try to unmap while writing zeros.

This patch removes the abuse of discard for writing zeroes from
file-posix and instead adds a new function that uses interfaces that are
actually meant to deallocate and zero out at the same time. Only if
those fail, it falls back to zeroing out without unmap. We never fall
back to a discard operation any more that may or may not result in
zeros.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/file-posix.c | 59 ++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 44 insertions(+), 15 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index XXXXXXX..XXXXXXX 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -XXX,XX +XXX,XX @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
     }
 #endif
 
-    bs->supported_zero_flags = s->discard_zeroes ? BDRV_REQ_MAY_UNMAP : 0;
+    bs->supported_zero_flags = BDRV_REQ_MAY_UNMAP;
     ret = 0;
 fail:
     if (filename && (bdrv_flags & BDRV_O_TEMPORARY)) {
@@ -XXX,XX +XXX,XX @@ static ssize_t handle_aiocb_write_zeroes(RawPosixAIOData *aiocb)
     return -ENOTSUP;
 }
 
+static ssize_t handle_aiocb_write_zeroes_unmap(RawPosixAIOData *aiocb)
+{
+    BDRVRawState *s G_GNUC_UNUSED = aiocb->bs->opaque;
+    int ret;
+
+    /* First try to write zeros and unmap at the same time */
+
+#ifdef CONFIG_FALLOCATE_PUNCH_HOLE
+    ret = do_fallocate(s->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
+                       aiocb->aio_offset, aiocb->aio_nbytes);
+    if (ret != -ENOTSUP) {
+        return ret;
+    }
+#endif
+
+#ifdef CONFIG_XFS
+    if (s->is_xfs) {
+        /* xfs_discard() guarantees that the discarded area reads as all-zero
+         * afterwards, so we can use it here. */
+        return xfs_discard(s, aiocb->aio_offset, aiocb->aio_nbytes);
+    }
+#endif
+
+    /* If we couldn't manage to unmap while guaranteed that the area reads as
+     * all-zero afterwards, just write zeroes without unmapping */
+    ret = handle_aiocb_write_zeroes(aiocb);
+    return ret;
+}
+
 #ifndef HAVE_COPY_FILE_RANGE
 static off_t copy_file_range(int in_fd, off_t *in_off, int out_fd,
                              off_t *out_off, size_t len, unsigned int flags)
@@ -XXX,XX +XXX,XX @@ static int aio_worker(void *arg)
     case QEMU_AIO_WRITE_ZEROES:
         ret = handle_aiocb_write_zeroes(aiocb);
         break;
+    case QEMU_AIO_WRITE_ZEROES | QEMU_AIO_DISCARD:
+        ret = handle_aiocb_write_zeroes_unmap(aiocb);
+        break;
     case QEMU_AIO_COPY_RANGE:
         ret = handle_aiocb_copy_range(aiocb);
         break;
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_pwrite_zeroes(
     int bytes, BdrvRequestFlags flags)
 {
     BDRVRawState *s = bs->opaque;
+    int operation = QEMU_AIO_WRITE_ZEROES;
 
-    if (!(flags & BDRV_REQ_MAY_UNMAP)) {
-        return paio_submit_co(bs, s->fd, offset, NULL, bytes,
-                              QEMU_AIO_WRITE_ZEROES);
-    } else if (s->discard_zeroes) {
-        return paio_submit_co(bs, s->fd, offset, NULL, bytes,
-                              QEMU_AIO_DISCARD);
+    if (flags & BDRV_REQ_MAY_UNMAP) {
+        operation |= QEMU_AIO_DISCARD;
     }
-    return -ENOTSUP;
+
+    return paio_submit_co(bs, s->fd, offset, NULL, bytes, operation);
 }
 
 static int raw_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int hdev_co_pwrite_zeroes(BlockDriverState *bs,
     int64_t offset, int bytes, BdrvRequestFlags flags)
 {
     BDRVRawState *s = bs->opaque;
+    int operation = QEMU_AIO_WRITE_ZEROES | QEMU_AIO_BLKDEV;
     int rc;
 
     rc = fd_open(bs);
     if (rc < 0) {
         return rc;
     }
-    if (!(flags & BDRV_REQ_MAY_UNMAP)) {
-        return paio_submit_co(bs, s->fd, offset, NULL, bytes,
-                              QEMU_AIO_WRITE_ZEROES|QEMU_AIO_BLKDEV);
-    } else if (s->discard_zeroes) {
-        return paio_submit_co(bs, s->fd, offset, NULL, bytes,
-                              QEMU_AIO_DISCARD|QEMU_AIO_BLKDEV);
+
+    if (flags & BDRV_REQ_MAY_UNMAP) {
+        operation |= QEMU_AIO_DISCARD;
     }
-    return -ENOTSUP;
+
+    return paio_submit_co(bs, s->fd, offset, NULL, bytes, operation);
 }
 
 static int coroutine_fn hdev_co_create_opts(const char *filename, QemuOpts *opts,
-- 
2.13.6

Like for query-block, the client needs to identify which BlockBackend
the returned data is for. Anonymous BlockBackends are identified by the
device model they are attached to. Add a 'qdev' field that contains the
qdev ID or QOM path of the attached device model.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 qapi/block-core.json | 14 ++++++++++----
 block/qapi.c         | 10 ++++++++++
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index XXXXXXX..XXXXXXX 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -XXX,XX +XXX,XX @@
 #
 # @node-name: The node name of the device. (Since 2.3)
 #
+# @qdev: The qdev ID, or if no ID is assigned, the QOM path of the block
+#        device. (since 3.0)
+#
 # @stats:  A @BlockDeviceStats for the device.
 #
 # @parent: This describes the file block device if it has one.
@@ -XXX,XX +XXX,XX @@
 # Since: 0.14.0
 ##
 { 'struct': 'BlockStats',
-  'data': {'*device': 'str', '*node-name': 'str',
+  'data': {'*device': 'str', '*qdev': 'str', '*node-name': 'str',
            'stats': 'BlockDeviceStats',
            '*parent': 'BlockStats',
            '*backing': 'BlockStats'} }
@@ -XXX,XX +XXX,XX @@
 #                "idle_time_ns":2953431879,
 #                "account_invalid":true,
 #                "account_failed":false
-#             }
+#             },
+#             "qdev": "/machine/unattached/device[23]"
 #          },
 #          {
 #             "device":"ide1-cd0",
@@ -XXX,XX +XXX,XX @@
 #                "wr_merged":0,
 #                "account_invalid":false,
 #                "account_failed":false
-#             }
+#             },
+#             "qdev": "/machine/unattached/device[24]"
 #          },
 #          {
 #             "device":"floppy0",
@@ -XXX,XX +XXX,XX @@
 #                "wr_merged":0,
 #                "account_invalid":false,
 #                "account_failed":false
-#             }
+#             },
+#             "qdev": "/machine/unattached/device[16]"
 #          },
 #          {
 #             "device":"sd0",
diff --git a/block/qapi.c b/block/qapi.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -XXX,XX +XXX,XX @@ BlockStatsList *qmp_query_blockstats(bool has_query_nodes,
             BlockStatsList *info = g_malloc0(sizeof(*info));
             AioContext *ctx = blk_get_aio_context(blk);
             BlockStats *s;
+            char *qdev;
 
             aio_context_acquire(ctx);
             s = bdrv_query_bds_stats(blk_bs(blk), true);
             s->has_device = true;
             s->device = g_strdup(blk_name(blk));
+
+            qdev = blk_get_attached_dev_id(blk);
+            if (qdev && *qdev) {
+                s->has_qdev = true;
+                s->qdev = qdev;
+            } else {
+                g_free(qdev);
+            }
+
             bdrv_query_blk_stats(s->stats, blk);
             aio_context_release(ctx);
 
-- 
2.13.6

Consistent with query-block, query-blockstats should not only include
named BlockBackends, but also those that are anonymous, but belong to a
device model.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block/qapi.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/block/qapi.c b/block/qapi.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -XXX,XX +XXX,XX @@ BlockStatsList *qmp_query_blockstats(bool has_query_nodes,
             p_next = &info->next;
         }
     } else {
-        for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
+        for (blk = blk_all_next(NULL); blk; blk = blk_all_next(blk)) {
             BlockStatsList *info = g_malloc0(sizeof(*info));
             AioContext *ctx = blk_get_aio_context(blk);
             BlockStats *s;
             char *qdev;
 
+            if (!*blk_name(blk) && !blk_get_attached_dev(blk)) {
+                continue;
+            }
+
             aio_context_acquire(ctx);
             s = bdrv_query_bds_stats(blk_bs(blk), true);
             s->has_device = true;
-- 
2.13.6

Make sure that query-blockstats returns information for every
BlockBackend that is named or attached to a device model (or both).

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/227     | 101 ++++++++++++++++++++++
 tests/qemu-iotests/227.out | 205 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 307 insertions(+)
 create mode 100755 tests/qemu-iotests/227
 create mode 100644 tests/qemu-iotests/227.out

diff --git a/tests/qemu-iotests/227 b/tests/qemu-iotests/227
new file mode 100755
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/227
@@ -XXX,XX +XXX,XX @@
+#!/bin/bash
+#
+# Test query-blockstats with different ways to create a BB
+#
+# Copyright (C) 2018 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=kwolf@redhat.com
+
+seq=$(basename $0)
+echo "QA output created by $seq"
+
+here=$PWD
+status=1	# failure is the default!
+
+_cleanup()
+{
+    _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt generic
+_supported_proto file
+_supported_os Linux
+
+function do_run_qemu()
+{
+    echo Testing: "$@"
+    $QEMU -nographic -qmp-pretty stdio -serial none "$@"
+    echo
+}
+
+function run_qemu()
+{
+    do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qmp \
+                          | _filter_qemu | _filter_imgfmt \
+                          | _filter_generated_node_ids
+}
+
+echo
+echo '=== blockstats with -drive if=virtio ==='
+echo
+
+run_qemu -drive driver=null-co,if=virtio <<EOF
+{ "execute": "qmp_capabilities" }
+{ "execute": "query-blockstats"}
+{ "execute": "quit" }
+EOF
+
+echo
+echo '=== blockstats with -drive if=none ==='
+echo
+
+run_qemu -drive driver=null-co,if=none <<EOF
+{ "execute": "qmp_capabilities" }
+{ "execute": "query-blockstats"}
+{ "execute": "quit" }
+EOF
+
+echo
+echo '=== blockstats with -blockdev ==='
+echo
+
+run_qemu -blockdev driver=null-co,node-name=null <<EOF
+{ "execute": "qmp_capabilities" }
+{ "execute": "query-blockstats"}
+{ "execute": "quit" }
+EOF
+
+echo
+echo '=== blockstats with -blockdev and -device ==='
+echo
+
+run_qemu -blockdev driver=null-co,node-name=null -device virtio-blk,drive=null,id=virtio0 <<EOF
+{ "execute": "qmp_capabilities" }
+{ "execute": "query-blockstats"}
+{ "execute": "quit" }
+EOF
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/227.out b/tests/qemu-iotests/227.out
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/227.out
@@ -XXX,XX +XXX,XX @@
+QA output created by 227
+
+=== blockstats with -drive if=virtio ===
+
+Testing: -drive driver=null-co,if=virtio
+{
+    QMP_VERSION
+}
+{
+    "return": {
+    }
+}
+{
+    "return": [
+        {
+            "device": "virtio0",
+            "stats": {
+                "flush_total_time_ns": 0,
+                "wr_highest_offset": 0,
+                "wr_total_time_ns": 0,
+                "failed_wr_operations": 0,
+                "failed_rd_operations": 0,
+                "wr_merged": 0,
+                "wr_bytes": 0,
+                "timed_stats": [
+                ],
+                "failed_flush_operations": 0,
+                "account_invalid": true,
+                "rd_total_time_ns": 0,
+                "flush_operations": 0,
+                "wr_operations": 0,
+                "rd_merged": 0,
+                "rd_bytes": 0,
+                "invalid_flush_operations": 0,
+                "account_failed": true,
+                "rd_operations": 0,
+                "invalid_wr_operations": 0,
+                "invalid_rd_operations": 0
+            },
+            "node-name": "NODE_NAME",
+            "qdev": "/machine/peripheral-anon/device[0]/virtio-backend"
+        }
+    ]
+}
+{
+    "return": {
+    }
+}
+{
+    "timestamp": {
+        "seconds":  TIMESTAMP,
+        "microseconds":  TIMESTAMP
+    },
+    "event": "SHUTDOWN",
+    "data": {
+        "guest": false
+    }
+}
+
+
+=== blockstats with -drive if=none ===
+
+Testing: -drive driver=null-co,if=none
+{
+    QMP_VERSION
+}
+{
+    "return": {
+    }
+}
+{
+    "return": [
+        {
+            "device": "none0",
+            "stats": {
+                "flush_total_time_ns": 0,
+                "wr_highest_offset": 0,
+                "wr_total_time_ns": 0,
+                "failed_wr_operations": 0,
+                "failed_rd_operations": 0,
+                "wr_merged": 0,
+                "wr_bytes": 0,
+                "timed_stats": [
+                ],
+                "failed_flush_operations": 0,
+                "account_invalid": true,
+                "rd_total_time_ns": 0,
+                "flush_operations": 0,
+                "wr_operations": 0,
+                "rd_merged": 0,
+                "rd_bytes": 0,
+                "invalid_flush_operations": 0,
+                "account_failed": true,
+                "rd_operations": 0,
+                "invalid_wr_operations": 0,
+                "invalid_rd_operations": 0
+            },
+            "node-name": "NODE_NAME"
+        }
+    ]
+}
+{
+    "return": {
+    }
+}
+{
+    "timestamp": {
+        "seconds":  TIMESTAMP,
+        "microseconds":  TIMESTAMP
+    },
+    "event": "SHUTDOWN",
+    "data": {
+        "guest": false
+    }
+}
+
+
+=== blockstats with -blockdev ===
+
+Testing: -blockdev driver=null-co,node-name=null
+{
+    QMP_VERSION
+}
+{
+    "return": {
+    }
+}
+{
+    "return": [
+    ]
+}
+{
+    "return": {
+    }
+}
+{
+    "timestamp": {
+        "seconds":  TIMESTAMP,
+        "microseconds":  TIMESTAMP
+    },
+    "event": "SHUTDOWN",
+    "data": {
+        "guest": false
+    }
+}
+
+
+=== blockstats with -blockdev and -device ===
+
+Testing: -blockdev driver=null-co,node-name=null -device virtio-blk,drive=null,id=virtio0
+{
+    QMP_VERSION
+}
+{
+    "return": {
+    }
+}
+{
+    "return": [
+        {
+            "device": "",
+            "stats": {
+                "flush_total_time_ns": 0,
+                "wr_highest_offset": 0,
+                "wr_total_time_ns": 0,
+                "failed_wr_operations": 0,
+                "failed_rd_operations": 0,
+                "wr_merged": 0,
+                "wr_bytes": 0,
+                "timed_stats": [
+                ],
+                "failed_flush_operations": 0,
+                "account_invalid": false,
+                "rd_total_time_ns": 0,
+                "flush_operations": 0,
+                "wr_operations": 0,
+                "rd_merged": 0,
+                "rd_bytes": 0,
+                "invalid_flush_operations": 0,
+                "account_failed": false,
+                "rd_operations": 0,
+                "invalid_wr_operations": 0,
+                "invalid_rd_operations": 0
+            },
+            "node-name": "null",
+            "qdev": "/machine/peripheral/virtio0/virtio-backend"
+        }
+    ]
+}
+{
+    "return": {
+    }
+}
+{
+    "timestamp": {
+        "seconds":  TIMESTAMP,
+        "microseconds":  TIMESTAMP
+    },
+    "event": "SHUTDOWN",
+    "data": {
+        "guest": false
+    }
+}
+
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -XXX,XX +XXX,XX @@
 223 rw auto quick
 225 rw auto quick
 226 auto quick
+227 auto quick
-- 
2.13.6