Series comparison

-[Qemu-devel] [PULL 00/15] Block patches
+[PULL 00/12] Block patches
-The following changes since commit 23919ddfd56135cad3cb468a8f54d5a595f024f4:
+The following changes since commit 171199f56f5f9bdf1e5d670d09ef1351d8f01bae:
-  Merge remote-tracking branch 'remotes/aperard/tags/pull-xen-20190827' into staging (2019-08-27 15:52:36 +0100)
+  Merge remote-tracking branch 'remotes/alistair/tags/pull-riscv-to-apply-20200619-3' into staging (2020-06-22 14:45:25 +0100)
 are available in the Git repository at:
-  https://github.com/XanClic/qemu.git tags/pull-block-2019-08-27
+  https://github.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to bb043c056cffcc2f3ce88bfdaf2e76e455c09e2c:
+for you to fetch changes up to 7838c67f22a81fcf669785cd6c0876438422071a:
-  iotests: Unify cache mode quoting (2019-08-27 19:48:44 +0200)
+  block/nvme: support nested aio_poll() (2020-06-23 15:46:08 +0100)
 ----------------------------------------------------------------
-Block patches:
+Pull request
 - qemu-io now accepts a file to read a write pattern from
 - Ensure that raw files have their first block allocated so we can probe
   the O_DIRECT alignment if necessary
 - Various fixes
 ----------------------------------------------------------------
-Denis Plotnikov (1):
-  qemu-io: add pattern file for write command
-Max Reitz (7):
+Daniele Buono (4):
-  iotests: Fix _filter_img_create()
+  coroutine: support SafeStack in ucontext backend
-  vmdk: Use bdrv_dirname() for relative extent paths
+  coroutine: add check for SafeStack in sigaltstack
-  iotests: Keep testing broken relative extent paths
+  configure: add flags to support SafeStack
-  vmdk: Reject invalid compressed writes
+  check-block: enable iotests with SafeStack
   iotests: Disable broken streamOptimized tests
   iotests: Disable 110 for vmdk.twoGbMaxExtentSparse
   iotests: Disable 126 for flat vmdk subformats
-Nir Soffer (3):
+Stefan Hajnoczi (8):
-  block: posix: Always allocate the first block
+  minikconf: explicitly set encoding to UTF-8
-  iotests: Test allocate_first_block() with O_DIRECT
+  block/nvme: poll queues without q->lock
-  iotests: Unify cache mode quoting
+  block/nvme: drop tautologous assertion
   block/nvme: don't access CQE after moving cq.head
   block/nvme: switch to a NVMeRequest freelist
   block/nvme: clarify that free_req_queue is protected by q->lock
   block/nvme: keep BDRVNVMeState pointer in NVMeQueuePair
   block/nvme: support nested aio_poll()
-Stefan Hajnoczi (1):
+ configure                    |  73 ++++++++++++
-  file-posix: fix request_alignment typo
+ include/qemu/coroutine_int.h |   5 +
+ block/nvme.c                 | 220 +++++++++++++++++++++++++----------
-Thomas Huth (2):
+ util/coroutine-sigaltstack.c |   4 +
-  iotests: Check for enabled drivers before testing them
+ util/coroutine-ucontext.c    |  28 +++++
-  tests/check-block: Skip iotests when sanitizers are enabled
+ block/trace-events           |   2 +-
+ scripts/minikconf.py         |   6 +-
-Vladimir Sementsov-Ogievskiy (1):
+ tests/check-block.sh         |  12 +-
-  block: fix permission update in bdrv_replace_node
+files changed, 284 insertions(+), 66 deletions(-)
  block.c                                       |  5 +-
  block/file-posix.c                            | 53 +++++++++-
  block/vmdk.c                                  | 64 ++++++++----
  qemu-io-cmds.c                                | 99 +++++++++++++++++--
  tests/check-block.sh                          |  5 +
  tests/qemu-iotests/002                        |  1 +
  tests/qemu-iotests/003                        |  1 +
  tests/qemu-iotests/005                        |  3 +-
  tests/qemu-iotests/009                        |  1 +
  tests/qemu-iotests/010                        |  1 +
  tests/qemu-iotests/011                        |  1 +
  tests/qemu-iotests/017                        |  3 +-
  tests/qemu-iotests/018                        |  3 +-
  tests/qemu-iotests/019                        |  3 +-
  tests/qemu-iotests/020                        |  3 +-
  tests/qemu-iotests/026                        |  4 +-
  tests/qemu-iotests/027                        |  1 +
  tests/qemu-iotests/032                        |  1 +
  tests/qemu-iotests/033                        |  1 +
  tests/qemu-iotests/034                        |  3 +-
  tests/qemu-iotests/037                        |  3 +-
  tests/qemu-iotests/039                        |  4 +-
  tests/qemu-iotests/052                        |  2 +-
  tests/qemu-iotests/059                        | 34 ++++++-
  tests/qemu-iotests/059.out                    | 26 +++--
  tests/qemu-iotests/063                        |  3 +-
  tests/qemu-iotests/071                        |  1 +
  tests/qemu-iotests/072                        |  1 +
  tests/qemu-iotests/081                        |  4 +-
  tests/qemu-iotests/091                        |  4 +-
  tests/qemu-iotests/099                        |  1 +
  tests/qemu-iotests/105                        |  3 +-
  tests/qemu-iotests/110                        |  3 +-
  tests/qemu-iotests/120                        |  1 +
  tests/qemu-iotests/126                        |  2 +
  tests/qemu-iotests/{150.out => 150.out.qcow2} |  0
  tests/qemu-iotests/150.out.raw                | 12 +++
  tests/qemu-iotests/162                        |  4 +-
  tests/qemu-iotests/175                        | 47 +++++++--
  tests/qemu-iotests/175.out                    | 16 ++-
  tests/qemu-iotests/178.out.qcow2              |  4 +-
  tests/qemu-iotests/184                        |  1 +
  tests/qemu-iotests/186                        |  1 +
  tests/qemu-iotests/197                        |  1 +
  tests/qemu-iotests/215                        |  1 +
  tests/qemu-iotests/221.out                    | 12 ++-
  tests/qemu-iotests/251                        |  1 +
  tests/qemu-iotests/253.out                    | 12 ++-
  tests/qemu-iotests/common.filter              |  4 +-
  tests/qemu-iotests/common.rc                  | 14 +++
 files changed, 391 insertions(+), 87 deletions(-)
  rename tests/qemu-iotests/{150.out => 150.out.qcow2} (100%)
  create mode 100644 tests/qemu-iotests/150.out.raw
 --
-.21.0
+.26.2

-[Qemu-devel] [PULL 15/15] iotests: Unify cache mode quoting
+[PULL 01/12] minikconf: explicitly set encoding to UTF-8
-From: Nir Soffer <nirsof@gmail.com>
+QEMU currently only has ASCII Kconfig files but Linux actually uses
 UTF-8. Explicitly specify the encoding and that we're doing text file
 I/O.
-Quoting cache mode is not needed, and most tests use unquoted values.
+It's unclear whether or not QEMU will ever need Unicode in its Kconfig
-Unify all test to use the same style.
+files. If we start using the help text then it will become an issue
 sooner or later. Make this change now for consistency with Linux
 Kconfig.
-Message-id: 20190827173432.7656-1-nsoffer@redhat.com
+Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com>
-Signed-off-by: Nir Soffer <nsoffer@redhat.com>
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
 Message-id: 20200521153616.307100-1-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- tests/qemu-iotests/026 | 4 ++--
+ scripts/minikconf.py | 6 +++---
- tests/qemu-iotests/039 | 4 ++--
+file changed, 3 insertions(+), 3 deletions(-)
  tests/qemu-iotests/052 | 2 +-
  tests/qemu-iotests/091 | 4 ++--
 files changed, 7 insertions(+), 7 deletions(-)
-diff --git a/tests/qemu-iotests/026 b/tests/qemu-iotests/026
+diff --git a/scripts/minikconf.py b/scripts/minikconf.py
 index XXXXXXX..XXXXXXX 100755
---- a/tests/qemu-iotests/026
+--- a/scripts/minikconf.py
-+++ b/tests/qemu-iotests/026
++++ b/scripts/minikconf.py
-@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
+@@ -XXX,XX +XXX,XX @@ class KconfigParser:
- # Currently only qcow2 supports rebasing
+         if incl_abs_fname in self.data.previously_included:
- _supported_fmt qcow2
+             return
- _supported_proto file
+         try:
--_default_cache_mode "writethrough"
+-            fp = open(incl_abs_fname, 'r')
--_supported_cache_modes "writethrough" "none"
++            fp = open(incl_abs_fname, 'rt', encoding='utf-8')
-+_default_cache_mode writethrough
+         except IOError as e:
-+_supported_cache_modes writethrough none
+             raise KconfigParserError(self,
- # The refcount table tests expect a certain minimum width for refcount entries
+                                 '%s: %s' % (e.strerror, include))
- # (so that the refcount table actually needs to grow); that minimum is 16 bits,
+@@ -XXX,XX +XXX,XX @@ if __name__ == '__main__':
- # being the default refcount entry width.
+             parser.do_assignment(name, value == 'y')
-diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039
+             external_vars.add(name[7:])
-index XXXXXXX..XXXXXXX 100755
+         else:
---- a/tests/qemu-iotests/039
+-            fp = open(arg, 'r')
-+++ b/tests/qemu-iotests/039
++            fp = open(arg, 'rt', encoding='utf-8')
-@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
+             parser.parse_file(fp)
- _supported_fmt qcow2
+             fp.close()
- _supported_proto file
- _supported_os Linux
+@@ -XXX,XX +XXX,XX @@ if __name__ == '__main__':
--_default_cache_mode "writethrough"
+         if key not in external_vars and config[key]:
--_supported_cache_modes "writethrough"
+             print ('CONFIG_%s=y' % key)
-+_default_cache_mode writethrough
-+_supported_cache_modes writethrough
+-    deps = open(argv[2], 'w')
++    deps = open(argv[2], 'wt', encoding='utf-8')
- size=128M
+     for fname in data.previously_included:
+         print ('%s: %s' % (argv[1], fname), file=deps)
-diff --git a/tests/qemu-iotests/052 b/tests/qemu-iotests/052
+     deps.close()
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/052
 +++ b/tests/qemu-iotests/052
@@ -XXX,XX +XXX,XX @@ _supported_fmt generic
  _supported_proto file
  # Don't do O_DIRECT on tmpfs
 -_supported_cache_modes "writeback" "writethrough" "unsafe"
 +_supported_cache_modes writeback writethrough unsafe
  size=128M
  _make_test_img $size
 diff --git a/tests/qemu-iotests/091 b/tests/qemu-iotests/091
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/091
 +++ b/tests/qemu-iotests/091
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
  _supported_fmt qcow2
  _supported_proto file
  _supported_os Linux
 -_default_cache_mode "none"
 -_supported_cache_modes "writethrough" "none" "writeback"
 +_default_cache_mode none
 +_supported_cache_modes writethrough none writeback
  size=1G
 --
-.21.0
+.26.2

-[Qemu-devel] [PULL 10/15] iotests: Disable 110 for vmdk.twoGbMaxExtentSparse
+[PULL 02/12] coroutine: support SafeStack in ucontext backend
-The error message for the test case where we have a quorum node for
+From: Daniele Buono <dbuono@linux.vnet.ibm.com>
 which no directory name can be generated is different: For
 twoGbMaxExtentSparse, it complains that it cannot open the extent file.
 For other (sub)formats, it just notes that it cannot determine the
 backing file path.  Both are fine, but just disable twoGbMaxExtentSparse
 for simplicity's sake.
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+LLVM's SafeStack instrumentation does not yet support programs that make
-Reviewed-by: John Snow <jsnow@redhat.com>
+use of the APIs in ucontext.h
-Message-id: 20190815153638.4600-7-mreitz@redhat.com
+With the current implementation of coroutine-ucontext, the resulting
-Reviewed-by: John Snow <jsnow@redhat.com>
+binary is incorrect, with different coroutines sharing the same unsafe
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+stack and producing undefined behavior at runtime.
 This fix allocates an additional unsafe stack area for each coroutine,
 and sets the new unsafe stack pointer before calling swapcontext() in
 qemu_coroutine_new.
 This is the only place where the pointer needs to be manually updated,
 since sigsetjmp/siglongjmp are already instrumented by LLVM to properly
 support SafeStack.
 The additional stack is then freed in qemu_coroutine_delete.
 Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
 Message-id: 20200529205122.714-2-dbuono@linux.vnet.ibm.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- tests/qemu-iotests/110 | 3 ++-
+ include/qemu/coroutine_int.h |  5 +++++
-file changed, 2 insertions(+), 1 deletion(-)
+ util/coroutine-ucontext.c    | 28 ++++++++++++++++++++++++++++
 files changed, 33 insertions(+)
-diff --git a/tests/qemu-iotests/110 b/tests/qemu-iotests/110
+diff --git a/include/qemu/coroutine_int.h b/include/qemu/coroutine_int.h
-index XXXXXXX..XXXXXXX 100755
+index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/110
+--- a/include/qemu/coroutine_int.h
-+++ b/tests/qemu-iotests/110
++++ b/include/qemu/coroutine_int.h
-@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
+@@ -XXX,XX +XXX,XX @@
- # Any format supporting backing files
+ #include "qemu/queue.h"
- _supported_fmt qed qcow qcow2 vmdk
+ #include "qemu/coroutine.h"
- _supported_proto file
--_unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat"
++#ifdef CONFIG_SAFESTACK
-+_unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat" \
++/* Pointer to the unsafe stack, defined by the compiler */
-+                     "subformat=twoGbMaxExtentSparse"
++extern __thread void *__safestack_unsafe_stack_ptr;
++#endif
- TEST_IMG_REL=$(basename "$TEST_IMG")
++
  #define COROUTINE_STACK_SIZE (1 << 20)
  typedef enum {
 diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
 index XXXXXXX..XXXXXXX 100644
 --- a/util/coroutine-ucontext.c
 +++ b/util/coroutine-ucontext.c
@@ -XXX,XX +XXX,XX @@ typedef struct {
      Coroutine base;
      void *stack;
      size_t stack_size;
 +#ifdef CONFIG_SAFESTACK
 +    /* Need an unsafe stack for each coroutine */
 +    void *unsafe_stack;
 +    size_t unsafe_stack_size;
 +#endif
      sigjmp_buf env;
      void *tsan_co_fiber;
@@ -XXX,XX +XXX,XX @@ Coroutine *qemu_coroutine_new(void)
      co = g_malloc0(sizeof(*co));
      co->stack_size = COROUTINE_STACK_SIZE;
      co->stack = qemu_alloc_stack(&co->stack_size);
 +#ifdef CONFIG_SAFESTACK
 +    co->unsafe_stack_size = COROUTINE_STACK_SIZE;
 +    co->unsafe_stack = qemu_alloc_stack(&co->unsafe_stack_size);
 +#endif
      co->base.entry_arg = &old_env; /* stash away our jmp_buf */
      uc.uc_link = &old_uc;
@@ -XXX,XX +XXX,XX @@ Coroutine *qemu_coroutine_new(void)
              COROUTINE_YIELD,
              &fake_stack_save,
              co->stack, co->stack_size, co->tsan_co_fiber);
 +
 +#ifdef CONFIG_SAFESTACK
 +        /*
 +         * Before we swap the context, set the new unsafe stack
 +         * The unsafe stack grows just like the normal stack, so start from
 +         * the last usable location of the memory area.
 +         * NOTE: we don't have to re-set the usp afterwards because we are
 +         * coming back to this context through a siglongjmp.
 +         * The compiler already wrapped the corresponding sigsetjmp call with
 +         * code that saves the usp on the (safe) stack before the call, and
 +         * restores it right after (which is where we return with siglongjmp).
 +         */
 +        void *usp = co->unsafe_stack + co->unsafe_stack_size;
 +        __safestack_unsafe_stack_ptr = usp;
 +#endif
 +
          swapcontext(&old_uc, &uc);
      }
@@ -XXX,XX +XXX,XX @@ void qemu_coroutine_delete(Coroutine *co_)
  #endif
      qemu_free_stack(co->stack, co->stack_size);
 +#ifdef CONFIG_SAFESTACK
 +    qemu_free_stack(co->unsafe_stack, co->unsafe_stack_size);
 +#endif
      g_free(co);
  }
 --
-.21.0
+.26.2

-[Qemu-devel] [PULL 13/15] iotests: Check for enabled drivers before testing them
+[PULL 03/12] coroutine: add check for SafeStack in sigaltstack
-From: Thomas Huth <thuth@redhat.com>
+From: Daniele Buono <dbuono@linux.vnet.ibm.com>
-It is possible to enable only a subset of the block drivers with the
+Current implementation of LLVM's SafeStack is not compatible with
-"--block-drv-rw-whitelist" option of the "configure" script. All other
+code that uses an alternate stack created with sigaltstack().
-drivers are marked as unusable (or only included as read-only with the
+Since coroutine-sigaltstack relies on sigaltstack(), it is not
-"--block-drv-ro-whitelist" option). If an iotest is now using such a
+compatible with SafeStack. The resulting binary is incorrect, with
-disabled block driver, it is failing - which is bad, since at least the
+different coroutines sharing the same unsafe stack and producing
-tests in the "auto" group should be able to deal with this situation.
+undefined behavior at runtime.
 Thus let's introduce a "_require_drivers" function that can be used by
 the shell tests to check for the availability of certain drivers first,
 and marks the test as "not run" if one of the drivers is missing.
-This patch mainly targets the test in the "auto" group which should
+In the future LLVM may provide a SafeStack implementation compatible with
-never fail in such a case, but also improves some of the other tests
+sigaltstack(). In the meantime, if SafeStack is desired, the coroutine
-along the way. Note that we also assume that the "qcow2" and "file"
+implementation from coroutine-ucontext should be used.
-drivers are always available - otherwise it does not make sense to
+As a safety check, add a control in coroutine-sigaltstack to throw a
-run "make check-block" at all (which only tests with qcow2 by default).
+preprocessor #error if SafeStack is enabled and we are trying to
 use coroutine-sigaltstack to implement coroutines.
-Signed-off-by: Thomas Huth <thuth@redhat.com>
+Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
-Message-id: 20190823133552.11680-1-thuth@redhat.com
+Message-id: 20200529205122.714-3-dbuono@linux.vnet.ibm.com
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- tests/qemu-iotests/071       |  1 +
+ util/coroutine-sigaltstack.c | 4 ++++
- tests/qemu-iotests/081       |  4 +---
+file changed, 4 insertions(+)
  tests/qemu-iotests/099       |  1 +
  tests/qemu-iotests/120       |  1 +
  tests/qemu-iotests/162       |  4 +---
  tests/qemu-iotests/184       |  1 +
  tests/qemu-iotests/186       |  1 +
  tests/qemu-iotests/common.rc | 14 ++++++++++++++
 files changed, 21 insertions(+), 6 deletions(-)
-diff --git a/tests/qemu-iotests/071 b/tests/qemu-iotests/071
+diff --git a/util/coroutine-sigaltstack.c b/util/coroutine-sigaltstack.c
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/071
 +++ b/tests/qemu-iotests/071
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
  _supported_fmt qcow2
  _supported_proto file
 +_require_drivers blkdebug blkverify
  do_run_qemu()
  {
 diff --git a/tests/qemu-iotests/081 b/tests/qemu-iotests/081
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/081
 +++ b/tests/qemu-iotests/081
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
  _supported_fmt raw
  _supported_proto file
  _supported_os Linux
 +_require_drivers quorum
  do_run_qemu()
  {
@@ -XXX,XX +XXX,XX @@ run_qemu()
                            | _filter_qemu_io | _filter_generated_node_ids
  }
 -test_quorum=$($QEMU_IMG --help|grep quorum)
 -[ "$test_quorum" = "" ] && _supported_fmt quorum
 -
  quorum="driver=raw,file.driver=quorum,file.vote-threshold=2"
  quorum="$quorum,file.children.0.file.filename=$TEST_DIR/1.raw"
  quorum="$quorum,file.children.1.file.filename=$TEST_DIR/2.raw"
 diff --git a/tests/qemu-iotests/099 b/tests/qemu-iotests/099
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/099
 +++ b/tests/qemu-iotests/099
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
  _supported_fmt qcow qcow2 qed vdi vhdx vmdk vpc
  _supported_proto file
  _supported_os Linux
 +_require_drivers blkdebug blkverify
  _unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat" \
      "subformat=twoGbMaxExtentSparse"
 diff --git a/tests/qemu-iotests/120 b/tests/qemu-iotests/120
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/120
 +++ b/tests/qemu-iotests/120
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
  _supported_fmt generic
  _supported_proto file
  _unsupported_fmt luks
 +_require_drivers raw
  _make_test_img 64M
 diff --git a/tests/qemu-iotests/162 b/tests/qemu-iotests/162
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/162
 +++ b/tests/qemu-iotests/162
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
  . ./common.filter
  _supported_fmt generic
 -
 -test_ssh=$($QEMU_IMG --help | grep '^Supported formats:.* ssh\( \|$\)')
 -[ "$test_ssh" = "" ] && _notrun "ssh support required"
 +_require_drivers ssh
  echo
  echo '=== NBD ==='
 diff --git a/tests/qemu-iotests/184 b/tests/qemu-iotests/184
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/184
 +++ b/tests/qemu-iotests/184
@@ -XXX,XX +XXX,XX @@ trap "exit \$status" 0 1 2 3 15
  . ./common.filter
  _supported_os Linux
 +_require_drivers throttle
  do_run_qemu()
  {
 diff --git a/tests/qemu-iotests/186 b/tests/qemu-iotests/186
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/186
 +++ b/tests/qemu-iotests/186
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
  _supported_fmt qcow2
  _supported_proto file
 +_require_drivers null-co
  if [ "$QEMU_DEFAULT_MACHINE" != "pc" ]; then
      _notrun "Requires a PC machine"
 diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
 index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/common.rc
+--- a/util/coroutine-sigaltstack.c
-+++ b/tests/qemu-iotests/common.rc
++++ b/util/coroutine-sigaltstack.c
-@@ -XXX,XX +XXX,XX @@ _require_command()
+@@ -XXX,XX +XXX,XX @@
-     [ -x "$c" ] || _notrun "$1 utility required, skipped this test"
+ #include "qemu-common.h"
- }
+ #include "qemu/coroutine_int.h"
-+# Check that a set of drivers has been whitelisted in the QEMU binary
++#ifdef CONFIG_SAFESTACK
-+#
++#error "SafeStack is not compatible with code run in alternate signal stacks"
-+_require_drivers()
++#endif
 +{
 +    available=$($QEMU -drive format=help | \
 +                sed -e '/Supported formats:/!d' -e 's/Supported formats://')
 +    for driver
 +    do
 +        if ! echo "$available" | grep -q " $driver\( \|$\)"; then
 +            _notrun "$driver not available"
 +        fi
 +    done
 +}
 +
- # make sure this script returns success
+ typedef struct {
- true
+     Coroutine base;
      void *stack;
 --
-.21.0
+.26.2

-[Qemu-devel] [PULL 07/15] iotests: Keep testing broken relative extent paths
+[PULL 04/12] configure: add flags to support SafeStack
-We had a test for a case where relative extent paths did not work, but
+From: Daniele Buono <dbuono@linux.vnet.ibm.com>
 unfortunately we just fixed the underlying problem, so it works now.
 This patch adds a new test case that still fails.
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+This patch adds a flag to enable/disable the SafeStack instrumentation
-Reviewed-by: John Snow <jsnow@redhat.com>
+provided by LLVM.
-Message-id: 20190815153638.4600-4-mreitz@redhat.com
-Reviewed-by: John Snow <jsnow@redhat.com>
+On enable, make sure that the compiler supports the flags, and that we
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+are using the proper coroutine implementation (coroutine-ucontext).
 On disable, explicitly disable the option if it was enabled by default.
 While SafeStack is supported only on Linux, NetBSD, FreeBSD and macOS,
 we are not checking for the O.S. since this is already done by LLVM.
 Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
 Message-id: 20200529205122.714-4-dbuono@linux.vnet.ibm.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- tests/qemu-iotests/059     | 27 +++++++++++++++++++++++++++
+ configure | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
- tests/qemu-iotests/059.out |  4 ++++
+file changed, 73 insertions(+)
 files changed, 31 insertions(+)
-diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
+diff --git a/configure b/configure
 index XXXXXXX..XXXXXXX 100755
---- a/tests/qemu-iotests/059
+--- a/configure
-+++ b/tests/qemu-iotests/059
++++ b/configure
-@@ -XXX,XX +XXX,XX @@ $QEMU_IMG convert -f qcow2 -O vmdk -o subformat=streamOptimized "$TEST_IMG.qcow2
+@@ -XXX,XX +XXX,XX @@ audio_win_int=""
+ libs_qga=""
- echo
+ debug_info="yes"
- echo "=== Testing monolithicFlat with internally generated JSON file name ==="
+ stack_protector=""
-+
++safe_stack=""
-+echo '--- blkdebug ---'
+ use_containers="yes"
- # Should work, because bdrv_dirname() works fine with blkdebug
+ gdb_bin=$(command -v "gdb-multiarch" || command -v "gdb")
- IMGOPTS="subformat=monolithicFlat" _make_test_img 64M
- $QEMU_IO -c "open -o driver=$IMGFMT,file.driver=blkdebug,file.image.filename=$TEST_IMG,file.inject-error.0.event=read_aio" \
+@@ -XXX,XX +XXX,XX @@ for opt do
-@@ -XXX,XX +XXX,XX @@ $QEMU_IO -c "open -o driver=$IMGFMT,file.driver=blkdebug,file.image.filename=$TE
+   ;;
-     | _filter_testdir | _filter_imgfmt | _filter_img_info
+   --disable-stack-protector) stack_protector="no"
- _cleanup_test_img
+   ;;
++  --enable-safe-stack) safe_stack="yes"
-+echo '--- quorum ---'
++  ;;
-+# Should not work, because bdrv_dirname() does not work with quorum
++  --disable-safe-stack) safe_stack="no"
-+IMGOPTS="subformat=monolithicFlat" _make_test_img 64M
++  ;;
-+cp "$TEST_IMG" "$TEST_IMG.orig"
+   --disable-curses) curses="no"
-+
+   ;;
-+filename="json:{
+   --enable-curses) curses="yes"
-+    \"driver\": \"$IMGFMT\",
+@@ -XXX,XX +XXX,XX @@ disabled with --disable-FEATURE, default is enabled if available:
-+    \"file\": {
+   debug-tcg       TCG debugging (default is disabled)
-+        \"driver\": \"quorum\",
+   debug-info      debugging information
-+        \"children\": [ {
+   sparse          sparse checker
-+            \"driver\": \"file\",
++  safe-stack      SafeStack Stack Smash Protection. Depends on
-+            \"filename\": \"$TEST_IMG\"
++                  clang/llvm >= 3.7 and requires coroutine backend ucontext.
-+        }, {
-+            \"driver\": \"file\",
+   gnutls          GNUTLS cryptography support
-+            \"filename\": \"$TEST_IMG.orig\"
+   nettle          nettle cryptography support
-+        } ],
+@@ -XXX,XX +XXX,XX @@ if test "$debug_stack_usage" = "yes"; then
-+        \"vote-threshold\": 1
+   fi
-+    } }"
+ fi
-+
-+filename=$(echo "$filename" | tr '\n' ' ' | sed -e 's/\s\+/ /g')
++##################################################
-+$QEMU_IMG info "$filename" 2>&1 \
++# SafeStack
 +    | sed -e "s/'json:[^']*'/\$QUORUM_FILE/g" \
 +    | _filter_testdir | _filter_imgfmt | _filter_img_info
 +
 +
- echo
++if test "$safe_stack" = "yes"; then
- echo "=== Testing version 3 ==="
++cat > $TMPC << EOF
- _use_sample_img iotest-version3.vmdk.bz2
++int main(int argc, char *argv[])
-diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
++{
-index XXXXXXX..XXXXXXX 100644
++#if ! __has_feature(safe_stack)
---- a/tests/qemu-iotests/059.out
++#error SafeStack Disabled
-+++ b/tests/qemu-iotests/059.out
++#endif
-@@ -XXX,XX +XXX,XX @@ wrote 512/512 bytes at offset 10240
++    return 0;
-bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
++}
++EOF
- === Testing monolithicFlat with internally generated JSON file name ===
++  flag="-fsanitize=safe-stack"
-+--- blkdebug ---
++  # Check that safe-stack is supported and enabled.
- Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
++  if compile_prog "-Werror $flag" "$flag"; then
- format name: IMGFMT
++    # Flag needed both at compilation and at linking
- cluster size: 0 bytes
++    QEMU_CFLAGS="$QEMU_CFLAGS $flag"
- vm state offset: 0 bytes
++    QEMU_LDFLAGS="$QEMU_LDFLAGS $flag"
-+--- quorum ---
++  else
-+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
++    error_exit "SafeStack not supported by your compiler"
-+qemu-img: Could not open $QUORUM_FILE: Cannot use relative paths with VMDK descriptor file $QUORUM_FILE: Cannot generate a base directory for quorum nodes
++  fi
++  if test "$coroutine" != "ucontext"; then
- === Testing version 3 ===
++    error_exit "SafeStack is only supported by the coroutine backend ucontext"
- image: TEST_DIR/iotest-version3.IMGFMT
++  fi
 +else
 +cat > $TMPC << EOF
 +int main(int argc, char *argv[])
 +{
 +#if defined(__has_feature)
 +#if __has_feature(safe_stack)
 +#error SafeStack Enabled
 +#endif
 +#endif
 +    return 0;
 +}
 +EOF
 +if test "$safe_stack" = "no"; then
 +  # Make sure that safe-stack is disabled
 +  if ! compile_prog "-Werror" ""; then
 +    # SafeStack was already enabled, try to explicitly remove the feature
 +    flag="-fno-sanitize=safe-stack"
 +    if ! compile_prog "-Werror $flag" "$flag"; then
 +      error_exit "Configure cannot disable SafeStack"
 +    fi
 +    QEMU_CFLAGS="$QEMU_CFLAGS $flag"
 +    QEMU_LDFLAGS="$QEMU_LDFLAGS $flag"
 +  fi
 +else # "$safe_stack" = ""
 +  # Set safe_stack to yes or no based on pre-existing flags
 +  if compile_prog "-Werror" ""; then
 +    safe_stack="no"
 +  else
 +    safe_stack="yes"
 +    if test "$coroutine" != "ucontext"; then
 +      error_exit "SafeStack is only supported by the coroutine backend ucontext"
 +    fi
 +  fi
 +fi
 +fi
  ##########################################
  # check if we have open_by_handle_at
@@ -XXX,XX +XXX,XX @@ echo "sparse enabled    $sparse"
  echo "strip binaries    $strip_opt"
  echo "profiler          $profiler"
  echo "static build      $static"
 +echo "safe stack        $safe_stack"
  if test "$darwin" = "yes" ; then
      echo "Cocoa support     $cocoa"
  fi
@@ -XXX,XX +XXX,XX @@ if test "$ccache_cpp2" = "yes"; then
    echo "export CCACHE_CPP2=y" >> $config_host_mak
  fi
 +if test "$safe_stack" = "yes"; then
 +  echo "CONFIG_SAFESTACK=y" >> $config_host_mak
 +fi
 +
  # If we're using a separate build tree, set it up now.
  # DIRS are directories which we simply mkdir in the build tree;
  # LINKS are things to symlink back into the source tree
 --
-.21.0
+.26.2

-[Qemu-devel] [PULL 14/15] tests/check-block: Skip iotests when sanitizers are enabled
+[PULL 05/12] check-block: enable iotests with SafeStack
-From: Thomas Huth <thuth@redhat.com>
+From: Daniele Buono <dbuono@linux.vnet.ibm.com>
-The sanitizers (especially the address sanitizer from Clang) are
+SafeStack is a stack protection technique implemented in llvm. It is
-sometimes printing out warnings or false positives - this spoils
+enabled with a -fsanitize flag.
-the output of the iotests, causing some of the tests to fail.
+iotests are currently disabled when any -fsanitize option is used,
-Thus let's skip the automatic iotests during "make check" when the
+because such options tend to produce additional warnings and false
-user configured QEMU with --enable-sanitizers.
+positives.
-Signed-off-by: Thomas Huth <thuth@redhat.com>
+While common -fsanitize options are used to verify the code and not
-Message-id: 20190823084203.29734-1-thuth@redhat.com
+added in production, SafeStack's main use is in production environments
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+to protect against stack smashing.
 Since SafeStack does not print any warning or false positive, enable
 iotests when SafeStack is the only -fsanitize option used.
 This is likely going to be a production binary and we want to make sure
 it works correctly.
 Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
 Message-id: 20200529205122.714-5-dbuono@linux.vnet.ibm.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- tests/check-block.sh | 5 +++++
+ tests/check-block.sh | 12 +++++++++++-
-file changed, 5 insertions(+)
+file changed, 11 insertions(+), 1 deletion(-)
 diff --git a/tests/check-block.sh b/tests/check-block.sh
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/check-block.sh
 +++ b/tests/check-block.sh
-@@ -XXX,XX +XXX,XX @@ if grep -q "TARGET_GPROF=y" *-softmmu/config-target.mak 2>/dev/null ; then
+@@ -XXX,XX +XXX,XX @@ if grep -q "CONFIG_GPROF=y" config-host.mak 2>/dev/null ; then
      exit 0
  fi
-+if grep -q "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null ; then
+-if grep -q "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null ; then
-+    echo "Sanitizers are enabled ==> Not running the qemu-iotests."
++# Disable tests with any sanitizer except for SafeStack
-+    exit 0
++CFLAGS=$( grep "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null )
-+fi
++SANITIZE_FLAGS=""
-+
++#Remove all occurrencies of -fsanitize=safe-stack
- if [ -z "$(find . -name 'qemu-system-*' -print)" ]; then
++for i in ${CFLAGS}; do
-     echo "No qemu-system binary available ==> Not running the qemu-iotests."
++        if [ "${i}" != "-fsanitize=safe-stack" ]; then
 +                SANITIZE_FLAGS="${SANITIZE_FLAGS} ${i}"
 +        fi
 +done
 +if echo ${SANITIZE_FLAGS} | grep -q "\-fsanitize" 2>/dev/null; then
 +    # Have a sanitize flag that is not allowed, stop
      echo "Sanitizers are enabled ==> Not running the qemu-iotests."
      exit 0
+ fi
 --
-.21.0
+.26.2

-[Qemu-devel] [PULL 08/15] vmdk: Reject invalid compressed writes
+[PULL 06/12] block/nvme: poll queues without q->lock
-Compressed writes generally have to write full clusters, not just in
+A lot of CPU time is spent simply locking/unlocking q->lock during
-theory but also in practice when it comes to vmdk's streamOptimized
+polling. Check for completion outside the lock to make q->lock disappear
-subformat.  It currently is just silently broken for writes with
+from the profile.
 non-zero in-cluster offsets:
-$ qemu-img create -f vmdk -o subformat=streamOptimized foo.vmdk 1M
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-$ qemu-io -c 'write 4k 4k' -c 'read 4k 4k' foo.vmdk
+Reviewed-by: Sergio Lopez <slp@redhat.com>
-wrote 4096/4096 bytes at offset 4096
+Message-id: 20200617132201.1832152-2-stefanha@redhat.com
-KiB, 1 ops; 00.01 sec (443.724 KiB/sec and 110.9309 ops/sec)
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-read failed: Invalid argument
+---
  block/nvme.c | 12 ++++++++++++
 file changed, 12 insertions(+)
-(The technical reason is that vmdk_write_extent() just writes the
+diff --git a/block/nvme.c b/block/nvme.c
 incomplete compressed data actually to offset 4k.  When reading the
 data, vmdk_read_extent() looks at offset 0 and finds the compressed data
 size to be 0, because that is what it reads from there.  This yields an
 error.)
 For incomplete writes with zero in-cluster offsets, the error path when
 reading the rest of the cluster is a bit different, but the result is
 the same:
 $ qemu-img create -f vmdk -o subformat=streamOptimized foo.vmdk 1M
 $ qemu-io -c 'write 0k 4k' -c 'read 4k 4k' foo.vmdk
 wrote 4096/4096 bytes at offset 0
 KiB, 1 ops; 00.01 sec (362.641 KiB/sec and 90.6603 ops/sec)
 read failed: Invalid argument
 (Here, vmdk_read_extent() finds the data and then sees that the
 uncompressed data is short.)
 It is better to reject invalid writes than to make the user believe they
 might have succeeded and then fail when trying to read it back.
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 Reviewed-by: John Snow <jsnow@redhat.com>
 Message-id: 20190815153638.4600-5-mreitz@redhat.com
 Reviewed-by: John Snow <jsnow@redhat.com>
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  block/vmdk.c | 10 ++++++++++
 file changed, 10 insertions(+)
 diff --git a/block/vmdk.c b/block/vmdk.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/vmdk.c
+--- a/block/nvme.c
-+++ b/block/vmdk.c
++++ b/block/nvme.c
-@@ -XXX,XX +XXX,XX @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset,
+@@ -XXX,XX +XXX,XX @@ static bool nvme_poll_queues(BDRVNVMeState *s)
-     if (extent->compressed) {
-         void *compressed_data;
+     for (i = 0; i < s->nr_queues; i++) {
+         NVMeQueuePair *q = s->queues[i];
-+        /* Only whole clusters */
++        const size_t cqe_offset = q->cq.head * NVME_CQ_ENTRY_BYTES;
-+        if (offset_in_cluster ||
++        NvmeCqe *cqe = (NvmeCqe *)&q->cq.queue[cqe_offset];
-+            n_bytes > (extent->cluster_sectors * SECTOR_SIZE) ||
++
-+            (n_bytes < (extent->cluster_sectors * SECTOR_SIZE) &&
++        /*
-+             offset + n_bytes != extent->end_sector * SECTOR_SIZE))
++         * Do an early check for completions. q->lock isn't needed because
-+        {
++         * nvme_process_completion() only runs in the event loop thread and
-+            ret = -EINVAL;
++         * cannot race with itself.
-+            goto out;
++         */
 +        if ((le16_to_cpu(cqe->status) & 0x1) == q->cq_phase) {
 +            continue;
 +        }
 +
-         if (!extent->has_marker) {
+         qemu_mutex_lock(&q->lock);
-             ret = -EINVAL;
+         while (nvme_process_completion(s, q)) {
-             goto out;
+             /* Keep polling */
 --
-.21.0
+.26.2

-[Qemu-devel] [PULL 11/15] iotests: Disable 126 for flat vmdk subformats
+[PULL 07/12] block/nvme: drop tautologous assertion
-iotest 126 requires backing file support, which flat vmdks cannot offer.
+nvme_process_completion() explicitly checks cid so the assertion that
-Skip this test for such subformats.
+follows is always true:
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+  if (cid == 0 || cid > NVME_QUEUE_SIZE) {
-Message-id: 20190815153638.4600-8-mreitz@redhat.com
+      ...
-Reviewed-by: John Snow <jsnow@redhat.com>
+      continue;
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+  }
   assert(cid <= NVME_QUEUE_SIZE);
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 Reviewed-by: Sergio Lopez <slp@redhat.com>
 Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
 Message-id: 20200617132201.1832152-3-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- tests/qemu-iotests/126 | 2 ++
+ block/nvme.c | 1 -
-file changed, 2 insertions(+)
+file changed, 1 deletion(-)
-diff --git a/tests/qemu-iotests/126 b/tests/qemu-iotests/126
+diff --git a/block/nvme.c b/block/nvme.c
-index XXXXXXX..XXXXXXX 100755
+index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/126
+--- a/block/nvme.c
-+++ b/tests/qemu-iotests/126
++++ b/block/nvme.c
-@@ -XXX,XX +XXX,XX @@ status=1    # failure is the default!
+@@ -XXX,XX +XXX,XX @@ static bool nvme_process_completion(BDRVNVMeState *s, NVMeQueuePair *q)
+                     cid);
- # Needs backing file support
+             continue;
- _supported_fmt qcow qcow2 qed vmdk
+         }
-+_unsupported_imgopts "subformat=monolithicFlat" \
+-        assert(cid <= NVME_QUEUE_SIZE);
-+                     "subformat=twoGbMaxExtentFlat"
+         trace_nvme_complete_command(s, q->index, cid);
- # This is the default protocol (and we want to test the difference between
+         preq = &q->reqs[cid - 1];
- # colons which separate a protocol prefix from the rest and colons which are
+         req = *preq;
  # just part of the filename, so we cannot test protocols which require a prefix)
 --
-.21.0
+.26.2

-[Qemu-devel] [PULL 09/15] iotests: Disable broken streamOptimized tests
+[PULL 08/12] block/nvme: don't access CQE after moving cq.head
-streamOptimized does not support writes that do not span exactly one
+Do not access a CQE after incrementing q->cq.head and releasing q->lock.
-cluster.  Furthermore, it cannot rewrite already allocated clusters.
+It is unlikely that this causes problems in practice but it's a latent
-As such, many iotests do not work with it.  Disable them.
+bug.
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+The reason why it should be safe at the moment is that completion
-Message-id: 20190815153638.4600-6-mreitz@redhat.com
+processing is not re-entrant and the CQ doorbell isn't written until the
-Reviewed-by: John Snow <jsnow@redhat.com>
+end of nvme_process_completion().
-Signed-off-by: Max Reitz <mreitz@redhat.com>
 Make this change now because QEMU expects completion processing to be
 re-entrant and later patches will do that.
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 Reviewed-by: Sergio Lopez <slp@redhat.com>
 Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
 Message-id: 20200617132201.1832152-4-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- tests/qemu-iotests/002 | 1 +
+ block/nvme.c | 5 ++++-
- tests/qemu-iotests/003 | 1 +
+file changed, 4 insertions(+), 1 deletion(-)
  tests/qemu-iotests/005 | 3 ++-
  tests/qemu-iotests/009 | 1 +
  tests/qemu-iotests/010 | 1 +
  tests/qemu-iotests/011 | 1 +
  tests/qemu-iotests/017 | 3 ++-
  tests/qemu-iotests/018 | 3 ++-
  tests/qemu-iotests/019 | 3 ++-
  tests/qemu-iotests/020 | 3 ++-
  tests/qemu-iotests/027 | 1 +
  tests/qemu-iotests/032 | 1 +
  tests/qemu-iotests/033 | 1 +
  tests/qemu-iotests/034 | 3 ++-
  tests/qemu-iotests/037 | 3 ++-
  tests/qemu-iotests/063 | 3 ++-
  tests/qemu-iotests/072 | 1 +
  tests/qemu-iotests/105 | 3 ++-
  tests/qemu-iotests/197 | 1 +
  tests/qemu-iotests/215 | 1 +
  tests/qemu-iotests/251 | 1 +
 files changed, 30 insertions(+), 9 deletions(-)
-diff --git a/tests/qemu-iotests/002 b/tests/qemu-iotests/002
+diff --git a/block/nvme.c b/block/nvme.c
-index XXXXXXX..XXXXXXX 100755
+index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/002
+--- a/block/nvme.c
-+++ b/tests/qemu-iotests/002
++++ b/block/nvme.c
-@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
+@@ -XXX,XX +XXX,XX @@ static bool nvme_process_completion(BDRVNVMeState *s, NVMeQueuePair *q)
+     q->busy = true;
- _supported_fmt generic
+     assert(q->inflight >= 0);
- _supported_proto generic
+     while (q->inflight) {
-+_unsupported_imgopts "subformat=streamOptimized"
++        int ret;
+         int16_t cid;
++
- size=128M
+         c = (NvmeCqe *)&q->cq.queue[q->cq.head * NVME_CQ_ENTRY_BYTES];
-diff --git a/tests/qemu-iotests/003 b/tests/qemu-iotests/003
+         if ((le16_to_cpu(c->status) & 0x1) == q->cq_phase) {
-index XXXXXXX..XXXXXXX 100755
+             break;
---- a/tests/qemu-iotests/003
+         }
-+++ b/tests/qemu-iotests/003
++        ret = nvme_translate_error(c);
-@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
+         q->cq.head = (q->cq.head + 1) % NVME_QUEUE_SIZE;
+         if (!q->cq.head) {
- _supported_fmt generic
+             q->cq_phase = !q->cq_phase;
- _supported_proto generic
+@@ -XXX,XX +XXX,XX @@ static bool nvme_process_completion(BDRVNVMeState *s, NVMeQueuePair *q)
-+_unsupported_imgopts "subformat=streamOptimized"
+         preq->busy = false;
+         preq->cb = preq->opaque = NULL;
- size=128M
+         qemu_mutex_unlock(&q->lock);
- offset=67M
+-        req.cb(req.opaque, nvme_translate_error(c));
-diff --git a/tests/qemu-iotests/005 b/tests/qemu-iotests/005
++        req.cb(req.opaque, ret);
-index XXXXXXX..XXXXXXX 100755
+         qemu_mutex_lock(&q->lock);
---- a/tests/qemu-iotests/005
+         q->inflight--;
-+++ b/tests/qemu-iotests/005
+         progress = true;
@@ -XXX,XX +XXX,XX @@ _supported_fmt generic
  _supported_proto generic
  _supported_os Linux
  _unsupported_imgopts "subformat=twoGbMaxExtentFlat" \
 -                     "subformat=twoGbMaxExtentSparse"
 +                     "subformat=twoGbMaxExtentSparse" \
 +                     "subformat=streamOptimized"
  # vpc is limited to 127GB, so we can't test it here
  if [ "$IMGFMT" = "vpc" ]; then
 diff --git a/tests/qemu-iotests/009 b/tests/qemu-iotests/009
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/009
 +++ b/tests/qemu-iotests/009
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
  _supported_fmt generic
  _supported_proto generic
 +_unsupported_imgopts "subformat=streamOptimized"
  size=6G
 diff --git a/tests/qemu-iotests/010 b/tests/qemu-iotests/010
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/010
 +++ b/tests/qemu-iotests/010
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
  _supported_fmt generic
  _supported_proto generic
 +_unsupported_imgopts "subformat=streamOptimized"
  size=6G
 diff --git a/tests/qemu-iotests/011 b/tests/qemu-iotests/011
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/011
 +++ b/tests/qemu-iotests/011
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
  _supported_fmt generic
  _supported_proto generic
 +_unsupported_imgopts "subformat=streamOptimized"
  size=6G
 diff --git a/tests/qemu-iotests/017 b/tests/qemu-iotests/017
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/017
 +++ b/tests/qemu-iotests/017
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
  _supported_fmt qcow qcow2 vmdk qed
  _supported_proto generic
  _unsupported_proto vxhs
 -_unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat"
 +_unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat" \
 +                     "subformat=streamOptimized"
  TEST_OFFSETS="0 4294967296"
 diff --git a/tests/qemu-iotests/018 b/tests/qemu-iotests/018
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/018
 +++ b/tests/qemu-iotests/018
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
  _supported_fmt qcow qcow2 vmdk qed
  _supported_proto file
  _supported_os Linux
 -_unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat"
 +_unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat" \
 +                     "streamOptimized"
  TEST_OFFSETS="0 4294967296"
 diff --git a/tests/qemu-iotests/019 b/tests/qemu-iotests/019
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/019
 +++ b/tests/qemu-iotests/019
@@ -XXX,XX +XXX,XX @@ _supported_proto file
  _supported_os Linux
  _unsupported_imgopts "subformat=monolithicFlat" \
                       "subformat=twoGbMaxExtentFlat" \
 -                     "subformat=twoGbMaxExtentSparse"
 +                     "subformat=twoGbMaxExtentSparse" \
 +                     "subformat=streamOptimized"
  TEST_OFFSETS="0 4294967296"
  CLUSTER_SIZE=65536
 diff --git a/tests/qemu-iotests/020 b/tests/qemu-iotests/020
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/020
 +++ b/tests/qemu-iotests/020
@@ -XXX,XX +XXX,XX @@ _supported_fmt qcow qcow2 vmdk qed
  _supported_proto file
  _unsupported_imgopts "subformat=monolithicFlat" \
                       "subformat=twoGbMaxExtentFlat" \
 -                     "subformat=twoGbMaxExtentSparse"
 +                     "subformat=twoGbMaxExtentSparse" \
 +                     "subformat=streamOptimized"
  TEST_OFFSETS="0 4294967296"
 diff --git a/tests/qemu-iotests/027 b/tests/qemu-iotests/027
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/027
 +++ b/tests/qemu-iotests/027
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
  _supported_fmt vmdk qcow qcow2 qed
  _supported_proto generic
 +_unsupported_imgopts "subformat=streamOptimized"
  size=128M
 diff --git a/tests/qemu-iotests/032 b/tests/qemu-iotests/032
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/032
 +++ b/tests/qemu-iotests/032
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
  # This works for any image format (though unlikely to segfault for raw)
  _supported_fmt generic
  _supported_proto generic
 +_unsupported_imgopts "subformat=streamOptimized"
  echo
  echo === Prepare image ===
 diff --git a/tests/qemu-iotests/033 b/tests/qemu-iotests/033
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/033
 +++ b/tests/qemu-iotests/033
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
  _supported_fmt generic
  _supported_proto generic
 +_unsupported_imgopts "subformat=streamOptimized"
  size=128M
 diff --git a/tests/qemu-iotests/034 b/tests/qemu-iotests/034
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/034
 +++ b/tests/qemu-iotests/034
@@ -XXX,XX +XXX,XX @@ _supported_proto file
  _supported_os Linux
  _unsupported_imgopts "subformat=monolithicFlat" \
                       "subformat=twoGbMaxExtentFlat" \
 -                     "subformat=twoGbMaxExtentSparse"
 +                     "subformat=twoGbMaxExtentSparse" \
 +                     "subformat=streamOptimized"
  CLUSTER_SIZE=4k
  size=128M
 diff --git a/tests/qemu-iotests/037 b/tests/qemu-iotests/037
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/037
 +++ b/tests/qemu-iotests/037
@@ -XXX,XX +XXX,XX @@ _supported_fmt qcow qcow2 vmdk qed
  _supported_proto file
  _unsupported_imgopts "subformat=monolithicFlat" \
                       "subformat=twoGbMaxExtentFlat" \
 -                     "subformat=twoGbMaxExtentSparse"
 +                     "subformat=twoGbMaxExtentSparse" \
 +                     "subformat=streamOptimized"
  CLUSTER_SIZE=4k
  size=128M
 diff --git a/tests/qemu-iotests/063 b/tests/qemu-iotests/063
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/063
 +++ b/tests/qemu-iotests/063
@@ -XXX,XX +XXX,XX @@ _supported_fmt qcow qcow2 vmdk qed raw
  _supported_proto file
  _unsupported_imgopts "subformat=monolithicFlat" \
                       "subformat=twoGbMaxExtentFlat" \
 -                     "subformat=twoGbMaxExtentSparse"
 +                     "subformat=twoGbMaxExtentSparse" \
 +                     "subformat=streamOptimized"
  _make_test_img 4M
 diff --git a/tests/qemu-iotests/072 b/tests/qemu-iotests/072
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/072
 +++ b/tests/qemu-iotests/072
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
  _supported_fmt vpc vmdk vhdx vdi qed qcow2 qcow
  _supported_proto file
 +_unsupported_imgopts "subformat=streamOptimized"
  IMG_SIZE=64M
 diff --git a/tests/qemu-iotests/105 b/tests/qemu-iotests/105
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/105
 +++ b/tests/qemu-iotests/105
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
  _supported_fmt qcow2 vmdk vhdx qed
  _supported_proto generic
  _unsupported_imgopts "subformat=twoGbMaxExtentFlat" \
 -                     "subformat=twoGbMaxExtentSparse"
 +                     "subformat=twoGbMaxExtentSparse" \
 +                     "subformat=streamOptimized"
  echo
  echo "creating large image"
 diff --git a/tests/qemu-iotests/197 b/tests/qemu-iotests/197
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/197
 +++ b/tests/qemu-iotests/197
@@ -XXX,XX +XXX,XX @@ _supported_fmt generic
  _supported_proto generic
  # LUKS support may be possible, but it complicates things.
  _unsupported_fmt luks
 +_unsupported_imgopts "subformat=streamOptimized"
  echo
  echo '=== Copy-on-read ==='
 diff --git a/tests/qemu-iotests/215 b/tests/qemu-iotests/215
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/215
 +++ b/tests/qemu-iotests/215
@@ -XXX,XX +XXX,XX @@ _supported_fmt generic
  _supported_proto generic
  # LUKS support may be possible, but it complicates things.
  _unsupported_fmt luks
 +_unsupported_imgopts "subformat=streamOptimized"
  echo
  echo '=== Copy-on-read ==='
 diff --git a/tests/qemu-iotests/251 b/tests/qemu-iotests/251
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/251
 +++ b/tests/qemu-iotests/251
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
  _supported_fmt generic
  _supported_proto file
  _supported_os Linux
 +_unsupported_imgopts "subformat=streamOptimized"
  if [ "$IMGOPTSSYNTAX" = "true" ]; then
      # We use json:{} filenames here, so we cannot work with additional options.
 --
-.21.0
+.26.2

-[Qemu-devel] [PULL 01/15] qemu-io: add pattern file for write command
+[PULL 09/12] block/nvme: switch to a NVMeRequest freelist
-From: Denis Plotnikov <dplotnikov@virtuozzo.com>
+There are three issues with the current NVMeRequest->busy field:
 . The busy field is accidentally accessed outside q->lock when request
    submission fails.
 . Waiters on free_req_queue are not woken when a request is returned
    early due to submission failure.
 . Finding a free request involves scanning all requests. This makes
    request submission O(n^2).
-The patch allows to provide a pattern file for write
+Switch to an O(1) freelist that is always accessed under the lock.
 command. There was no similar ability before.
-Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
+Also differentiate between NVME_QUEUE_SIZE, the actual SQ/CQ size, and
-Message-id: 20190820164616.4072-1-dplotnikov@virtuozzo.com
+NVME_NUM_REQS, the number of usable requests. This makes the code
-Reviewed-by: Eric Blake <eblake@redhat.com>
+simpler than using NVME_QUEUE_SIZE everywhere and having to keep in mind
-[mreitz: Keep optstring in alphabetical order]
+that one slot is reserved.
-Signed-off-by: Max Reitz <mreitz@redhat.com>
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 Reviewed-by: Sergio Lopez <slp@redhat.com>
 Message-id: 20200617132201.1832152-5-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- qemu-io-cmds.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++---
+ block/nvme.c | 81 ++++++++++++++++++++++++++++++++++------------------
-file changed, 93 insertions(+), 6 deletions(-)
+file changed, 54 insertions(+), 27 deletions(-)
-diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
+diff --git a/block/nvme.c b/block/nvme.c
 index XXXXXXX..XXXXXXX 100644
---- a/qemu-io-cmds.c
+--- a/block/nvme.c
-+++ b/qemu-io-cmds.c
++++ b/block/nvme.c
-@@ -XXX,XX +XXX,XX @@ static void qemu_io_free(void *p)
+@@ -XXX,XX +XXX,XX @@
-     qemu_vfree(p);
+ #define NVME_QUEUE_SIZE 128
  #define NVME_BAR_SIZE 8192
 +/*
 + * We have to leave one slot empty as that is the full queue case where
 + * head == tail + 1.
 + */
 +#define NVME_NUM_REQS (NVME_QUEUE_SIZE - 1)
 +
  typedef struct {
      int32_t  head, tail;
      uint8_t  *queue;
@@ -XXX,XX +XXX,XX @@ typedef struct {
      int cid;
      void *prp_list_page;
      uint64_t prp_list_iova;
 -    bool busy;
 +    int free_req_next; /* q->reqs[] index of next free req */
  } NVMeRequest;
  typedef struct {
@@ -XXX,XX +XXX,XX @@ typedef struct {
      /* Fields protected by @lock */
      NVMeQueue   sq, cq;
      int         cq_phase;
 -    NVMeRequest reqs[NVME_QUEUE_SIZE];
 +    int         free_req_head;
 +    NVMeRequest reqs[NVME_NUM_REQS];
      bool        busy;
      int         need_kick;
      int         inflight;
@@ -XXX,XX +XXX,XX @@ static NVMeQueuePair *nvme_create_queue_pair(BlockDriverState *bs,
      qemu_mutex_init(&q->lock);
      q->index = idx;
      qemu_co_queue_init(&q->free_req_queue);
 -    q->prp_list_pages = qemu_blockalign0(bs, s->page_size * NVME_QUEUE_SIZE);
 +    q->prp_list_pages = qemu_blockalign0(bs, s->page_size * NVME_NUM_REQS);
      r = qemu_vfio_dma_map(s->vfio, q->prp_list_pages,
 -                          s->page_size * NVME_QUEUE_SIZE,
 +                          s->page_size * NVME_NUM_REQS,
                            false, &prp_list_iova);
      if (r) {
          goto fail;
      }
 -    for (i = 0; i < NVME_QUEUE_SIZE; i++) {
 +    q->free_req_head = -1;
 +    for (i = 0; i < NVME_NUM_REQS; i++) {
          NVMeRequest *req = &q->reqs[i];
          req->cid = i + 1;
 +        req->free_req_next = q->free_req_head;
 +        q->free_req_head = i;
          req->prp_list_page = q->prp_list_pages + i * s->page_size;
          req->prp_list_iova = prp_list_iova + i * s->page_size;
      }
 +
      nvme_init_queue(bs, &q->sq, size, NVME_SQ_ENTRY_BYTES, &local_err);
      if (local_err) {
          error_propagate(errp, local_err);
@@ -XXX,XX +XXX,XX @@ static void nvme_kick(BDRVNVMeState *s, NVMeQueuePair *q)
   */
  static NVMeRequest *nvme_get_free_req(NVMeQueuePair *q)
  {
 -    int i;
 -    NVMeRequest *req = NULL;
 +    NVMeRequest *req;
      qemu_mutex_lock(&q->lock);
 -    while (q->inflight + q->need_kick > NVME_QUEUE_SIZE - 2) {
 -        /* We have to leave one slot empty as that is the full queue case (head
 -         * == tail + 1). */
 +
 +    while (q->free_req_head == -1) {
          if (qemu_in_coroutine()) {
              trace_nvme_free_req_queue_wait(q);
              qemu_co_queue_wait(&q->free_req_queue, &q->lock);
@@ -XXX,XX +XXX,XX @@ static NVMeRequest *nvme_get_free_req(NVMeQueuePair *q)
              return NULL;
          }
      }
 -    for (i = 0; i < NVME_QUEUE_SIZE; i++) {
 -        if (!q->reqs[i].busy) {
 -            q->reqs[i].busy = true;
 -            req = &q->reqs[i];
 -            break;
 -        }
 -    }
 -    /* We have checked inflight and need_kick while holding q->lock, so one
 -     * free req must be available. */
 -    assert(req);
 +
 +    req = &q->reqs[q->free_req_head];
 +    q->free_req_head = req->free_req_next;
 +    req->free_req_next = -1;
 +
      qemu_mutex_unlock(&q->lock);
      return req;
  }
-+/*
++/* With q->lock */
-+ * qemu_io_alloc_from_file()
++static void nvme_put_free_req_locked(NVMeQueuePair *q, NVMeRequest *req)
 + *
 + * Allocates the buffer and populates it with the content of the given file
 + * up to @len bytes. If the file length is less than @len, then the buffer
 + * is populated with the file content cyclically.
 + *
 + * @blk - the block backend where the buffer content is going to be written to
 + * @len - the buffer length
 + * @file_name - the file to read the content from
 + *
 + * Returns: the buffer pointer on success
 + *          NULL on error
 + */
 +static void *qemu_io_alloc_from_file(BlockBackend *blk, size_t len,
 +                                     const char *file_name)
 +{
-+    char *buf, *buf_origin;
++    req->free_req_next = q->free_req_head;
-+    FILE *f = fopen(file_name, "r");
++    q->free_req_head = req - q->reqs;
 +    int pattern_len;
 +
 +    if (!f) {
 +        perror(file_name);
 +        return NULL;
 +    }
 +
 +    if (qemuio_misalign) {
 +        len += MISALIGN_OFFSET;
 +    }
 +
 +    buf_origin = buf = blk_blockalign(blk, len);
 +
 +    if (qemuio_misalign) {
 +        buf_origin += MISALIGN_OFFSET;
 +        buf += MISALIGN_OFFSET;
 +        len -= MISALIGN_OFFSET;
 +    }
 +
 +    pattern_len = fread(buf_origin, 1, len, f);
 +
 +    if (ferror(f)) {
 +        perror(file_name);
 +        goto error;
 +    }
 +
 +    if (pattern_len == 0) {
 +        fprintf(stderr, "%s: file is empty\n", file_name);
 +        goto error;
 +    }
 +
 +    fclose(f);
 +
 +    if (len > pattern_len) {
 +        len -= pattern_len;
 +        buf += pattern_len;
 +
 +        while (len > 0) {
 +            size_t len_to_copy = MIN(pattern_len, len);
 +
 +            memcpy(buf, buf_origin, len_to_copy);
 +
 +            len -= len_to_copy;
 +            buf += len_to_copy;
 +        }
 +    }
 +
 +    return buf_origin;
 +
 +error:
 +    qemu_io_free(buf_origin);
 +    return NULL;
 +}
 +
- static void dump_buffer(const void *buffer, int64_t offset, int64_t len)
++/* With q->lock */
 +static void nvme_wake_free_req_locked(BDRVNVMeState *s, NVMeQueuePair *q)
 +{
 +    if (!qemu_co_queue_empty(&q->free_req_queue)) {
 +        replay_bh_schedule_oneshot_event(s->aio_context,
 +                nvme_free_req_queue_cb, q);
 +    }
 +}
 +
 +/* Insert a request in the freelist and wake waiters */
 +static void nvme_put_free_req_and_wake(BDRVNVMeState *s,  NVMeQueuePair *q,
 +                                       NVMeRequest *req)
 +{
 +    qemu_mutex_lock(&q->lock);
 +    nvme_put_free_req_locked(q, req);
 +    nvme_wake_free_req_locked(s, q);
 +    qemu_mutex_unlock(&q->lock);
 +}
 +
  static inline int nvme_translate_error(const NvmeCqe *c)
  {
-     uint64_t i;
+     uint16_t status = (le16_to_cpu(c->status) >> 1) & 0xFF;
-@@ -XXX,XX +XXX,XX @@ static void write_help(void)
+@@ -XXX,XX +XXX,XX @@ static bool nvme_process_completion(BDRVNVMeState *s, NVMeQueuePair *q)
- " -n, -- with -z, don't allow slow fallback\n"
+         req = *preq;
- " -p, -- ignored for backwards compatibility\n"
+         assert(req.cid == cid);
- " -P, -- use different pattern to fill file\n"
+         assert(req.cb);
-+" -s, -- use a pattern file to fill the write buffer\n"
+-        preq->busy = false;
- " -C, -- report statistics in a machine parsable format\n"
++        nvme_put_free_req_locked(q, preq);
- " -q, -- quiet mode, do not show I/O statistics\n"
+         preq->cb = preq->opaque = NULL;
- " -u, -- with -z, allow unmapping\n"
+         qemu_mutex_unlock(&q->lock);
-@@ -XXX,XX +XXX,XX @@ static const cmdinfo_t write_cmd = {
+         req.cb(req.opaque, ret);
-     .perm       = BLK_PERM_WRITE,
+@@ -XXX,XX +XXX,XX @@ static bool nvme_process_completion(BDRVNVMeState *s, NVMeQueuePair *q)
-     .argmin     = 2,
+         /* Notify the device so it can post more completions. */
-     .argmax     = -1,
+         smp_mb_release();
--    .args       = "[-bcCfnquz] [-P pattern] off len",
+         *q->cq.doorbell = cpu_to_le32(q->cq.head);
-+    .args       = "[-bcCfnquz] [-P pattern | -s source_file] off len",
+-        if (!qemu_co_queue_empty(&q->free_req_queue)) {
-     .oneline    = "writes a number of bytes at a specified offset",
+-            replay_bh_schedule_oneshot_event(s->aio_context,
-     .help       = write_help,
+-                                             nvme_free_req_queue_cb, q);
- };
+-        }
-@@ -XXX,XX +XXX,XX @@ static int write_f(BlockBackend *blk, int argc, char **argv)
++        nvme_wake_free_req_locked(s, q);
  {
      struct timespec t1, t2;
      bool Cflag = false, qflag = false, bflag = false;
 -    bool Pflag = false, zflag = false, cflag = false;
 +    bool Pflag = false, zflag = false, cflag = false, sflag = false;
      int flags = 0;
      int c, cnt, ret;
      char *buf = NULL;
@@ -XXX,XX +XXX,XX @@ static int write_f(BlockBackend *blk, int argc, char **argv)
      /* Some compilers get confused and warn if this is not initialized.  */
      int64_t total = 0;
      int pattern = 0xcd;
 +    const char *file_name = NULL;
 -    while ((c = getopt(argc, argv, "bcCfnpP:quz")) != -1) {
 +    while ((c = getopt(argc, argv, "bcCfnpP:qs:uz")) != -1) {
          switch (c) {
          case 'b':
              bflag = true;
@@ -XXX,XX +XXX,XX @@ static int write_f(BlockBackend *blk, int argc, char **argv)
          case 'q':
              qflag = true;
              break;
 +        case 's':
 +            sflag = true;
 +            file_name = optarg;
 +            break;
          case 'u':
              flags |= BDRV_REQ_MAY_UNMAP;
              break;
@@ -XXX,XX +XXX,XX @@ static int write_f(BlockBackend *blk, int argc, char **argv)
          return -EINVAL;
      }
+     q->busy = false;
--    if (zflag && Pflag) {
+     return progress;
--        printf("-z and -P cannot be specified at the same time\n");
+@@ -XXX,XX +XXX,XX @@ static coroutine_fn int nvme_co_prw_aligned(BlockDriverState *bs,
-+    if (zflag + Pflag + sflag > 1) {
+     r = nvme_cmd_map_qiov(bs, &cmd, req, qiov);
-+        printf("Only one of -z, -P, and -s "
+     qemu_co_mutex_unlock(&s->dma_map_lock);
-+               "can be specified at the same time\n");
+     if (r) {
-         return -EINVAL;
+-        req->busy = false;
 +        nvme_put_free_req_and_wake(s, ioq, req);
          return r;
      }
+     nvme_submit_command(s, ioq, req, &cmd, nvme_rw_cb, &data);
-@@ -XXX,XX +XXX,XX @@ static int write_f(BlockBackend *blk, int argc, char **argv)
+@@ -XXX,XX +XXX,XX @@ static int coroutine_fn nvme_co_pdiscard(BlockDriverState *bs,
      qemu_co_mutex_unlock(&s->dma_map_lock);
      if (ret) {
 -        req->busy = false;
 +        nvme_put_free_req_and_wake(s, ioq, req);
          goto out;
      }
-     if (!zflag) {
--        buf = qemu_io_alloc(blk, count, pattern);
-+        if (sflag) {
-+            buf = qemu_io_alloc_from_file(blk, count, file_name);
-+            if (!buf) {
-+                return -EINVAL;
-+            }
-+        } else {
-+            buf = qemu_io_alloc(blk, count, pattern);
-+        }
-     }
-     clock_gettime(CLOCK_MONOTONIC, &t1);
 --
-.21.0
+.26.2

-[Qemu-devel] [PULL 02/15] block: fix permission update in bdrv_replace_node
+Deleted patch
-From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-It's wrong to OR shared permissions. It may lead to crash on further
-permission updates.
-Also, no needs to consider previously calculated permissions, as at
-this point we already bind all new parents and bdrv_get_cumulative_perm
-result is enough. So fix the bug by just set permissions by
-bdrv_get_cumulative_perm result.
-Bug was introduced in long ago 234ac1a9025, in 2.9.
-Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Message-id: 20190824100740.61635-1-vsementsov@virtuozzo.com
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block.c | 5 ++---
-file changed, 2 insertions(+), 3 deletions(-)
-diff --git a/block.c b/block.c
-index XXXXXXX..XXXXXXX 100644
---- a/block.c
-+++ b/block.c
-@@ -XXX,XX +XXX,XX @@ void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
- {
-     BdrvChild *c, *next;
-     GSList *list = NULL, *p;
--    uint64_t old_perm, old_shared;
-     uint64_t perm = 0, shared = BLK_PERM_ALL;
-     int ret;
-@@ -XXX,XX +XXX,XX @@ void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
-         bdrv_unref(from);
-     }
--    bdrv_get_cumulative_perm(to, &old_perm, &old_shared);
--    bdrv_set_perm(to, old_perm | perm, old_shared | shared);
-+    bdrv_get_cumulative_perm(to, &perm, &shared);
-+    bdrv_set_perm(to, perm, shared);
- out:
-     g_slist_free(list);
---
-.21.0

-[Qemu-devel] [PULL 03/15] block: posix: Always allocate the first block
+Deleted patch
-From: Nir Soffer <nirsof@gmail.com>
-When creating an image with preallocation "off" or "falloc", the first
-block of the image is typically not allocated. When using Gluster
-storage backed by XFS filesystem, reading this block using direct I/O
-succeeds regardless of request length, fooling alignment detection.
-In this case we fallback to a safe value (4096) instead of the optimal
-value (512), which may lead to unneeded data copying when aligning
-requests.  Allocating the first block avoids the fallback.
-Since we allocate the first block even with preallocation=off, we no
-longer create images with zero disk size:
-    $ ./qemu-img create -f raw test.raw 1g
-    Formatting 'test.raw', fmt=raw size=1073741824
-    $ ls -lhs test.raw
-.0K -rw-r--r--. 1 nsoffer nsoffer 1.0G Aug 16 23:48 test.raw
-And converting the image requires additional cluster:
-    $ ./qemu-img measure -f raw -O qcow2 test.raw
-    required size: 458752
-    fully allocated size: 1074135040
-When using format like vmdk with multiple files per image, we allocate
-one block per file:
-    $ ./qemu-img create -f vmdk -o subformat=twoGbMaxExtentFlat test.vmdk 4g
-    Formatting 'test.vmdk', fmt=vmdk size=4294967296 compat6=off hwversion=undefined subformat=twoGbMaxExtentFlat
-    $ ls -lhs test*.vmdk
-.0K -rw-r--r--. 1 nsoffer nsoffer 2.0G Aug 27 03:23 test-f001.vmdk
-.0K -rw-r--r--. 1 nsoffer nsoffer 2.0G Aug 27 03:23 test-f002.vmdk
-.0K -rw-r--r--. 1 nsoffer nsoffer  353 Aug 27 03:23 test.vmdk
-I did quick performance test for copying disks with qemu-img convert to
-new raw target image to Gluster storage with sector size of 512 bytes:
-    for i in $(seq 10); do
-        rm -f dst.raw
-        sleep 10
-        time ./qemu-img convert -f raw -O raw -t none -T none src.raw dst.raw
-    done
-Here is a table comparing the total time spent:
-Type    Before(s)   After(s)    Diff(%)
----------------------------------------
-real      530.028    469.123      -11.4
-user       17.204     10.768      -37.4
-sys        17.881      7.011      -60.7
-We can see very clear improvement in CPU usage.
-Signed-off-by: Nir Soffer <nsoffer@redhat.com>
-Message-id: 20190827010528.8818-2-nsoffer@redhat.com
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/file-posix.c                            | 51 +++++++++++++++++++
- tests/qemu-iotests/059.out                    |  2 +-
- tests/qemu-iotests/{150.out => 150.out.qcow2} |  0
- tests/qemu-iotests/150.out.raw                | 12 +++++
- tests/qemu-iotests/175                        | 19 ++++---
- tests/qemu-iotests/175.out                    |  8 +--
- tests/qemu-iotests/178.out.qcow2              |  4 +-
- tests/qemu-iotests/221.out                    | 12 +++--
- tests/qemu-iotests/253.out                    | 12 +++--
-files changed, 99 insertions(+), 21 deletions(-)
- rename tests/qemu-iotests/{150.out => 150.out.qcow2} (100%)
- create mode 100644 tests/qemu-iotests/150.out.raw
-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 handle_aiocb_discard(void *opaque)
-     return ret;
- }
-+/*
-+ * Help alignment probing by allocating the first block.
-+ *
-+ * When reading with direct I/O from unallocated area on Gluster backed by XFS,
-+ * reading succeeds regardless of request length. In this case we fallback to
-+ * safe alignment which is not optimal. Allocating the first block avoids this
-+ * fallback.
-+ *
-+ * fd may be opened with O_DIRECT, but we don't know the buffer alignment or
-+ * request alignment, so we use safe values.
-+ *
-+ * Returns: 0 on success, -errno on failure. Since this is an optimization,
-+ * caller may ignore failures.
-+ */
-+static int allocate_first_block(int fd, size_t max_size)
-+{
-+    size_t write_size = (max_size < MAX_BLOCKSIZE)
-+        ? BDRV_SECTOR_SIZE
-+        : MAX_BLOCKSIZE;
-+    size_t max_align = MAX(MAX_BLOCKSIZE, getpagesize());
-+    void *buf;
-+    ssize_t n;
-+    int ret;
-+
-+    buf = qemu_memalign(max_align, write_size);
-+    memset(buf, 0, write_size);
-+
-+    do {
-+        n = pwrite(fd, buf, write_size, 0);
-+    } while (n == -1 && errno == EINTR);
-+
-+    ret = (n == -1) ? -errno : 0;
-+
-+    qemu_vfree(buf);
-+    return ret;
-+}
-+
- static int handle_aiocb_truncate(void *opaque)
- {
-     RawPosixAIOData *aiocb = opaque;
-@@ -XXX,XX +XXX,XX @@ static int handle_aiocb_truncate(void *opaque)
-                 /* posix_fallocate() doesn't set errno. */
-                 error_setg_errno(errp, -result,
-                                  "Could not preallocate new data");
-+            } else if (current_length == 0) {
-+                /*
-+                 * posix_fallocate() uses fallocate() if the filesystem
-+                 * supports it, or fallback to manually writing zeroes. If
-+                 * fallocate() was used, unaligned reads from the fallocated
-+                 * area in raw_probe_alignment() will succeed, hence we need to
-+                 * allocate the first block.
-+                 *
-+                 * Optimize future alignment probing; ignore failures.
-+                 */
-+                allocate_first_block(fd, offset);
-             }
-         } else {
-             result = 0;
-@@ -XXX,XX +XXX,XX @@ static int handle_aiocb_truncate(void *opaque)
-         if (ftruncate(fd, offset) != 0) {
-             result = -errno;
-             error_setg_errno(errp, -result, "Could not resize file");
-+        } else if (current_length == 0 && offset > current_length) {
-+            /* Optimize future alignment probing; ignore failures. */
-+            allocate_first_block(fd, offset);
-         }
-         return result;
-     default:
-diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/059.out
-+++ b/tests/qemu-iotests/059.out
-@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824000 subformat=twoGbMax
- image: TEST_DIR/t.vmdk
- file format: vmdk
- virtual size: 0.977 TiB (1073741824000 bytes)
--disk size: 16 KiB
-+disk size: 1.97 MiB
- Format specific information:
-     cid: XXXXXXXX
-     parent cid: XXXXXXXX
-diff --git a/tests/qemu-iotests/150.out b/tests/qemu-iotests/150.out.qcow2
-similarity index 100%
-rename from tests/qemu-iotests/150.out
-rename to tests/qemu-iotests/150.out.qcow2
-diff --git a/tests/qemu-iotests/150.out.raw b/tests/qemu-iotests/150.out.raw
-new file mode 100644
-index XXXXXXX..XXXXXXX
---- /dev/null
-+++ b/tests/qemu-iotests/150.out.raw
-@@ -XXX,XX +XXX,XX @@
-+QA output created by 150
-+
-+=== Mapping sparse conversion ===
-+
-+Offset          Length          File
-+0               0x1000          TEST_DIR/t.IMGFMT
-+
-+=== Mapping non-sparse conversion ===
-+
-+Offset          Length          File
-+0               0x100000        TEST_DIR/t.IMGFMT
-+*** done
-diff --git a/tests/qemu-iotests/175 b/tests/qemu-iotests/175
-index XXXXXXX..XXXXXXX 100755
---- a/tests/qemu-iotests/175
-+++ b/tests/qemu-iotests/175
-@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
- # the file size.  This function hides the resulting difference in the
- # stat -c '%b' output.
- # Parameter 1: Number of blocks an empty file occupies
--# Parameter 2: Image size in bytes
-+# Parameter 2: Minimal number of blocks in an image
-+# Parameter 3: Image size in bytes
- _filter_blocks()
- {
-     extra_blocks=$1
--    img_size=$2
-+    min_blocks=$2
-+    img_size=$3
--    sed -e "s/blocks=$extra_blocks\\(\$\\|[^0-9]\\)/nothing allocated/" \
--        -e "s/blocks=$((extra_blocks + img_size / 512))\\(\$\\|[^0-9]\\)/everything allocated/"
-+    sed -e "s/blocks=$min_blocks\\(\$\\|[^0-9]\\)/min allocation/" \
-+        -e "s/blocks=$((extra_blocks + img_size / 512))\\(\$\\|[^0-9]\\)/max allocation/"
- }
- # get standard environment, filters and checks
-@@ -XXX,XX +XXX,XX @@ size=$((1 * 1024 * 1024))
- touch "$TEST_DIR/empty"
- extra_blocks=$(stat -c '%b' "$TEST_DIR/empty")
-+# We always write the first byte; check how many blocks this filesystem
-+# allocates to match empty image alloation.
-+printf "\0" > "$TEST_DIR/empty"
-+min_blocks=$(stat -c '%b' "$TEST_DIR/empty")
-+
- echo
- echo "== creating image with default preallocation =="
- _make_test_img $size | _filter_imgfmt
--stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $size
-+stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $min_blocks $size
- for mode in off full falloc; do
-     echo
-     echo "== creating image with preallocation $mode =="
-     IMGOPTS=preallocation=$mode _make_test_img $size | _filter_imgfmt
--    stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $size
-+    stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $min_blocks $size
- done
- # success, all done
-diff --git a/tests/qemu-iotests/175.out b/tests/qemu-iotests/175.out
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/175.out
-+++ b/tests/qemu-iotests/175.out
-@@ -XXX,XX +XXX,XX @@ QA output created by 175
- == creating image with default preallocation ==
- Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
--size=1048576, nothing allocated
-+size=1048576, min allocation
- == creating image with preallocation off ==
- Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=off
--size=1048576, nothing allocated
-+size=1048576, min allocation
- == creating image with preallocation full ==
- Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=full
--size=1048576, everything allocated
-+size=1048576, max allocation
- == creating image with preallocation falloc ==
- Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=falloc
--size=1048576, everything allocated
-+size=1048576, max allocation
-  *** done
-diff --git a/tests/qemu-iotests/178.out.qcow2 b/tests/qemu-iotests/178.out.qcow2
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/178.out.qcow2
-+++ b/tests/qemu-iotests/178.out.qcow2
-@@ -XXX,XX +XXX,XX @@ converted image file size in bytes: 196608
- == raw input image with data (human) ==
- Formatting 'TEST_DIR/t.qcow2', fmt=IMGFMT size=1073741824
--required size: 393216
-+required size: 458752
- fully allocated size: 1074135040
- wrote 512/512 bytes at offset 512
-bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-@@ -XXX,XX +XXX,XX @@ converted image file size in bytes: 196608
- Formatting 'TEST_DIR/t.qcow2', fmt=IMGFMT size=1073741824
- {
--    "required": 393216,
-+    "required": 458752,
-     "fully-allocated": 1074135040
- }
- wrote 512/512 bytes at offset 512
-diff --git a/tests/qemu-iotests/221.out b/tests/qemu-iotests/221.out
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/221.out
-+++ b/tests/qemu-iotests/221.out
-@@ -XXX,XX +XXX,XX @@ QA output created by 221
- === Check mapping of unaligned raw image ===
- Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=65537
--[{ "start": 0, "length": 66048, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]
--[{ "start": 0, "length": 66048, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]
-+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
-+{ "start": 4096, "length": 61952, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]
-+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
-+{ "start": 4096, "length": 61952, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]
- wrote 1/1 bytes at offset 65536
-bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
--[{ "start": 0, "length": 65536, "depth": 0, "zero": true, "data": false, "offset": OFFSET},
-+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
-+{ "start": 4096, "length": 61440, "depth": 0, "zero": true, "data": false, "offset": OFFSET},
- { "start": 65536, "length": 1, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
- { "start": 65537, "length": 511, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]
--[{ "start": 0, "length": 65536, "depth": 0, "zero": true, "data": false, "offset": OFFSET},
-+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
-+{ "start": 4096, "length": 61440, "depth": 0, "zero": true, "data": false, "offset": OFFSET},
- { "start": 65536, "length": 1, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
- { "start": 65537, "length": 511, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]
- *** done
-diff --git a/tests/qemu-iotests/253.out b/tests/qemu-iotests/253.out
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/253.out
-+++ b/tests/qemu-iotests/253.out
-@@ -XXX,XX +XXX,XX @@ QA output created by 253
- === Check mapping of unaligned raw image ===
- Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048575
--[{ "start": 0, "length": 1048576, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]
--[{ "start": 0, "length": 1048576, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]
-+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
-+{ "start": 4096, "length": 1044480, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]
-+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
-+{ "start": 4096, "length": 1044480, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]
- wrote 65535/65535 bytes at offset 983040
-.999 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
--[{ "start": 0, "length": 983040, "depth": 0, "zero": true, "data": false, "offset": OFFSET},
-+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
-+{ "start": 4096, "length": 978944, "depth": 0, "zero": true, "data": false, "offset": OFFSET},
- { "start": 983040, "length": 65536, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
--[{ "start": 0, "length": 983040, "depth": 0, "zero": true, "data": false, "offset": OFFSET},
-+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
-+{ "start": 4096, "length": 978944, "depth": 0, "zero": true, "data": false, "offset": OFFSET},
- { "start": 983040, "length": 65536, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
- *** done
---
-.21.0

-[Qemu-devel] [PULL 12/15] file-posix: fix request_alignment typo
+[PULL 10/12] block/nvme: clarify that free_req_queue is protected by q->lock
-From: Stefan Hajnoczi <stefanha@redhat.com>
+Existing users access free_req_queue under q->lock. Document this.
-Fixes: a6b257a08e3d72219f03e461a52152672fec0612
-       ("file-posix: Handle undetectable alignment")
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-Message-id: 20190827101328.4062-1-stefanha@redhat.com
+Reviewed-by: Sergio Lopez <slp@redhat.com>
-Reviewed-by: Thomas Huth <thuth@redhat.com>
+Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+Message-id: 20200617132201.1832152-6-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/file-posix.c | 2 +-
+ block/nvme.c | 2 +-
 file changed, 1 insertion(+), 1 deletion(-)
-diff --git a/block/file-posix.c b/block/file-posix.c
+diff --git a/block/nvme.c b/block/nvme.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/file-posix.c
+--- a/block/nvme.c
-+++ b/block/file-posix.c
++++ b/block/nvme.c
-@@ -XXX,XX +XXX,XX @@ static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
+@@ -XXX,XX +XXX,XX @@ typedef struct {
-         for (i = 0; i < ARRAY_SIZE(alignments); i++) {
+ } NVMeRequest;
-             align = alignments[i];
-             if (raw_is_io_aligned(fd, buf + align, max_align)) {
+ typedef struct {
--                /* Fallback to request_aligment. */
+-    CoQueue     free_req_queue;
-+                /* Fallback to request_alignment. */
+     QemuMutex   lock;
-                 s->buf_align = (align != 1) ? align : bs->bl.request_alignment;
-                 break;
+     /* Fields protected by BQL */
-             }
+@@ -XXX,XX +XXX,XX @@ typedef struct {
      uint8_t     *prp_list_pages;
      /* Fields protected by @lock */
 +    CoQueue     free_req_queue;
      NVMeQueue   sq, cq;
      int         cq_phase;
      int         free_req_head;
 --
-.21.0
+.26.2

-[Qemu-devel] [PULL 06/15] vmdk: Use bdrv_dirname() for relative extent paths
+[PULL 11/12] block/nvme: keep BDRVNVMeState pointer in NVMeQueuePair
-This makes iotest 033 pass with e.g. subformat=monolithicFlat.  It also
+Passing around both BDRVNVMeState and NVMeQueuePair is unwieldy. Reduce
-turns a former error in 059 into success.
+the number of function arguments by keeping the BDRVNVMeState pointer in
 NVMeQueuePair. This will come in handly when a BH is introduced in a
 later patch and only one argument can be passed to it.
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-Message-id: 20190815153638.4600-3-mreitz@redhat.com
+Reviewed-by: Sergio Lopez <slp@redhat.com>
-Reviewed-by: John Snow <jsnow@redhat.com>
+Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+Message-id: 20200617132201.1832152-7-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/vmdk.c               | 54 ++++++++++++++++++++++++--------------
+ block/nvme.c | 70 ++++++++++++++++++++++++++++------------------------
- tests/qemu-iotests/059     |  7 +++--
+file changed, 38 insertions(+), 32 deletions(-)
  tests/qemu-iotests/059.out |  4 ++-
 files changed, 42 insertions(+), 23 deletions(-)
-diff --git a/block/vmdk.c b/block/vmdk.c
+diff --git a/block/nvme.c b/block/nvme.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/vmdk.c
+--- a/block/nvme.c
-+++ b/block/vmdk.c
++++ b/block/nvme.c
-@@ -XXX,XX +XXX,XX @@ static const char *next_line(const char *s)
+@@ -XXX,XX +XXX,XX @@
- }
+  */
+ #define NVME_NUM_REQS (NVME_QUEUE_SIZE - 1)
- static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
--                              const char *desc_file_path, QDict *options,
++typedef struct BDRVNVMeState BDRVNVMeState;
--                              Error **errp)
++
-+                              QDict *options, Error **errp)
+ typedef struct {
- {
+     int32_t  head, tail;
-     int ret;
+     uint8_t  *queue;
-     int matches;
+@@ -XXX,XX +XXX,XX @@ typedef struct {
-@@ -XXX,XX +XXX,XX @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
+ typedef struct {
-     const char *p, *np;
+     QemuMutex   lock;
-     int64_t sectors = 0;
-     int64_t flat_offset;
++    /* Read from I/O code path, initialized under BQL */
-+    char *desc_file_dir = NULL;
++    BDRVNVMeState   *s;
-     char *extent_path;
++    int             index;
-     BdrvChild *extent_file;
++
-     BDRVVmdkState *s = bs->opaque;
+     /* Fields protected by BQL */
-@@ -XXX,XX +XXX,XX @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
+-    int         index;
-             continue;
+     uint8_t     *prp_list_pages;
      /* Fields protected by @lock */
@@ -XXX,XX +XXX,XX @@ typedef volatile struct {
  QEMU_BUILD_BUG_ON(offsetof(NVMeRegs, doorbells) != 0x1000);
 -typedef struct {
 +struct BDRVNVMeState {
      AioContext *aio_context;
      QEMUVFIOState *vfio;
      NVMeRegs *regs;
@@ -XXX,XX +XXX,XX @@ typedef struct {
      /* PCI address (required for nvme_refresh_filename()) */
      char *device;
 -} BDRVNVMeState;
 +};
  #define NVME_BLOCK_OPT_DEVICE "device"
  #define NVME_BLOCK_OPT_NAMESPACE "namespace"
@@ -XXX,XX +XXX,XX @@ static void nvme_init_queue(BlockDriverState *bs, NVMeQueue *q,
      }
  }
 -static void nvme_free_queue_pair(BlockDriverState *bs, NVMeQueuePair *q)
 +static void nvme_free_queue_pair(NVMeQueuePair *q)
  {
      qemu_vfree(q->prp_list_pages);
      qemu_vfree(q->sq.queue);
@@ -XXX,XX +XXX,XX @@ static NVMeQueuePair *nvme_create_queue_pair(BlockDriverState *bs,
      uint64_t prp_list_iova;
      qemu_mutex_init(&q->lock);
 +    q->s = s;
      q->index = idx;
      qemu_co_queue_init(&q->free_req_queue);
      q->prp_list_pages = qemu_blockalign0(bs, s->page_size * NVME_NUM_REQS);
@@ -XXX,XX +XXX,XX @@ static NVMeQueuePair *nvme_create_queue_pair(BlockDriverState *bs,
      return q;
  fail:
 -    nvme_free_queue_pair(bs, q);
 +    nvme_free_queue_pair(q);
      return NULL;
  }
  /* With q->lock */
 -static void nvme_kick(BDRVNVMeState *s, NVMeQueuePair *q)
 +static void nvme_kick(NVMeQueuePair *q)
  {
 +    BDRVNVMeState *s = q->s;
 +
      if (s->plugged || !q->need_kick) {
          return;
      }
@@ -XXX,XX +XXX,XX @@ static void nvme_put_free_req_locked(NVMeQueuePair *q, NVMeRequest *req)
  }
  /* With q->lock */
 -static void nvme_wake_free_req_locked(BDRVNVMeState *s, NVMeQueuePair *q)
 +static void nvme_wake_free_req_locked(NVMeQueuePair *q)
  {
      if (!qemu_co_queue_empty(&q->free_req_queue)) {
 -        replay_bh_schedule_oneshot_event(s->aio_context,
 +        replay_bh_schedule_oneshot_event(q->s->aio_context,
                  nvme_free_req_queue_cb, q);
      }
  }
  /* Insert a request in the freelist and wake waiters */
 -static void nvme_put_free_req_and_wake(BDRVNVMeState *s,  NVMeQueuePair *q,
 -                                       NVMeRequest *req)
 +static void nvme_put_free_req_and_wake(NVMeQueuePair *q, NVMeRequest *req)
  {
      qemu_mutex_lock(&q->lock);
      nvme_put_free_req_locked(q, req);
 -    nvme_wake_free_req_locked(s, q);
 +    nvme_wake_free_req_locked(q);
      qemu_mutex_unlock(&q->lock);
  }
@@ -XXX,XX +XXX,XX @@ static inline int nvme_translate_error(const NvmeCqe *c)
  }
  /* With q->lock */
 -static bool nvme_process_completion(BDRVNVMeState *s, NVMeQueuePair *q)
 +static bool nvme_process_completion(NVMeQueuePair *q)
  {
 +    BDRVNVMeState *s = q->s;
      bool progress = false;
      NVMeRequest *preq;
      NVMeRequest req;
@@ -XXX,XX +XXX,XX @@ static bool nvme_process_completion(BDRVNVMeState *s, NVMeQueuePair *q)
          /* Notify the device so it can post more completions. */
          smp_mb_release();
          *q->cq.doorbell = cpu_to_le32(q->cq.head);
 -        nvme_wake_free_req_locked(s, q);
 +        nvme_wake_free_req_locked(q);
      }
      q->busy = false;
      return progress;
@@ -XXX,XX +XXX,XX @@ static void nvme_trace_command(const NvmeCmd *cmd)
      }
  }
 -static void nvme_submit_command(BDRVNVMeState *s, NVMeQueuePair *q,
 -                                NVMeRequest *req,
 +static void nvme_submit_command(NVMeQueuePair *q, NVMeRequest *req,
                                  NvmeCmd *cmd, BlockCompletionFunc cb,
                                  void *opaque)
  {
@@ -XXX,XX +XXX,XX @@ static void nvme_submit_command(BDRVNVMeState *s, NVMeQueuePair *q,
      req->opaque = opaque;
      cmd->cid = cpu_to_le32(req->cid);
 -    trace_nvme_submit_command(s, q->index, req->cid);
 +    trace_nvme_submit_command(q->s, q->index, req->cid);
      nvme_trace_command(cmd);
      qemu_mutex_lock(&q->lock);
      memcpy((uint8_t *)q->sq.queue +
             q->sq.tail * NVME_SQ_ENTRY_BYTES, cmd, sizeof(*cmd));
      q->sq.tail = (q->sq.tail + 1) % NVME_QUEUE_SIZE;
      q->need_kick++;
 -    nvme_kick(s, q);
 -    nvme_process_completion(s, q);
 +    nvme_kick(q);
 +    nvme_process_completion(q);
      qemu_mutex_unlock(&q->lock);
  }
@@ -XXX,XX +XXX,XX @@ static int nvme_cmd_sync(BlockDriverState *bs, NVMeQueuePair *q,
                           NvmeCmd *cmd)
  {
      NVMeRequest *req;
 -    BDRVNVMeState *s = bs->opaque;
      int ret = -EINPROGRESS;
      req = nvme_get_free_req(q);
      if (!req) {
          return -EBUSY;
      }
 -    nvme_submit_command(s, q, req, cmd, nvme_cmd_sync_cb, &ret);
 +    nvme_submit_command(q, req, cmd, nvme_cmd_sync_cb, &ret);
      BDRV_POLL_WHILE(bs, ret == -EINPROGRESS);
      return ret;
@@ -XXX,XX +XXX,XX @@ static bool nvme_poll_queues(BDRVNVMeState *s)
          }
--        if (!path_is_absolute(fname) && !path_has_protocol(fname) &&
+         qemu_mutex_lock(&q->lock);
--            !desc_file_path[0])
+-        while (nvme_process_completion(s, q)) {
--        {
++        while (nvme_process_completion(q)) {
--            bdrv_refresh_filename(bs->file->bs);
+             /* Keep polling */
--            error_setg(errp, "Cannot use relative extent paths with VMDK "
+             progress = true;
 -                       "descriptor file '%s'", bs->file->bs->filename);
 -            return -EINVAL;
 -        }
 +        if (path_is_absolute(fname)) {
 +            extent_path = g_strdup(fname);
 +        } else {
 +            if (!desc_file_dir) {
 +                desc_file_dir = bdrv_dirname(bs->file->bs, errp);
 +                if (!desc_file_dir) {
 +                    bdrv_refresh_filename(bs->file->bs);
 +                    error_prepend(errp, "Cannot use relative paths with VMDK "
 +                                  "descriptor file '%s': ",
 +                                  bs->file->bs->filename);
 +                    ret = -EINVAL;
 +                    goto out;
 +                }
 +            }
 -        extent_path = path_combine(desc_file_path, fname);
 +            extent_path = g_strconcat(desc_file_dir, fname, NULL);
 +        }
          ret = snprintf(extent_opt_prefix, 32, "extents.%d", s->num_extents);
          assert(ret < 32);
@@ -XXX,XX +XXX,XX @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
          g_free(extent_path);
          if (local_err) {
              error_propagate(errp, local_err);
 -            return -EINVAL;
 +            ret = -EINVAL;
 +            goto out;
          }
+@@ -XXX,XX +XXX,XX @@ static bool nvme_add_io_queue(BlockDriverState *bs, Error **errp)
-         /* save to extents array */
+     };
-@@ -XXX,XX +XXX,XX @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
+     if (nvme_cmd_sync(bs, s->queues[0], &cmd)) {
-, 0, 0, 0, 0, &extent, errp);
+         error_setg(errp, "Failed to create io queue [%d]", n);
-             if (ret < 0) {
+-        nvme_free_queue_pair(bs, q);
-                 bdrv_unref_child(bs, extent_file);
++        nvme_free_queue_pair(q);
--                return ret;
+         return false;
-+                goto out;
+     }
-             }
+     cmd = (NvmeCmd) {
-             extent->flat_start_offset = flat_offset << 9;
+@@ -XXX,XX +XXX,XX @@ static bool nvme_add_io_queue(BlockDriverState *bs, Error **errp)
-         } else if (!strcmp(type, "SPARSE") || !strcmp(type, "VMFSSPARSE")) {
+     };
-@@ -XXX,XX +XXX,XX @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
+     if (nvme_cmd_sync(bs, s->queues[0], &cmd)) {
-             g_free(buf);
+         error_setg(errp, "Failed to create io queue [%d]", n);
-             if (ret) {
+-        nvme_free_queue_pair(bs, q);
-                 bdrv_unref_child(bs, extent_file);
++        nvme_free_queue_pair(q);
--                return ret;
+         return false;
-+                goto out;
+     }
-             }
+     s->queues = g_renew(NVMeQueuePair *, s->queues, n + 1);
-             extent = &s->extents[s->num_extents - 1];
+@@ -XXX,XX +XXX,XX @@ static void nvme_close(BlockDriverState *bs)
-         } else if (!strcmp(type, "SESPARSE")) {
+     BDRVNVMeState *s = bs->opaque;
-             ret = vmdk_open_se_sparse(bs, extent_file, bs->open_flags, errp);
-             if (ret) {
+     for (i = 0; i < s->nr_queues; ++i) {
-                 bdrv_unref_child(bs, extent_file);
+-        nvme_free_queue_pair(bs, s->queues[i]);
--                return ret;
++        nvme_free_queue_pair(s->queues[i]);
-+                goto out;
+     }
-             }
+     g_free(s->queues);
-             extent = &s->extents[s->num_extents - 1];
+     aio_set_event_notifier(bdrv_get_aio_context(bs), &s->irq_notifier,
-         } else {
+@@ -XXX,XX +XXX,XX @@ static coroutine_fn int nvme_co_prw_aligned(BlockDriverState *bs,
-             error_setg(errp, "Unsupported extent type '%s'", type);
+     r = nvme_cmd_map_qiov(bs, &cmd, req, qiov);
-             bdrv_unref_child(bs, extent_file);
+     qemu_co_mutex_unlock(&s->dma_map_lock);
--            return -ENOTSUP;
+     if (r) {
-+            ret = -ENOTSUP;
+-        nvme_put_free_req_and_wake(s, ioq, req);
-+            goto out;
++        nvme_put_free_req_and_wake(ioq, req);
-         }
+         return r;
-         extent->type = g_strdup(type);
+     }
-     }
+-    nvme_submit_command(s, ioq, req, &cmd, nvme_rw_cb, &data);
--    return 0;
++    nvme_submit_command(ioq, req, &cmd, nvme_rw_cb, &data);
-+
-+    ret = 0;
+     data.co = qemu_coroutine_self();
-+    goto out;
+     while (data.ret == -EINPROGRESS) {
+@@ -XXX,XX +XXX,XX @@ static coroutine_fn int nvme_co_flush(BlockDriverState *bs)
- invalid:
+     assert(s->nr_queues > 1);
-     np = next_line(p);
+     req = nvme_get_free_req(ioq);
-@@ -XXX,XX +XXX,XX @@ invalid:
+     assert(req);
-         np--;
+-    nvme_submit_command(s, ioq, req, &cmd, nvme_rw_cb, &data);
-     }
++    nvme_submit_command(ioq, req, &cmd, nvme_rw_cb, &data);
-     error_setg(errp, "Invalid extent line: %.*s", (int)(np - p), p);
--    return -EINVAL;
+     data.co = qemu_coroutine_self();
-+    ret = -EINVAL;
+     if (data.ret == -EINPROGRESS) {
-+
+@@ -XXX,XX +XXX,XX @@ static coroutine_fn int nvme_co_pwrite_zeroes(BlockDriverState *bs,
-+out:
+     req = nvme_get_free_req(ioq);
-+    g_free(desc_file_dir);
+     assert(req);
-+    return ret;
- }
+-    nvme_submit_command(s, ioq, req, &cmd, nvme_rw_cb, &data);
++    nvme_submit_command(ioq, req, &cmd, nvme_rw_cb, &data);
- static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf,
-@@ -XXX,XX +XXX,XX @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf,
+     data.co = qemu_coroutine_self();
-     }
+     while (data.ret == -EINPROGRESS) {
-     s->create_type = g_strdup(ct);
+@@ -XXX,XX +XXX,XX @@ static int coroutine_fn nvme_co_pdiscard(BlockDriverState *bs,
-     s->desc_offset = 0;
+     qemu_co_mutex_unlock(&s->dma_map_lock);
--    ret = vmdk_parse_extents(buf, bs, bs->file->bs->exact_filename, options,
--                             errp);
+     if (ret) {
-+    ret = vmdk_parse_extents(buf, bs, options, errp);
+-        nvme_put_free_req_and_wake(s, ioq, req);
- exit:
++        nvme_put_free_req_and_wake(ioq, req);
-     return ret;
+         goto out;
- }
+     }
-diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
-index XXXXXXX..XXXXXXX 100755
+     trace_nvme_dsm(s, offset, bytes);
---- a/tests/qemu-iotests/059
-+++ b/tests/qemu-iotests/059
+-    nvme_submit_command(s, ioq, req, &cmd, nvme_rw_cb, &data);
-@@ -XXX,XX +XXX,XX @@ $QEMU_IMG convert -f qcow2 -O vmdk -o subformat=streamOptimized "$TEST_IMG.qcow2
++    nvme_submit_command(ioq, req, &cmd, nvme_rw_cb, &data);
- echo
+     data.co = qemu_coroutine_self();
- echo "=== Testing monolithicFlat with internally generated JSON file name ==="
+     while (data.ret == -EINPROGRESS) {
-+# Should work, because bdrv_dirname() works fine with blkdebug
+@@ -XXX,XX +XXX,XX @@ static void nvme_aio_unplug(BlockDriverState *bs)
- IMGOPTS="subformat=monolithicFlat" _make_test_img 64M
+     for (i = 1; i < s->nr_queues; i++) {
--$QEMU_IO -c "open -o driver=$IMGFMT,file.driver=blkdebug,file.image.filename=$TEST_IMG,file.inject-error.0.event=read_aio" 2>&1 \
+         NVMeQueuePair *q = s->queues[i];
--    | _filter_testdir | _filter_imgfmt
+         qemu_mutex_lock(&q->lock);
-+$QEMU_IO -c "open -o driver=$IMGFMT,file.driver=blkdebug,file.image.filename=$TEST_IMG,file.inject-error.0.event=read_aio" \
+-        nvme_kick(s, q);
-+         -c info \
+-        nvme_process_completion(s, q);
-+    2>&1 \
++        nvme_kick(q);
-+    | _filter_testdir | _filter_imgfmt | _filter_img_info
++        nvme_process_completion(q);
- _cleanup_test_img
+         qemu_mutex_unlock(&q->lock);
+     }
- echo
+ }
 diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
 index XXXXXXX..XXXXXXX 100644
 --- a/tests/qemu-iotests/059.out
 +++ b/tests/qemu-iotests/059.out
@@ -XXX,XX +XXX,XX @@ wrote 512/512 bytes at offset 10240
  === Testing monolithicFlat with internally generated JSON file name ===
  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 -qemu-io: can't open: Cannot use relative extent paths with VMDK descriptor file 'json:{"image": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, "driver": "blkdebug", "inject-error.0.event": "read_aio"}'
 +format name: IMGFMT
 +cluster size: 0 bytes
 +vm state offset: 0 bytes
  === Testing version 3 ===
  image: TEST_DIR/iotest-version3.IMGFMT
 --
-.21.0
+.26.2

-[Qemu-devel] [PULL 04/15] iotests: Test allocate_first_block() with O_DIRECT
+[PULL 12/12] block/nvme: support nested aio_poll()
-From: Nir Soffer <nirsof@gmail.com>
+QEMU block drivers are supposed to support aio_poll() from I/O
 completion callback functions. This means completion processing must be
 re-entrant.
-Using block_resize we can test allocate_first_block() with file
+The standard approach is to schedule a BH during completion processing
-descriptor opened with O_DIRECT, ensuring that it works for any size
+and cancel it at the end of processing. If aio_poll() is invoked by a
-larger than 4096 bytes.
+callback function then the BH will run. The BH continues the suspended
 completion processing.
-Testing smaller sizes is tricky as the result depends on the filesystem
+All of this means that request A's cb() can synchronously wait for
-used for testing. For example on NFS any size will work since O_DIRECT
+request B to complete. Previously the nvme block driver would hang
-does not require any alignment.
+because it didn't process completions from nested aio_poll().
-Signed-off-by: Nir Soffer <nsoffer@redhat.com>
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
+Reviewed-by: Sergio Lopez <slp@redhat.com>
-Message-id: 20190827010528.8818-3-nsoffer@redhat.com
+Message-id: 20200617132201.1832152-8-stefanha@redhat.com
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- tests/qemu-iotests/175     | 28 ++++++++++++++++++++++++++++
+ block/nvme.c       | 67 ++++++++++++++++++++++++++++++++++++++++------
- tests/qemu-iotests/175.out |  8 ++++++++
+ block/trace-events |  2 +-
-files changed, 36 insertions(+)
+files changed, 60 insertions(+), 9 deletions(-)
-diff --git a/tests/qemu-iotests/175 b/tests/qemu-iotests/175
+diff --git a/block/nvme.c b/block/nvme.c
-index XXXXXXX..XXXXXXX 100755
+index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/175
+--- a/block/nvme.c
-+++ b/tests/qemu-iotests/175
++++ b/block/nvme.c
-@@ -XXX,XX +XXX,XX @@ _filter_blocks()
+@@ -XXX,XX +XXX,XX @@ typedef struct {
-         -e "s/blocks=$((extra_blocks + img_size / 512))\\(\$\\|[^0-9]\\)/max allocation/"
+     int         cq_phase;
      int         free_req_head;
      NVMeRequest reqs[NVME_NUM_REQS];
 -    bool        busy;
      int         need_kick;
      int         inflight;
 +
 +    /* Thread-safe, no lock necessary */
 +    QEMUBH      *completion_bh;
  } NVMeQueuePair;
  /* Memory mapped registers */
@@ -XXX,XX +XXX,XX @@ struct BDRVNVMeState {
  #define NVME_BLOCK_OPT_DEVICE "device"
  #define NVME_BLOCK_OPT_NAMESPACE "namespace"
 +static void nvme_process_completion_bh(void *opaque);
 +
  static QemuOptsList runtime_opts = {
      .name = "nvme",
      .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
@@ -XXX,XX +XXX,XX @@ static void nvme_init_queue(BlockDriverState *bs, NVMeQueue *q,
  static void nvme_free_queue_pair(NVMeQueuePair *q)
  {
 +    if (q->completion_bh) {
 +        qemu_bh_delete(q->completion_bh);
 +    }
      qemu_vfree(q->prp_list_pages);
      qemu_vfree(q->sq.queue);
      qemu_vfree(q->cq.queue);
@@ -XXX,XX +XXX,XX @@ static NVMeQueuePair *nvme_create_queue_pair(BlockDriverState *bs,
      q->index = idx;
      qemu_co_queue_init(&q->free_req_queue);
      q->prp_list_pages = qemu_blockalign0(bs, s->page_size * NVME_NUM_REQS);
 +    q->completion_bh = aio_bh_new(bdrv_get_aio_context(bs),
 +                                  nvme_process_completion_bh, q);
      r = qemu_vfio_dma_map(s->vfio, q->prp_list_pages,
                            s->page_size * NVME_NUM_REQS,
                            false, &prp_list_iova);
@@ -XXX,XX +XXX,XX @@ static bool nvme_process_completion(NVMeQueuePair *q)
      NvmeCqe *c;
      trace_nvme_process_completion(s, q->index, q->inflight);
 -    if (q->busy || s->plugged) {
 -        trace_nvme_process_completion_queue_busy(s, q->index);
 +    if (s->plugged) {
 +        trace_nvme_process_completion_queue_plugged(s, q->index);
          return false;
      }
 -    q->busy = true;
 +
 +    /*
 +     * Support re-entrancy when a request cb() function invokes aio_poll().
 +     * Pending completions must be visible to aio_poll() so that a cb()
 +     * function can wait for the completion of another request.
 +     *
 +     * The aio_poll() loop will execute our BH and we'll resume completion
 +     * processing there.
 +     */
 +    qemu_bh_schedule(q->completion_bh);
 +
      assert(q->inflight >= 0);
      while (q->inflight) {
          int ret;
@@ -XXX,XX +XXX,XX @@ static bool nvme_process_completion(NVMeQueuePair *q)
          assert(req.cb);
          nvme_put_free_req_locked(q, preq);
          preq->cb = preq->opaque = NULL;
 -        qemu_mutex_unlock(&q->lock);
 -        req.cb(req.opaque, ret);
 -        qemu_mutex_lock(&q->lock);
          q->inflight--;
 +        qemu_mutex_unlock(&q->lock);
 +        req.cb(req.opaque, ret);
 +        qemu_mutex_lock(&q->lock);
          progress = true;
      }
      if (progress) {
@@ -XXX,XX +XXX,XX @@ static bool nvme_process_completion(NVMeQueuePair *q)
          *q->cq.doorbell = cpu_to_le32(q->cq.head);
          nvme_wake_free_req_locked(q);
      }
 -    q->busy = false;
 +
 +    qemu_bh_cancel(q->completion_bh);
 +
      return progress;
  }
-+# Resize image using block_resize.
++static void nvme_process_completion_bh(void *opaque)
 +# Parameter 1: image path
 +# Parameter 2: new size
 +_block_resize()
 +{
-+    local path=$1
++    NVMeQueuePair *q = opaque;
 +    local size=$2
 +
-+    $QEMU -qmp stdio -nographic -nodefaults \
++    /*
-+        -blockdev file,node-name=file,filename=$path,cache.direct=on \
++     * We're being invoked because a nvme_process_completion() cb() function
-+        <<EOF
++     * called aio_poll(). The callback may be waiting for further completions
-+{'execute': 'qmp_capabilities'}
++     * so notify the device that it has space to fill in more completions now.
-+{'execute': 'block_resize', 'arguments': {'node-name': 'file', 'size': $size}}
++     */
-+{'execute': 'quit'}
++    smp_mb_release();
-+EOF
++    *q->cq.doorbell = cpu_to_le32(q->cq.head);
 +    nvme_wake_free_req_locked(q);
 +
 +    nvme_process_completion(q);
 +}
 +
- # get standard environment, filters and checks
+ static void nvme_trace_command(const NvmeCmd *cmd)
- . ./common.rc
+ {
- . ./common.filter
+     int i;
-@@ -XXX,XX +XXX,XX @@ _supported_fmt raw
+@@ -XXX,XX +XXX,XX @@ static void nvme_detach_aio_context(BlockDriverState *bs)
- _supported_proto file
+ {
- _supported_os Linux
+     BDRVNVMeState *s = bs->opaque;
-+_default_cache_mode none
++    for (int i = 0; i < s->nr_queues; i++) {
-+_supported_cache_modes none directsync
++        NVMeQueuePair *q = s->queues[i];
 +
- size=$((1 * 1024 * 1024))
++        qemu_bh_delete(q->completion_bh);
++        q->completion_bh = NULL;
- touch "$TEST_DIR/empty"
++    }
@@ -XXX,XX +XXX,XX @@ for mode in off full falloc; do
      stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $min_blocks $size
  done
 +for new_size in 4096 1048576; do
 +    echo
 +    echo "== resize empty image with block_resize =="
 +    _make_test_img 0 | _filter_imgfmt
 +    _block_resize $TEST_IMG $new_size >/dev/null
 +    stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $min_blocks $new_size
 +done
 +
- # success, all done
+     aio_set_event_notifier(bdrv_get_aio_context(bs), &s->irq_notifier,
- echo "*** done"
+                            false, NULL, NULL);
- rm -f $seq.full
+ }
-diff --git a/tests/qemu-iotests/175.out b/tests/qemu-iotests/175.out
+@@ -XXX,XX +XXX,XX @@ static void nvme_attach_aio_context(BlockDriverState *bs,
      s->aio_context = new_context;
      aio_set_event_notifier(new_context, &s->irq_notifier,
                             false, nvme_handle_event, nvme_poll_cb);
 +
 +    for (int i = 0; i < s->nr_queues; i++) {
 +        NVMeQueuePair *q = s->queues[i];
 +
 +        q->completion_bh =
 +            aio_bh_new(new_context, nvme_process_completion_bh, q);
 +    }
  }
  static void nvme_aio_plug(BlockDriverState *bs)
 diff --git a/block/trace-events b/block/trace-events
 index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/175.out
+--- a/block/trace-events
-+++ b/tests/qemu-iotests/175.out
++++ b/block/trace-events
-@@ -XXX,XX +XXX,XX @@ size=1048576, max allocation
+@@ -XXX,XX +XXX,XX @@ nvme_kick(void *s, int queue) "s %p queue %d"
- == creating image with preallocation falloc ==
+ nvme_dma_flush_queue_wait(void *s) "s %p"
- Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=falloc
+ nvme_error(int cmd_specific, int sq_head, int sqid, int cid, int status) "cmd_specific %d sq_head %d sqid %d cid %d status 0x%x"
- size=1048576, max allocation
+ nvme_process_completion(void *s, int index, int inflight) "s %p queue %d inflight %d"
-+
+-nvme_process_completion_queue_busy(void *s, int index) "s %p queue %d"
-+== resize empty image with block_resize ==
++nvme_process_completion_queue_plugged(void *s, int index) "s %p queue %d"
-+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=0
+ nvme_complete_command(void *s, int index, int cid) "s %p queue %d cid %d"
-+size=4096, min allocation
+ nvme_submit_command(void *s, int index, int cid) "s %p queue %d cid %d"
-+
+ nvme_submit_command_raw(int c0, int c1, int c2, int c3, int c4, int c5, int c6, int c7) "%02x %02x %02x %02x %02x %02x %02x %02x"
 +== resize empty image with block_resize ==
 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=0
 +size=1048576, min allocation
   *** done
 --
-.21.0
+.26.2

-[Qemu-devel] [PULL 05/15] iotests: Fix _filter_img_create()
+Deleted patch
-fe646693acc changed qemu-img create's output so that it no longer prints
-single quotes around parameter values.  The subformat and adapter_type
-filters in _filter_img_create() have never been adapted to that change.
-Fixes: fe646693acc13ac48b98435d14149ab04dc597bc
-Signed-off-by: Max Reitz <mreitz@redhat.com>
-Reviewed-by: John Snow <jsnow@redhat.com>
-Message-id: 20190815153638.4600-2-mreitz@redhat.com
-Reviewed-by: John Snow <jsnow@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- tests/qemu-iotests/059.out       | 16 ++++++++--------
- tests/qemu-iotests/common.filter |  4 ++--
-files changed, 10 insertions(+), 10 deletions(-)
-diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/059.out
-+++ b/tests/qemu-iotests/059.out
-@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
- qemu-io: can't open device TEST_DIR/t.vmdk: L1 size too big
- === Testing monolithicFlat creation and opening ===
--Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2147483648 subformat=monolithicFlat
-+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2147483648
- image: TEST_DIR/t.IMGFMT
- file format: IMGFMT
- virtual size: 2 GiB (2147483648 bytes)
- === Testing monolithicFlat with zeroed_grain ===
- qemu-img: TEST_DIR/t.IMGFMT: Flat image can't enable zeroed grain
--Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2147483648 subformat=monolithicFlat
-+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2147483648
- === Testing big twoGbMaxExtentFlat ===
--Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824000 subformat=twoGbMaxExtentFlat
-+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824000
- image: TEST_DIR/t.vmdk
- file format: vmdk
- virtual size: 0.977 TiB (1073741824000 bytes)
-@@ -XXX,XX +XXX,XX @@ Format specific information:
- qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Invalid extent line: RW 12582912 VMFS "dummy.IMGFMT" 1
- === Testing truncated sparse ===
--Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=107374182400 subformat=monolithicSparse
-+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=107374182400
- qemu-img: Could not open 'TEST_DIR/t.IMGFMT': File truncated, expecting at least 13172736 bytes
- === Converting to streamOptimized from image with small cluster size===
-@@ -XXX,XX +XXX,XX @@ wrote 512/512 bytes at offset 10240
-bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
- === Testing monolithicFlat with internally generated JSON file name ===
--Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 subformat=monolithicFlat
-+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
- qemu-io: can't open: Cannot use relative extent paths with VMDK descriptor file 'json:{"image": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, "driver": "blkdebug", "inject-error.0.event": "read_aio"}'
- === Testing version 3 ===
-@@ -XXX,XX +XXX,XX @@ read 512/512 bytes at offset 64931328
-bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
- === Testing 4TB monolithicFlat creation and IO ===
--Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4398046511104 subformat=monolithicFlat
-+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4398046511104
- image: TEST_DIR/t.IMGFMT
- file format: IMGFMT
- virtual size: 4 TiB (4398046511104 bytes)
-@@ -XXX,XX +XXX,XX @@ read 1024/1024 bytes at offset 966367641600
-KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
- === Testing qemu-img map on extents ===
--Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33285996544 subformat=monolithicSparse
-+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33285996544
- wrote 1024/1024 bytes at offset 65024
-KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
- wrote 1024/1024 bytes at offset 2147483136
-@@ -XXX,XX +XXX,XX @@ Offset          Length          Mapped to       File
-0x20000         0x3f0000        TEST_DIR/t.vmdk
-x7fff0000      0x20000         0x410000        TEST_DIR/t.vmdk
-x140000000     0x10000         0x430000        TEST_DIR/t.vmdk
--Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33285996544 subformat=twoGbMaxExtentSparse
-+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33285996544
- wrote 1024/1024 bytes at offset 65024
-KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
- wrote 1024/1024 bytes at offset 2147483136
-diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
-index XXXXXXX..XXXXXXX 100644
---- a/tests/qemu-iotests/common.filter
-+++ b/tests/qemu-iotests/common.filter
-@@ -XXX,XX +XXX,XX @@ _filter_img_create()
-         -e "s# compat6=\\(on\\|off\\)##g" \
-         -e "s# static=\\(on\\|off\\)##g" \
-         -e "s# zeroed_grain=\\(on\\|off\\)##g" \
--        -e "s# subformat='[^']*'##g" \
--        -e "s# adapter_type='[^']*'##g" \
-+        -e "s# subformat=[^ ]*##g" \
-+        -e "s# adapter_type=[^ ]*##g" \
-         -e "s# hwversion=[^ ]*##g" \
-         -e "s# lazy_refcounts=\\(on\\|off\\)##g" \
-         -e "s# block_size=[0-9]\\+##g" \
---
-.21.0

The following changes since commit 23919ddfd56135cad3cb468a8f54d5a595f024f4:

Merge remote-tracking branch 'remotes/aperard/tags/pull-xen-20190827' into staging (2019-08-27 15:52:36 +0100)

are available in the Git repository at:

https://github.com/XanClic/qemu.git tags/pull-block-2019-08-27

for you to fetch changes up to bb043c056cffcc2f3ce88bfdaf2e76e455c09e2c:

iotests: Unify cache mode quoting (2019-08-27 19:48:44 +0200)

----------------------------------------------------------------
Block patches:
- qemu-io now accepts a file to read a write pattern from
- Ensure that raw files have their first block allocated so we can probe
  the O_DIRECT alignment if necessary
- Various fixes

----------------------------------------------------------------
Denis Plotnikov (1):
  qemu-io: add pattern file for write command

Max Reitz (7):
  iotests: Fix _filter_img_create()
  vmdk: Use bdrv_dirname() for relative extent paths
  iotests: Keep testing broken relative extent paths
  vmdk: Reject invalid compressed writes
  iotests: Disable broken streamOptimized tests
  iotests: Disable 110 for vmdk.twoGbMaxExtentSparse
  iotests: Disable 126 for flat vmdk subformats

Nir Soffer (3):
  block: posix: Always allocate the first block
  iotests: Test allocate_first_block() with O_DIRECT
  iotests: Unify cache mode quoting

Stefan Hajnoczi (1):
  file-posix: fix request_alignment typo

Thomas Huth (2):
  iotests: Check for enabled drivers before testing them
  tests/check-block: Skip iotests when sanitizers are enabled

Vladimir Sementsov-Ogievskiy (1):
  block: fix permission update in bdrv_replace_node

block.c                                       |  5 +-
 block/file-posix.c                            | 53 +++++++++-
 block/vmdk.c                                  | 64 ++++++++----
 qemu-io-cmds.c                                | 99 +++++++++++++++++--
 tests/check-block.sh                          |  5 +
 tests/qemu-iotests/002                        |  1 +
 tests/qemu-iotests/003                        |  1 +
 tests/qemu-iotests/005                        |  3 +-
 tests/qemu-iotests/009                        |  1 +
 tests/qemu-iotests/010                        |  1 +
 tests/qemu-iotests/011                        |  1 +
 tests/qemu-iotests/017                        |  3 +-
 tests/qemu-iotests/018                        |  3 +-
 tests/qemu-iotests/019                        |  3 +-
 tests/qemu-iotests/020                        |  3 +-
 tests/qemu-iotests/026                        |  4 +-
 tests/qemu-iotests/027                        |  1 +
 tests/qemu-iotests/032                        |  1 +
 tests/qemu-iotests/033                        |  1 +
 tests/qemu-iotests/034                        |  3 +-
 tests/qemu-iotests/037                        |  3 +-
 tests/qemu-iotests/039                        |  4 +-
 tests/qemu-iotests/052                        |  2 +-
 tests/qemu-iotests/059                        | 34 ++++++-
 tests/qemu-iotests/059.out                    | 26 +++--
 tests/qemu-iotests/063                        |  3 +-
 tests/qemu-iotests/071                        |  1 +
 tests/qemu-iotests/072                        |  1 +
 tests/qemu-iotests/081                        |  4 +-
 tests/qemu-iotests/091                        |  4 +-
 tests/qemu-iotests/099                        |  1 +
 tests/qemu-iotests/105                        |  3 +-
 tests/qemu-iotests/110                        |  3 +-
 tests/qemu-iotests/120                        |  1 +
 tests/qemu-iotests/126                        |  2 +
 tests/qemu-iotests/{150.out => 150.out.qcow2} |  0
 tests/qemu-iotests/150.out.raw                | 12 +++
 tests/qemu-iotests/162                        |  4 +-
 tests/qemu-iotests/175                        | 47 +++++++--
 tests/qemu-iotests/175.out                    | 16 ++-
 tests/qemu-iotests/178.out.qcow2              |  4 +-
 tests/qemu-iotests/184                        |  1 +
 tests/qemu-iotests/186                        |  1 +
 tests/qemu-iotests/197                        |  1 +
 tests/qemu-iotests/215                        |  1 +
 tests/qemu-iotests/221.out                    | 12 ++-
 tests/qemu-iotests/251                        |  1 +
 tests/qemu-iotests/253.out                    | 12 ++-
 tests/qemu-iotests/common.filter              |  4 +-
 tests/qemu-iotests/common.rc                  | 14 +++
 50 files changed, 391 insertions(+), 87 deletions(-)
 rename tests/qemu-iotests/{150.out => 150.out.qcow2} (100%)
 create mode 100644 tests/qemu-iotests/150.out.raw

-- 
2.21.0

From: Denis Plotnikov <dplotnikov@virtuozzo.com>

The patch allows to provide a pattern file for write
command. There was no similar ability before.

Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
Message-id: 20190820164616.4072-1-dplotnikov@virtuozzo.com
Reviewed-by: Eric Blake <eblake@redhat.com>
[mreitz: Keep optstring in alphabetical order]
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-io-cmds.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 93 insertions(+), 6 deletions(-)

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index XXXXXXX..XXXXXXX 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -XXX,XX +XXX,XX @@ static void qemu_io_free(void *p)
     qemu_vfree(p);
 }
 
+/*
+ * qemu_io_alloc_from_file()
+ *
+ * Allocates the buffer and populates it with the content of the given file
+ * up to @len bytes. If the file length is less than @len, then the buffer
+ * is populated with the file content cyclically.
+ *
+ * @blk - the block backend where the buffer content is going to be written to
+ * @len - the buffer length
+ * @file_name - the file to read the content from
+ *
+ * Returns: the buffer pointer on success
+ *          NULL on error
+ */
+static void *qemu_io_alloc_from_file(BlockBackend *blk, size_t len,
+                                     const char *file_name)
+{
+    char *buf, *buf_origin;
+    FILE *f = fopen(file_name, "r");
+    int pattern_len;
+
+    if (!f) {
+        perror(file_name);
+        return NULL;
+    }
+
+    if (qemuio_misalign) {
+        len += MISALIGN_OFFSET;
+    }
+
+    buf_origin = buf = blk_blockalign(blk, len);
+
+    if (qemuio_misalign) {
+        buf_origin += MISALIGN_OFFSET;
+        buf += MISALIGN_OFFSET;
+        len -= MISALIGN_OFFSET;
+    }
+
+    pattern_len = fread(buf_origin, 1, len, f);
+
+    if (ferror(f)) {
+        perror(file_name);
+        goto error;
+    }
+
+    if (pattern_len == 0) {
+        fprintf(stderr, "%s: file is empty\n", file_name);
+        goto error;
+    }
+
+    fclose(f);
+
+    if (len > pattern_len) {
+        len -= pattern_len;
+        buf += pattern_len;
+
+        while (len > 0) {
+            size_t len_to_copy = MIN(pattern_len, len);
+
+            memcpy(buf, buf_origin, len_to_copy);
+
+            len -= len_to_copy;
+            buf += len_to_copy;
+        }
+    }
+
+    return buf_origin;
+
+error:
+    qemu_io_free(buf_origin);
+    return NULL;
+}
+
 static void dump_buffer(const void *buffer, int64_t offset, int64_t len)
 {
     uint64_t i;
@@ -XXX,XX +XXX,XX @@ static void write_help(void)
 " -n, -- with -z, don't allow slow fallback\n"
 " -p, -- ignored for backwards compatibility\n"
 " -P, -- use different pattern to fill file\n"
+" -s, -- use a pattern file to fill the write buffer\n"
 " -C, -- report statistics in a machine parsable format\n"
 " -q, -- quiet mode, do not show I/O statistics\n"
 " -u, -- with -z, allow unmapping\n"
@@ -XXX,XX +XXX,XX @@ static const cmdinfo_t write_cmd = {
     .perm       = BLK_PERM_WRITE,
     .argmin     = 2,
     .argmax     = -1,
-    .args       = "[-bcCfnquz] [-P pattern] off len",
+    .args       = "[-bcCfnquz] [-P pattern | -s source_file] off len",
     .oneline    = "writes a number of bytes at a specified offset",
     .help       = write_help,
 };
@@ -XXX,XX +XXX,XX @@ static int write_f(BlockBackend *blk, int argc, char **argv)
 {
     struct timespec t1, t2;
     bool Cflag = false, qflag = false, bflag = false;
-    bool Pflag = false, zflag = false, cflag = false;
+    bool Pflag = false, zflag = false, cflag = false, sflag = false;
     int flags = 0;
     int c, cnt, ret;
     char *buf = NULL;
@@ -XXX,XX +XXX,XX @@ static int write_f(BlockBackend *blk, int argc, char **argv)
     /* Some compilers get confused and warn if this is not initialized.  */
     int64_t total = 0;
     int pattern = 0xcd;
+    const char *file_name = NULL;
 
-    while ((c = getopt(argc, argv, "bcCfnpP:quz")) != -1) {
+    while ((c = getopt(argc, argv, "bcCfnpP:qs:uz")) != -1) {
         switch (c) {
         case 'b':
             bflag = true;
@@ -XXX,XX +XXX,XX @@ static int write_f(BlockBackend *blk, int argc, char **argv)
         case 'q':
             qflag = true;
             break;
+        case 's':
+            sflag = true;
+            file_name = optarg;
+            break;
         case 'u':
             flags |= BDRV_REQ_MAY_UNMAP;
             break;
@@ -XXX,XX +XXX,XX @@ static int write_f(BlockBackend *blk, int argc, char **argv)
         return -EINVAL;
     }
 
-    if (zflag && Pflag) {
-        printf("-z and -P cannot be specified at the same time\n");
+    if (zflag + Pflag + sflag > 1) {
+        printf("Only one of -z, -P, and -s "
+               "can be specified at the same time\n");
         return -EINVAL;
     }
 
@@ -XXX,XX +XXX,XX @@ static int write_f(BlockBackend *blk, int argc, char **argv)
     }
 
     if (!zflag) {
-        buf = qemu_io_alloc(blk, count, pattern);
+        if (sflag) {
+            buf = qemu_io_alloc_from_file(blk, count, file_name);
+            if (!buf) {
+                return -EINVAL;
+            }
+        } else {
+            buf = qemu_io_alloc(blk, count, pattern);
+        }
     }
 
     clock_gettime(CLOCK_MONOTONIC, &t1);
-- 
2.21.0

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

It's wrong to OR shared permissions. It may lead to crash on further
permission updates.
Also, no needs to consider previously calculated permissions, as at
this point we already bind all new parents and bdrv_get_cumulative_perm
result is enough. So fix the bug by just set permissions by
bdrv_get_cumulative_perm result.

Bug was introduced in long ago 234ac1a9025, in 2.9.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190824100740.61635-1-vsementsov@virtuozzo.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index XXXXXXX..XXXXXXX 100644
--- a/block.c
+++ b/block.c
@@ -XXX,XX +XXX,XX @@ void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
 {
     BdrvChild *c, *next;
     GSList *list = NULL, *p;
-    uint64_t old_perm, old_shared;
     uint64_t perm = 0, shared = BLK_PERM_ALL;
     int ret;
 
@@ -XXX,XX +XXX,XX @@ void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
         bdrv_unref(from);
     }
 
-    bdrv_get_cumulative_perm(to, &old_perm, &old_shared);
-    bdrv_set_perm(to, old_perm | perm, old_shared | shared);
+    bdrv_get_cumulative_perm(to, &perm, &shared);
+    bdrv_set_perm(to, perm, shared);
 
 out:
     g_slist_free(list);
-- 
2.21.0

From: Nir Soffer <nirsof@gmail.com>

When creating an image with preallocation "off" or "falloc", the first
block of the image is typically not allocated. When using Gluster
storage backed by XFS filesystem, reading this block using direct I/O
succeeds regardless of request length, fooling alignment detection.

In this case we fallback to a safe value (4096) instead of the optimal
value (512), which may lead to unneeded data copying when aligning
requests.  Allocating the first block avoids the fallback.

Since we allocate the first block even with preallocation=off, we no
longer create images with zero disk size:

$ ./qemu-img create -f raw test.raw 1g
    Formatting 'test.raw', fmt=raw size=1073741824

$ ls -lhs test.raw
    4.0K -rw-r--r--. 1 nsoffer nsoffer 1.0G Aug 16 23:48 test.raw

And converting the image requires additional cluster:

$ ./qemu-img measure -f raw -O qcow2 test.raw
    required size: 458752
    fully allocated size: 1074135040

When using format like vmdk with multiple files per image, we allocate
one block per file:

$ ./qemu-img create -f vmdk -o subformat=twoGbMaxExtentFlat test.vmdk 4g
    Formatting 'test.vmdk', fmt=vmdk size=4294967296 compat6=off hwversion=undefined subformat=twoGbMaxExtentFlat

$ ls -lhs test*.vmdk
    4.0K -rw-r--r--. 1 nsoffer nsoffer 2.0G Aug 27 03:23 test-f001.vmdk
    4.0K -rw-r--r--. 1 nsoffer nsoffer 2.0G Aug 27 03:23 test-f002.vmdk
    4.0K -rw-r--r--. 1 nsoffer nsoffer  353 Aug 27 03:23 test.vmdk

I did quick performance test for copying disks with qemu-img convert to
new raw target image to Gluster storage with sector size of 512 bytes:

for i in $(seq 10); do
        rm -f dst.raw
        sleep 10
        time ./qemu-img convert -f raw -O raw -t none -T none src.raw dst.raw
    done

Here is a table comparing the total time spent:

Type    Before(s)   After(s)    Diff(%)
---------------------------------------
real      530.028    469.123      -11.4
user       17.204     10.768      -37.4
sys        17.881      7.011      -60.7

We can see very clear improvement in CPU usage.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
Message-id: 20190827010528.8818-2-nsoffer@redhat.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/file-posix.c                            | 51 +++++++++++++++++++
 tests/qemu-iotests/059.out                    |  2 +-
 tests/qemu-iotests/{150.out => 150.out.qcow2} |  0
 tests/qemu-iotests/150.out.raw                | 12 +++++
 tests/qemu-iotests/175                        | 19 ++++---
 tests/qemu-iotests/175.out                    |  8 +--
 tests/qemu-iotests/178.out.qcow2              |  4 +-
 tests/qemu-iotests/221.out                    | 12 +++--
 tests/qemu-iotests/253.out                    | 12 +++--
 9 files changed, 99 insertions(+), 21 deletions(-)
 rename tests/qemu-iotests/{150.out => 150.out.qcow2} (100%)
 create mode 100644 tests/qemu-iotests/150.out.raw

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 handle_aiocb_discard(void *opaque)
     return ret;
 }
 
+/*
+ * Help alignment probing by allocating the first block.
+ *
+ * When reading with direct I/O from unallocated area on Gluster backed by XFS,
+ * reading succeeds regardless of request length. In this case we fallback to
+ * safe alignment which is not optimal. Allocating the first block avoids this
+ * fallback.
+ *
+ * fd may be opened with O_DIRECT, but we don't know the buffer alignment or
+ * request alignment, so we use safe values.
+ *
+ * Returns: 0 on success, -errno on failure. Since this is an optimization,
+ * caller may ignore failures.
+ */
+static int allocate_first_block(int fd, size_t max_size)
+{
+    size_t write_size = (max_size < MAX_BLOCKSIZE)
+        ? BDRV_SECTOR_SIZE
+        : MAX_BLOCKSIZE;
+    size_t max_align = MAX(MAX_BLOCKSIZE, getpagesize());
+    void *buf;
+    ssize_t n;
+    int ret;
+
+    buf = qemu_memalign(max_align, write_size);
+    memset(buf, 0, write_size);
+
+    do {
+        n = pwrite(fd, buf, write_size, 0);
+    } while (n == -1 && errno == EINTR);
+
+    ret = (n == -1) ? -errno : 0;
+
+    qemu_vfree(buf);
+    return ret;
+}
+
 static int handle_aiocb_truncate(void *opaque)
 {
     RawPosixAIOData *aiocb = opaque;
@@ -XXX,XX +XXX,XX @@ static int handle_aiocb_truncate(void *opaque)
                 /* posix_fallocate() doesn't set errno. */
                 error_setg_errno(errp, -result,
                                  "Could not preallocate new data");
+            } else if (current_length == 0) {
+                /*
+                 * posix_fallocate() uses fallocate() if the filesystem
+                 * supports it, or fallback to manually writing zeroes. If
+                 * fallocate() was used, unaligned reads from the fallocated
+                 * area in raw_probe_alignment() will succeed, hence we need to
+                 * allocate the first block.
+                 *
+                 * Optimize future alignment probing; ignore failures.
+                 */
+                allocate_first_block(fd, offset);
             }
         } else {
             result = 0;
@@ -XXX,XX +XXX,XX @@ static int handle_aiocb_truncate(void *opaque)
         if (ftruncate(fd, offset) != 0) {
             result = -errno;
             error_setg_errno(errp, -result, "Could not resize file");
+        } else if (current_length == 0 && offset > current_length) {
+            /* Optimize future alignment probing; ignore failures. */
+            allocate_first_block(fd, offset);
         }
         return result;
     default:
diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824000 subformat=twoGbMax
 image: TEST_DIR/t.vmdk
 file format: vmdk
 virtual size: 0.977 TiB (1073741824000 bytes)
-disk size: 16 KiB
+disk size: 1.97 MiB
 Format specific information:
     cid: XXXXXXXX
     parent cid: XXXXXXXX
diff --git a/tests/qemu-iotests/150.out b/tests/qemu-iotests/150.out.qcow2
similarity index 100%
rename from tests/qemu-iotests/150.out
rename to tests/qemu-iotests/150.out.qcow2
diff --git a/tests/qemu-iotests/150.out.raw b/tests/qemu-iotests/150.out.raw
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/150.out.raw
@@ -XXX,XX +XXX,XX @@
+QA output created by 150
+
+=== Mapping sparse conversion ===
+
+Offset          Length          File
+0               0x1000          TEST_DIR/t.IMGFMT
+
+=== Mapping non-sparse conversion ===
+
+Offset          Length          File
+0               0x100000        TEST_DIR/t.IMGFMT
+*** done
diff --git a/tests/qemu-iotests/175 b/tests/qemu-iotests/175
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/175
+++ b/tests/qemu-iotests/175
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 # the file size.  This function hides the resulting difference in the
 # stat -c '%b' output.
 # Parameter 1: Number of blocks an empty file occupies
-# Parameter 2: Image size in bytes
+# Parameter 2: Minimal number of blocks in an image
+# Parameter 3: Image size in bytes
 _filter_blocks()
 {
     extra_blocks=$1
-    img_size=$2
+    min_blocks=$2
+    img_size=$3
 
-    sed -e "s/blocks=$extra_blocks\$\$\\|[^0-9]\$/nothing allocated/" \
-        -e "s/blocks=$((extra_blocks + img_size / 512))\$\$\\|[^0-9]\$/everything allocated/"
+    sed -e "s/blocks=$min_blocks\$\$\\|[^0-9]\$/min allocation/" \
+        -e "s/blocks=$((extra_blocks + img_size / 512))\$\$\\|[^0-9]\$/max allocation/"
 }
 
 # get standard environment, filters and checks
@@ -XXX,XX +XXX,XX @@ size=$((1 * 1024 * 1024))
 touch "$TEST_DIR/empty"
 extra_blocks=$(stat -c '%b' "$TEST_DIR/empty")
 
+# We always write the first byte; check how many blocks this filesystem
+# allocates to match empty image alloation.
+printf "\0" > "$TEST_DIR/empty"
+min_blocks=$(stat -c '%b' "$TEST_DIR/empty")
+
 echo
 echo "== creating image with default preallocation =="
 _make_test_img $size | _filter_imgfmt
-stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $size
+stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $min_blocks $size
 
 for mode in off full falloc; do
     echo
     echo "== creating image with preallocation $mode =="
     IMGOPTS=preallocation=$mode _make_test_img $size | _filter_imgfmt
-    stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $size
+    stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $min_blocks $size
 done
 
 # success, all done
diff --git a/tests/qemu-iotests/175.out b/tests/qemu-iotests/175.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/175.out
+++ b/tests/qemu-iotests/175.out
@@ -XXX,XX +XXX,XX @@ QA output created by 175
 
 == creating image with default preallocation ==
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
-size=1048576, nothing allocated
+size=1048576, min allocation
 
 == creating image with preallocation off ==
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=off
-size=1048576, nothing allocated
+size=1048576, min allocation
 
 == creating image with preallocation full ==
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=full
-size=1048576, everything allocated
+size=1048576, max allocation
 
 == creating image with preallocation falloc ==
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=falloc
-size=1048576, everything allocated
+size=1048576, max allocation
  *** done
diff --git a/tests/qemu-iotests/178.out.qcow2 b/tests/qemu-iotests/178.out.qcow2
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/178.out.qcow2
+++ b/tests/qemu-iotests/178.out.qcow2
@@ -XXX,XX +XXX,XX @@ converted image file size in bytes: 196608
 == raw input image with data (human) ==
 
 Formatting 'TEST_DIR/t.qcow2', fmt=IMGFMT size=1073741824
-required size: 393216
+required size: 458752
 fully allocated size: 1074135040
 wrote 512/512 bytes at offset 512
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -XXX,XX +XXX,XX @@ converted image file size in bytes: 196608
 
 Formatting 'TEST_DIR/t.qcow2', fmt=IMGFMT size=1073741824
 {
-    "required": 393216,
+    "required": 458752,
     "fully-allocated": 1074135040
 }
 wrote 512/512 bytes at offset 512
diff --git a/tests/qemu-iotests/221.out b/tests/qemu-iotests/221.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/221.out
+++ b/tests/qemu-iotests/221.out
@@ -XXX,XX +XXX,XX @@ QA output created by 221
 === Check mapping of unaligned raw image ===
 
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=65537
-[{ "start": 0, "length": 66048, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]
-[{ "start": 0, "length": 66048, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]
+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
+{ "start": 4096, "length": 61952, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]
+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
+{ "start": 4096, "length": 61952, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]
 wrote 1/1 bytes at offset 65536
 1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-[{ "start": 0, "length": 65536, "depth": 0, "zero": true, "data": false, "offset": OFFSET},
+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
+{ "start": 4096, "length": 61440, "depth": 0, "zero": true, "data": false, "offset": OFFSET},
 { "start": 65536, "length": 1, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
 { "start": 65537, "length": 511, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]
-[{ "start": 0, "length": 65536, "depth": 0, "zero": true, "data": false, "offset": OFFSET},
+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
+{ "start": 4096, "length": 61440, "depth": 0, "zero": true, "data": false, "offset": OFFSET},
 { "start": 65536, "length": 1, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
 { "start": 65537, "length": 511, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]
 *** done
diff --git a/tests/qemu-iotests/253.out b/tests/qemu-iotests/253.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/253.out
+++ b/tests/qemu-iotests/253.out
@@ -XXX,XX +XXX,XX @@ QA output created by 253
 === Check mapping of unaligned raw image ===
 
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048575
-[{ "start": 0, "length": 1048576, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]
-[{ "start": 0, "length": 1048576, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]
+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
+{ "start": 4096, "length": 1044480, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]
+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
+{ "start": 4096, "length": 1044480, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]
 wrote 65535/65535 bytes at offset 983040
 63.999 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-[{ "start": 0, "length": 983040, "depth": 0, "zero": true, "data": false, "offset": OFFSET},
+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
+{ "start": 4096, "length": 978944, "depth": 0, "zero": true, "data": false, "offset": OFFSET},
 { "start": 983040, "length": 65536, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
-[{ "start": 0, "length": 983040, "depth": 0, "zero": true, "data": false, "offset": OFFSET},
+[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
+{ "start": 4096, "length": 978944, "depth": 0, "zero": true, "data": false, "offset": OFFSET},
 { "start": 983040, "length": 65536, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
 *** done
-- 
2.21.0

From: Nir Soffer <nirsof@gmail.com>

Using block_resize we can test allocate_first_block() with file
descriptor opened with O_DIRECT, ensuring that it works for any size
larger than 4096 bytes.

Testing smaller sizes is tricky as the result depends on the filesystem
used for testing. For example on NFS any size will work since O_DIRECT
does not require any alignment.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 20190827010528.8818-3-nsoffer@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/175     | 28 ++++++++++++++++++++++++++++
 tests/qemu-iotests/175.out |  8 ++++++++
 2 files changed, 36 insertions(+)

diff --git a/tests/qemu-iotests/175 b/tests/qemu-iotests/175
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/175
+++ b/tests/qemu-iotests/175
@@ -XXX,XX +XXX,XX @@ _filter_blocks()
         -e "s/blocks=$((extra_blocks + img_size / 512))\$\$\\|[^0-9]\$/max allocation/"
 }
 
+# Resize image using block_resize.
+# Parameter 1: image path
+# Parameter 2: new size
+_block_resize()
+{
+    local path=$1
+    local size=$2
+
+    $QEMU -qmp stdio -nographic -nodefaults \
+        -blockdev file,node-name=file,filename=$path,cache.direct=on \
+        <<EOF
+{'execute': 'qmp_capabilities'}
+{'execute': 'block_resize', 'arguments': {'node-name': 'file', 'size': $size}}
+{'execute': 'quit'}
+EOF
+}
+
 # get standard environment, filters and checks
 . ./common.rc
 . ./common.filter
@@ -XXX,XX +XXX,XX @@ _supported_fmt raw
 _supported_proto file
 _supported_os Linux
 
+_default_cache_mode none
+_supported_cache_modes none directsync
+
 size=$((1 * 1024 * 1024))
 
 touch "$TEST_DIR/empty"
@@ -XXX,XX +XXX,XX @@ for mode in off full falloc; do
     stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $min_blocks $size
 done
 
+for new_size in 4096 1048576; do
+    echo
+    echo "== resize empty image with block_resize =="
+    _make_test_img 0 | _filter_imgfmt
+    _block_resize $TEST_IMG $new_size >/dev/null
+    stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $min_blocks $new_size
+done
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/175.out b/tests/qemu-iotests/175.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/175.out
+++ b/tests/qemu-iotests/175.out
@@ -XXX,XX +XXX,XX @@ size=1048576, max allocation
 == creating image with preallocation falloc ==
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=falloc
 size=1048576, max allocation
+
+== resize empty image with block_resize ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=0
+size=4096, min allocation
+
+== resize empty image with block_resize ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=0
+size=1048576, min allocation
  *** done
-- 
2.21.0

fe646693acc changed qemu-img create's output so that it no longer prints
single quotes around parameter values.  The subformat and adapter_type
filters in _filter_img_create() have never been adapted to that change.

Fixes: fe646693acc13ac48b98435d14149ab04dc597bc
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 20190815153638.4600-2-mreitz@redhat.com
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/059.out       | 16 ++++++++--------
 tests/qemu-iotests/common.filter |  4 ++--
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 qemu-io: can't open device TEST_DIR/t.vmdk: L1 size too big
 
 === Testing monolithicFlat creation and opening ===
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2147483648 subformat=monolithicFlat
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2147483648
 image: TEST_DIR/t.IMGFMT
 file format: IMGFMT
 virtual size: 2 GiB (2147483648 bytes)
 
 === Testing monolithicFlat with zeroed_grain ===
 qemu-img: TEST_DIR/t.IMGFMT: Flat image can't enable zeroed grain
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2147483648 subformat=monolithicFlat
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2147483648
 
 === Testing big twoGbMaxExtentFlat ===
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824000 subformat=twoGbMaxExtentFlat
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824000
 image: TEST_DIR/t.vmdk
 file format: vmdk
 virtual size: 0.977 TiB (1073741824000 bytes)
@@ -XXX,XX +XXX,XX @@ Format specific information:
 qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Invalid extent line: RW 12582912 VMFS "dummy.IMGFMT" 1
 
 === Testing truncated sparse ===
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=107374182400 subformat=monolithicSparse
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=107374182400
 qemu-img: Could not open 'TEST_DIR/t.IMGFMT': File truncated, expecting at least 13172736 bytes
 
 === Converting to streamOptimized from image with small cluster size===
@@ -XXX,XX +XXX,XX @@ wrote 512/512 bytes at offset 10240
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 === Testing monolithicFlat with internally generated JSON file name ===
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 subformat=monolithicFlat
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 qemu-io: can't open: Cannot use relative extent paths with VMDK descriptor file 'json:{"image": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, "driver": "blkdebug", "inject-error.0.event": "read_aio"}'
 
 === Testing version 3 ===
@@ -XXX,XX +XXX,XX @@ read 512/512 bytes at offset 64931328
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 === Testing 4TB monolithicFlat creation and IO ===
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4398046511104 subformat=monolithicFlat
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4398046511104
 image: TEST_DIR/t.IMGFMT
 file format: IMGFMT
 virtual size: 4 TiB (4398046511104 bytes)
@@ -XXX,XX +XXX,XX @@ read 1024/1024 bytes at offset 966367641600
 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 === Testing qemu-img map on extents ===
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33285996544 subformat=monolithicSparse
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33285996544
 wrote 1024/1024 bytes at offset 65024
 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 wrote 1024/1024 bytes at offset 2147483136
@@ -XXX,XX +XXX,XX @@ Offset          Length          Mapped to       File
 0               0x20000         0x3f0000        TEST_DIR/t.vmdk
 0x7fff0000      0x20000         0x410000        TEST_DIR/t.vmdk
 0x140000000     0x10000         0x430000        TEST_DIR/t.vmdk
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33285996544 subformat=twoGbMaxExtentSparse
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33285996544
 wrote 1024/1024 bytes at offset 65024
 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 wrote 1024/1024 bytes at offset 2147483136
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -XXX,XX +XXX,XX @@ _filter_img_create()
         -e "s# compat6=\$on\\|off\$##g" \
         -e "s# static=\$on\\|off\$##g" \
         -e "s# zeroed_grain=\$on\\|off\$##g" \
-        -e "s# subformat='[^']*'##g" \
-        -e "s# adapter_type='[^']*'##g" \
+        -e "s# subformat=[^ ]*##g" \
+        -e "s# adapter_type=[^ ]*##g" \
         -e "s# hwversion=[^ ]*##g" \
         -e "s# lazy_refcounts=\$on\\|off\$##g" \
         -e "s# block_size=[0-9]\\+##g" \
-- 
2.21.0

This makes iotest 033 pass with e.g. subformat=monolithicFlat.  It also
turns a former error in 059 into success.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20190815153638.4600-3-mreitz@redhat.com
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/vmdk.c               | 54 ++++++++++++++++++++++++--------------
 tests/qemu-iotests/059     |  7 +++--
 tests/qemu-iotests/059.out |  4 ++-
 3 files changed, 42 insertions(+), 23 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index XXXXXXX..XXXXXXX 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -XXX,XX +XXX,XX @@ static const char *next_line(const char *s)
 }
 
 static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
-                              const char *desc_file_path, QDict *options,
-                              Error **errp)
+                              QDict *options, Error **errp)
 {
     int ret;
     int matches;
@@ -XXX,XX +XXX,XX @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
     const char *p, *np;
     int64_t sectors = 0;
     int64_t flat_offset;
+    char *desc_file_dir = NULL;
     char *extent_path;
     BdrvChild *extent_file;
     BDRVVmdkState *s = bs->opaque;
@@ -XXX,XX +XXX,XX @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
             continue;
         }
 
-        if (!path_is_absolute(fname) && !path_has_protocol(fname) &&
-            !desc_file_path[0])
-        {
-            bdrv_refresh_filename(bs->file->bs);
-            error_setg(errp, "Cannot use relative extent paths with VMDK "
-                       "descriptor file '%s'", bs->file->bs->filename);
-            return -EINVAL;
-        }
+        if (path_is_absolute(fname)) {
+            extent_path = g_strdup(fname);
+        } else {
+            if (!desc_file_dir) {
+                desc_file_dir = bdrv_dirname(bs->file->bs, errp);
+                if (!desc_file_dir) {
+                    bdrv_refresh_filename(bs->file->bs);
+                    error_prepend(errp, "Cannot use relative paths with VMDK "
+                                  "descriptor file '%s': ",
+                                  bs->file->bs->filename);
+                    ret = -EINVAL;
+                    goto out;
+                }
+            }
 
-        extent_path = path_combine(desc_file_path, fname);
+            extent_path = g_strconcat(desc_file_dir, fname, NULL);
+        }
 
         ret = snprintf(extent_opt_prefix, 32, "extents.%d", s->num_extents);
         assert(ret < 32);
@@ -XXX,XX +XXX,XX @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
         g_free(extent_path);
         if (local_err) {
             error_propagate(errp, local_err);
-            return -EINVAL;
+            ret = -EINVAL;
+            goto out;
         }
 
         /* save to extents array */
@@ -XXX,XX +XXX,XX @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
                             0, 0, 0, 0, 0, &extent, errp);
             if (ret < 0) {
                 bdrv_unref_child(bs, extent_file);
-                return ret;
+                goto out;
             }
             extent->flat_start_offset = flat_offset << 9;
         } else if (!strcmp(type, "SPARSE") || !strcmp(type, "VMFSSPARSE")) {
@@ -XXX,XX +XXX,XX @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
             g_free(buf);
             if (ret) {
                 bdrv_unref_child(bs, extent_file);
-                return ret;
+                goto out;
             }
             extent = &s->extents[s->num_extents - 1];
         } else if (!strcmp(type, "SESPARSE")) {
             ret = vmdk_open_se_sparse(bs, extent_file, bs->open_flags, errp);
             if (ret) {
                 bdrv_unref_child(bs, extent_file);
-                return ret;
+                goto out;
             }
             extent = &s->extents[s->num_extents - 1];
         } else {
             error_setg(errp, "Unsupported extent type '%s'", type);
             bdrv_unref_child(bs, extent_file);
-            return -ENOTSUP;
+            ret = -ENOTSUP;
+            goto out;
         }
         extent->type = g_strdup(type);
     }
-    return 0;
+
+    ret = 0;
+    goto out;
 
 invalid:
     np = next_line(p);
@@ -XXX,XX +XXX,XX @@ invalid:
         np--;
     }
     error_setg(errp, "Invalid extent line: %.*s", (int)(np - p), p);
-    return -EINVAL;
+    ret = -EINVAL;
+
+out:
+    g_free(desc_file_dir);
+    return ret;
 }
 
 static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf,
@@ -XXX,XX +XXX,XX @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf,
     }
     s->create_type = g_strdup(ct);
     s->desc_offset = 0;
-    ret = vmdk_parse_extents(buf, bs, bs->file->bs->exact_filename, options,
-                             errp);
+    ret = vmdk_parse_extents(buf, bs, options, errp);
 exit:
     return ret;
 }
diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/059
+++ b/tests/qemu-iotests/059
@@ -XXX,XX +XXX,XX @@ $QEMU_IMG convert -f qcow2 -O vmdk -o subformat=streamOptimized "$TEST_IMG.qcow2
 
 echo
 echo "=== Testing monolithicFlat with internally generated JSON file name ==="
+# Should work, because bdrv_dirname() works fine with blkdebug
 IMGOPTS="subformat=monolithicFlat" _make_test_img 64M
-$QEMU_IO -c "open -o driver=$IMGFMT,file.driver=blkdebug,file.image.filename=$TEST_IMG,file.inject-error.0.event=read_aio" 2>&1 \
-    | _filter_testdir | _filter_imgfmt
+$QEMU_IO -c "open -o driver=$IMGFMT,file.driver=blkdebug,file.image.filename=$TEST_IMG,file.inject-error.0.event=read_aio" \
+         -c info \
+    2>&1 \
+    | _filter_testdir | _filter_imgfmt | _filter_img_info
 _cleanup_test_img
 
 echo
diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -XXX,XX +XXX,XX @@ wrote 512/512 bytes at offset 10240
 
 === Testing monolithicFlat with internally generated JSON file name ===
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-qemu-io: can't open: Cannot use relative extent paths with VMDK descriptor file 'json:{"image": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, "driver": "blkdebug", "inject-error.0.event": "read_aio"}'
+format name: IMGFMT
+cluster size: 0 bytes
+vm state offset: 0 bytes
 
 === Testing version 3 ===
 image: TEST_DIR/iotest-version3.IMGFMT
-- 
2.21.0

We had a test for a case where relative extent paths did not work, but
unfortunately we just fixed the underlying problem, so it works now.
This patch adds a new test case that still fails.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 20190815153638.4600-4-mreitz@redhat.com
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/059     | 27 +++++++++++++++++++++++++++
 tests/qemu-iotests/059.out |  4 ++++
 2 files changed, 31 insertions(+)

diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/059
+++ b/tests/qemu-iotests/059
@@ -XXX,XX +XXX,XX @@ $QEMU_IMG convert -f qcow2 -O vmdk -o subformat=streamOptimized "$TEST_IMG.qcow2
 
 echo
 echo "=== Testing monolithicFlat with internally generated JSON file name ==="
+
+echo '--- blkdebug ---'
 # Should work, because bdrv_dirname() works fine with blkdebug
 IMGOPTS="subformat=monolithicFlat" _make_test_img 64M
 $QEMU_IO -c "open -o driver=$IMGFMT,file.driver=blkdebug,file.image.filename=$TEST_IMG,file.inject-error.0.event=read_aio" \
@@ -XXX,XX +XXX,XX @@ $QEMU_IO -c "open -o driver=$IMGFMT,file.driver=blkdebug,file.image.filename=$TE
     | _filter_testdir | _filter_imgfmt | _filter_img_info
 _cleanup_test_img
 
+echo '--- quorum ---'
+# Should not work, because bdrv_dirname() does not work with quorum
+IMGOPTS="subformat=monolithicFlat" _make_test_img 64M
+cp "$TEST_IMG" "$TEST_IMG.orig"
+
+filename="json:{
+    \"driver\": \"$IMGFMT\",
+    \"file\": {
+        \"driver\": \"quorum\",
+        \"children\": [ {
+            \"driver\": \"file\",
+            \"filename\": \"$TEST_IMG\"
+        }, {
+            \"driver\": \"file\",
+            \"filename\": \"$TEST_IMG.orig\"
+        } ],
+        \"vote-threshold\": 1
+    } }"
+
+filename=$(echo "$filename" | tr '\n' ' ' | sed -e 's/\s\+/ /g')
+$QEMU_IMG info "$filename" 2>&1 \
+    | sed -e "s/'json:[^']*'/\$QUORUM_FILE/g" \
+    | _filter_testdir | _filter_imgfmt | _filter_img_info
+
+
 echo
 echo "=== Testing version 3 ==="
 _use_sample_img iotest-version3.vmdk.bz2
diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -XXX,XX +XXX,XX @@ wrote 512/512 bytes at offset 10240
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 === Testing monolithicFlat with internally generated JSON file name ===
+--- blkdebug ---
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 format name: IMGFMT
 cluster size: 0 bytes
 vm state offset: 0 bytes
+--- quorum ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+qemu-img: Could not open $QUORUM_FILE: Cannot use relative paths with VMDK descriptor file $QUORUM_FILE: Cannot generate a base directory for quorum nodes
 
 === Testing version 3 ===
 image: TEST_DIR/iotest-version3.IMGFMT
-- 
2.21.0

Compressed writes generally have to write full clusters, not just in
theory but also in practice when it comes to vmdk's streamOptimized
subformat.  It currently is just silently broken for writes with
non-zero in-cluster offsets:

$ qemu-img create -f vmdk -o subformat=streamOptimized foo.vmdk 1M
$ qemu-io -c 'write 4k 4k' -c 'read 4k 4k' foo.vmdk
wrote 4096/4096 bytes at offset 4096
4 KiB, 1 ops; 00.01 sec (443.724 KiB/sec and 110.9309 ops/sec)
read failed: Invalid argument

(The technical reason is that vmdk_write_extent() just writes the
incomplete compressed data actually to offset 4k.  When reading the
data, vmdk_read_extent() looks at offset 0 and finds the compressed data
size to be 0, because that is what it reads from there.  This yields an
error.)

For incomplete writes with zero in-cluster offsets, the error path when
reading the rest of the cluster is a bit different, but the result is
the same:

$ qemu-img create -f vmdk -o subformat=streamOptimized foo.vmdk 1M
$ qemu-io -c 'write 0k 4k' -c 'read 4k 4k' foo.vmdk
wrote 4096/4096 bytes at offset 0
4 KiB, 1 ops; 00.01 sec (362.641 KiB/sec and 90.6603 ops/sec)
read failed: Invalid argument

(Here, vmdk_read_extent() finds the data and then sees that the
uncompressed data is short.)

It is better to reject invalid writes than to make the user believe they
might have succeeded and then fail when trying to read it back.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 20190815153638.4600-5-mreitz@redhat.com
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/vmdk.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/block/vmdk.c b/block/vmdk.c
index XXXXXXX..XXXXXXX 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -XXX,XX +XXX,XX @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset,
     if (extent->compressed) {
         void *compressed_data;
 
+        /* Only whole clusters */
+        if (offset_in_cluster ||
+            n_bytes > (extent->cluster_sectors * SECTOR_SIZE) ||
+            (n_bytes < (extent->cluster_sectors * SECTOR_SIZE) &&
+             offset + n_bytes != extent->end_sector * SECTOR_SIZE))
+        {
+            ret = -EINVAL;
+            goto out;
+        }
+
         if (!extent->has_marker) {
             ret = -EINVAL;
             goto out;
-- 
2.21.0

streamOptimized does not support writes that do not span exactly one
cluster.  Furthermore, it cannot rewrite already allocated clusters.
As such, many iotests do not work with it.  Disable them.

diff --git a/tests/qemu-iotests/002 b/tests/qemu-iotests/002
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/002
+++ b/tests/qemu-iotests/002
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt generic
 _supported_proto generic
+_unsupported_imgopts "subformat=streamOptimized"
 
 
 size=128M
diff --git a/tests/qemu-iotests/003 b/tests/qemu-iotests/003
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/003
+++ b/tests/qemu-iotests/003
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt generic
 _supported_proto generic
+_unsupported_imgopts "subformat=streamOptimized"
 
 size=128M
 offset=67M
diff --git a/tests/qemu-iotests/005 b/tests/qemu-iotests/005
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/005
+++ b/tests/qemu-iotests/005
@@ -XXX,XX +XXX,XX @@ _supported_fmt generic
 _supported_proto generic
 _supported_os Linux
 _unsupported_imgopts "subformat=twoGbMaxExtentFlat" \
-                     "subformat=twoGbMaxExtentSparse"
+                     "subformat=twoGbMaxExtentSparse" \
+                     "subformat=streamOptimized"
 
 # vpc is limited to 127GB, so we can't test it here
 if [ "$IMGFMT" = "vpc" ]; then
diff --git a/tests/qemu-iotests/009 b/tests/qemu-iotests/009
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/009
+++ b/tests/qemu-iotests/009
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt generic
 _supported_proto generic
+_unsupported_imgopts "subformat=streamOptimized"
 
 
 size=6G
diff --git a/tests/qemu-iotests/010 b/tests/qemu-iotests/010
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/010
+++ b/tests/qemu-iotests/010
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt generic
 _supported_proto generic
+_unsupported_imgopts "subformat=streamOptimized"
 
 
 size=6G
diff --git a/tests/qemu-iotests/011 b/tests/qemu-iotests/011
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/011
+++ b/tests/qemu-iotests/011
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt generic
 _supported_proto generic
+_unsupported_imgopts "subformat=streamOptimized"
 
 
 size=6G
diff --git a/tests/qemu-iotests/017 b/tests/qemu-iotests/017
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/017
+++ b/tests/qemu-iotests/017
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt qcow qcow2 vmdk qed
 _supported_proto generic
 _unsupported_proto vxhs
-_unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat"
+_unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat" \
+                     "subformat=streamOptimized"
 
 TEST_OFFSETS="0 4294967296"
 
diff --git a/tests/qemu-iotests/018 b/tests/qemu-iotests/018
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/018
+++ b/tests/qemu-iotests/018
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt qcow qcow2 vmdk qed
 _supported_proto file
 _supported_os Linux
-_unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat"
+_unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat" \
+                     "streamOptimized"
 
 TEST_OFFSETS="0 4294967296"
 
diff --git a/tests/qemu-iotests/019 b/tests/qemu-iotests/019
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/019
+++ b/tests/qemu-iotests/019
@@ -XXX,XX +XXX,XX @@ _supported_proto file
 _supported_os Linux
 _unsupported_imgopts "subformat=monolithicFlat" \
                      "subformat=twoGbMaxExtentFlat" \
-                     "subformat=twoGbMaxExtentSparse"
+                     "subformat=twoGbMaxExtentSparse" \
+                     "subformat=streamOptimized"
 
 TEST_OFFSETS="0 4294967296"
 CLUSTER_SIZE=65536
diff --git a/tests/qemu-iotests/020 b/tests/qemu-iotests/020
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/020
+++ b/tests/qemu-iotests/020
@@ -XXX,XX +XXX,XX @@ _supported_fmt qcow qcow2 vmdk qed
 _supported_proto file
 _unsupported_imgopts "subformat=monolithicFlat" \
                      "subformat=twoGbMaxExtentFlat" \
-                     "subformat=twoGbMaxExtentSparse"
+                     "subformat=twoGbMaxExtentSparse" \
+                     "subformat=streamOptimized"
 
 TEST_OFFSETS="0 4294967296"
 
diff --git a/tests/qemu-iotests/027 b/tests/qemu-iotests/027
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/027
+++ b/tests/qemu-iotests/027
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt vmdk qcow qcow2 qed
 _supported_proto generic
+_unsupported_imgopts "subformat=streamOptimized"
 
 
 size=128M
diff --git a/tests/qemu-iotests/032 b/tests/qemu-iotests/032
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/032
+++ b/tests/qemu-iotests/032
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 # This works for any image format (though unlikely to segfault for raw)
 _supported_fmt generic
 _supported_proto generic
+_unsupported_imgopts "subformat=streamOptimized"
 
 echo
 echo === Prepare image ===
diff --git a/tests/qemu-iotests/033 b/tests/qemu-iotests/033
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/033
+++ b/tests/qemu-iotests/033
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt generic
 _supported_proto generic
+_unsupported_imgopts "subformat=streamOptimized"
 
 
 size=128M
diff --git a/tests/qemu-iotests/034 b/tests/qemu-iotests/034
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/034
+++ b/tests/qemu-iotests/034
@@ -XXX,XX +XXX,XX @@ _supported_proto file
 _supported_os Linux
 _unsupported_imgopts "subformat=monolithicFlat" \
                      "subformat=twoGbMaxExtentFlat" \
-                     "subformat=twoGbMaxExtentSparse"
+                     "subformat=twoGbMaxExtentSparse" \
+                     "subformat=streamOptimized"
 
 CLUSTER_SIZE=4k
 size=128M
diff --git a/tests/qemu-iotests/037 b/tests/qemu-iotests/037
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/037
+++ b/tests/qemu-iotests/037
@@ -XXX,XX +XXX,XX @@ _supported_fmt qcow qcow2 vmdk qed
 _supported_proto file
 _unsupported_imgopts "subformat=monolithicFlat" \
                      "subformat=twoGbMaxExtentFlat" \
-                     "subformat=twoGbMaxExtentSparse"
+                     "subformat=twoGbMaxExtentSparse" \
+                     "subformat=streamOptimized"
 
 CLUSTER_SIZE=4k
 size=128M
diff --git a/tests/qemu-iotests/063 b/tests/qemu-iotests/063
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/063
+++ b/tests/qemu-iotests/063
@@ -XXX,XX +XXX,XX @@ _supported_fmt qcow qcow2 vmdk qed raw
 _supported_proto file
 _unsupported_imgopts "subformat=monolithicFlat" \
                      "subformat=twoGbMaxExtentFlat" \
-                     "subformat=twoGbMaxExtentSparse"
+                     "subformat=twoGbMaxExtentSparse" \
+                     "subformat=streamOptimized"
 
 _make_test_img 4M
 
diff --git a/tests/qemu-iotests/072 b/tests/qemu-iotests/072
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/072
+++ b/tests/qemu-iotests/072
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt vpc vmdk vhdx vdi qed qcow2 qcow
 _supported_proto file
+_unsupported_imgopts "subformat=streamOptimized"
 
 IMG_SIZE=64M
 
diff --git a/tests/qemu-iotests/105 b/tests/qemu-iotests/105
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/105
+++ b/tests/qemu-iotests/105
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt qcow2 vmdk vhdx qed
 _supported_proto generic
 _unsupported_imgopts "subformat=twoGbMaxExtentFlat" \
-                     "subformat=twoGbMaxExtentSparse"
+                     "subformat=twoGbMaxExtentSparse" \
+                     "subformat=streamOptimized"
 
 echo
 echo "creating large image"
diff --git a/tests/qemu-iotests/197 b/tests/qemu-iotests/197
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/197
+++ b/tests/qemu-iotests/197
@@ -XXX,XX +XXX,XX @@ _supported_fmt generic
 _supported_proto generic
 # LUKS support may be possible, but it complicates things.
 _unsupported_fmt luks
+_unsupported_imgopts "subformat=streamOptimized"
 
 echo
 echo '=== Copy-on-read ==='
diff --git a/tests/qemu-iotests/215 b/tests/qemu-iotests/215
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/215
+++ b/tests/qemu-iotests/215
@@ -XXX,XX +XXX,XX @@ _supported_fmt generic
 _supported_proto generic
 # LUKS support may be possible, but it complicates things.
 _unsupported_fmt luks
+_unsupported_imgopts "subformat=streamOptimized"
 
 echo
 echo '=== Copy-on-read ==='
diff --git a/tests/qemu-iotests/251 b/tests/qemu-iotests/251
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/251
+++ b/tests/qemu-iotests/251
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt generic
 _supported_proto file
 _supported_os Linux
+_unsupported_imgopts "subformat=streamOptimized"
 
 if [ "$IMGOPTSSYNTAX" = "true" ]; then
     # We use json:{} filenames here, so we cannot work with additional options.
-- 
2.21.0

The error message for the test case where we have a quorum node for
which no directory name can be generated is different: For
twoGbMaxExtentSparse, it complains that it cannot open the extent file.
For other (sub)formats, it just notes that it cannot determine the
backing file path.  Both are fine, but just disable twoGbMaxExtentSparse
for simplicity's sake.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 20190815153638.4600-7-mreitz@redhat.com
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/110 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/110 b/tests/qemu-iotests/110
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/110
+++ b/tests/qemu-iotests/110
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 # Any format supporting backing files
 _supported_fmt qed qcow qcow2 vmdk
 _supported_proto file
-_unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat"
+_unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat" \
+                     "subformat=twoGbMaxExtentSparse"
 
 TEST_IMG_REL=$(basename "$TEST_IMG")
 
-- 
2.21.0

From: Stefan Hajnoczi <stefanha@redhat.com>

Fixes: a6b257a08e3d72219f03e461a52152672fec0612
       ("file-posix: Handle undetectable alignment")
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20190827101328.4062-1-stefanha@redhat.com
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/file-posix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
         for (i = 0; i < ARRAY_SIZE(alignments); i++) {
             align = alignments[i];
             if (raw_is_io_aligned(fd, buf + align, max_align)) {
-                /* Fallback to request_aligment. */
+                /* Fallback to request_alignment. */
                 s->buf_align = (align != 1) ? align : bs->bl.request_alignment;
                 break;
             }
-- 
2.21.0

From: Thomas Huth <thuth@redhat.com>

It is possible to enable only a subset of the block drivers with the
"--block-drv-rw-whitelist" option of the "configure" script. All other
drivers are marked as unusable (or only included as read-only with the
"--block-drv-ro-whitelist" option). If an iotest is now using such a
disabled block driver, it is failing - which is bad, since at least the
tests in the "auto" group should be able to deal with this situation.
Thus let's introduce a "_require_drivers" function that can be used by
the shell tests to check for the availability of certain drivers first,
and marks the test as "not run" if one of the drivers is missing.

This patch mainly targets the test in the "auto" group which should
never fail in such a case, but also improves some of the other tests
along the way. Note that we also assume that the "qcow2" and "file"
drivers are always available - otherwise it does not make sense to
run "make check-block" at all (which only tests with qcow2 by default).

Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-id: 20190823133552.11680-1-thuth@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/071       |  1 +
 tests/qemu-iotests/081       |  4 +---
 tests/qemu-iotests/099       |  1 +
 tests/qemu-iotests/120       |  1 +
 tests/qemu-iotests/162       |  4 +---
 tests/qemu-iotests/184       |  1 +
 tests/qemu-iotests/186       |  1 +
 tests/qemu-iotests/common.rc | 14 ++++++++++++++
 8 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/071 b/tests/qemu-iotests/071
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/071
+++ b/tests/qemu-iotests/071
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt qcow2
 _supported_proto file
+_require_drivers blkdebug blkverify
 
 do_run_qemu()
 {
diff --git a/tests/qemu-iotests/081 b/tests/qemu-iotests/081
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/081
+++ b/tests/qemu-iotests/081
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt raw
 _supported_proto file
 _supported_os Linux
+_require_drivers quorum
 
 do_run_qemu()
 {
@@ -XXX,XX +XXX,XX @@ run_qemu()
                           | _filter_qemu_io | _filter_generated_node_ids
 }
 
-test_quorum=$($QEMU_IMG --help|grep quorum)
-[ "$test_quorum" = "" ] && _supported_fmt quorum
-
 quorum="driver=raw,file.driver=quorum,file.vote-threshold=2"
 quorum="$quorum,file.children.0.file.filename=$TEST_DIR/1.raw"
 quorum="$quorum,file.children.1.file.filename=$TEST_DIR/2.raw"
diff --git a/tests/qemu-iotests/099 b/tests/qemu-iotests/099
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/099
+++ b/tests/qemu-iotests/099
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt qcow qcow2 qed vdi vhdx vmdk vpc
 _supported_proto file
 _supported_os Linux
+_require_drivers blkdebug blkverify
 _unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat" \
     "subformat=twoGbMaxExtentSparse"
 
diff --git a/tests/qemu-iotests/120 b/tests/qemu-iotests/120
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/120
+++ b/tests/qemu-iotests/120
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt generic
 _supported_proto file
 _unsupported_fmt luks
+_require_drivers raw
 
 _make_test_img 64M
 
diff --git a/tests/qemu-iotests/162 b/tests/qemu-iotests/162
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/162
+++ b/tests/qemu-iotests/162
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common.filter
 
 _supported_fmt generic
-
-test_ssh=$($QEMU_IMG --help | grep '^Supported formats:.* ssh$ \|$$')
-[ "$test_ssh" = "" ] && _notrun "ssh support required"
+_require_drivers ssh
 
 echo
 echo '=== NBD ==='
diff --git a/tests/qemu-iotests/184 b/tests/qemu-iotests/184
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/184
+++ b/tests/qemu-iotests/184
@@ -XXX,XX +XXX,XX @@ trap "exit \$status" 0 1 2 3 15
 . ./common.filter
 
 _supported_os Linux
+_require_drivers throttle
 
 do_run_qemu()
 {
diff --git a/tests/qemu-iotests/186 b/tests/qemu-iotests/186
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/186
+++ b/tests/qemu-iotests/186
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt qcow2
 _supported_proto file
+_require_drivers null-co
 
 if [ "$QEMU_DEFAULT_MACHINE" != "pc" ]; then
     _notrun "Requires a PC machine"
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -XXX,XX +XXX,XX @@ _require_command()
     [ -x "$c" ] || _notrun "$1 utility required, skipped this test"
 }
 
+# Check that a set of drivers has been whitelisted in the QEMU binary
+#
+_require_drivers()
+{
+    available=$($QEMU -drive format=help | \
+                sed -e '/Supported formats:/!d' -e 's/Supported formats://')
+    for driver
+    do
+        if ! echo "$available" | grep -q " $driver$ \|$$"; then
+            _notrun "$driver not available"
+        fi
+    done
+}
+
 # make sure this script returns success
 true
-- 
2.21.0

From: Thomas Huth <thuth@redhat.com>

The sanitizers (especially the address sanitizer from Clang) are
sometimes printing out warnings or false positives - this spoils
the output of the iotests, causing some of the tests to fail.
Thus let's skip the automatic iotests during "make check" when the
user configured QEMU with --enable-sanitizers.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-id: 20190823084203.29734-1-thuth@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/check-block.sh | 5 +++++
 1 file changed, 5 insertions(+)

From: Nir Soffer <nirsof@gmail.com>

Quoting cache mode is not needed, and most tests use unquoted values.
Unify all test to use the same style.

Message-id: 20190827173432.7656-1-nsoffer@redhat.com
Signed-off-by: Nir Soffer <nsoffer@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/026 | 4 ++--
 tests/qemu-iotests/039 | 4 ++--
 tests/qemu-iotests/052 | 2 +-
 tests/qemu-iotests/091 | 4 ++--
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/qemu-iotests/026 b/tests/qemu-iotests/026
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/026
+++ b/tests/qemu-iotests/026
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 # Currently only qcow2 supports rebasing
 _supported_fmt qcow2
 _supported_proto file
-_default_cache_mode "writethrough"
-_supported_cache_modes "writethrough" "none"
+_default_cache_mode writethrough
+_supported_cache_modes writethrough none
 # The refcount table tests expect a certain minimum width for refcount entries
 # (so that the refcount table actually needs to grow); that minimum is 16 bits,
 # being the default refcount entry width.
diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/039
+++ b/tests/qemu-iotests/039
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt qcow2
 _supported_proto file
 _supported_os Linux
-_default_cache_mode "writethrough"
-_supported_cache_modes "writethrough"
+_default_cache_mode writethrough
+_supported_cache_modes writethrough
 
 size=128M
 
diff --git a/tests/qemu-iotests/052 b/tests/qemu-iotests/052
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/052
+++ b/tests/qemu-iotests/052
@@ -XXX,XX +XXX,XX @@ _supported_fmt generic
 _supported_proto file
 
 # Don't do O_DIRECT on tmpfs
-_supported_cache_modes "writeback" "writethrough" "unsafe"
+_supported_cache_modes writeback writethrough unsafe
 
 size=128M
 _make_test_img $size
diff --git a/tests/qemu-iotests/091 b/tests/qemu-iotests/091
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/091
+++ b/tests/qemu-iotests/091
@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt qcow2
 _supported_proto file
 _supported_os Linux
-_default_cache_mode "none"
-_supported_cache_modes "writethrough" "none" "writeback"
+_default_cache_mode none
+_supported_cache_modes writethrough none writeback
 
 size=1G
 
-- 
2.21.0

The following changes since commit 171199f56f5f9bdf1e5d670d09ef1351d8f01bae:

Merge remote-tracking branch 'remotes/alistair/tags/pull-riscv-to-apply-20200619-3' into staging (2020-06-22 14:45:25 +0100)

are available in the Git repository at:

https://github.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to 7838c67f22a81fcf669785cd6c0876438422071a:

block/nvme: support nested aio_poll() (2020-06-23 15:46:08 +0100)

----------------------------------------------------------------
Pull request

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

Daniele Buono (4):
  coroutine: support SafeStack in ucontext backend
  coroutine: add check for SafeStack in sigaltstack
  configure: add flags to support SafeStack
  check-block: enable iotests with SafeStack

Stefan Hajnoczi (8):
  minikconf: explicitly set encoding to UTF-8
  block/nvme: poll queues without q->lock
  block/nvme: drop tautologous assertion
  block/nvme: don't access CQE after moving cq.head
  block/nvme: switch to a NVMeRequest freelist
  block/nvme: clarify that free_req_queue is protected by q->lock
  block/nvme: keep BDRVNVMeState pointer in NVMeQueuePair
  block/nvme: support nested aio_poll()

-- 
2.26.2

QEMU currently only has ASCII Kconfig files but Linux actually uses
UTF-8. Explicitly specify the encoding and that we're doing text file
I/O.

It's unclear whether or not QEMU will ever need Unicode in its Kconfig
files. If we start using the help text then it will become an issue
sooner or later. Make this change now for consistency with Linux
Kconfig.

Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20200521153616.307100-1-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 scripts/minikconf.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/minikconf.py b/scripts/minikconf.py
index XXXXXXX..XXXXXXX 100755
--- a/scripts/minikconf.py
+++ b/scripts/minikconf.py
@@ -XXX,XX +XXX,XX @@ class KconfigParser:
         if incl_abs_fname in self.data.previously_included:
             return
         try:
-            fp = open(incl_abs_fname, 'r')
+            fp = open(incl_abs_fname, 'rt', encoding='utf-8')
         except IOError as e:
             raise KconfigParserError(self,
                                 '%s: %s' % (e.strerror, include))
@@ -XXX,XX +XXX,XX @@ if __name__ == '__main__':
             parser.do_assignment(name, value == 'y')
             external_vars.add(name[7:])
         else:
-            fp = open(arg, 'r')
+            fp = open(arg, 'rt', encoding='utf-8')
             parser.parse_file(fp)
             fp.close()
 
@@ -XXX,XX +XXX,XX @@ if __name__ == '__main__':
         if key not in external_vars and config[key]:
             print ('CONFIG_%s=y' % key)
 
-    deps = open(argv[2], 'w')
+    deps = open(argv[2], 'wt', encoding='utf-8')
     for fname in data.previously_included:
         print ('%s: %s' % (argv[1], fname), file=deps)
     deps.close()
-- 
2.26.2

From: Daniele Buono <dbuono@linux.vnet.ibm.com>

LLVM's SafeStack instrumentation does not yet support programs that make
use of the APIs in ucontext.h
With the current implementation of coroutine-ucontext, the resulting
binary is incorrect, with different coroutines sharing the same unsafe
stack and producing undefined behavior at runtime.
This fix allocates an additional unsafe stack area for each coroutine,
and sets the new unsafe stack pointer before calling swapcontext() in
qemu_coroutine_new.
This is the only place where the pointer needs to be manually updated,
since sigsetjmp/siglongjmp are already instrumented by LLVM to properly
support SafeStack.
The additional stack is then freed in qemu_coroutine_delete.

Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
Message-id: 20200529205122.714-2-dbuono@linux.vnet.ibm.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/qemu/coroutine_int.h |  5 +++++
 util/coroutine-ucontext.c    | 28 ++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/include/qemu/coroutine_int.h b/include/qemu/coroutine_int.h
index XXXXXXX..XXXXXXX 100644
--- a/include/qemu/coroutine_int.h
+++ b/include/qemu/coroutine_int.h
@@ -XXX,XX +XXX,XX @@
 #include "qemu/queue.h"
 #include "qemu/coroutine.h"
 
+#ifdef CONFIG_SAFESTACK
+/* Pointer to the unsafe stack, defined by the compiler */
+extern __thread void *__safestack_unsafe_stack_ptr;
+#endif
+
 #define COROUTINE_STACK_SIZE (1 << 20)
 
 typedef enum {
diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
index XXXXXXX..XXXXXXX 100644
--- a/util/coroutine-ucontext.c
+++ b/util/coroutine-ucontext.c
@@ -XXX,XX +XXX,XX @@ typedef struct {
     Coroutine base;
     void *stack;
     size_t stack_size;
+#ifdef CONFIG_SAFESTACK
+    /* Need an unsafe stack for each coroutine */
+    void *unsafe_stack;
+    size_t unsafe_stack_size;
+#endif
     sigjmp_buf env;
 
     void *tsan_co_fiber;
@@ -XXX,XX +XXX,XX @@ Coroutine *qemu_coroutine_new(void)
     co = g_malloc0(sizeof(*co));
     co->stack_size = COROUTINE_STACK_SIZE;
     co->stack = qemu_alloc_stack(&co->stack_size);
+#ifdef CONFIG_SAFESTACK
+    co->unsafe_stack_size = COROUTINE_STACK_SIZE;
+    co->unsafe_stack = qemu_alloc_stack(&co->unsafe_stack_size);
+#endif
     co->base.entry_arg = &old_env; /* stash away our jmp_buf */
 
     uc.uc_link = &old_uc;
@@ -XXX,XX +XXX,XX @@ Coroutine *qemu_coroutine_new(void)
             COROUTINE_YIELD,
             &fake_stack_save,
             co->stack, co->stack_size, co->tsan_co_fiber);
+
+#ifdef CONFIG_SAFESTACK
+        /*
+         * Before we swap the context, set the new unsafe stack
+         * The unsafe stack grows just like the normal stack, so start from
+         * the last usable location of the memory area.
+         * NOTE: we don't have to re-set the usp afterwards because we are
+         * coming back to this context through a siglongjmp.
+         * The compiler already wrapped the corresponding sigsetjmp call with
+         * code that saves the usp on the (safe) stack before the call, and
+         * restores it right after (which is where we return with siglongjmp).
+         */
+        void *usp = co->unsafe_stack + co->unsafe_stack_size;
+        __safestack_unsafe_stack_ptr = usp;
+#endif
+
         swapcontext(&old_uc, &uc);
     }
 
@@ -XXX,XX +XXX,XX @@ void qemu_coroutine_delete(Coroutine *co_)
 #endif
 
     qemu_free_stack(co->stack, co->stack_size);
+#ifdef CONFIG_SAFESTACK
+    qemu_free_stack(co->unsafe_stack, co->unsafe_stack_size);
+#endif
     g_free(co);
 }
 
-- 
2.26.2

From: Daniele Buono <dbuono@linux.vnet.ibm.com>

Current implementation of LLVM's SafeStack is not compatible with
code that uses an alternate stack created with sigaltstack().
Since coroutine-sigaltstack relies on sigaltstack(), it is not
compatible with SafeStack. The resulting binary is incorrect, with
different coroutines sharing the same unsafe stack and producing
undefined behavior at runtime.

In the future LLVM may provide a SafeStack implementation compatible with
sigaltstack(). In the meantime, if SafeStack is desired, the coroutine
implementation from coroutine-ucontext should be used.
As a safety check, add a control in coroutine-sigaltstack to throw a
preprocessor #error if SafeStack is enabled and we are trying to
use coroutine-sigaltstack to implement coroutines.

Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
Message-id: 20200529205122.714-3-dbuono@linux.vnet.ibm.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/coroutine-sigaltstack.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/util/coroutine-sigaltstack.c b/util/coroutine-sigaltstack.c
index XXXXXXX..XXXXXXX 100644
--- a/util/coroutine-sigaltstack.c
+++ b/util/coroutine-sigaltstack.c
@@ -XXX,XX +XXX,XX @@
 #include "qemu-common.h"
 #include "qemu/coroutine_int.h"
 
+#ifdef CONFIG_SAFESTACK
+#error "SafeStack is not compatible with code run in alternate signal stacks"
+#endif
+
 typedef struct {
     Coroutine base;
     void *stack;
-- 
2.26.2

From: Daniele Buono <dbuono@linux.vnet.ibm.com>

This patch adds a flag to enable/disable the SafeStack instrumentation
provided by LLVM.

On enable, make sure that the compiler supports the flags, and that we
are using the proper coroutine implementation (coroutine-ucontext).
On disable, explicitly disable the option if it was enabled by default.

While SafeStack is supported only on Linux, NetBSD, FreeBSD and macOS,
we are not checking for the O.S. since this is already done by LLVM.

Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
Message-id: 20200529205122.714-4-dbuono@linux.vnet.ibm.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 configure | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/configure b/configure
index XXXXXXX..XXXXXXX 100755
--- a/configure
+++ b/configure
@@ -XXX,XX +XXX,XX @@ audio_win_int=""
 libs_qga=""
 debug_info="yes"
 stack_protector=""
+safe_stack=""
 use_containers="yes"
 gdb_bin=$(command -v "gdb-multiarch" || command -v "gdb")
 
@@ -XXX,XX +XXX,XX @@ for opt do
   ;;
   --disable-stack-protector) stack_protector="no"
   ;;
+  --enable-safe-stack) safe_stack="yes"
+  ;;
+  --disable-safe-stack) safe_stack="no"
+  ;;
   --disable-curses) curses="no"
   ;;
   --enable-curses) curses="yes"
@@ -XXX,XX +XXX,XX @@ disabled with --disable-FEATURE, default is enabled if available:
   debug-tcg       TCG debugging (default is disabled)
   debug-info      debugging information
   sparse          sparse checker
+  safe-stack      SafeStack Stack Smash Protection. Depends on
+                  clang/llvm >= 3.7 and requires coroutine backend ucontext.
 
   gnutls          GNUTLS cryptography support
   nettle          nettle cryptography support
@@ -XXX,XX +XXX,XX @@ if test "$debug_stack_usage" = "yes"; then
   fi
 fi
 
+##################################################
+# SafeStack
+
+
+if test "$safe_stack" = "yes"; then
+cat > $TMPC << EOF
+int main(int argc, char *argv[])
+{
+#if ! __has_feature(safe_stack)
+#error SafeStack Disabled
+#endif
+    return 0;
+}
+EOF
+  flag="-fsanitize=safe-stack"
+  # Check that safe-stack is supported and enabled.
+  if compile_prog "-Werror $flag" "$flag"; then
+    # Flag needed both at compilation and at linking
+    QEMU_CFLAGS="$QEMU_CFLAGS $flag"
+    QEMU_LDFLAGS="$QEMU_LDFLAGS $flag"
+  else
+    error_exit "SafeStack not supported by your compiler"
+  fi
+  if test "$coroutine" != "ucontext"; then
+    error_exit "SafeStack is only supported by the coroutine backend ucontext"
+  fi
+else
+cat > $TMPC << EOF
+int main(int argc, char *argv[])
+{
+#if defined(__has_feature)
+#if __has_feature(safe_stack)
+#error SafeStack Enabled
+#endif
+#endif
+    return 0;
+}
+EOF
+if test "$safe_stack" = "no"; then
+  # Make sure that safe-stack is disabled
+  if ! compile_prog "-Werror" ""; then
+    # SafeStack was already enabled, try to explicitly remove the feature
+    flag="-fno-sanitize=safe-stack"
+    if ! compile_prog "-Werror $flag" "$flag"; then
+      error_exit "Configure cannot disable SafeStack"
+    fi
+    QEMU_CFLAGS="$QEMU_CFLAGS $flag"
+    QEMU_LDFLAGS="$QEMU_LDFLAGS $flag"
+  fi
+else # "$safe_stack" = ""
+  # Set safe_stack to yes or no based on pre-existing flags
+  if compile_prog "-Werror" ""; then
+    safe_stack="no"
+  else
+    safe_stack="yes"
+    if test "$coroutine" != "ucontext"; then
+      error_exit "SafeStack is only supported by the coroutine backend ucontext"
+    fi
+  fi
+fi
+fi
 
 ##########################################
 # check if we have open_by_handle_at
@@ -XXX,XX +XXX,XX @@ echo "sparse enabled    $sparse"
 echo "strip binaries    $strip_opt"
 echo "profiler          $profiler"
 echo "static build      $static"
+echo "safe stack        $safe_stack"
 if test "$darwin" = "yes" ; then
     echo "Cocoa support     $cocoa"
 fi
@@ -XXX,XX +XXX,XX @@ if test "$ccache_cpp2" = "yes"; then
   echo "export CCACHE_CPP2=y" >> $config_host_mak
 fi
 
+if test "$safe_stack" = "yes"; then
+  echo "CONFIG_SAFESTACK=y" >> $config_host_mak
+fi
+
 # If we're using a separate build tree, set it up now.
 # DIRS are directories which we simply mkdir in the build tree;
 # LINKS are things to symlink back into the source tree
-- 
2.26.2

From: Daniele Buono <dbuono@linux.vnet.ibm.com>

SafeStack is a stack protection technique implemented in llvm. It is
enabled with a -fsanitize flag.
iotests are currently disabled when any -fsanitize option is used,
because such options tend to produce additional warnings and false
positives.

While common -fsanitize options are used to verify the code and not
added in production, SafeStack's main use is in production environments
to protect against stack smashing.

Since SafeStack does not print any warning or false positive, enable
iotests when SafeStack is the only -fsanitize option used.
This is likely going to be a production binary and we want to make sure
it works correctly.

Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
Message-id: 20200529205122.714-5-dbuono@linux.vnet.ibm.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/check-block.sh | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/tests/check-block.sh b/tests/check-block.sh
index XXXXXXX..XXXXXXX 100755
--- a/tests/check-block.sh
+++ b/tests/check-block.sh
@@ -XXX,XX +XXX,XX @@ if grep -q "CONFIG_GPROF=y" config-host.mak 2>/dev/null ; then
     exit 0
 fi
 
-if grep -q "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null ; then
+# Disable tests with any sanitizer except for SafeStack
+CFLAGS=$( grep "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null )
+SANITIZE_FLAGS=""
+#Remove all occurrencies of -fsanitize=safe-stack
+for i in ${CFLAGS}; do
+        if [ "${i}" != "-fsanitize=safe-stack" ]; then
+                SANITIZE_FLAGS="${SANITIZE_FLAGS} ${i}"
+        fi
+done
+if echo ${SANITIZE_FLAGS} | grep -q "\-fsanitize" 2>/dev/null; then
+    # Have a sanitize flag that is not allowed, stop
     echo "Sanitizers are enabled ==> Not running the qemu-iotests."
     exit 0
 fi
-- 
2.26.2

A lot of CPU time is spent simply locking/unlocking q->lock during
polling. Check for completion outside the lock to make q->lock disappear
from the profile.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Sergio Lopez <slp@redhat.com>
Message-id: 20200617132201.1832152-2-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/nvme.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/block/nvme.c b/block/nvme.c
index XXXXXXX..XXXXXXX 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -XXX,XX +XXX,XX @@ static bool nvme_poll_queues(BDRVNVMeState *s)
 
     for (i = 0; i < s->nr_queues; i++) {
         NVMeQueuePair *q = s->queues[i];
+        const size_t cqe_offset = q->cq.head * NVME_CQ_ENTRY_BYTES;
+        NvmeCqe *cqe = (NvmeCqe *)&q->cq.queue[cqe_offset];
+
+        /*
+         * Do an early check for completions. q->lock isn't needed because
+         * nvme_process_completion() only runs in the event loop thread and
+         * cannot race with itself.
+         */
+        if ((le16_to_cpu(cqe->status) & 0x1) == q->cq_phase) {
+            continue;
+        }
+
         qemu_mutex_lock(&q->lock);
         while (nvme_process_completion(s, q)) {
             /* Keep polling */
-- 
2.26.2

Do not access a CQE after incrementing q->cq.head and releasing q->lock.
It is unlikely that this causes problems in practice but it's a latent
bug.

The reason why it should be safe at the moment is that completion
processing is not re-entrant and the CQ doorbell isn't written until the
end of nvme_process_completion().

Make this change now because QEMU expects completion processing to be
re-entrant and later patches will do that.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Sergio Lopez <slp@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20200617132201.1832152-4-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/nvme.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/block/nvme.c b/block/nvme.c
index XXXXXXX..XXXXXXX 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -XXX,XX +XXX,XX @@ static bool nvme_process_completion(BDRVNVMeState *s, NVMeQueuePair *q)
     q->busy = true;
     assert(q->inflight >= 0);
     while (q->inflight) {
+        int ret;
         int16_t cid;
+
         c = (NvmeCqe *)&q->cq.queue[q->cq.head * NVME_CQ_ENTRY_BYTES];
         if ((le16_to_cpu(c->status) & 0x1) == q->cq_phase) {
             break;
         }
+        ret = nvme_translate_error(c);
         q->cq.head = (q->cq.head + 1) % NVME_QUEUE_SIZE;
         if (!q->cq.head) {
             q->cq_phase = !q->cq_phase;
@@ -XXX,XX +XXX,XX @@ static bool nvme_process_completion(BDRVNVMeState *s, NVMeQueuePair *q)
         preq->busy = false;
         preq->cb = preq->opaque = NULL;
         qemu_mutex_unlock(&q->lock);
-        req.cb(req.opaque, nvme_translate_error(c));
+        req.cb(req.opaque, ret);
         qemu_mutex_lock(&q->lock);
         q->inflight--;
         progress = true;
-- 
2.26.2

There are three issues with the current NVMeRequest->busy field:
1. The busy field is accidentally accessed outside q->lock when request
   submission fails.
2. Waiters on free_req_queue are not woken when a request is returned
   early due to submission failure.
2. Finding a free request involves scanning all requests. This makes
   request submission O(n^2).

Switch to an O(1) freelist that is always accessed under the lock.

Also differentiate between NVME_QUEUE_SIZE, the actual SQ/CQ size, and
NVME_NUM_REQS, the number of usable requests. This makes the code
simpler than using NVME_QUEUE_SIZE everywhere and having to keep in mind
that one slot is reserved.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Sergio Lopez <slp@redhat.com>
Message-id: 20200617132201.1832152-5-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/nvme.c | 81 ++++++++++++++++++++++++++++++++++------------------
 1 file changed, 54 insertions(+), 27 deletions(-)

diff --git a/block/nvme.c b/block/nvme.c
index XXXXXXX..XXXXXXX 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -XXX,XX +XXX,XX @@
 #define NVME_QUEUE_SIZE 128
 #define NVME_BAR_SIZE 8192
 
+/*
+ * We have to leave one slot empty as that is the full queue case where
+ * head == tail + 1.
+ */
+#define NVME_NUM_REQS (NVME_QUEUE_SIZE - 1)
+
 typedef struct {
     int32_t  head, tail;
     uint8_t  *queue;
@@ -XXX,XX +XXX,XX @@ typedef struct {
     int cid;
     void *prp_list_page;
     uint64_t prp_list_iova;
-    bool busy;
+    int free_req_next; /* q->reqs[] index of next free req */
 } NVMeRequest;
 
 typedef struct {
@@ -XXX,XX +XXX,XX @@ typedef struct {
     /* Fields protected by @lock */
     NVMeQueue   sq, cq;
     int         cq_phase;
-    NVMeRequest reqs[NVME_QUEUE_SIZE];
+    int         free_req_head;
+    NVMeRequest reqs[NVME_NUM_REQS];
     bool        busy;
     int         need_kick;
     int         inflight;
@@ -XXX,XX +XXX,XX @@ static NVMeQueuePair *nvme_create_queue_pair(BlockDriverState *bs,
     qemu_mutex_init(&q->lock);
     q->index = idx;
     qemu_co_queue_init(&q->free_req_queue);
-    q->prp_list_pages = qemu_blockalign0(bs, s->page_size * NVME_QUEUE_SIZE);
+    q->prp_list_pages = qemu_blockalign0(bs, s->page_size * NVME_NUM_REQS);
     r = qemu_vfio_dma_map(s->vfio, q->prp_list_pages,
-                          s->page_size * NVME_QUEUE_SIZE,
+                          s->page_size * NVME_NUM_REQS,
                           false, &prp_list_iova);
     if (r) {
         goto fail;
     }
-    for (i = 0; i < NVME_QUEUE_SIZE; i++) {
+    q->free_req_head = -1;
+    for (i = 0; i < NVME_NUM_REQS; i++) {
         NVMeRequest *req = &q->reqs[i];
         req->cid = i + 1;
+        req->free_req_next = q->free_req_head;
+        q->free_req_head = i;
         req->prp_list_page = q->prp_list_pages + i * s->page_size;
         req->prp_list_iova = prp_list_iova + i * s->page_size;
     }
+
     nvme_init_queue(bs, &q->sq, size, NVME_SQ_ENTRY_BYTES, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
@@ -XXX,XX +XXX,XX @@ static void nvme_kick(BDRVNVMeState *s, NVMeQueuePair *q)
  */
 static NVMeRequest *nvme_get_free_req(NVMeQueuePair *q)
 {
-    int i;
-    NVMeRequest *req = NULL;
+    NVMeRequest *req;
 
     qemu_mutex_lock(&q->lock);
-    while (q->inflight + q->need_kick > NVME_QUEUE_SIZE - 2) {
-        /* We have to leave one slot empty as that is the full queue case (head
-         * == tail + 1). */
+
+    while (q->free_req_head == -1) {
         if (qemu_in_coroutine()) {
             trace_nvme_free_req_queue_wait(q);
             qemu_co_queue_wait(&q->free_req_queue, &q->lock);
@@ -XXX,XX +XXX,XX @@ static NVMeRequest *nvme_get_free_req(NVMeQueuePair *q)
             return NULL;
         }
     }
-    for (i = 0; i < NVME_QUEUE_SIZE; i++) {
-        if (!q->reqs[i].busy) {
-            q->reqs[i].busy = true;
-            req = &q->reqs[i];
-            break;
-        }
-    }
-    /* We have checked inflight and need_kick while holding q->lock, so one
-     * free req must be available. */
-    assert(req);
+
+    req = &q->reqs[q->free_req_head];
+    q->free_req_head = req->free_req_next;
+    req->free_req_next = -1;
+
     qemu_mutex_unlock(&q->lock);
     return req;
 }
 
+/* With q->lock */
+static void nvme_put_free_req_locked(NVMeQueuePair *q, NVMeRequest *req)
+{
+    req->free_req_next = q->free_req_head;
+    q->free_req_head = req - q->reqs;
+}
+
+/* With q->lock */
+static void nvme_wake_free_req_locked(BDRVNVMeState *s, NVMeQueuePair *q)
+{
+    if (!qemu_co_queue_empty(&q->free_req_queue)) {
+        replay_bh_schedule_oneshot_event(s->aio_context,
+                nvme_free_req_queue_cb, q);
+    }
+}
+
+/* Insert a request in the freelist and wake waiters */
+static void nvme_put_free_req_and_wake(BDRVNVMeState *s,  NVMeQueuePair *q,
+                                       NVMeRequest *req)
+{
+    qemu_mutex_lock(&q->lock);
+    nvme_put_free_req_locked(q, req);
+    nvme_wake_free_req_locked(s, q);
+    qemu_mutex_unlock(&q->lock);
+}
+
 static inline int nvme_translate_error(const NvmeCqe *c)
 {
     uint16_t status = (le16_to_cpu(c->status) >> 1) & 0xFF;
@@ -XXX,XX +XXX,XX @@ static bool nvme_process_completion(BDRVNVMeState *s, NVMeQueuePair *q)
         req = *preq;
         assert(req.cid == cid);
         assert(req.cb);
-        preq->busy = false;
+        nvme_put_free_req_locked(q, preq);
         preq->cb = preq->opaque = NULL;
         qemu_mutex_unlock(&q->lock);
         req.cb(req.opaque, ret);
@@ -XXX,XX +XXX,XX @@ static bool nvme_process_completion(BDRVNVMeState *s, NVMeQueuePair *q)
         /* Notify the device so it can post more completions. */
         smp_mb_release();
         *q->cq.doorbell = cpu_to_le32(q->cq.head);
-        if (!qemu_co_queue_empty(&q->free_req_queue)) {
-            replay_bh_schedule_oneshot_event(s->aio_context,
-                                             nvme_free_req_queue_cb, q);
-        }
+        nvme_wake_free_req_locked(s, q);
     }
     q->busy = false;
     return progress;
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int nvme_co_prw_aligned(BlockDriverState *bs,
     r = nvme_cmd_map_qiov(bs, &cmd, req, qiov);
     qemu_co_mutex_unlock(&s->dma_map_lock);
     if (r) {
-        req->busy = false;
+        nvme_put_free_req_and_wake(s, ioq, req);
         return r;
     }
     nvme_submit_command(s, ioq, req, &cmd, nvme_rw_cb, &data);
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn nvme_co_pdiscard(BlockDriverState *bs,
     qemu_co_mutex_unlock(&s->dma_map_lock);
 
     if (ret) {
-        req->busy = false;
+        nvme_put_free_req_and_wake(s, ioq, req);
         goto out;
     }
 
-- 
2.26.2

Passing around both BDRVNVMeState and NVMeQueuePair is unwieldy. Reduce
the number of function arguments by keeping the BDRVNVMeState pointer in
NVMeQueuePair. This will come in handly when a BH is introduced in a
later patch and only one argument can be passed to it.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Sergio Lopez <slp@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20200617132201.1832152-7-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/nvme.c | 70 ++++++++++++++++++++++++++++------------------------
 1 file changed, 38 insertions(+), 32 deletions(-)

diff --git a/block/nvme.c b/block/nvme.c
index XXXXXXX..XXXXXXX 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -XXX,XX +XXX,XX @@
  */
 #define NVME_NUM_REQS (NVME_QUEUE_SIZE - 1)
 
+typedef struct BDRVNVMeState BDRVNVMeState;
+
 typedef struct {
     int32_t  head, tail;
     uint8_t  *queue;
@@ -XXX,XX +XXX,XX @@ typedef struct {
 typedef struct {
     QemuMutex   lock;
 
+    /* Read from I/O code path, initialized under BQL */
+    BDRVNVMeState   *s;
+    int             index;
+
     /* Fields protected by BQL */
-    int         index;
     uint8_t     *prp_list_pages;
 
     /* Fields protected by @lock */
@@ -XXX,XX +XXX,XX @@ typedef volatile struct {
 
 QEMU_BUILD_BUG_ON(offsetof(NVMeRegs, doorbells) != 0x1000);
 
-typedef struct {
+struct BDRVNVMeState {
     AioContext *aio_context;
     QEMUVFIOState *vfio;
     NVMeRegs *regs;
@@ -XXX,XX +XXX,XX @@ typedef struct {
 
     /* PCI address (required for nvme_refresh_filename()) */
     char *device;
-} BDRVNVMeState;
+};
 
 #define NVME_BLOCK_OPT_DEVICE "device"
 #define NVME_BLOCK_OPT_NAMESPACE "namespace"
@@ -XXX,XX +XXX,XX @@ static void nvme_init_queue(BlockDriverState *bs, NVMeQueue *q,
     }
 }
 
-static void nvme_free_queue_pair(BlockDriverState *bs, NVMeQueuePair *q)
+static void nvme_free_queue_pair(NVMeQueuePair *q)
 {
     qemu_vfree(q->prp_list_pages);
     qemu_vfree(q->sq.queue);
@@ -XXX,XX +XXX,XX @@ static NVMeQueuePair *nvme_create_queue_pair(BlockDriverState *bs,
     uint64_t prp_list_iova;
 
     qemu_mutex_init(&q->lock);
+    q->s = s;
     q->index = idx;
     qemu_co_queue_init(&q->free_req_queue);
     q->prp_list_pages = qemu_blockalign0(bs, s->page_size * NVME_NUM_REQS);
@@ -XXX,XX +XXX,XX @@ static NVMeQueuePair *nvme_create_queue_pair(BlockDriverState *bs,
 
     return q;
 fail:
-    nvme_free_queue_pair(bs, q);
+    nvme_free_queue_pair(q);
     return NULL;
 }
 
 /* With q->lock */
-static void nvme_kick(BDRVNVMeState *s, NVMeQueuePair *q)
+static void nvme_kick(NVMeQueuePair *q)
 {
+    BDRVNVMeState *s = q->s;
+
     if (s->plugged || !q->need_kick) {
         return;
     }
@@ -XXX,XX +XXX,XX @@ static void nvme_put_free_req_locked(NVMeQueuePair *q, NVMeRequest *req)
 }
 
 /* With q->lock */
-static void nvme_wake_free_req_locked(BDRVNVMeState *s, NVMeQueuePair *q)
+static void nvme_wake_free_req_locked(NVMeQueuePair *q)
 {
     if (!qemu_co_queue_empty(&q->free_req_queue)) {
-        replay_bh_schedule_oneshot_event(s->aio_context,
+        replay_bh_schedule_oneshot_event(q->s->aio_context,
                 nvme_free_req_queue_cb, q);
     }
 }
 
 /* Insert a request in the freelist and wake waiters */
-static void nvme_put_free_req_and_wake(BDRVNVMeState *s,  NVMeQueuePair *q,
-                                       NVMeRequest *req)
+static void nvme_put_free_req_and_wake(NVMeQueuePair *q, NVMeRequest *req)
 {
     qemu_mutex_lock(&q->lock);
     nvme_put_free_req_locked(q, req);
-    nvme_wake_free_req_locked(s, q);
+    nvme_wake_free_req_locked(q);
     qemu_mutex_unlock(&q->lock);
 }
 
@@ -XXX,XX +XXX,XX @@ static inline int nvme_translate_error(const NvmeCqe *c)
 }
 
 /* With q->lock */
-static bool nvme_process_completion(BDRVNVMeState *s, NVMeQueuePair *q)
+static bool nvme_process_completion(NVMeQueuePair *q)
 {
+    BDRVNVMeState *s = q->s;
     bool progress = false;
     NVMeRequest *preq;
     NVMeRequest req;
@@ -XXX,XX +XXX,XX @@ static bool nvme_process_completion(BDRVNVMeState *s, NVMeQueuePair *q)
         /* Notify the device so it can post more completions. */
         smp_mb_release();
         *q->cq.doorbell = cpu_to_le32(q->cq.head);
-        nvme_wake_free_req_locked(s, q);
+        nvme_wake_free_req_locked(q);
     }
     q->busy = false;
     return progress;
@@ -XXX,XX +XXX,XX @@ static void nvme_trace_command(const NvmeCmd *cmd)
     }
 }
 
-static void nvme_submit_command(BDRVNVMeState *s, NVMeQueuePair *q,
-                                NVMeRequest *req,
+static void nvme_submit_command(NVMeQueuePair *q, NVMeRequest *req,
                                 NvmeCmd *cmd, BlockCompletionFunc cb,
                                 void *opaque)
 {
@@ -XXX,XX +XXX,XX @@ static void nvme_submit_command(BDRVNVMeState *s, NVMeQueuePair *q,
     req->opaque = opaque;
     cmd->cid = cpu_to_le32(req->cid);
 
-    trace_nvme_submit_command(s, q->index, req->cid);
+    trace_nvme_submit_command(q->s, q->index, req->cid);
     nvme_trace_command(cmd);
     qemu_mutex_lock(&q->lock);
     memcpy((uint8_t *)q->sq.queue +
            q->sq.tail * NVME_SQ_ENTRY_BYTES, cmd, sizeof(*cmd));
     q->sq.tail = (q->sq.tail + 1) % NVME_QUEUE_SIZE;
     q->need_kick++;
-    nvme_kick(s, q);
-    nvme_process_completion(s, q);
+    nvme_kick(q);
+    nvme_process_completion(q);
     qemu_mutex_unlock(&q->lock);
 }
 
@@ -XXX,XX +XXX,XX @@ static int nvme_cmd_sync(BlockDriverState *bs, NVMeQueuePair *q,
                          NvmeCmd *cmd)
 {
     NVMeRequest *req;
-    BDRVNVMeState *s = bs->opaque;
     int ret = -EINPROGRESS;
     req = nvme_get_free_req(q);
     if (!req) {
         return -EBUSY;
     }
-    nvme_submit_command(s, q, req, cmd, nvme_cmd_sync_cb, &ret);
+    nvme_submit_command(q, req, cmd, nvme_cmd_sync_cb, &ret);
 
     BDRV_POLL_WHILE(bs, ret == -EINPROGRESS);
     return ret;
@@ -XXX,XX +XXX,XX @@ static bool nvme_poll_queues(BDRVNVMeState *s)
         }
 
         qemu_mutex_lock(&q->lock);
-        while (nvme_process_completion(s, q)) {
+        while (nvme_process_completion(q)) {
             /* Keep polling */
             progress = true;
         }
@@ -XXX,XX +XXX,XX @@ static bool nvme_add_io_queue(BlockDriverState *bs, Error **errp)
     };
     if (nvme_cmd_sync(bs, s->queues[0], &cmd)) {
         error_setg(errp, "Failed to create io queue [%d]", n);
-        nvme_free_queue_pair(bs, q);
+        nvme_free_queue_pair(q);
         return false;
     }
     cmd = (NvmeCmd) {
@@ -XXX,XX +XXX,XX @@ static bool nvme_add_io_queue(BlockDriverState *bs, Error **errp)
     };
     if (nvme_cmd_sync(bs, s->queues[0], &cmd)) {
         error_setg(errp, "Failed to create io queue [%d]", n);
-        nvme_free_queue_pair(bs, q);
+        nvme_free_queue_pair(q);
         return false;
     }
     s->queues = g_renew(NVMeQueuePair *, s->queues, n + 1);
@@ -XXX,XX +XXX,XX @@ static void nvme_close(BlockDriverState *bs)
     BDRVNVMeState *s = bs->opaque;
 
     for (i = 0; i < s->nr_queues; ++i) {
-        nvme_free_queue_pair(bs, s->queues[i]);
+        nvme_free_queue_pair(s->queues[i]);
     }
     g_free(s->queues);
     aio_set_event_notifier(bdrv_get_aio_context(bs), &s->irq_notifier,
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int nvme_co_prw_aligned(BlockDriverState *bs,
     r = nvme_cmd_map_qiov(bs, &cmd, req, qiov);
     qemu_co_mutex_unlock(&s->dma_map_lock);
     if (r) {
-        nvme_put_free_req_and_wake(s, ioq, req);
+        nvme_put_free_req_and_wake(ioq, req);
         return r;
     }
-    nvme_submit_command(s, ioq, req, &cmd, nvme_rw_cb, &data);
+    nvme_submit_command(ioq, req, &cmd, nvme_rw_cb, &data);
 
     data.co = qemu_coroutine_self();
     while (data.ret == -EINPROGRESS) {
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int nvme_co_flush(BlockDriverState *bs)
     assert(s->nr_queues > 1);
     req = nvme_get_free_req(ioq);
     assert(req);
-    nvme_submit_command(s, ioq, req, &cmd, nvme_rw_cb, &data);
+    nvme_submit_command(ioq, req, &cmd, nvme_rw_cb, &data);
 
     data.co = qemu_coroutine_self();
     if (data.ret == -EINPROGRESS) {
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int nvme_co_pwrite_zeroes(BlockDriverState *bs,
     req = nvme_get_free_req(ioq);
     assert(req);
 
-    nvme_submit_command(s, ioq, req, &cmd, nvme_rw_cb, &data);
+    nvme_submit_command(ioq, req, &cmd, nvme_rw_cb, &data);
 
     data.co = qemu_coroutine_self();
     while (data.ret == -EINPROGRESS) {
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn nvme_co_pdiscard(BlockDriverState *bs,
     qemu_co_mutex_unlock(&s->dma_map_lock);
 
     if (ret) {
-        nvme_put_free_req_and_wake(s, ioq, req);
+        nvme_put_free_req_and_wake(ioq, req);
         goto out;
     }
 
     trace_nvme_dsm(s, offset, bytes);
 
-    nvme_submit_command(s, ioq, req, &cmd, nvme_rw_cb, &data);
+    nvme_submit_command(ioq, req, &cmd, nvme_rw_cb, &data);
 
     data.co = qemu_coroutine_self();
     while (data.ret == -EINPROGRESS) {
@@ -XXX,XX +XXX,XX @@ static void nvme_aio_unplug(BlockDriverState *bs)
     for (i = 1; i < s->nr_queues; i++) {
         NVMeQueuePair *q = s->queues[i];
         qemu_mutex_lock(&q->lock);
-        nvme_kick(s, q);
-        nvme_process_completion(s, q);
+        nvme_kick(q);
+        nvme_process_completion(q);
         qemu_mutex_unlock(&q->lock);
     }
 }
-- 
2.26.2

QEMU block drivers are supposed to support aio_poll() from I/O
completion callback functions. This means completion processing must be
re-entrant.

The standard approach is to schedule a BH during completion processing
and cancel it at the end of processing. If aio_poll() is invoked by a
callback function then the BH will run. The BH continues the suspended
completion processing.

All of this means that request A's cb() can synchronously wait for
request B to complete. Previously the nvme block driver would hang
because it didn't process completions from nested aio_poll().

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Sergio Lopez <slp@redhat.com>
Message-id: 20200617132201.1832152-8-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/nvme.c       | 67 ++++++++++++++++++++++++++++++++++++++++------
 block/trace-events |  2 +-
 2 files changed, 60 insertions(+), 9 deletions(-)

diff --git a/block/nvme.c b/block/nvme.c
index XXXXXXX..XXXXXXX 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -XXX,XX +XXX,XX @@ typedef struct {
     int         cq_phase;
     int         free_req_head;
     NVMeRequest reqs[NVME_NUM_REQS];
-    bool        busy;
     int         need_kick;
     int         inflight;
+
+    /* Thread-safe, no lock necessary */
+    QEMUBH      *completion_bh;
 } NVMeQueuePair;
 
 /* Memory mapped registers */
@@ -XXX,XX +XXX,XX @@ struct BDRVNVMeState {
 #define NVME_BLOCK_OPT_DEVICE "device"
 #define NVME_BLOCK_OPT_NAMESPACE "namespace"
 
+static void nvme_process_completion_bh(void *opaque);
+
 static QemuOptsList runtime_opts = {
     .name = "nvme",
     .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
@@ -XXX,XX +XXX,XX @@ static void nvme_init_queue(BlockDriverState *bs, NVMeQueue *q,
 
 static void nvme_free_queue_pair(NVMeQueuePair *q)
 {
+    if (q->completion_bh) {
+        qemu_bh_delete(q->completion_bh);
+    }
     qemu_vfree(q->prp_list_pages);
     qemu_vfree(q->sq.queue);
     qemu_vfree(q->cq.queue);
@@ -XXX,XX +XXX,XX @@ static NVMeQueuePair *nvme_create_queue_pair(BlockDriverState *bs,
     q->index = idx;
     qemu_co_queue_init(&q->free_req_queue);
     q->prp_list_pages = qemu_blockalign0(bs, s->page_size * NVME_NUM_REQS);
+    q->completion_bh = aio_bh_new(bdrv_get_aio_context(bs),
+                                  nvme_process_completion_bh, q);
     r = qemu_vfio_dma_map(s->vfio, q->prp_list_pages,
                           s->page_size * NVME_NUM_REQS,
                           false, &prp_list_iova);
@@ -XXX,XX +XXX,XX @@ static bool nvme_process_completion(NVMeQueuePair *q)
     NvmeCqe *c;
 
     trace_nvme_process_completion(s, q->index, q->inflight);
-    if (q->busy || s->plugged) {
-        trace_nvme_process_completion_queue_busy(s, q->index);
+    if (s->plugged) {
+        trace_nvme_process_completion_queue_plugged(s, q->index);
         return false;
     }
-    q->busy = true;
+
+    /*
+     * Support re-entrancy when a request cb() function invokes aio_poll().
+     * Pending completions must be visible to aio_poll() so that a cb()
+     * function can wait for the completion of another request.
+     *
+     * The aio_poll() loop will execute our BH and we'll resume completion
+     * processing there.
+     */
+    qemu_bh_schedule(q->completion_bh);
+
     assert(q->inflight >= 0);
     while (q->inflight) {
         int ret;
@@ -XXX,XX +XXX,XX @@ static bool nvme_process_completion(NVMeQueuePair *q)
         assert(req.cb);
         nvme_put_free_req_locked(q, preq);
         preq->cb = preq->opaque = NULL;
-        qemu_mutex_unlock(&q->lock);
-        req.cb(req.opaque, ret);
-        qemu_mutex_lock(&q->lock);
         q->inflight--;
+        qemu_mutex_unlock(&q->lock);
+        req.cb(req.opaque, ret);
+        qemu_mutex_lock(&q->lock);
         progress = true;
     }
     if (progress) {
@@ -XXX,XX +XXX,XX @@ static bool nvme_process_completion(NVMeQueuePair *q)
         *q->cq.doorbell = cpu_to_le32(q->cq.head);
         nvme_wake_free_req_locked(q);
     }
-    q->busy = false;
+
+    qemu_bh_cancel(q->completion_bh);
+
     return progress;
 }
 
+static void nvme_process_completion_bh(void *opaque)
+{
+    NVMeQueuePair *q = opaque;
+
+    /*
+     * We're being invoked because a nvme_process_completion() cb() function
+     * called aio_poll(). The callback may be waiting for further completions
+     * so notify the device that it has space to fill in more completions now.
+     */
+    smp_mb_release();
+    *q->cq.doorbell = cpu_to_le32(q->cq.head);
+    nvme_wake_free_req_locked(q);
+
+    nvme_process_completion(q);
+}
+
 static void nvme_trace_command(const NvmeCmd *cmd)
 {
     int i;
@@ -XXX,XX +XXX,XX @@ static void nvme_detach_aio_context(BlockDriverState *bs)
 {
     BDRVNVMeState *s = bs->opaque;
 
+    for (int i = 0; i < s->nr_queues; i++) {
+        NVMeQueuePair *q = s->queues[i];
+
+        qemu_bh_delete(q->completion_bh);
+        q->completion_bh = NULL;
+    }
+
     aio_set_event_notifier(bdrv_get_aio_context(bs), &s->irq_notifier,
                            false, NULL, NULL);
 }
@@ -XXX,XX +XXX,XX @@ static void nvme_attach_aio_context(BlockDriverState *bs,
     s->aio_context = new_context;
     aio_set_event_notifier(new_context, &s->irq_notifier,
                            false, nvme_handle_event, nvme_poll_cb);
+
+    for (int i = 0; i < s->nr_queues; i++) {
+        NVMeQueuePair *q = s->queues[i];
+
+        q->completion_bh =
+            aio_bh_new(new_context, nvme_process_completion_bh, q);
+    }
 }
 
 static void nvme_aio_plug(BlockDriverState *bs)
diff --git a/block/trace-events b/block/trace-events
index XXXXXXX..XXXXXXX 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -XXX,XX +XXX,XX @@ nvme_kick(void *s, int queue) "s %p queue %d"
 nvme_dma_flush_queue_wait(void *s) "s %p"
 nvme_error(int cmd_specific, int sq_head, int sqid, int cid, int status) "cmd_specific %d sq_head %d sqid %d cid %d status 0x%x"
 nvme_process_completion(void *s, int index, int inflight) "s %p queue %d inflight %d"
-nvme_process_completion_queue_busy(void *s, int index) "s %p queue %d"
+nvme_process_completion_queue_plugged(void *s, int index) "s %p queue %d"
 nvme_complete_command(void *s, int index, int cid) "s %p queue %d cid %d"
 nvme_submit_command(void *s, int index, int cid) "s %p queue %d cid %d"
 nvme_submit_command_raw(int c0, int c1, int c2, int c3, int c4, int c5, int c6, int c7) "%02x %02x %02x %02x %02x %02x %02x %02x"
-- 
2.26.2