Series comparison

-[Qemu-devel] [PULL 00/14] Block layer patches
+[PULL v3 0/4] Block layer patches
-The following changes since commit 5704c36d25ee84e7129722cb0db53df9faefe943:
+The following changes since commit 4f59102571fce49af180cfc6d4cdd2b5df7bdb14:
-  Merge remote-tracking branch 'remotes/kraxel/tags/fixes-31-20181112-pull-request' into staging (2018-11-12 15:55:40 +0000)
+  Merge remote-tracking branch 'remotes/amarkovic/tags/mips-queue-oct-01-2019' into staging (2019-10-01 16:21:42 +0100)
 are available in the Git repository at:
   git://repo.or.cz/qemu/kevin.git tags/for-upstream
-for you to fetch changes up to 1a42e5d8298d1b0f90d2254e7d559391dd3a45ca:
+for you to fetch changes up to 7e693a0500455edab21754573c32b7146138cffd:
-  Merge remote-tracking branch 'mreitz/tags/pull-block-2018-11-12' into queue-block (2018-11-12 17:57:32 +0100)
+  iotests: Remove Python 2 compatibility code (2019-10-04 11:59:16 +0200)
 ----------------------------------------------------------------
 Block layer patches:
-- file-posix: Don't waste a file descriptor for locking, don't lock the
+- Fix internal snapshots with typical -blockdev setups
-  same bit multiple times
+- iotests: Require Python 3.6 or later
 - nvme: Fix double free and memory leak
 - Misc error handling fixes
 - Added NULL checks found by static analysis
 - Allow more block drivers to not be included in the qemu build
 ----------------------------------------------------------------
-Fam Zheng (4):
+Kevin Wolf (4):
-      file-posix: Use error API properly
+      block/snapshot: Restrict set of snapshot nodes
-      file-posix: Skip effectiveless OFD lock operations
+      iotests: Test internal snapshots with -blockdev
-      file-posix: Drop s->lock_fd
+      iotests: Require Python 3.6 or later
-      tests: Add unit tests for image locking
+      iotests: Remove Python 2 compatibility code
-Jeff Cody (1):
+ block/snapshot.c                 |  26 ++++--
-      block: Make more block drivers compile-time configurable
+ tests/qemu-iotests/044           |   3 -
  tests/qemu-iotests/163           |   3 -
  tests/qemu-iotests/267           | 168 ++++++++++++++++++++++++++++++++++++
  tests/qemu-iotests/267.out       | 182 +++++++++++++++++++++++++++++++++++++++
  tests/qemu-iotests/check         |  13 ++-
  tests/qemu-iotests/common.filter |  11 ++-
  tests/qemu-iotests/group         |   1 +
  tests/qemu-iotests/iotests.py    |  13 +--
 files changed, 392 insertions(+), 28 deletions(-)
  create mode 100755 tests/qemu-iotests/267
  create mode 100644 tests/qemu-iotests/267.out
-Kevin Wolf (1):
-      Merge remote-tracking branch 'mreitz/tags/pull-block-2018-11-12' into queue-block
-Li Qiang (2):
-      nvme: don't unref ctrl_mem when device unrealized
-      nvme: free cmbuf in nvme_exit
-Liam Merwick (5):
-      job: Fix off-by-one assert checks for JobSTT and JobVerbTable
-      block: Null pointer dereference in blk_root_get_parent_desc()
-      qemu-img: assert block_job_get() does not return NULL in img_commit()
-      block: Fix potential Null pointer dereferences in vvfat.c
-      qcow2: Read outside array bounds in qcow2_pre_write_overlap_check()
-Peter Maydell (1):
-      blockdev: Consistently use snapshot_node_name in external_snapshot_prepare()
-zhenwei pi (1):
-      blockdev: handle error on block latency histogram set error
- configure                  |  91 ++++++++++++++++++++++++++
- block/block-backend.c      |   3 +-
- block/file-posix.c         | 122 ++++++++++++++++++++---------------
- block/qcow2-refcount.c     |  18 +++---
- block/vvfat.c              |  46 ++++++++-----
- blockdev.c                 |  21 ++++--
- hw/block/nvme.c            |   6 +-
- job.c                      |   4 +-
- qemu-img.c                 |   1 +
- tests/test-image-locking.c | 157 +++++++++++++++++++++++++++++++++++++++++++++
- block/Makefile.objs        |  22 +++++--
- tests/Makefile.include     |   2 +
-files changed, 400 insertions(+), 93 deletions(-)
- create mode 100644 tests/test-image-locking.c

-[Qemu-devel] [PULL 01/14] file-posix: Use error API properly
+Deleted patch
-From: Fam Zheng <famz@redhat.com>
-Use error_report for situations that affect user operation (i.e.  we're
-actually returning error), and warn_report/warn_report_err when some
-less critical error happened but the user operation can still carry on.
-For raw_normalize_devicepath, add Error parameter to propagate to
-its callers.
-Suggested-by: Markus Armbruster <armbru@redhat.com>
-Signed-off-by: Fam Zheng <famz@redhat.com>
-Signed-off-by: Kevin Wolf <kwolf@redhat.com>
----
- block/file-posix.c | 39 ++++++++++++++++-----------------------
-file changed, 16 insertions(+), 23 deletions(-)
-diff --git a/block/file-posix.c b/block/file-posix.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/file-posix.c
-+++ b/block/file-posix.c
-@@ -XXX,XX +XXX,XX @@ static int cdrom_reopen(BlockDriverState *bs);
- #endif
- #if defined(__NetBSD__)
--static int raw_normalize_devicepath(const char **filename)
-+static int raw_normalize_devicepath(const char **filename, Error **errp)
- {
-     static char namebuf[PATH_MAX];
-     const char *dp, *fname;
-@@ -XXX,XX +XXX,XX @@ static int raw_normalize_devicepath(const char **filename)
-     fname = *filename;
-     dp = strrchr(fname, '/');
-     if (lstat(fname, &sb) < 0) {
--        fprintf(stderr, "%s: stat failed: %s\n",
--            fname, strerror(errno));
-+        error_setg_errno(errp, errno, "%s: stat failed", fname);
-         return -errno;
-     }
-@@ -XXX,XX +XXX,XX @@ static int raw_normalize_devicepath(const char **filename)
-         snprintf(namebuf, PATH_MAX, "%.*s/r%s",
-             (int)(dp - fname), fname, dp + 1);
-     }
--    fprintf(stderr, "%s is a block device", fname);
-     *filename = namebuf;
--    fprintf(stderr, ", using %s\n", *filename);
-+    warn_report("%s is a block device, using %s", fname, *filename);
-     return 0;
- }
- #else
--static int raw_normalize_devicepath(const char **filename)
-+static int raw_normalize_devicepath(const char **filename, Error **errp)
- {
-     return 0;
- }
-@@ -XXX,XX +XXX,XX @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
-     filename = qemu_opt_get(opts, "filename");
--    ret = raw_normalize_devicepath(&filename);
-+    ret = raw_normalize_devicepath(&filename, errp);
-     if (ret != 0) {
--        error_setg_errno(errp, -ret, "Could not normalize device path");
-         goto fail;
-     }
-@@ -XXX,XX +XXX,XX @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
-     case ON_OFF_AUTO_ON:
-         s->use_lock = true;
-         if (!qemu_has_ofd_lock()) {
--            fprintf(stderr,
--                    "File lock requested but OFD locking syscall is "
--                    "unavailable, falling back to POSIX file locks.\n"
--                    "Due to the implementation, locks can be lost "
--                    "unexpectedly.\n");
-+            warn_report("File lock requested but OFD locking syscall is "
-+                        "unavailable, falling back to POSIX file locks");
-+            error_printf("Due to the implementation, locks can be lost "
-+                         "unexpectedly.\n");
-         }
-         break;
-     case ON_OFF_AUTO_OFF:
-@@ -XXX,XX +XXX,XX @@ static int raw_handle_perm_lock(BlockDriverState *bs,
-             /* Theoretically the above call only unlocks bytes and it cannot
-              * fail. Something weird happened, report it.
-              */
--            error_report_err(local_err);
-+            warn_report_err(local_err);
-         }
-         break;
-     case RAW_PL_COMMIT:
-@@ -XXX,XX +XXX,XX @@ static int raw_handle_perm_lock(BlockDriverState *bs,
-             /* Theoretically the above call only unlocks bytes and it cannot
-              * fail. Something weird happened, report it.
-              */
--            error_report_err(local_err);
-+            warn_report_err(local_err);
-         }
-         break;
-     }
-@@ -XXX,XX +XXX,XX @@ static int raw_reopen_prepare(BDRVReopenState *state,
-     /* If we cannot use fcntl, or fcntl failed, fall back to qemu_open() */
-     if (rs->fd == -1) {
-         const char *normalized_filename = state->bs->filename;
--        ret = raw_normalize_devicepath(&normalized_filename);
--        if (ret < 0) {
--            error_setg_errno(errp, -ret, "Could not normalize device path");
--        } else {
-+        ret = raw_normalize_devicepath(&normalized_filename, errp);
-+        if (ret >= 0) {
-             assert(!(rs->open_flags & O_CREAT));
-             rs->fd = qemu_open(normalized_filename, rs->open_flags);
-             if (rs->fd == -1) {
-@@ -XXX,XX +XXX,XX @@ static int aio_worker(void *arg)
-         ret = handle_aiocb_truncate(aiocb);
-         break;
-     default:
--        fprintf(stderr, "invalid aio request (0x%x)\n", aiocb->aio_type);
-+        error_report("invalid aio request (0x%x)", aiocb->aio_type);
-         ret = -EINVAL;
-         break;
-     }
-@@ -XXX,XX +XXX,XX @@ out_unlock:
-          * not mean the whole creation operation has failed.  So
-          * report it the user for their convenience, but do not report
-          * it to the caller. */
--        error_report_err(local_err);
-+        warn_report_err(local_err);
-     }
- out_close:
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn hdev_co_create_opts(const char *filename, QemuOpts *opts
-     (void)has_prefix;
--    ret = raw_normalize_devicepath(&filename);
-+    ret = raw_normalize_devicepath(&filename, errp);
-     if (ret < 0) {
--        error_setg_errno(errp, -ret, "Could not normalize device path");
-         return ret;
-     }
---
-.19.1

-[Qemu-devel] [PULL 13/14] block: Fix potential Null pointer dereferences in vvfat.c
+[PULL v3 1/4] block/snapshot: Restrict set of snapshot nodes
-From: Liam Merwick <Liam.Merwick@oracle.com>
+Nodes involved in internal snapshots were those that were returned by
 bdrv_next(), inserted and not read-only. bdrv_next() in turn returns all
 nodes that are either the root node of a BlockBackend or monitor-owned
 nodes.
-The calls to find_mapping_for_cluster() may return NULL but it
+With the typical -drive use, this worked well enough. However, in the
-isn't always checked for before dereferencing the value returned.
+typical -blockdev case, the user defines one node per option, making all
-Additionally, add some asserts to cover cases where NULL can't
+nodes monitor-owned nodes. This includes protocol nodes etc. which often
-be returned but which might not be obvious at first glance.
+are not snapshottable, so "savevm" only returns an error.
-Signed-off-by: Liam Merwick <Liam.Merwick@oracle.com>
+Change the conditions so that internal snapshot still include all nodes
-Message-id: 1541453919-25973-5-git-send-email-Liam.Merwick@oracle.com
+that have a BlockBackend attached (we definitely want to snapshot
-[mreitz: Dropped superfluous check of "mapping" following an assertion
+anything attached to a guest device and probably also the built-in NBD
-         that it is not NULL, and fixed some indentation]
+server; snapshotting block job BlockBackends is more of an accident, but
-Signed-off-by: Max Reitz <mreitz@redhat.com>
+a preexisting one), but other monitor-owned nodes are only included if
 they have no parents.
 This makes internal snapshots usable again with typical -blockdev
 configurations.
 Cc: qemu-stable@nongnu.org
 Signed-off-by: Kevin Wolf <kwolf@redhat.com>
 Reviewed-by: Eric Blake <eblake@redhat.com>
 Reviewed-by: Peter Krempa <pkrempa@redhat.com>
 Tested-by: Peter Krempa <pkrempa@redhat.com>
 ---
- block/vvfat.c | 46 ++++++++++++++++++++++++++++++----------------
+ block/snapshot.c | 26 +++++++++++++++++++-------
-file changed, 30 insertions(+), 16 deletions(-)
+file changed, 19 insertions(+), 7 deletions(-)
-diff --git a/block/vvfat.c b/block/vvfat.c
+diff --git a/block/snapshot.c b/block/snapshot.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/vvfat.c
+--- a/block/snapshot.c
-+++ b/block/vvfat.c
++++ b/block/snapshot.c
-@@ -XXX,XX +XXX,XX @@ static inline void array_free(array_t* array)
+@@ -XXX,XX +XXX,XX @@
- /* does not automatically grow */
+ #include "qapi/qmp/qerror.h"
- static inline void* array_get(array_t* array,unsigned int index) {
+ #include "qapi/qmp/qstring.h"
-     assert(index < array->next);
+ #include "qemu/option.h"
-+    assert(array->pointer);
++#include "sysemu/block-backend.h"
-     return array->pointer + index * array->item_size;
  QemuOptsList internal_snapshot_opts = {
      .name = "snapshot",
@@ -XXX,XX +XXX,XX @@ int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
      return ret;
  }
--static inline int array_ensure_allocated(array_t* array, int index)
++static bool bdrv_all_snapshots_includes_bs(BlockDriverState *bs)
-+static inline void array_ensure_allocated(array_t *array, int index)
++{
- {
++    if (!bdrv_is_inserted(bs) || bdrv_is_read_only(bs)) {
-     if((index + 1) * array->item_size > array->size) {
++        return false;
          int new_size = (index + 32) * array->item_size;
          array->pointer = g_realloc(array->pointer, new_size);
 -        if (!array->pointer)
 -            return -1;
 +        assert(array->pointer);
          memset(array->pointer + array->size, 0, new_size - array->size);
          array->size = new_size;
          array->next = index + 1;
      }
 -
 -    return 0;
  }
  static inline void* array_get_next(array_t* array) {
      unsigned int next = array->next;
 -    if (array_ensure_allocated(array, next) < 0)
 -        return NULL;
 -
 +    array_ensure_allocated(array, next);
      array->next = next + 1;
      return array_get(array, next);
  }
@@ -XXX,XX +XXX,XX @@ static int commit_direntries(BDRVVVFATState* s,
      direntry_t* direntry = array_get(&(s->directory), dir_index);
      uint32_t first_cluster = dir_index == 0 ? 0 : begin_of_direntry(direntry);
      mapping_t* mapping = find_mapping_for_cluster(s, first_cluster);
 -
      int factor = 0x10 * s->sectors_per_cluster;
      int old_cluster_count, new_cluster_count;
 -    int current_dir_index = mapping->info.dir.first_dir_index;
 -    int first_dir_index = current_dir_index;
 +    int current_dir_index;
 +    int first_dir_index;
      int ret, i;
      uint32_t c;
 -DLOG(fprintf(stderr, "commit_direntries for %s, parent_mapping_index %d\n", mapping->path, parent_mapping_index));
 -
      assert(direntry);
      assert(mapping);
      assert(mapping->begin == first_cluster);
@@ -XXX,XX +XXX,XX @@ DLOG(fprintf(stderr, "commit_direntries for %s, parent_mapping_index %d\n", mapp
      assert(mapping->mode & MODE_DIRECTORY);
      assert(dir_index == 0 || is_directory(direntry));
 +    DLOG(fprintf(stderr, "commit_direntries for %s, parent_mapping_index %d\n",
 +                 mapping->path, parent_mapping_index));
 +
 +    current_dir_index = mapping->info.dir.first_dir_index;
 +    first_dir_index = current_dir_index;
      mapping->info.dir.parent_mapping_index = parent_mapping_index;
      if (first_cluster == 0) {
@@ -XXX,XX +XXX,XX @@ DLOG(fprintf(stderr, "commit_direntries for %s, parent_mapping_index %d\n", mapp
          direntry = array_get(&(s->directory), first_dir_index + i);
          if (is_directory(direntry) && !is_dot(direntry)) {
              mapping = find_mapping_for_cluster(s, first_cluster);
 +            if (mapping == NULL) {
 +                return -1;
 +            }
              assert(mapping->mode & MODE_DIRECTORY);
              ret = commit_direntries(s, first_dir_index + i,
                  array_index(&(s->mapping), mapping));
@@ -XXX,XX +XXX,XX @@ static int commit_one_file(BDRVVVFATState* s,
      assert(offset < size);
      assert((offset % s->cluster_size) == 0);
 +    if (mapping == NULL) {
 +        return -1;
 +    }
 +
-     for (i = s->cluster_size; i < offset; i += s->cluster_size)
++    /* Include all nodes that are either in use by a BlockBackend, or that
-         c = modified_fat_get(s, c);
++     * aren't attached to any node, but owned by the monitor. */
++    return bdrv_has_blk(bs) || QLIST_EMPTY(&bs->parents);
-@@ -XXX,XX +XXX,XX @@ static int handle_renames_and_mkdirs(BDRVVVFATState* s)
++}
-         if (commit->action == ACTION_RENAME) {
-             mapping_t* mapping = find_mapping_for_cluster(s,
+ /* Group operations. All block drivers are involved.
-                     commit->param.rename.cluster);
+  * These functions will properly handle dataplane (take aio_context_acquire
--            char* old_path = mapping->path;
+@@ -XXX,XX +XXX,XX @@ bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs)
-+            char *old_path;
+         AioContext *ctx = bdrv_get_aio_context(bs);
-+            if (mapping == NULL) {
+         aio_context_acquire(ctx);
-+                return -1;
+-        if (bdrv_is_inserted(bs) && !bdrv_is_read_only(bs)) {
-+            }
++        if (bdrv_all_snapshots_includes_bs(bs)) {
-+            old_path = mapping->path;
+             ok = bdrv_can_snapshot(bs);
-             assert(commit->path);
+         }
-             mapping->path = commit->path;
+         aio_context_release(ctx);
-             if (rename(old_path, mapping->path))
+@@ -XXX,XX +XXX,XX @@ int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bad_bs,
-@@ -XXX,XX +XXX,XX @@ static int handle_renames_and_mkdirs(BDRVVVFATState* s)
+         AioContext *ctx = bdrv_get_aio_context(bs);
-                         direntry_t* d = direntry + i;
+         aio_context_acquire(ctx);
-                         if (is_file(d) || (is_directory(d) && !is_dot(d))) {
+-        if (bdrv_can_snapshot(bs) &&
-+                            int l;
+-                bdrv_snapshot_find(bs, snapshot, name) >= 0) {
-+                            char *new_path;
++        if (bdrv_all_snapshots_includes_bs(bs) &&
-                             mapping_t* m = find_mapping_for_cluster(s,
++            bdrv_snapshot_find(bs, snapshot, name) >= 0)
-                                     begin_of_direntry(d));
++        {
--                            int l = strlen(m->path);
+             ret = bdrv_snapshot_delete(bs, snapshot->id_str,
--                            char* new_path = g_malloc(l + diff + 1);
+                                        snapshot->name, err);
-+                            if (m == NULL) {
+         }
-+                                return -1;
+@@ -XXX,XX +XXX,XX @@ int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs,
-+                            }
+         AioContext *ctx = bdrv_get_aio_context(bs);
-+                            l = strlen(m->path);
-+                            new_path = g_malloc(l + diff + 1);
+         aio_context_acquire(ctx);
+-        if (bdrv_can_snapshot(bs)) {
-                             assert(!strncmp(m->path, mapping->path, l2));
++        if (bdrv_all_snapshots_includes_bs(bs)) {
+             ret = bdrv_snapshot_goto(bs, name, errp);
          }
          aio_context_release(ctx);
@@ -XXX,XX +XXX,XX @@ int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs)
          AioContext *ctx = bdrv_get_aio_context(bs);
          aio_context_acquire(ctx);
 -        if (bdrv_can_snapshot(bs)) {
 +        if (bdrv_all_snapshots_includes_bs(bs)) {
              err = bdrv_snapshot_find(bs, &sn, name);
          }
          aio_context_release(ctx);
@@ -XXX,XX +XXX,XX @@ int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn,
          if (bs == vm_state_bs) {
              sn->vm_state_size = vm_state_size;
              err = bdrv_snapshot_create(bs, sn);
 -        } else if (bdrv_can_snapshot(bs)) {
 +        } else if (bdrv_all_snapshots_includes_bs(bs)) {
              sn->vm_state_size = 0;
              err = bdrv_snapshot_create(bs, sn);
          }
@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_all_find_vmstate_bs(void)
          bool found;
          aio_context_acquire(ctx);
 -        found = bdrv_can_snapshot(bs);
 +        found = bdrv_all_snapshots_includes_bs(bs) && bdrv_can_snapshot(bs);
          aio_context_release(ctx);
          if (found) {
 --
-.19.1
+.20.1

-[Qemu-devel] [PULL 08/14] tests: Add unit tests for image locking
+[PULL v3 2/4] iotests: Test internal snapshots with -blockdev
-From: Fam Zheng <famz@redhat.com>
+Signed-off-by: Kevin Wolf <kwolf@redhat.com>
 Reviewed-by: Peter Krempa <pkrempa@redhat.com>
 Tested-by: Peter Krempa <pkrempa@redhat.com>
 ---
  tests/qemu-iotests/267           | 168 ++++++++++++++++++++++++++++
  tests/qemu-iotests/267.out       | 182 +++++++++++++++++++++++++++++++
  tests/qemu-iotests/common.filter |  11 +-
  tests/qemu-iotests/group         |   1 +
 files changed, 358 insertions(+), 4 deletions(-)
  create mode 100755 tests/qemu-iotests/267
  create mode 100644 tests/qemu-iotests/267.out
-Signed-off-by: Fam Zheng <famz@redhat.com>
+diff --git a/tests/qemu-iotests/267 b/tests/qemu-iotests/267
-Signed-off-by: Kevin Wolf <kwolf@redhat.com>
+new file mode 100755
----
+index XXXXXXX..XXXXXXX
- tests/test-image-locking.c | 157 +++++++++++++++++++++++++++++++++++++
+--- /dev/null
- tests/Makefile.include     |   2 +
++++ b/tests/qemu-iotests/267
-files changed, 159 insertions(+)
+@@ -XXX,XX +XXX,XX @@
- create mode 100644 tests/test-image-locking.c
++#!/usr/bin/env bash
++#
-diff --git a/tests/test-image-locking.c b/tests/test-image-locking.c
++# Test which nodes are involved in internal snapshots
 +#
 +# Copyright (C) 2019 Red Hat, Inc.
 +#
 +# This program is free software; you can redistribute it and/or modify
 +# it under the terms of the GNU General Public License as published by
 +# the Free Software Foundation; either version 2 of the License, or
 +# (at your option) any later version.
 +#
 +# This program is distributed in the hope that it will be useful,
 +# but WITHOUT ANY WARRANTY; without even the implied warranty of
 +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 +# GNU General Public License for more details.
 +#
 +# You should have received a copy of the GNU General Public License
 +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
 +#
 +
 +# creator
 +owner=kwolf@redhat.com
 +
 +seq=`basename $0`
 +echo "QA output created by $seq"
 +
 +status=1    # failure is the default!
 +
 +_cleanup()
 +{
 +    _cleanup_test_img
 +    rm -f "$TEST_DIR/nbd"
 +}
 +trap "_cleanup; exit \$status" 0 1 2 3 15
 +
 +# get standard environment, filters and checks
 +. ./common.rc
 +. ./common.filter
 +
 +_supported_fmt qcow2
 +_supported_proto file
 +_supported_os Linux
 +
 +# Internal snapshots are (currently) impossible with refcount_bits=1
 +_unsupported_imgopts 'refcount_bits=1[^0-9]'
 +
 +do_run_qemu()
 +{
 +    echo Testing: "$@"
 +    (
 +        if ! test -t 0; then
 +            while read cmd; do
 +                echo $cmd
 +            done
 +        fi
 +        echo quit
 +    ) | $QEMU -nographic -monitor stdio -nodefaults "$@"
 +    echo
 +}
 +
 +run_qemu()
 +{
 +    do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qemu | _filter_hmp |
 +        _filter_generated_node_ids | _filter_imgfmt | _filter_vmstate_size
 +}
 +
 +size=128M
 +
 +run_test()
 +{
 +    _make_test_img $size
 +    printf "savevm snap0\ninfo snapshots\nloadvm snap0\n" | run_qemu "$@" | _filter_date
 +}
 +
 +
 +echo
 +echo "=== No block devices at all ==="
 +echo
 +
 +run_test
 +
 +echo
 +echo "=== -drive if=none ==="
 +echo
 +
 +run_test -drive driver=file,file="$TEST_IMG",if=none
 +run_test -drive driver=$IMGFMT,file="$TEST_IMG",if=none
 +run_test -drive driver=$IMGFMT,file="$TEST_IMG",if=none -device virtio-blk,drive=none0
 +
 +echo
 +echo "=== -drive if=virtio ==="
 +echo
 +
 +run_test -drive driver=file,file="$TEST_IMG",if=virtio
 +run_test -drive driver=$IMGFMT,file="$TEST_IMG",if=virtio
 +
 +echo
 +echo "=== Simple -blockdev ==="
 +echo
 +
 +run_test -blockdev driver=file,filename="$TEST_IMG",node-name=file
 +run_test -blockdev driver=file,filename="$TEST_IMG",node-name=file \
 +         -blockdev driver=$IMGFMT,file=file,node-name=fmt
 +run_test -blockdev driver=file,filename="$TEST_IMG",node-name=file \
 +         -blockdev driver=raw,file=file,node-name=raw \
 +         -blockdev driver=$IMGFMT,file=raw,node-name=fmt
 +
 +echo
 +echo "=== -blockdev with a filter on top ==="
 +echo
 +
 +run_test -blockdev driver=file,filename="$TEST_IMG",node-name=file \
 +         -blockdev driver=$IMGFMT,file=file,node-name=fmt \
 +         -blockdev driver=copy-on-read,file=fmt,node-name=filter
 +
 +echo
 +echo "=== -blockdev with a backing file ==="
 +echo
 +
 +TEST_IMG="$TEST_IMG.base" _make_test_img $size
 +
 +IMGOPTS="backing_file=$TEST_IMG.base" \
 +run_test -blockdev driver=file,filename="$TEST_IMG.base",node-name=backing-file \
 +         -blockdev driver=file,filename="$TEST_IMG",node-name=file \
 +         -blockdev driver=$IMGFMT,file=file,backing=backing-file,node-name=fmt
 +
 +IMGOPTS="backing_file=$TEST_IMG.base" \
 +run_test -blockdev driver=file,filename="$TEST_IMG.base",node-name=backing-file \
 +         -blockdev driver=$IMGFMT,file=backing-file,node-name=backing-fmt \
 +         -blockdev driver=file,filename="$TEST_IMG",node-name=file \
 +         -blockdev driver=$IMGFMT,file=file,backing=backing-fmt,node-name=fmt
 +
 +# A snapshot should be present on the overlay, but not the backing file
 +echo Internal snapshots on overlay:
 +$QEMU_IMG snapshot -l "$TEST_IMG" | _filter_date | _filter_vmstate_size
 +
 +echo Internal snapshots on backing file:
 +$QEMU_IMG snapshot -l "$TEST_IMG.base" | _filter_date | _filter_vmstate_size
 +
 +echo
 +echo "=== -blockdev with NBD server on the backing file ==="
 +echo
 +
 +IMGOPTS="backing_file=$TEST_IMG.base" _make_test_img $size
 +cat <<EOF |
 +nbd_server_start unix:$TEST_DIR/nbd
 +nbd_server_add -w backing-fmt
 +savevm snap0
 +info snapshots
 +loadvm snap0
 +EOF
 +run_qemu -blockdev driver=file,filename="$TEST_IMG.base",node-name=backing-file \
 +         -blockdev driver=$IMGFMT,file=backing-file,node-name=backing-fmt \
 +         -blockdev driver=file,filename="$TEST_IMG",node-name=file \
 +         -blockdev driver=$IMGFMT,file=file,backing=backing-fmt,node-name=fmt |
 +         _filter_date
 +
 +# This time, a snapshot should be created on both files
 +echo Internal snapshots on overlay:
 +$QEMU_IMG snapshot -l "$TEST_IMG" | _filter_date | _filter_vmstate_size
 +
 +echo Internal snapshots on backing file:
 +$QEMU_IMG snapshot -l "$TEST_IMG.base" | _filter_date | _filter_vmstate_size
 +
 +# success, all done
 +echo "*** done"
 +rm -f $seq.full
 +status=0
 diff --git a/tests/qemu-iotests/267.out b/tests/qemu-iotests/267.out
 new file mode 100644
 index XXXXXXX..XXXXXXX
 --- /dev/null
-+++ b/tests/test-image-locking.c
++++ b/tests/qemu-iotests/267.out
 @@ -XXX,XX +XXX,XX @@
-+/*
++QA output created by 267
-+ * Image locking tests
++
-+ *
++=== No block devices at all ===
-+ * Copyright (c) 2018 Red Hat Inc.
++
-+ *
++Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
-+ * Author: Fam Zheng <famz@redhat.com>
++Testing:
-+ *
++QEMU X.Y.Z monitor - type 'help' for more information
-+ * Permission is hereby granted, free of charge, to any person obtaining a copy
++(qemu) savevm snap0
-+ * of this software and associated documentation files (the "Software"), to deal
++Error: No block device can accept snapshots
-+ * in the Software without restriction, including without limitation the rights
++(qemu) info snapshots
-+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
++No available block device supports snapshots
-+ * copies of the Software, and to permit persons to whom the Software is
++(qemu) loadvm snap0
-+ * furnished to do so, subject to the following conditions:
++Error: No block device supports snapshots
-+ *
++(qemu) quit
-+ * The above copyright notice and this permission notice shall be included in
++
-+ * all copies or substantial portions of the Software.
++
-+ *
++=== -drive if=none ===
-+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
++
-+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
++Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
-+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
++Testing: -drive driver=file,file=TEST_DIR/t.IMGFMT,if=none
-+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
++QEMU X.Y.Z monitor - type 'help' for more information
-+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
++(qemu) savevm snap0
-+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
++Error: Device 'none0' is writable but does not support snapshots
-+ * THE SOFTWARE.
++(qemu) info snapshots
-+ */
++No available block device supports snapshots
-+
++(qemu) loadvm snap0
-+#include "qemu/osdep.h"
++Error: Device 'none0' is writable but does not support snapshots
-+#include "block/block.h"
++(qemu) quit
-+#include "sysemu/block-backend.h"
++
-+#include "qapi/error.h"
++Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
-+#include "qapi/qmp/qdict.h"
++Testing: -drive driver=IMGFMT,file=TEST_DIR/t.IMGFMT,if=none
-+
++QEMU X.Y.Z monitor - type 'help' for more information
-+static BlockBackend *open_image(const char *path,
++(qemu) savevm snap0
-+                                uint64_t perm, uint64_t shared_perm,
++(qemu) info snapshots
-+                                Error **errp)
++List of snapshots present on all disks:
 +ID        TAG                 VM SIZE                DATE       VM CLOCK
 +--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
 +(qemu) loadvm snap0
 +(qemu) quit
 +
 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 +Testing: -drive driver=IMGFMT,file=TEST_DIR/t.IMGFMT,if=none -device virtio-blk,drive=none0
 +QEMU X.Y.Z monitor - type 'help' for more information
 +(qemu) savevm snap0
 +(qemu) info snapshots
 +List of snapshots present on all disks:
 +ID        TAG                 VM SIZE                DATE       VM CLOCK
 +--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
 +(qemu) loadvm snap0
 +(qemu) quit
 +
 +
 +=== -drive if=virtio ===
 +
 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 +Testing: -drive driver=file,file=TEST_DIR/t.IMGFMT,if=virtio
 +QEMU X.Y.Z monitor - type 'help' for more information
 +(qemu) savevm snap0
 +Error: Device 'virtio0' is writable but does not support snapshots
 +(qemu) info snapshots
 +No available block device supports snapshots
 +(qemu) loadvm snap0
 +Error: Device 'virtio0' is writable but does not support snapshots
 +(qemu) quit
 +
 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 +Testing: -drive driver=IMGFMT,file=TEST_DIR/t.IMGFMT,if=virtio
 +QEMU X.Y.Z monitor - type 'help' for more information
 +(qemu) savevm snap0
 +(qemu) info snapshots
 +List of snapshots present on all disks:
 +ID        TAG                 VM SIZE                DATE       VM CLOCK
 +--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
 +(qemu) loadvm snap0
 +(qemu) quit
 +
 +
 +=== Simple -blockdev ===
 +
 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 +Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file
 +QEMU X.Y.Z monitor - type 'help' for more information
 +(qemu) savevm snap0
 +Error: Device '' is writable but does not support snapshots
 +(qemu) info snapshots
 +No available block device supports snapshots
 +(qemu) loadvm snap0
 +Error: Device '' is writable but does not support snapshots
 +(qemu) quit
 +
 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 +Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=IMGFMT,file=file,node-name=fmt
 +QEMU X.Y.Z monitor - type 'help' for more information
 +(qemu) savevm snap0
 +(qemu) info snapshots
 +List of snapshots present on all disks:
 +ID        TAG                 VM SIZE                DATE       VM CLOCK
 +--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
 +(qemu) loadvm snap0
 +(qemu) quit
 +
 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 +Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=raw,file=file,node-name=raw -blockdev driver=IMGFMT,file=raw,node-name=fmt
 +QEMU X.Y.Z monitor - type 'help' for more information
 +(qemu) savevm snap0
 +(qemu) info snapshots
 +List of snapshots present on all disks:
 +ID        TAG                 VM SIZE                DATE       VM CLOCK
 +--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
 +(qemu) loadvm snap0
 +(qemu) quit
 +
 +
 +=== -blockdev with a filter on top ===
 +
 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 +Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=IMGFMT,file=file,node-name=fmt -blockdev driver=copy-on-read,file=fmt,node-name=filter
 +QEMU X.Y.Z monitor - type 'help' for more information
 +(qemu) savevm snap0
 +(qemu) info snapshots
 +List of snapshots present on all disks:
 +ID        TAG                 VM SIZE                DATE       VM CLOCK
 +--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
 +(qemu) loadvm snap0
 +(qemu) quit
 +
 +
 +=== -blockdev with a backing file ===
 +
 +Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728
 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
 +Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT.base,node-name=backing-file -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=IMGFMT,file=file,backing=backing-file,node-name=fmt
 +QEMU X.Y.Z monitor - type 'help' for more information
 +(qemu) savevm snap0
 +(qemu) info snapshots
 +List of snapshots present on all disks:
 +ID        TAG                 VM SIZE                DATE       VM CLOCK
 +--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
 +(qemu) loadvm snap0
 +(qemu) quit
 +
 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
 +Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT.base,node-name=backing-file -blockdev driver=IMGFMT,file=backing-file,node-name=backing-fmt -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=IMGFMT,file=file,backing=backing-fmt,node-name=fmt
 +QEMU X.Y.Z monitor - type 'help' for more information
 +(qemu) savevm snap0
 +(qemu) info snapshots
 +List of snapshots present on all disks:
 +ID        TAG                 VM SIZE                DATE       VM CLOCK
 +--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
 +(qemu) loadvm snap0
 +(qemu) quit
 +
 +Internal snapshots on overlay:
 +Snapshot list:
 +ID        TAG                 VM SIZE                DATE       VM CLOCK
 +1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
 +Internal snapshots on backing file:
 +
 +=== -blockdev with NBD server on the backing file ===
 +
 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
 +Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT.base,node-name=backing-file -blockdev driver=IMGFMT,file=backing-file,node-name=backing-fmt -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=IMGFMT,file=file,backing=backing-fmt,node-name=fmt
 +QEMU X.Y.Z monitor - type 'help' for more information
 +(qemu) nbd_server_start unix:TEST_DIR/nbd
 +(qemu) nbd_server_add -w backing-fmt
 +(qemu) savevm snap0
 +(qemu) info snapshots
 +List of snapshots present on all disks:
 +ID        TAG                 VM SIZE                DATE       VM CLOCK
 +--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
 +(qemu) loadvm snap0
 +(qemu) quit
 +
 +Internal snapshots on overlay:
 +Snapshot list:
 +ID        TAG                 VM SIZE                DATE       VM CLOCK
 +1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
 +Internal snapshots on backing file:
 +Snapshot list:
 +ID        TAG                 VM SIZE                DATE       VM CLOCK
 +1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
 +*** done
 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 @@
  # standard filters
  #
 -# ctime(3) dates
 -#
  _filter_date()
  {
 -    $SED \
 -        -e 's/[A-Z][a-z][a-z] [A-z][a-z][a-z]  *[0-9][0-9]* [0-9][0-9]:[0-9][0-9]:[0-9][0-9] [0-9][0-9][0-9][0-9]$/DATE/'
 +    $SED -re 's/[0-9]{4}-[0-9]{2}-[0-9]{2} [0-9]{2}:[0-9]{2}:[0-9]{2}/yyyy-mm-dd hh:mm:ss/'
 +}
 +
 +_filter_vmstate_size()
 +{
-+    Error *local_err = NULL;
++    $SED -r -e 's/[0-9. ]{5} [KMGT]iB/     SIZE/' \
-+    BlockBackend *blk;
++            -e 's/[0-9. ]{5} B/   SIZE/'
-+    QDict *options = qdict_new();
+ }
-+
-+    qdict_put_str(options, "driver", "raw");
+ _filter_generated_node_ids()
-+    blk = blk_new_open(path, NULL, options, BDRV_O_RDWR, &local_err);
+diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
 +    if (blk) {
 +        g_assert_null(local_err);
 +        if (blk_set_perm(blk, perm, shared_perm, errp)) {
 +            blk_unref(blk);
 +            blk = NULL;
 +        }
 +    } else {
 +        error_propagate(errp, local_err);
 +    }
 +    return blk;
 +}
 +
 +static void check_locked_bytes(int fd, uint64_t perm_locks,
 +                               uint64_t shared_perm_locks)
 +{
 +    int i;
 +
 +    if (!perm_locks && !shared_perm_locks) {
 +        g_assert(!qemu_lock_fd_test(fd, 0, 0, true));
 +        return;
 +    }
 +    for (i = 0; (1ULL << i) <= BLK_PERM_ALL; i++) {
 +        uint64_t bit = (1ULL << i);
 +        bool perm_expected = !!(bit & perm_locks);
 +        bool shared_perm_expected = !!(bit & shared_perm_locks);
 +        g_assert_cmpint(perm_expected, ==,
 +                        !!qemu_lock_fd_test(fd, 100 + i, 1, true));
 +        g_assert_cmpint(shared_perm_expected, ==,
 +                        !!qemu_lock_fd_test(fd, 200 + i, 1, true));
 +    }
 +}
 +
 +static void test_image_locking_basic(void)
 +{
 +    BlockBackend *blk1, *blk2, *blk3;
 +    char img_path[] = "/tmp/qtest.XXXXXX";
 +    uint64_t perm, shared_perm;
 +
 +    int fd = mkstemp(img_path);
 +    assert(fd >= 0);
 +
 +    perm = BLK_PERM_WRITE | BLK_PERM_CONSISTENT_READ;
 +    shared_perm = BLK_PERM_ALL;
 +    blk1 = open_image(img_path, perm, shared_perm, &error_abort);
 +    g_assert(blk1);
 +
 +    check_locked_bytes(fd, perm, ~shared_perm);
 +
 +    /* compatible perm between blk1 and blk2 */
 +    blk2 = open_image(img_path, perm | BLK_PERM_RESIZE, shared_perm, NULL);
 +    g_assert(blk2);
 +    check_locked_bytes(fd, perm | BLK_PERM_RESIZE, ~shared_perm);
 +
 +    /* incompatible perm with already open blk1 and blk2 */
 +    blk3 = open_image(img_path, perm, BLK_PERM_WRITE_UNCHANGED, NULL);
 +    g_assert_null(blk3);
 +
 +    blk_unref(blk2);
 +
 +    /* Check that extra bytes in blk2 are correctly unlocked */
 +    check_locked_bytes(fd, perm, ~shared_perm);
 +
 +    blk_unref(blk1);
 +
 +    /* Image is unused, no lock there */
 +    check_locked_bytes(fd, 0, 0);
 +    blk3 = open_image(img_path, perm, BLK_PERM_WRITE_UNCHANGED, &error_abort);
 +    g_assert(blk3);
 +    blk_unref(blk3);
 +    close(fd);
 +    unlink(img_path);
 +}
 +
 +static void test_set_perm_abort(void)
 +{
 +    BlockBackend *blk1, *blk2;
 +    char img_path[] = "/tmp/qtest.XXXXXX";
 +    uint64_t perm, shared_perm;
 +    int r;
 +    int fd = mkstemp(img_path);
 +    assert(fd >= 0);
 +
 +    perm = BLK_PERM_WRITE | BLK_PERM_CONSISTENT_READ;
 +    shared_perm = BLK_PERM_ALL;
 +    blk1 = open_image(img_path, perm, shared_perm, &error_abort);
 +    g_assert(blk1);
 +
 +    blk2 = open_image(img_path, perm, shared_perm, &error_abort);
 +    g_assert(blk2);
 +
 +    check_locked_bytes(fd, perm, ~shared_perm);
 +
 +    /* A failed blk_set_perm mustn't change perm status (locked bytes) */
 +    r = blk_set_perm(blk2, perm | BLK_PERM_RESIZE, BLK_PERM_WRITE_UNCHANGED,
 +                     NULL);
 +    g_assert_cmpint(r, !=, 0);
 +    check_locked_bytes(fd, perm, ~shared_perm);
 +    blk_unref(blk1);
 +    blk_unref(blk2);
 +}
 +
 +int main(int argc, char **argv)
 +{
 +    bdrv_init();
 +    qemu_init_main_loop(&error_abort);
 +
 +    g_test_init(&argc, &argv, NULL);
 +
 +    if (qemu_has_ofd_lock()) {
 +        g_test_add_func("/image-locking/basic", test_image_locking_basic);
 +        g_test_add_func("/image-locking/set-perm-abort", test_set_perm_abort);
 +    }
 +
 +    return g_test_run();
 +}
 diff --git a/tests/Makefile.include b/tests/Makefile.include
 index XXXXXXX..XXXXXXX 100644
---- a/tests/Makefile.include
+--- a/tests/qemu-iotests/group
-+++ b/tests/Makefile.include
++++ b/tests/qemu-iotests/group
-@@ -XXX,XX +XXX,XX @@ check-unit-y += tests/test-bdrv-drain$(EXESUF)
+@@ -XXX,XX +XXX,XX @@
- check-unit-y += tests/test-blockjob$(EXESUF)
+rw quick
- check-unit-y += tests/test-blockjob-txn$(EXESUF)
+rw auto quick
- check-unit-y += tests/test-block-backend$(EXESUF)
+rw quick
-+check-unit-y += tests/test-image-locking$(EXESUF)
++267 rw auto quick snapshot
  check-unit-y += tests/test-x86-cpuid$(EXESUF)
  # all code tested by test-x86-cpuid is inside topology.h
  ifeq ($(CONFIG_SOFTMMU),y)
@@ -XXX,XX +XXX,XX @@ tests/test-bdrv-drain$(EXESUF): tests/test-bdrv-drain.o $(test-block-obj-y) $(te
  tests/test-blockjob$(EXESUF): tests/test-blockjob.o $(test-block-obj-y) $(test-util-obj-y)
  tests/test-blockjob-txn$(EXESUF): tests/test-blockjob-txn.o $(test-block-obj-y) $(test-util-obj-y)
  tests/test-block-backend$(EXESUF): tests/test-block-backend.o $(test-block-obj-y) $(test-util-obj-y)
 +tests/test-image-locking$(EXESUF): tests/test-image-locking.o $(test-block-obj-y) $(test-util-obj-y)
  tests/test-thread-pool$(EXESUF): tests/test-thread-pool.o $(test-block-obj-y)
  tests/test-iov$(EXESUF): tests/test-iov.o $(test-util-obj-y)
  tests/test-hbitmap$(EXESUF): tests/test-hbitmap.o $(test-util-obj-y) $(test-crypto-obj-y)
 --
-.19.1
+.20.1

-[Qemu-devel] [PULL 09/14] block: Make more block drivers compile-time configurable
+[PULL v3 3/4] iotests: Require Python 3.6 or later
-From: Jeff Cody <jcody@redhat.com>
+Running iotests is not required to build QEMU, so we can have stricter
 version requirements for Python here and can make use of new features
 and drop compatibility code earlier.
-This adds configure options to control the following block drivers:
+This makes qemu-iotests skip all Python tests if a Python version before
 .6 is used for the build.
-* Bochs
+Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
-* Cloop
+Signed-off-by: Kevin Wolf <kwolf@redhat.com>
-* Dmg
+Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
-* Qcow (V1)
+Reviewed-by: Thomas Huth <thuth@redhat.com>
-* Vdi
+Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
-* Vvfat
+Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-* qed
+---
-* parallels
+ tests/qemu-iotests/check | 13 ++++++++++++-
-* sheepdog
+file changed, 12 insertions(+), 1 deletion(-)
-Each of these defaults to being enabled.
+diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
 Signed-off-by: Jeff Cody <jcody@redhat.com>
 Signed-off-by: Markus Armbruster <armbru@redhat.com>
 Message-id: 20181107063644.2254-1-armbru@redhat.com
 Signed-off-by: Max Reitz <mreitz@redhat.com>
 ---
  configure           | 91 +++++++++++++++++++++++++++++++++++++++++++++
  block/Makefile.objs | 22 ++++++++---
 files changed, 107 insertions(+), 6 deletions(-)
 diff --git a/configure b/configure
 index XXXXXXX..XXXXXXX 100755
---- a/configure
+--- a/tests/qemu-iotests/check
-+++ b/configure
++++ b/tests/qemu-iotests/check
-@@ -XXX,XX +XXX,XX @@ tcmalloc="no"
+@@ -XXX,XX +XXX,XX @@ then
- jemalloc="no"
+     export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper"
  replication="yes"
  vxhs=""
 +bochs="yes"
 +cloop="yes"
 +dmg="yes"
 +qcow1="yes"
 +vdi="yes"
 +vvfat="yes"
 +qed="yes"
 +parallels="yes"
 +sheepdog="yes"
  libxml2=""
  docker="no"
  debug_mutex="no"
@@ -XXX,XX +XXX,XX @@ for opt do
    ;;
    --enable-vxhs) vxhs="yes"
    ;;
 +  --disable-bochs) bochs="no"
 +  ;;
 +  --enable-bochs) bochs="yes"
 +  ;;
 +  --disable-cloop) cloop="no"
 +  ;;
 +  --enable-cloop) cloop="yes"
 +  ;;
 +  --disable-dmg) dmg="no"
 +  ;;
 +  --enable-dmg) dmg="yes"
 +  ;;
 +  --disable-qcow1) qcow1="no"
 +  ;;
 +  --enable-qcow1) qcow1="yes"
 +  ;;
 +  --disable-vdi) vdi="no"
 +  ;;
 +  --enable-vdi) vdi="yes"
 +  ;;
 +  --disable-vvfat) vvfat="no"
 +  ;;
 +  --enable-vvfat) vvfat="yes"
 +  ;;
 +  --disable-qed) qed="no"
 +  ;;
 +  --enable-qed) qed="yes"
 +  ;;
 +  --disable-parallels) parallels="no"
 +  ;;
 +  --enable-parallels) parallels="yes"
 +  ;;
 +  --disable-sheepdog) sheepdog="no"
 +  ;;
 +  --enable-sheepdog) sheepdog="yes"
 +  ;;
    --disable-vhost-user) vhost_user="no"
    ;;
    --enable-vhost-user)
@@ -XXX,XX +XXX,XX @@ disabled with --disable-FEATURE, default is enabled if available:
    qom-cast-debug  cast debugging support
    tools           build qemu-io, qemu-nbd and qemu-image tools
    vxhs            Veritas HyperScale vDisk backend support
 +  bochs           bochs image format support
 +  cloop           cloop image format support
 +  dmg             dmg image format support
 +  qcow1           qcow v1 image format support
 +  vdi             vdi image format support
 +  vvfat           vvfat image format support
 +  qed             qed image format support
 +  parallels       parallels image format support
 +  sheepdog        sheepdog block driver support
    crypto-afalg    Linux AF_ALG crypto backend driver
    vhost-user      vhost-user support
    capstone        capstone disassembler support
@@ -XXX,XX +XXX,XX @@ echo "jemalloc support  $jemalloc"
  echo "avx2 optimization $avx2_opt"
  echo "replication support $replication"
  echo "VxHS block device $vxhs"
 +echo "bochs support     $bochs"
 +echo "cloop support     $cloop"
 +echo "dmg support       $dmg"
 +echo "qcow v1 support   $qcow1"
 +echo "vdi support       $vdi"
 +echo "vvfat support     $vvfat"
 +echo "qed support       $qed"
 +echo "parallels support $parallels"
 +echo "sheepdog support  $sheepdog"
  echo "capstone          $capstone"
  echo "docker            $docker"
  echo "libpmem support   $libpmem"
@@ -XXX,XX +XXX,XX @@ if test "$libpmem" = "yes" ; then
    echo "CONFIG_LIBPMEM=y" >> $config_host_mak
  fi
-+if test "$bochs" = "yes" ; then
++python_usable=false
-+  echo "CONFIG_BOCHS=y" >> $config_host_mak
++if $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)'
-+fi
++then
-+if test "$cloop" = "yes" ; then
++    python_usable=true
 +  echo "CONFIG_CLOOP=y" >> $config_host_mak
 +fi
 +if test "$dmg" = "yes" ; then
 +  echo "CONFIG_DMG=y" >> $config_host_mak
 +fi
 +if test "$qcow1" = "yes" ; then
 +  echo "CONFIG_QCOW1=y" >> $config_host_mak
 +fi
 +if test "$vdi" = "yes" ; then
 +  echo "CONFIG_VDI=y" >> $config_host_mak
 +fi
 +if test "$vvfat" = "yes" ; then
 +  echo "CONFIG_VVFAT=y" >> $config_host_mak
 +fi
 +if test "$qed" = "yes" ; then
 +  echo "CONFIG_QED=y" >> $config_host_mak
 +fi
 +if test "$parallels" = "yes" ; then
 +  echo "CONFIG_PARALLELS=y" >> $config_host_mak
 +fi
 +if test "$sheepdog" = "yes" ; then
 +  echo "CONFIG_SHEEPDOG=y" >> $config_host_mak
 +fi
 +
- if test "$tcg_interpreter" = "yes"; then
+ default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p')
-   QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
+ default_alias_machine=$($QEMU_PROG -machine help | \
- elif test "$ARCH" = "sparc64" ; then
+    sed -n "/(alias of $default_machine)/ { s/ .*//p; q; }")
-diff --git a/block/Makefile.objs b/block/Makefile.objs
+@@ -XXX,XX +XXX,XX @@ do
-index XXXXXXX..XXXXXXX 100644
+         start=$(_wallclock)
---- a/block/Makefile.objs
-+++ b/block/Makefile.objs
+         if [ "$(head -n 1 "$source_iotests/$seq")" == "#!/usr/bin/env python" ]; then
-@@ -XXX,XX +XXX,XX @@
+-            run_command="$PYTHON $seq"
--block-obj-y += raw-format.o qcow.o vdi.o vmdk.o cloop.o bochs.o vpc.o vvfat.o dmg.o
++            if $python_usable; then
-+block-obj-y += raw-format.o vmdk.o vpc.o
++                run_command="$PYTHON $seq"
-+block-obj-$(CONFIG_QCOW1) += qcow.o
++            else
-+block-obj-$(CONFIG_VDI) += vdi.o
++                run_command="false"
-+block-obj-$(CONFIG_CLOOP) += cloop.o
++                echo "Unsupported Python version" > $seq.notrun
-+block-obj-$(CONFIG_BOCHS) += bochs.o
++            fi
-+block-obj-$(CONFIG_VVFAT) += vvfat.o
+         else
-+block-obj-$(CONFIG_DMG) += dmg.o
+             run_command="./$seq"
-+
+         fi
  block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-cache.o qcow2-bitmap.o
 -block-obj-y += qed.o qed-l2-cache.o qed-table.o qed-cluster.o
 -block-obj-y += qed-check.o
 +block-obj-$(CONFIG_QED) += qed.o qed-l2-cache.o qed-table.o qed-cluster.o
 +block-obj-$(CONFIG_QED) += qed-check.o
  block-obj-y += vhdx.o vhdx-endian.o vhdx-log.o
  block-obj-y += quorum.o
 -block-obj-y += parallels.o blkdebug.o blkverify.o blkreplay.o
 +block-obj-y += blkdebug.o blkverify.o blkreplay.o
 +block-obj-$(CONFIG_PARALLELS) += parallels.o
  block-obj-y += blklogwrites.o
  block-obj-y += block-backend.o snapshot.o qapi.o
  block-obj-$(CONFIG_WIN32) += file-win32.o win32-aio.o
@@ -XXX,XX +XXX,XX @@ block-obj-y += null.o mirror.o commit.o io.o create.o
  block-obj-y += throttle-groups.o
  block-obj-$(CONFIG_LINUX) += nvme.o
 -block-obj-y += nbd.o nbd-client.o sheepdog.o
 +block-obj-y += nbd.o nbd-client.o
 +block-obj-$(CONFIG_SHEEPDOG) += sheepdog.o
  block-obj-$(CONFIG_LIBISCSI) += iscsi.o
  block-obj-$(if $(CONFIG_LIBISCSI),y,n) += iscsi-opts.o
  block-obj-$(CONFIG_LIBNFS) += nfs.o
@@ -XXX,XX +XXX,XX @@ gluster.o-libs     := $(GLUSTERFS_LIBS)
  vxhs.o-libs        := $(VXHS_LIBS)
  ssh.o-cflags       := $(LIBSSH2_CFLAGS)
  ssh.o-libs         := $(LIBSSH2_LIBS)
 -block-obj-$(if $(CONFIG_BZIP2),m,n) += dmg-bz2.o
 +block-obj-dmg-bz2-$(CONFIG_BZIP2) += dmg-bz2.o
 +block-obj-$(if $(CONFIG_DMG),m,n) += $(block-obj-dmg-bz2-y)
  dmg-bz2.o-libs     := $(BZIP2_LIBS)
  qcow.o-libs        := -lz
  linux-aio.o-libs   := -laio
 --
-.19.1
+.20.1

-[Qemu-devel] [PULL 02/14] blockdev: handle error on block latency histogram set error
+[PULL v3 4/4] iotests: Remove Python 2 compatibility code
-From: zhenwei pi <pizhenwei@bytedance.com>
+Some scripts check the Python version number and have two code paths to
 accomodate both Python 2 and 3. Remove the code specific to Python 2 and
 assert the minimum version of 3.6 instead (check skips Python tests in
 this case, so the assertion would only ever trigger if a Python script
 is executed manually).
-Function block_latency_histogram_set may return error, but qapi ignore this.
+Signed-off-by: Kevin Wolf <kwolf@redhat.com>
-This can be reproduced easily by qmp command:
+Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
-virsh qemu-monitor-command INSTANCE '{"execute":"x-block-latency-histogram-set",
+Reviewed-by: Thomas Huth <thuth@redhat.com>
-"arguments":{"device":"drive-virtio-disk1","boundaries":[10,200,40]}}'
+Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
-In fact this command does not work, but we still get success result.
+Reviewed-by: John Snow <jsnow@redhat.com>
 Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 ---
  tests/qemu-iotests/044        |  3 ---
  tests/qemu-iotests/163        |  3 ---
  tests/qemu-iotests/iotests.py | 13 +++----------
 files changed, 3 insertions(+), 16 deletions(-)
-qmp_x_block_latency_histogram_set is a batch setting API, report error ASAP.
+diff --git a/tests/qemu-iotests/044 b/tests/qemu-iotests/044
+index XXXXXXX..XXXXXXX 100755
-Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
+--- a/tests/qemu-iotests/044
-Signed-off-by: Kevin Wolf <kwolf@redhat.com>
++++ b/tests/qemu-iotests/044
----
+@@ -XXX,XX +XXX,XX @@ import struct
- blockdev.c | 19 ++++++++++++++++---
+ import subprocess
-file changed, 16 insertions(+), 3 deletions(-)
+ import sys
-diff --git a/blockdev.c b/blockdev.c
+-if sys.version_info.major == 2:
 -    range = xrange
 -
  test_img = os.path.join(iotests.test_dir, 'test.img')
  class TestRefcountTableGrowth(iotests.QMPTestCase):
 diff --git a/tests/qemu-iotests/163 b/tests/qemu-iotests/163
 index XXXXXXX..XXXXXXX 100755
 --- a/tests/qemu-iotests/163
 +++ b/tests/qemu-iotests/163
@@ -XXX,XX +XXX,XX @@
  import os, random, iotests, struct, qcow2, sys
  from iotests import qemu_img, qemu_io, image_size
 -if sys.version_info.major == 2:
 -    range = xrange
 -
  test_img = os.path.join(iotests.test_dir, 'test.img')
  check_img = os.path.join(iotests.test_dir, 'check.img')
 diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
 index XXXXXXX..XXXXXXX 100644
---- a/blockdev.c
+--- a/tests/qemu-iotests/iotests.py
-+++ b/blockdev.c
++++ b/tests/qemu-iotests/iotests.py
-@@ -XXX,XX +XXX,XX @@ void qmp_x_block_latency_histogram_set(
+@@ -XXX,XX +XXX,XX @@ from collections import OrderedDict
- {
+ sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
-     BlockBackend *blk = blk_by_name(device);
+ from qemu import qtest
-     BlockAcctStats *stats;
-+    int ret;
++assert sys.version_info >= (3,6)
-     if (!blk) {
+ # This will not work if arguments contain spaces but is necessary if we
-         error_setg(errp, "Device '%s' not found", device);
+ # want to support the override options that ./check supports.
-@@ -XXX,XX +XXX,XX @@ void qmp_x_block_latency_histogram_set(
+@@ -XXX,XX +XXX,XX @@ def image_size(img):
-     }
+     return json.loads(r)['virtual-size']
-     if (has_boundaries || has_boundaries_read) {
+ def is_str(val):
--        block_latency_histogram_set(
+-    if sys.version_info.major >= 3:
-+        ret = block_latency_histogram_set(
+-        return isinstance(val, str)
-             stats, BLOCK_ACCT_READ,
+-    else:
-             has_boundaries_read ? boundaries_read : boundaries);
+-        return isinstance(val, str) or isinstance(val, unicode)
-+        if (ret) {
++    return isinstance(val, str)
-+            error_setg(errp, "Device '%s' set read boundaries fail", device);
-+            return;
+ test_dir_re = re.compile(r"%s" % test_dir)
-+        }
+ def filter_test_dir(msg):
-     }
+@@ -XXX,XX +XXX,XX @@ def execute_test(test_function=None,
+     else:
-     if (has_boundaries || has_boundaries_write) {
+         # We need to filter out the time taken from the output so that
--        block_latency_histogram_set(
+         # qemu-iotest can reliably diff the results against master output.
-+        ret = block_latency_histogram_set(
+-        if sys.version_info.major >= 3:
-             stats, BLOCK_ACCT_WRITE,
+-            output = io.StringIO()
-             has_boundaries_write ? boundaries_write : boundaries);
+-        else:
-+        if (ret) {
+-            # io.StringIO is for unicode strings, which is not what
-+            error_setg(errp, "Device '%s' set write boundaries fail", device);
+-            # 2.x's test runner emits.
-+            return;
+-            output = io.BytesIO()
-+        }
++        output = io.StringIO()
-     }
+     logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
      if (has_boundaries || has_boundaries_flush) {
 -        block_latency_histogram_set(
 +        ret = block_latency_histogram_set(
              stats, BLOCK_ACCT_FLUSH,
              has_boundaries_flush ? boundaries_flush : boundaries);
 +        if (ret) {
 +            error_setg(errp, "Device '%s' set flush boundaries fail", device);
 +            return;
 +        }
      }
  }
 --
-.19.1
+.20.1

-[Qemu-devel] [PULL 03/14] blockdev: Consistently use snapshot_node_name in external_snapshot_prepare()
+Deleted patch
-From: Peter Maydell <peter.maydell@linaro.org>
-In the function external_snapshot_prepare() we have a
-BlockdevSnapshotSync struct, which has the usual combination
-of has_snapshot_node_name and snapshot_node_name fields for an
-optional field. We set up a local variable
-        const char *snapshot_node_name =
-            s->has_snapshot_node_name ? s->snapshot_node_name : NULL;
-and then mostly use "if (!snapshot_node_name)" for checking
-whether we have a snapshot node name. The exception is that in
-one place we check s->has_snapshot_node_name instead. This
-confuses Coverity (CID 1396473), which thinks it might be
-possible to get here with s->has_snapshot_node_name true but
-snapshot_node_name NULL, and warns that the call to
-qdict_put_str() will segfault in that case.
-Make the code consistent and unconfuse Coverity by using
-the same check for this conditional that we do in the rest
-of the surrounding code.
-Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
-Reviewed-by: Alberto Garcia <berto@igalia.com>
-Signed-off-by: Kevin Wolf <kwolf@redhat.com>
----
- blockdev.c | 2 +-
-file changed, 1 insertion(+), 1 deletion(-)
-diff --git a/blockdev.c b/blockdev.c
-index XXXXXXX..XXXXXXX 100644
---- a/blockdev.c
-+++ b/blockdev.c
-@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
-         }
-         options = qdict_new();
--        if (s->has_snapshot_node_name) {
-+        if (snapshot_node_name) {
-             qdict_put_str(options, "node-name", snapshot_node_name);
-         }
-         qdict_put_str(options, "driver", format);
---
-.19.1

-[Qemu-devel] [PULL 04/14] nvme: don't unref ctrl_mem when device unrealized
+Deleted patch
-From: Li Qiang <liq3ea@gmail.com>
-Currently, when hotplug/unhotplug nvme device, it will cause an
-assert in object.c. Following is the backtrack:
-ERROR:qom/object.c:981:object_unref: assertion failed: (obj->ref > 0)
-Thread 2 "qemu-system-x86" received signal SIGABRT, Aborted.
-[Switching to Thread 0x7fffcbd32700 (LWP 18844)]
-x00007fffdb9e4fff in raise () from /lib/x86_64-linux-gnu/libc.so.6
-(gdb) bt
-/lib/x86_64-linux-gnu/libglib-2.0.so.0
-/lib/x86_64-linux-gnu/libglib-2.0.so.0
-qom/object.c:981
-/home/liqiang02/qemu-upstream/qemu/memory.c:1732
-/home/liqiang02/qemu-upstream/qemu/memory.c:285
-util/qemu-thread-posix.c:504
-/lib/x86_64-linux-gnu/libpthread.so.0
-This is caused by memory_region_unref in nvme_exit.
-Remove it to make the PCIdevice refcount correct.
-Signed-off-by: Li Qiang <liq3ea@gmail.com>
-Reviewed-by: Igor Mammedov <imammedo@redhat.com>
-Signed-off-by: Kevin Wolf <kwolf@redhat.com>
----
- hw/block/nvme.c | 3 ---
-file changed, 3 deletions(-)
-diff --git a/hw/block/nvme.c b/hw/block/nvme.c
-index XXXXXXX..XXXXXXX 100644
---- a/hw/block/nvme.c
-+++ b/hw/block/nvme.c
-@@ -XXX,XX +XXX,XX @@ static void nvme_exit(PCIDevice *pci_dev)
-     g_free(n->namespaces);
-     g_free(n->cq);
-     g_free(n->sq);
--    if (n->cmbsz) {
--        memory_region_unref(&n->ctrl_mem);
--    }
-     msix_uninit_exclusive_bar(pci_dev);
- }
---
-.19.1

-[Qemu-devel] [PULL 05/14] nvme: free cmbuf in nvme_exit
+Deleted patch
-From: Li Qiang <liq3ea@gmail.com>
-This avoid a memory leak in unhotplug nvme device.
-Signed-off-by: Li Qiang <liq3ea@gmail.com>
-Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
-Signed-off-by: Kevin Wolf <kwolf@redhat.com>
----
- hw/block/nvme.c | 3 +++
-file changed, 3 insertions(+)
-diff --git a/hw/block/nvme.c b/hw/block/nvme.c
-index XXXXXXX..XXXXXXX 100644
---- a/hw/block/nvme.c
-+++ b/hw/block/nvme.c
-@@ -XXX,XX +XXX,XX @@ static void nvme_exit(PCIDevice *pci_dev)
-     g_free(n->cq);
-     g_free(n->sq);
-+    if (n->cmb_size_mb) {
-+        g_free(n->cmbuf);
-+    }
-     msix_uninit_exclusive_bar(pci_dev);
- }
---
-.19.1

-[Qemu-devel] [PULL 06/14] file-posix: Skip effectiveless OFD lock operations
+Deleted patch
-From: Fam Zheng <famz@redhat.com>
-If we know we've already locked the bytes, don't do it again; similarly
-don't unlock a byte if we haven't locked it. This doesn't change the
-behavior, but fixes a corner case explained below.
-Libvirt had an error handling bug that an image can get its (ownership,
-file mode, SELinux) permissions changed (RHBZ 1584982) by mistake behind
-QEMU. Specifically, an image in use by Libvirt VM has:
-    $ ls -lhZ b.img
-    -rw-r--r--. qemu qemu system_u:object_r:svirt_image_t:s0:c600,c690 b.img
-Trying to attach it a second time won't work because of image locking.
-And after the error, it becomes:
-    $ ls -lhZ b.img
-    -rw-r--r--. root root system_u:object_r:virt_image_t:s0 b.img
-Then, we won't be able to do OFD lock operations with the existing fd.
-In other words, the code such as in blk_detach_dev:
-    blk_set_perm(blk, 0, BLK_PERM_ALL, &error_abort);
-can abort() QEMU, out of environmental changes.
-This patch is an easy fix to this and the change is regardlessly
-reasonable, so do it.
-Signed-off-by: Fam Zheng <famz@redhat.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Signed-off-by: Kevin Wolf <kwolf@redhat.com>
----
- block/file-posix.c | 54 +++++++++++++++++++++++++++++++++++++---------
-file changed, 44 insertions(+), 10 deletions(-)
-diff --git a/block/file-posix.c b/block/file-posix.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/file-posix.c
-+++ b/block/file-posix.c
-@@ -XXX,XX +XXX,XX @@ typedef struct BDRVRawState {
-     uint64_t perm;
-     uint64_t shared_perm;
-+    /* The perms bits whose corresponding bytes are already locked in
-+     * s->lock_fd. */
-+    uint64_t locked_perm;
-+    uint64_t locked_shared_perm;
-+
- #ifdef CONFIG_XFS
-     bool is_xfs:1;
- #endif
-@@ -XXX,XX +XXX,XX @@ typedef enum {
-  * file; if @unlock == true, also unlock the unneeded bytes.
-  * @shared_perm_lock_bits is the mask of all permissions that are NOT shared.
-  */
--static int raw_apply_lock_bytes(int fd,
-+static int raw_apply_lock_bytes(BDRVRawState *s, int fd,
-                                 uint64_t perm_lock_bits,
-                                 uint64_t shared_perm_lock_bits,
-                                 bool unlock, Error **errp)
- {
-     int ret;
-     int i;
-+    uint64_t locked_perm, locked_shared_perm;
-+
-+    if (s) {
-+        locked_perm = s->locked_perm;
-+        locked_shared_perm = s->locked_shared_perm;
-+    } else {
-+        /*
-+         * We don't have the previous bits, just lock/unlock for each of the
-+         * requested bits.
-+         */
-+        if (unlock) {
-+            locked_perm = BLK_PERM_ALL;
-+            locked_shared_perm = BLK_PERM_ALL;
-+        } else {
-+            locked_perm = 0;
-+            locked_shared_perm = 0;
-+        }
-+    }
-     PERM_FOREACH(i) {
-         int off = RAW_LOCK_PERM_BASE + i;
--        if (perm_lock_bits & (1ULL << i)) {
-+        uint64_t bit = (1ULL << i);
-+        if ((perm_lock_bits & bit) && !(locked_perm & bit)) {
-             ret = qemu_lock_fd(fd, off, 1, false);
-             if (ret) {
-                 error_setg(errp, "Failed to lock byte %d", off);
-                 return ret;
-+            } else if (s) {
-+                s->locked_perm |= bit;
-             }
--        } else if (unlock) {
-+        } else if (unlock && (locked_perm & bit) && !(perm_lock_bits & bit)) {
-             ret = qemu_unlock_fd(fd, off, 1);
-             if (ret) {
-                 error_setg(errp, "Failed to unlock byte %d", off);
-                 return ret;
-+            } else if (s) {
-+                s->locked_perm &= ~bit;
-             }
-         }
-     }
-     PERM_FOREACH(i) {
-         int off = RAW_LOCK_SHARED_BASE + i;
--        if (shared_perm_lock_bits & (1ULL << i)) {
-+        uint64_t bit = (1ULL << i);
-+        if ((shared_perm_lock_bits & bit) && !(locked_shared_perm & bit)) {
-             ret = qemu_lock_fd(fd, off, 1, false);
-             if (ret) {
-                 error_setg(errp, "Failed to lock byte %d", off);
-                 return ret;
-+            } else if (s) {
-+                s->locked_shared_perm |= bit;
-             }
--        } else if (unlock) {
-+        } else if (unlock && (locked_shared_perm & bit) &&
-+                   !(shared_perm_lock_bits & bit)) {
-             ret = qemu_unlock_fd(fd, off, 1);
-             if (ret) {
-                 error_setg(errp, "Failed to unlock byte %d", off);
-                 return ret;
-+            } else if (s) {
-+                s->locked_shared_perm &= ~bit;
-             }
-         }
-     }
-@@ -XXX,XX +XXX,XX @@ static int raw_handle_perm_lock(BlockDriverState *bs,
-     switch (op) {
-     case RAW_PL_PREPARE:
--        ret = raw_apply_lock_bytes(s->lock_fd, s->perm | new_perm,
-+        ret = raw_apply_lock_bytes(s, s->lock_fd, s->perm | new_perm,
-                                    ~s->shared_perm | ~new_shared,
-                                    false, errp);
-         if (!ret) {
-@@ -XXX,XX +XXX,XX @@ static int raw_handle_perm_lock(BlockDriverState *bs,
-         op = RAW_PL_ABORT;
-         /* fall through to unlock bytes. */
-     case RAW_PL_ABORT:
--        raw_apply_lock_bytes(s->lock_fd, s->perm, ~s->shared_perm,
-+        raw_apply_lock_bytes(s, s->lock_fd, s->perm, ~s->shared_perm,
-                              true, &local_err);
-         if (local_err) {
-             /* Theoretically the above call only unlocks bytes and it cannot
-@@ -XXX,XX +XXX,XX @@ static int raw_handle_perm_lock(BlockDriverState *bs,
-         }
-         break;
-     case RAW_PL_COMMIT:
--        raw_apply_lock_bytes(s->lock_fd, new_perm, ~new_shared,
-+        raw_apply_lock_bytes(s, s->lock_fd, new_perm, ~new_shared,
-                              true, &local_err);
-         if (local_err) {
-             /* Theoretically the above call only unlocks bytes and it cannot
-@@ -XXX,XX +XXX,XX @@ raw_co_create(BlockdevCreateOptions *options, Error **errp)
-     shared = BLK_PERM_ALL & ~BLK_PERM_RESIZE;
-     /* Step one: Take locks */
--    result = raw_apply_lock_bytes(fd, perm, ~shared, false, errp);
-+    result = raw_apply_lock_bytes(NULL, fd, perm, ~shared, false, errp);
-     if (result < 0) {
-         goto out_close;
-     }
-@@ -XXX,XX +XXX,XX @@ raw_co_create(BlockdevCreateOptions *options, Error **errp)
-     }
- out_unlock:
--    raw_apply_lock_bytes(fd, 0, 0, true, &local_err);
-+    raw_apply_lock_bytes(NULL, fd, 0, 0, true, &local_err);
-     if (local_err) {
-         /* The above call should not fail, and if it does, that does
-          * not mean the whole creation operation has failed.  So
---
-.19.1

-[Qemu-devel] [PULL 07/14] file-posix: Drop s->lock_fd
+Deleted patch
-From: Fam Zheng <famz@redhat.com>
-The lock_fd field is not strictly necessary because transferring locked
-bytes from old fd to the new one shouldn't fail anyway. This spares the
-user one fd per image.
-Signed-off-by: Fam Zheng <famz@redhat.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Signed-off-by: Kevin Wolf <kwolf@redhat.com>
----
- block/file-posix.c | 37 +++++++++++++------------------------
-file changed, 13 insertions(+), 24 deletions(-)
-diff --git a/block/file-posix.c b/block/file-posix.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/file-posix.c
-+++ b/block/file-posix.c
-@@ -XXX,XX +XXX,XX @@ do { \
- typedef struct BDRVRawState {
-     int fd;
--    int lock_fd;
-     bool use_lock;
-     int type;
-     int open_flags;
-@@ -XXX,XX +XXX,XX @@ typedef struct BDRVRawState {
-     uint64_t shared_perm;
-     /* The perms bits whose corresponding bytes are already locked in
--     * s->lock_fd. */
-+     * s->fd. */
-     uint64_t locked_perm;
-     uint64_t locked_shared_perm;
-@@ -XXX,XX +XXX,XX @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
-     }
-     s->fd = fd;
--    s->lock_fd = -1;
--    if (s->use_lock) {
--        fd = qemu_open(filename, s->open_flags);
--        if (fd < 0) {
--            ret = -errno;
--            error_setg_errno(errp, errno, "Could not open '%s' for locking",
--                             filename);
--            qemu_close(s->fd);
--            goto fail;
--        }
--        s->lock_fd = fd;
--    }
-     s->perm = 0;
-     s->shared_perm = BLK_PERM_ALL;
-@@ -XXX,XX +XXX,XX @@ static int raw_handle_perm_lock(BlockDriverState *bs,
-         return 0;
-     }
--    assert(s->lock_fd > 0);
--
-     switch (op) {
-     case RAW_PL_PREPARE:
--        ret = raw_apply_lock_bytes(s, s->lock_fd, s->perm | new_perm,
-+        ret = raw_apply_lock_bytes(s, s->fd, s->perm | new_perm,
-                                    ~s->shared_perm | ~new_shared,
-                                    false, errp);
-         if (!ret) {
--            ret = raw_check_lock_bytes(s->lock_fd, new_perm, new_shared, errp);
-+            ret = raw_check_lock_bytes(s->fd, new_perm, new_shared, errp);
-             if (!ret) {
-                 return 0;
-             }
-@@ -XXX,XX +XXX,XX @@ static int raw_handle_perm_lock(BlockDriverState *bs,
-         op = RAW_PL_ABORT;
-         /* fall through to unlock bytes. */
-     case RAW_PL_ABORT:
--        raw_apply_lock_bytes(s, s->lock_fd, s->perm, ~s->shared_perm,
-+        raw_apply_lock_bytes(s, s->fd, s->perm, ~s->shared_perm,
-                              true, &local_err);
-         if (local_err) {
-             /* Theoretically the above call only unlocks bytes and it cannot
-@@ -XXX,XX +XXX,XX @@ static int raw_handle_perm_lock(BlockDriverState *bs,
-         }
-         break;
-     case RAW_PL_COMMIT:
--        raw_apply_lock_bytes(s, s->lock_fd, new_perm, ~new_shared,
-+        raw_apply_lock_bytes(s, s->fd, new_perm, ~new_shared,
-                              true, &local_err);
-         if (local_err) {
-             /* Theoretically the above call only unlocks bytes and it cannot
-@@ -XXX,XX +XXX,XX @@ static void raw_reopen_commit(BDRVReopenState *state)
- {
-     BDRVRawReopenState *rs = state->opaque;
-     BDRVRawState *s = state->bs->opaque;
-+    Error *local_err = NULL;
-     s->check_cache_dropped = rs->check_cache_dropped;
-     s->open_flags = rs->open_flags;
-+    /* Copy locks to the new fd before closing the old one. */
-+    raw_apply_lock_bytes(NULL, rs->fd, s->locked_perm,
-+                         ~s->locked_shared_perm, false, &local_err);
-+    if (local_err) {
-+        /* shouldn't fail in a sane host, but report it just in case. */
-+        error_report_err(local_err);
-+    }
-     qemu_close(s->fd);
-     s->fd = rs->fd;
-@@ -XXX,XX +XXX,XX @@ static void raw_close(BlockDriverState *bs)
-         qemu_close(s->fd);
-         s->fd = -1;
-     }
--    if (s->lock_fd >= 0) {
--        qemu_close(s->lock_fd);
--        s->lock_fd = -1;
--    }
- }
- /**
---
-.19.1

-[Qemu-devel] [PULL 10/14] job: Fix off-by-one assert checks for JobSTT and JobVerbTable
+Deleted patch
-From: Liam Merwick <Liam.Merwick@oracle.com>
-In the assert checking the array dereference of JobVerbTable[verb]
-in job_apply_verb() the check of the index, verb, allows an overrun
-because an index equal to the array size is permitted.
-Similarly, in the assert check of JobSTT[s0][s1] with index s1
-in job_state_transition(), an off-by-one overrun is not flagged
-either.
-This is not a run-time issue as there are no callers actually
-passing in the max value.
-Signed-off-by: Liam Merwick <Liam.Merwick@oracle.com>
-Reviewed-by: Darren Kenny <Darren.Kenny@oracle.com>
-Reviewed-by: Mark Kanda <Mark.Kanda@oracle.com>
-Reviewed-by: Eric Blake <eblake@redhat.com>
-Reviewed-by: John Snow <jsnow@redhat.com>
-Message-id: 1541453919-25973-2-git-send-email-Liam.Merwick@oracle.com
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- job.c | 4 ++--
-file changed, 2 insertions(+), 2 deletions(-)
-diff --git a/job.c b/job.c
-index XXXXXXX..XXXXXXX 100644
---- a/job.c
-+++ b/job.c
-@@ -XXX,XX +XXX,XX @@ bool job_is_internal(Job *job)
- static void job_state_transition(Job *job, JobStatus s1)
- {
-     JobStatus s0 = job->status;
--    assert(s1 >= 0 && s1 <= JOB_STATUS__MAX);
-+    assert(s1 >= 0 && s1 < JOB_STATUS__MAX);
-     trace_job_state_transition(job, job->ret,
-                                JobSTT[s0][s1] ? "allowed" : "disallowed",
-                                JobStatus_str(s0), JobStatus_str(s1));
-@@ -XXX,XX +XXX,XX @@ static void job_state_transition(Job *job, JobStatus s1)
- int job_apply_verb(Job *job, JobVerb verb, Error **errp)
- {
-     JobStatus s0 = job->status;
--    assert(verb >= 0 && verb <= JOB_VERB__MAX);
-+    assert(verb >= 0 && verb < JOB_VERB__MAX);
-     trace_job_apply_verb(job, JobStatus_str(s0), JobVerb_str(verb),
-                          JobVerbTable[verb][s0] ? "allowed" : "prohibited");
-     if (JobVerbTable[verb][s0]) {
---
-.19.1

-[Qemu-devel] [PULL 11/14] block: Null pointer dereference in blk_root_get_parent_desc()
+Deleted patch
-From: Liam Merwick <Liam.Merwick@oracle.com>
-The dev_id returned by the call to blk_get_attached_dev_id() in
-blk_root_get_parent_desc() can be NULL (an internal call to
-object_get_canonical_path may have returned NULL).
-Instead of just checking this case before before dereferencing,
-adjust blk_get_attached_dev_id() to return the empty string if no
-object path can be found (similar to the case when blk->dev is NULL
-and an empty string is returned).
-Signed-off-by: Liam Merwick <Liam.Merwick@oracle.com>
-Message-id: 1541453919-25973-3-git-send-email-Liam.Merwick@oracle.com
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/block-backend.c | 3 ++-
-file changed, 2 insertions(+), 1 deletion(-)
-diff --git a/block/block-backend.c b/block/block-backend.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/block-backend.c
-+++ b/block/block-backend.c
-@@ -XXX,XX +XXX,XX @@ char *blk_get_attached_dev_id(BlockBackend *blk)
-     } else if (dev->id) {
-         return g_strdup(dev->id);
-     }
--    return object_get_canonical_path(OBJECT(dev));
-+
-+    return object_get_canonical_path(OBJECT(dev)) ?: g_strdup("");
- }
- /*
---
-.19.1

-[Qemu-devel] [PULL 12/14] qemu-img: assert block_job_get() does not return NULL in img_commit()
+Deleted patch
-From: Liam Merwick <Liam.Merwick@oracle.com>
-Although the function block_job_get() can return NULL, it would be a
-serious bug if it did so (because the job yields before executing anything
-(if it started successfully); but otherwise, commit_active_start() would
-have returned an error).  However, as a precaution, before dereferencing
-the 'job' pointer in img_commit() assert it is not NULL.
-Signed-off-by: Liam Merwick <Liam.Merwick@oracle.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-id: 1541453919-25973-4-git-send-email-Liam.Merwick@oracle.com
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- qemu-img.c | 1 +
-file changed, 1 insertion(+)
-diff --git a/qemu-img.c b/qemu-img.c
-index XXXXXXX..XXXXXXX 100644
---- a/qemu-img.c
-+++ b/qemu-img.c
-@@ -XXX,XX +XXX,XX @@ static int img_commit(int argc, char **argv)
-     }
-     job = block_job_get("commit");
-+    assert(job);
-     run_block_job(job, &local_err);
-     if (local_err) {
-         goto unref_backing;
---
-.19.1

-[Qemu-devel] [PULL 14/14] qcow2: Read outside array bounds in qcow2_pre_write_overlap_check()
+Deleted patch
-From: Liam Merwick <Liam.Merwick@oracle.com>
-The commit for 0e4e4318eaa5 increments QCOW2_OL_MAX_BITNR but does not
-add an array entry for QCOW2_OL_BITMAP_DIRECTORY_BITNR to metadata_ol_names[].
-As a result, an array dereference of metadata_ol_names[8] in
-qcow2_pre_write_overlap_check() could result in a read outside of the array bounds.
-Fixes: 0e4e4318eaa5 ('qcow2: add overlap check for bitmap directory')
-Cc: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-Signed-off-by: Liam Merwick <Liam.Merwick@oracle.com>
-Reviewed-by: Eric Blake <eblake@redhat.com>
-Reviewed-by: Max Reitz <mreitz@redhat.com>
-Message-id: 1541453919-25973-6-git-send-email-Liam.Merwick@oracle.com
-Signed-off-by: Max Reitz <mreitz@redhat.com>
----
- block/qcow2-refcount.c | 18 ++++++++++--------
-file changed, 10 insertions(+), 8 deletions(-)
-diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/qcow2-refcount.c
-+++ b/block/qcow2-refcount.c
-@@ -XXX,XX +XXX,XX @@ int qcow2_check_metadata_overlap(BlockDriverState *bs, int ign, int64_t offset,
- }
- static const char *metadata_ol_names[] = {
--    [QCOW2_OL_MAIN_HEADER_BITNR]    = "qcow2_header",
--    [QCOW2_OL_ACTIVE_L1_BITNR]      = "active L1 table",
--    [QCOW2_OL_ACTIVE_L2_BITNR]      = "active L2 table",
--    [QCOW2_OL_REFCOUNT_TABLE_BITNR] = "refcount table",
--    [QCOW2_OL_REFCOUNT_BLOCK_BITNR] = "refcount block",
--    [QCOW2_OL_SNAPSHOT_TABLE_BITNR] = "snapshot table",
--    [QCOW2_OL_INACTIVE_L1_BITNR]    = "inactive L1 table",
--    [QCOW2_OL_INACTIVE_L2_BITNR]    = "inactive L2 table",
-+    [QCOW2_OL_MAIN_HEADER_BITNR]        = "qcow2_header",
-+    [QCOW2_OL_ACTIVE_L1_BITNR]          = "active L1 table",
-+    [QCOW2_OL_ACTIVE_L2_BITNR]          = "active L2 table",
-+    [QCOW2_OL_REFCOUNT_TABLE_BITNR]     = "refcount table",
-+    [QCOW2_OL_REFCOUNT_BLOCK_BITNR]     = "refcount block",
-+    [QCOW2_OL_SNAPSHOT_TABLE_BITNR]     = "snapshot table",
-+    [QCOW2_OL_INACTIVE_L1_BITNR]        = "inactive L1 table",
-+    [QCOW2_OL_INACTIVE_L2_BITNR]        = "inactive L2 table",
-+    [QCOW2_OL_BITMAP_DIRECTORY_BITNR]   = "bitmap directory",
- };
-+QEMU_BUILD_BUG_ON(QCOW2_OL_MAX_BITNR != ARRAY_SIZE(metadata_ol_names));
- /*
-  * First performs a check for metadata overlaps (through
---
-.19.1

The following changes since commit 5704c36d25ee84e7129722cb0db53df9faefe943:

Merge remote-tracking branch 'remotes/kraxel/tags/fixes-31-20181112-pull-request' into staging (2018-11-12 15:55:40 +0000)

are available in the Git repository at:

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

for you to fetch changes up to 1a42e5d8298d1b0f90d2254e7d559391dd3a45ca:

Merge remote-tracking branch 'mreitz/tags/pull-block-2018-11-12' into queue-block (2018-11-12 17:57:32 +0100)

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

- file-posix: Don't waste a file descriptor for locking, don't lock the
  same bit multiple times
- nvme: Fix double free and memory leak
- Misc error handling fixes
- Added NULL checks found by static analysis
- Allow more block drivers to not be included in the qemu build

----------------------------------------------------------------
Fam Zheng (4):
      file-posix: Use error API properly
      file-posix: Skip effectiveless OFD lock operations
      file-posix: Drop s->lock_fd
      tests: Add unit tests for image locking

Jeff Cody (1):
      block: Make more block drivers compile-time configurable

Kevin Wolf (1):
      Merge remote-tracking branch 'mreitz/tags/pull-block-2018-11-12' into queue-block

Li Qiang (2):
      nvme: don't unref ctrl_mem when device unrealized
      nvme: free cmbuf in nvme_exit

Liam Merwick (5):
      job: Fix off-by-one assert checks for JobSTT and JobVerbTable
      block: Null pointer dereference in blk_root_get_parent_desc()
      qemu-img: assert block_job_get() does not return NULL in img_commit()
      block: Fix potential Null pointer dereferences in vvfat.c
      qcow2: Read outside array bounds in qcow2_pre_write_overlap_check()

Peter Maydell (1):
      blockdev: Consistently use snapshot_node_name in external_snapshot_prepare()

zhenwei pi (1):
      blockdev: handle error on block latency histogram set error

From: Fam Zheng <famz@redhat.com>

Use error_report for situations that affect user operation (i.e.  we're
actually returning error), and warn_report/warn_report_err when some
less critical error happened but the user operation can still carry on.

For raw_normalize_devicepath, add Error parameter to propagate to
its callers.

Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/file-posix.c | 39 ++++++++++++++++-----------------------
 1 file changed, 16 insertions(+), 23 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index XXXXXXX..XXXXXXX 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -XXX,XX +XXX,XX @@ static int cdrom_reopen(BlockDriverState *bs);
 #endif
 
 #if defined(__NetBSD__)
-static int raw_normalize_devicepath(const char **filename)
+static int raw_normalize_devicepath(const char **filename, Error **errp)
 {
     static char namebuf[PATH_MAX];
     const char *dp, *fname;
@@ -XXX,XX +XXX,XX @@ static int raw_normalize_devicepath(const char **filename)
     fname = *filename;
     dp = strrchr(fname, '/');
     if (lstat(fname, &sb) < 0) {
-        fprintf(stderr, "%s: stat failed: %s\n",
-            fname, strerror(errno));
+        error_setg_errno(errp, errno, "%s: stat failed", fname);
         return -errno;
     }
 
@@ -XXX,XX +XXX,XX @@ static int raw_normalize_devicepath(const char **filename)
         snprintf(namebuf, PATH_MAX, "%.*s/r%s",
             (int)(dp - fname), fname, dp + 1);
     }
-    fprintf(stderr, "%s is a block device", fname);
     *filename = namebuf;
-    fprintf(stderr, ", using %s\n", *filename);
+    warn_report("%s is a block device, using %s", fname, *filename);
 
     return 0;
 }
 #else
-static int raw_normalize_devicepath(const char **filename)
+static int raw_normalize_devicepath(const char **filename, Error **errp)
 {
     return 0;
 }
@@ -XXX,XX +XXX,XX @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
 
     filename = qemu_opt_get(opts, "filename");
 
-    ret = raw_normalize_devicepath(&filename);
+    ret = raw_normalize_devicepath(&filename, errp);
     if (ret != 0) {
-        error_setg_errno(errp, -ret, "Could not normalize device path");
         goto fail;
     }
 
@@ -XXX,XX +XXX,XX @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
     case ON_OFF_AUTO_ON:
         s->use_lock = true;
         if (!qemu_has_ofd_lock()) {
-            fprintf(stderr,
-                    "File lock requested but OFD locking syscall is "
-                    "unavailable, falling back to POSIX file locks.\n"
-                    "Due to the implementation, locks can be lost "
-                    "unexpectedly.\n");
+            warn_report("File lock requested but OFD locking syscall is "
+                        "unavailable, falling back to POSIX file locks");
+            error_printf("Due to the implementation, locks can be lost "
+                         "unexpectedly.\n");
         }
         break;
     case ON_OFF_AUTO_OFF:
@@ -XXX,XX +XXX,XX @@ static int raw_handle_perm_lock(BlockDriverState *bs,
             /* Theoretically the above call only unlocks bytes and it cannot
              * fail. Something weird happened, report it.
              */
-            error_report_err(local_err);
+            warn_report_err(local_err);
         }
         break;
     case RAW_PL_COMMIT:
@@ -XXX,XX +XXX,XX @@ static int raw_handle_perm_lock(BlockDriverState *bs,
             /* Theoretically the above call only unlocks bytes and it cannot
              * fail. Something weird happened, report it.
              */
-            error_report_err(local_err);
+            warn_report_err(local_err);
         }
         break;
     }
@@ -XXX,XX +XXX,XX @@ static int raw_reopen_prepare(BDRVReopenState *state,
     /* If we cannot use fcntl, or fcntl failed, fall back to qemu_open() */
     if (rs->fd == -1) {
         const char *normalized_filename = state->bs->filename;
-        ret = raw_normalize_devicepath(&normalized_filename);
-        if (ret < 0) {
-            error_setg_errno(errp, -ret, "Could not normalize device path");
-        } else {
+        ret = raw_normalize_devicepath(&normalized_filename, errp);
+        if (ret >= 0) {
             assert(!(rs->open_flags & O_CREAT));
             rs->fd = qemu_open(normalized_filename, rs->open_flags);
             if (rs->fd == -1) {
@@ -XXX,XX +XXX,XX @@ static int aio_worker(void *arg)
         ret = handle_aiocb_truncate(aiocb);
         break;
     default:
-        fprintf(stderr, "invalid aio request (0x%x)\n", aiocb->aio_type);
+        error_report("invalid aio request (0x%x)", aiocb->aio_type);
         ret = -EINVAL;
         break;
     }
@@ -XXX,XX +XXX,XX @@ out_unlock:
          * not mean the whole creation operation has failed.  So
          * report it the user for their convenience, but do not report
          * it to the caller. */
-        error_report_err(local_err);
+        warn_report_err(local_err);
     }
 
 out_close:
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn hdev_co_create_opts(const char *filename, QemuOpts *opts
 
     (void)has_prefix;
 
-    ret = raw_normalize_devicepath(&filename);
+    ret = raw_normalize_devicepath(&filename, errp);
     if (ret < 0) {
-        error_setg_errno(errp, -ret, "Could not normalize device path");
         return ret;
     }
 
-- 
2.19.1

From: zhenwei pi <pizhenwei@bytedance.com>

Function block_latency_histogram_set may return error, but qapi ignore this.
This can be reproduced easily by qmp command:
virsh qemu-monitor-command INSTANCE '{"execute":"x-block-latency-histogram-set",
"arguments":{"device":"drive-virtio-disk1","boundaries":[10,200,40]}}'
In fact this command does not work, but we still get success result.

qmp_x_block_latency_histogram_set is a batch setting API, report error ASAP.

Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index XXXXXXX..XXXXXXX 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -XXX,XX +XXX,XX @@ void qmp_x_block_latency_histogram_set(
 {
     BlockBackend *blk = blk_by_name(device);
     BlockAcctStats *stats;
+    int ret;
 
     if (!blk) {
         error_setg(errp, "Device '%s' not found", device);
@@ -XXX,XX +XXX,XX @@ void qmp_x_block_latency_histogram_set(
     }
 
     if (has_boundaries || has_boundaries_read) {
-        block_latency_histogram_set(
+        ret = block_latency_histogram_set(
             stats, BLOCK_ACCT_READ,
             has_boundaries_read ? boundaries_read : boundaries);
+        if (ret) {
+            error_setg(errp, "Device '%s' set read boundaries fail", device);
+            return;
+        }
     }
 
     if (has_boundaries || has_boundaries_write) {
-        block_latency_histogram_set(
+        ret = block_latency_histogram_set(
             stats, BLOCK_ACCT_WRITE,
             has_boundaries_write ? boundaries_write : boundaries);
+        if (ret) {
+            error_setg(errp, "Device '%s' set write boundaries fail", device);
+            return;
+        }
     }
 
     if (has_boundaries || has_boundaries_flush) {
-        block_latency_histogram_set(
+        ret = block_latency_histogram_set(
             stats, BLOCK_ACCT_FLUSH,
             has_boundaries_flush ? boundaries_flush : boundaries);
+        if (ret) {
+            error_setg(errp, "Device '%s' set flush boundaries fail", device);
+            return;
+        }
     }
 }
 
-- 
2.19.1

From: Peter Maydell <peter.maydell@linaro.org>

In the function external_snapshot_prepare() we have a
BlockdevSnapshotSync struct, which has the usual combination
of has_snapshot_node_name and snapshot_node_name fields for an
optional field. We set up a local variable
        const char *snapshot_node_name =
            s->has_snapshot_node_name ? s->snapshot_node_name : NULL;

and then mostly use "if (!snapshot_node_name)" for checking
whether we have a snapshot node name. The exception is that in
one place we check s->has_snapshot_node_name instead. This
confuses Coverity (CID 1396473), which thinks it might be
possible to get here with s->has_snapshot_node_name true but
snapshot_node_name NULL, and warns that the call to
qdict_put_str() will segfault in that case.

Make the code consistent and unconfuse Coverity by using
the same check for this conditional that we do in the rest
of the surrounding code.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/blockdev.c b/blockdev.c
index XXXXXXX..XXXXXXX 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
         }
 
         options = qdict_new();
-        if (s->has_snapshot_node_name) {
+        if (snapshot_node_name) {
             qdict_put_str(options, "node-name", snapshot_node_name);
         }
         qdict_put_str(options, "driver", format);
-- 
2.19.1

From: Li Qiang <liq3ea@gmail.com>

Currently, when hotplug/unhotplug nvme device, it will cause an
assert in object.c. Following is the backtrack:

ERROR:qom/object.c:981:object_unref: assertion failed: (obj->ref > 0)

Thread 2 "qemu-system-x86" received signal SIGABRT, Aborted.
[Switching to Thread 0x7fffcbd32700 (LWP 18844)]
0x00007fffdb9e4fff in raise () from /lib/x86_64-linux-gnu/libc.so.6
(gdb) bt
/lib/x86_64-linux-gnu/libglib-2.0.so.0
/lib/x86_64-linux-gnu/libglib-2.0.so.0
qom/object.c:981
/home/liqiang02/qemu-upstream/qemu/memory.c:1732
/home/liqiang02/qemu-upstream/qemu/memory.c:285
util/qemu-thread-posix.c:504
/lib/x86_64-linux-gnu/libpthread.so.0

This is caused by memory_region_unref in nvme_exit.

Remove it to make the PCIdevice refcount correct.

Signed-off-by: Li Qiang <liq3ea@gmail.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/block/nvme.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -XXX,XX +XXX,XX @@ static void nvme_exit(PCIDevice *pci_dev)
     g_free(n->namespaces);
     g_free(n->cq);
     g_free(n->sq);
-    if (n->cmbsz) {
-        memory_region_unref(&n->ctrl_mem);
-    }
 
     msix_uninit_exclusive_bar(pci_dev);
 }
-- 
2.19.1

From: Fam Zheng <famz@redhat.com>

If we know we've already locked the bytes, don't do it again; similarly
don't unlock a byte if we haven't locked it. This doesn't change the
behavior, but fixes a corner case explained below.

Libvirt had an error handling bug that an image can get its (ownership,
file mode, SELinux) permissions changed (RHBZ 1584982) by mistake behind
QEMU. Specifically, an image in use by Libvirt VM has:

$ ls -lhZ b.img
    -rw-r--r--. qemu qemu system_u:object_r:svirt_image_t:s0:c600,c690 b.img

Trying to attach it a second time won't work because of image locking.
And after the error, it becomes:

$ ls -lhZ b.img
    -rw-r--r--. root root system_u:object_r:virt_image_t:s0 b.img

Then, we won't be able to do OFD lock operations with the existing fd.
In other words, the code such as in blk_detach_dev:

blk_set_perm(blk, 0, BLK_PERM_ALL, &error_abort);

can abort() QEMU, out of environmental changes.

This patch is an easy fix to this and the change is regardlessly
reasonable, so do it.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/file-posix.c | 54 +++++++++++++++++++++++++++++++++++++---------
 1 file changed, 44 insertions(+), 10 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index XXXXXXX..XXXXXXX 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -XXX,XX +XXX,XX @@ typedef struct BDRVRawState {
     uint64_t perm;
     uint64_t shared_perm;
 
+    /* The perms bits whose corresponding bytes are already locked in
+     * s->lock_fd. */
+    uint64_t locked_perm;
+    uint64_t locked_shared_perm;
+
 #ifdef CONFIG_XFS
     bool is_xfs:1;
 #endif
@@ -XXX,XX +XXX,XX @@ typedef enum {
  * file; if @unlock == true, also unlock the unneeded bytes.
  * @shared_perm_lock_bits is the mask of all permissions that are NOT shared.
  */
-static int raw_apply_lock_bytes(int fd,
+static int raw_apply_lock_bytes(BDRVRawState *s, int fd,
                                 uint64_t perm_lock_bits,
                                 uint64_t shared_perm_lock_bits,
                                 bool unlock, Error **errp)
 {
     int ret;
     int i;
+    uint64_t locked_perm, locked_shared_perm;
+
+    if (s) {
+        locked_perm = s->locked_perm;
+        locked_shared_perm = s->locked_shared_perm;
+    } else {
+        /*
+         * We don't have the previous bits, just lock/unlock for each of the
+         * requested bits.
+         */
+        if (unlock) {
+            locked_perm = BLK_PERM_ALL;
+            locked_shared_perm = BLK_PERM_ALL;
+        } else {
+            locked_perm = 0;
+            locked_shared_perm = 0;
+        }
+    }
 
     PERM_FOREACH(i) {
         int off = RAW_LOCK_PERM_BASE + i;
-        if (perm_lock_bits & (1ULL << i)) {
+        uint64_t bit = (1ULL << i);
+        if ((perm_lock_bits & bit) && !(locked_perm & bit)) {
             ret = qemu_lock_fd(fd, off, 1, false);
             if (ret) {
                 error_setg(errp, "Failed to lock byte %d", off);
                 return ret;
+            } else if (s) {
+                s->locked_perm |= bit;
             }
-        } else if (unlock) {
+        } else if (unlock && (locked_perm & bit) && !(perm_lock_bits & bit)) {
             ret = qemu_unlock_fd(fd, off, 1);
             if (ret) {
                 error_setg(errp, "Failed to unlock byte %d", off);
                 return ret;
+            } else if (s) {
+                s->locked_perm &= ~bit;
             }
         }
     }
     PERM_FOREACH(i) {
         int off = RAW_LOCK_SHARED_BASE + i;
-        if (shared_perm_lock_bits & (1ULL << i)) {
+        uint64_t bit = (1ULL << i);
+        if ((shared_perm_lock_bits & bit) && !(locked_shared_perm & bit)) {
             ret = qemu_lock_fd(fd, off, 1, false);
             if (ret) {
                 error_setg(errp, "Failed to lock byte %d", off);
                 return ret;
+            } else if (s) {
+                s->locked_shared_perm |= bit;
             }
-        } else if (unlock) {
+        } else if (unlock && (locked_shared_perm & bit) &&
+                   !(shared_perm_lock_bits & bit)) {
             ret = qemu_unlock_fd(fd, off, 1);
             if (ret) {
                 error_setg(errp, "Failed to unlock byte %d", off);
                 return ret;
+            } else if (s) {
+                s->locked_shared_perm &= ~bit;
             }
         }
     }
@@ -XXX,XX +XXX,XX @@ static int raw_handle_perm_lock(BlockDriverState *bs,
 
     switch (op) {
     case RAW_PL_PREPARE:
-        ret = raw_apply_lock_bytes(s->lock_fd, s->perm | new_perm,
+        ret = raw_apply_lock_bytes(s, s->lock_fd, s->perm | new_perm,
                                    ~s->shared_perm | ~new_shared,
                                    false, errp);
         if (!ret) {
@@ -XXX,XX +XXX,XX @@ static int raw_handle_perm_lock(BlockDriverState *bs,
         op = RAW_PL_ABORT;
         /* fall through to unlock bytes. */
     case RAW_PL_ABORT:
-        raw_apply_lock_bytes(s->lock_fd, s->perm, ~s->shared_perm,
+        raw_apply_lock_bytes(s, s->lock_fd, s->perm, ~s->shared_perm,
                              true, &local_err);
         if (local_err) {
             /* Theoretically the above call only unlocks bytes and it cannot
@@ -XXX,XX +XXX,XX @@ static int raw_handle_perm_lock(BlockDriverState *bs,
         }
         break;
     case RAW_PL_COMMIT:
-        raw_apply_lock_bytes(s->lock_fd, new_perm, ~new_shared,
+        raw_apply_lock_bytes(s, s->lock_fd, new_perm, ~new_shared,
                              true, &local_err);
         if (local_err) {
             /* Theoretically the above call only unlocks bytes and it cannot
@@ -XXX,XX +XXX,XX @@ raw_co_create(BlockdevCreateOptions *options, Error **errp)
     shared = BLK_PERM_ALL & ~BLK_PERM_RESIZE;
 
     /* Step one: Take locks */
-    result = raw_apply_lock_bytes(fd, perm, ~shared, false, errp);
+    result = raw_apply_lock_bytes(NULL, fd, perm, ~shared, false, errp);
     if (result < 0) {
         goto out_close;
     }
@@ -XXX,XX +XXX,XX @@ raw_co_create(BlockdevCreateOptions *options, Error **errp)
     }
 
 out_unlock:
-    raw_apply_lock_bytes(fd, 0, 0, true, &local_err);
+    raw_apply_lock_bytes(NULL, fd, 0, 0, true, &local_err);
     if (local_err) {
         /* The above call should not fail, and if it does, that does
          * not mean the whole creation operation has failed.  So
-- 
2.19.1

From: Fam Zheng <famz@redhat.com>

The lock_fd field is not strictly necessary because transferring locked
bytes from old fd to the new one shouldn't fail anyway. This spares the
user one fd per image.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/file-posix.c | 37 +++++++++++++------------------------
 1 file changed, 13 insertions(+), 24 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index XXXXXXX..XXXXXXX 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -XXX,XX +XXX,XX @@ do { \
 
 typedef struct BDRVRawState {
     int fd;
-    int lock_fd;
     bool use_lock;
     int type;
     int open_flags;
@@ -XXX,XX +XXX,XX @@ typedef struct BDRVRawState {
     uint64_t shared_perm;
 
     /* The perms bits whose corresponding bytes are already locked in
-     * s->lock_fd. */
+     * s->fd. */
     uint64_t locked_perm;
     uint64_t locked_shared_perm;
 
@@ -XXX,XX +XXX,XX @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
     }
     s->fd = fd;
 
-    s->lock_fd = -1;
-    if (s->use_lock) {
-        fd = qemu_open(filename, s->open_flags);
-        if (fd < 0) {
-            ret = -errno;
-            error_setg_errno(errp, errno, "Could not open '%s' for locking",
-                             filename);
-            qemu_close(s->fd);
-            goto fail;
-        }
-        s->lock_fd = fd;
-    }
     s->perm = 0;
     s->shared_perm = BLK_PERM_ALL;
 
@@ -XXX,XX +XXX,XX @@ static int raw_handle_perm_lock(BlockDriverState *bs,
         return 0;
     }
 
-    assert(s->lock_fd > 0);
-
     switch (op) {
     case RAW_PL_PREPARE:
-        ret = raw_apply_lock_bytes(s, s->lock_fd, s->perm | new_perm,
+        ret = raw_apply_lock_bytes(s, s->fd, s->perm | new_perm,
                                    ~s->shared_perm | ~new_shared,
                                    false, errp);
         if (!ret) {
-            ret = raw_check_lock_bytes(s->lock_fd, new_perm, new_shared, errp);
+            ret = raw_check_lock_bytes(s->fd, new_perm, new_shared, errp);
             if (!ret) {
                 return 0;
             }
@@ -XXX,XX +XXX,XX @@ static int raw_handle_perm_lock(BlockDriverState *bs,
         op = RAW_PL_ABORT;
         /* fall through to unlock bytes. */
     case RAW_PL_ABORT:
-        raw_apply_lock_bytes(s, s->lock_fd, s->perm, ~s->shared_perm,
+        raw_apply_lock_bytes(s, s->fd, s->perm, ~s->shared_perm,
                              true, &local_err);
         if (local_err) {
             /* Theoretically the above call only unlocks bytes and it cannot
@@ -XXX,XX +XXX,XX @@ static int raw_handle_perm_lock(BlockDriverState *bs,
         }
         break;
     case RAW_PL_COMMIT:
-        raw_apply_lock_bytes(s, s->lock_fd, new_perm, ~new_shared,
+        raw_apply_lock_bytes(s, s->fd, new_perm, ~new_shared,
                              true, &local_err);
         if (local_err) {
             /* Theoretically the above call only unlocks bytes and it cannot
@@ -XXX,XX +XXX,XX @@ static void raw_reopen_commit(BDRVReopenState *state)
 {
     BDRVRawReopenState *rs = state->opaque;
     BDRVRawState *s = state->bs->opaque;
+    Error *local_err = NULL;
 
     s->check_cache_dropped = rs->check_cache_dropped;
     s->open_flags = rs->open_flags;
 
+    /* Copy locks to the new fd before closing the old one. */
+    raw_apply_lock_bytes(NULL, rs->fd, s->locked_perm,
+                         ~s->locked_shared_perm, false, &local_err);
+    if (local_err) {
+        /* shouldn't fail in a sane host, but report it just in case. */
+        error_report_err(local_err);
+    }
     qemu_close(s->fd);
     s->fd = rs->fd;
 
@@ -XXX,XX +XXX,XX @@ static void raw_close(BlockDriverState *bs)
         qemu_close(s->fd);
         s->fd = -1;
     }
-    if (s->lock_fd >= 0) {
-        qemu_close(s->lock_fd);
-        s->lock_fd = -1;
-    }
 }
 
 /**
-- 
2.19.1

From: Fam Zheng <famz@redhat.com>

Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/test-image-locking.c | 157 +++++++++++++++++++++++++++++++++++++
 tests/Makefile.include     |   2 +
 2 files changed, 159 insertions(+)
 create mode 100644 tests/test-image-locking.c

diff --git a/tests/test-image-locking.c b/tests/test-image-locking.c
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/test-image-locking.c
@@ -XXX,XX +XXX,XX @@
+/*
+ * Image locking tests
+ *
+ * Copyright (c) 2018 Red Hat Inc.
+ *
+ * Author: Fam Zheng <famz@redhat.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "block/block.h"
+#include "sysemu/block-backend.h"
+#include "qapi/error.h"
+#include "qapi/qmp/qdict.h"
+
+static BlockBackend *open_image(const char *path,
+                                uint64_t perm, uint64_t shared_perm,
+                                Error **errp)
+{
+    Error *local_err = NULL;
+    BlockBackend *blk;
+    QDict *options = qdict_new();
+
+    qdict_put_str(options, "driver", "raw");
+    blk = blk_new_open(path, NULL, options, BDRV_O_RDWR, &local_err);
+    if (blk) {
+        g_assert_null(local_err);
+        if (blk_set_perm(blk, perm, shared_perm, errp)) {
+            blk_unref(blk);
+            blk = NULL;
+        }
+    } else {
+        error_propagate(errp, local_err);
+    }
+    return blk;
+}
+
+static void check_locked_bytes(int fd, uint64_t perm_locks,
+                               uint64_t shared_perm_locks)
+{
+    int i;
+
+    if (!perm_locks && !shared_perm_locks) {
+        g_assert(!qemu_lock_fd_test(fd, 0, 0, true));
+        return;
+    }
+    for (i = 0; (1ULL << i) <= BLK_PERM_ALL; i++) {
+        uint64_t bit = (1ULL << i);
+        bool perm_expected = !!(bit & perm_locks);
+        bool shared_perm_expected = !!(bit & shared_perm_locks);
+        g_assert_cmpint(perm_expected, ==,
+                        !!qemu_lock_fd_test(fd, 100 + i, 1, true));
+        g_assert_cmpint(shared_perm_expected, ==,
+                        !!qemu_lock_fd_test(fd, 200 + i, 1, true));
+    }
+}
+
+static void test_image_locking_basic(void)
+{
+    BlockBackend *blk1, *blk2, *blk3;
+    char img_path[] = "/tmp/qtest.XXXXXX";
+    uint64_t perm, shared_perm;
+
+    int fd = mkstemp(img_path);
+    assert(fd >= 0);
+
+    perm = BLK_PERM_WRITE | BLK_PERM_CONSISTENT_READ;
+    shared_perm = BLK_PERM_ALL;
+    blk1 = open_image(img_path, perm, shared_perm, &error_abort);
+    g_assert(blk1);
+
+    check_locked_bytes(fd, perm, ~shared_perm);
+
+    /* compatible perm between blk1 and blk2 */
+    blk2 = open_image(img_path, perm | BLK_PERM_RESIZE, shared_perm, NULL);
+    g_assert(blk2);
+    check_locked_bytes(fd, perm | BLK_PERM_RESIZE, ~shared_perm);
+
+    /* incompatible perm with already open blk1 and blk2 */
+    blk3 = open_image(img_path, perm, BLK_PERM_WRITE_UNCHANGED, NULL);
+    g_assert_null(blk3);
+
+    blk_unref(blk2);
+
+    /* Check that extra bytes in blk2 are correctly unlocked */
+    check_locked_bytes(fd, perm, ~shared_perm);
+
+    blk_unref(blk1);
+
+    /* Image is unused, no lock there */
+    check_locked_bytes(fd, 0, 0);
+    blk3 = open_image(img_path, perm, BLK_PERM_WRITE_UNCHANGED, &error_abort);
+    g_assert(blk3);
+    blk_unref(blk3);
+    close(fd);
+    unlink(img_path);
+}
+
+static void test_set_perm_abort(void)
+{
+    BlockBackend *blk1, *blk2;
+    char img_path[] = "/tmp/qtest.XXXXXX";
+    uint64_t perm, shared_perm;
+    int r;
+    int fd = mkstemp(img_path);
+    assert(fd >= 0);
+
+    perm = BLK_PERM_WRITE | BLK_PERM_CONSISTENT_READ;
+    shared_perm = BLK_PERM_ALL;
+    blk1 = open_image(img_path, perm, shared_perm, &error_abort);
+    g_assert(blk1);
+
+    blk2 = open_image(img_path, perm, shared_perm, &error_abort);
+    g_assert(blk2);
+
+    check_locked_bytes(fd, perm, ~shared_perm);
+
+    /* A failed blk_set_perm mustn't change perm status (locked bytes) */
+    r = blk_set_perm(blk2, perm | BLK_PERM_RESIZE, BLK_PERM_WRITE_UNCHANGED,
+                     NULL);
+    g_assert_cmpint(r, !=, 0);
+    check_locked_bytes(fd, perm, ~shared_perm);
+    blk_unref(blk1);
+    blk_unref(blk2);
+}
+
+int main(int argc, char **argv)
+{
+    bdrv_init();
+    qemu_init_main_loop(&error_abort);
+
+    g_test_init(&argc, &argv, NULL);
+
+    if (qemu_has_ofd_lock()) {
+        g_test_add_func("/image-locking/basic", test_image_locking_basic);
+        g_test_add_func("/image-locking/set-perm-abort", test_set_perm_abort);
+    }
+
+    return g_test_run();
+}
diff --git a/tests/Makefile.include b/tests/Makefile.include
index XXXXXXX..XXXXXXX 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -XXX,XX +XXX,XX @@ check-unit-y += tests/test-bdrv-drain$(EXESUF)
 check-unit-y += tests/test-blockjob$(EXESUF)
 check-unit-y += tests/test-blockjob-txn$(EXESUF)
 check-unit-y += tests/test-block-backend$(EXESUF)
+check-unit-y += tests/test-image-locking$(EXESUF)
 check-unit-y += tests/test-x86-cpuid$(EXESUF)
 # all code tested by test-x86-cpuid is inside topology.h
 ifeq ($(CONFIG_SOFTMMU),y)
@@ -XXX,XX +XXX,XX @@ tests/test-bdrv-drain$(EXESUF): tests/test-bdrv-drain.o $(test-block-obj-y) $(te
 tests/test-blockjob$(EXESUF): tests/test-blockjob.o $(test-block-obj-y) $(test-util-obj-y)
 tests/test-blockjob-txn$(EXESUF): tests/test-blockjob-txn.o $(test-block-obj-y) $(test-util-obj-y)
 tests/test-block-backend$(EXESUF): tests/test-block-backend.o $(test-block-obj-y) $(test-util-obj-y)
+tests/test-image-locking$(EXESUF): tests/test-image-locking.o $(test-block-obj-y) $(test-util-obj-y)
 tests/test-thread-pool$(EXESUF): tests/test-thread-pool.o $(test-block-obj-y)
 tests/test-iov$(EXESUF): tests/test-iov.o $(test-util-obj-y)
 tests/test-hbitmap$(EXESUF): tests/test-hbitmap.o $(test-util-obj-y) $(test-crypto-obj-y)
-- 
2.19.1

From: Jeff Cody <jcody@redhat.com>

This adds configure options to control the following block drivers:

* Bochs
* Cloop
* Dmg
* Qcow (V1)
* Vdi
* Vvfat
* qed
* parallels
* sheepdog

Each of these defaults to being enabled.

Signed-off-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-id: 20181107063644.2254-1-armbru@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 configure           | 91 +++++++++++++++++++++++++++++++++++++++++++++
 block/Makefile.objs | 22 ++++++++---
 2 files changed, 107 insertions(+), 6 deletions(-)

diff --git a/configure b/configure
index XXXXXXX..XXXXXXX 100755
--- a/configure
+++ b/configure
@@ -XXX,XX +XXX,XX @@ tcmalloc="no"
 jemalloc="no"
 replication="yes"
 vxhs=""
+bochs="yes"
+cloop="yes"
+dmg="yes"
+qcow1="yes"
+vdi="yes"
+vvfat="yes"
+qed="yes"
+parallels="yes"
+sheepdog="yes"
 libxml2=""
 docker="no"
 debug_mutex="no"
@@ -XXX,XX +XXX,XX @@ for opt do
   ;;
   --enable-vxhs) vxhs="yes"
   ;;
+  --disable-bochs) bochs="no"
+  ;;
+  --enable-bochs) bochs="yes"
+  ;;
+  --disable-cloop) cloop="no"
+  ;;
+  --enable-cloop) cloop="yes"
+  ;;
+  --disable-dmg) dmg="no"
+  ;;
+  --enable-dmg) dmg="yes"
+  ;;
+  --disable-qcow1) qcow1="no"
+  ;;
+  --enable-qcow1) qcow1="yes"
+  ;;
+  --disable-vdi) vdi="no"
+  ;;
+  --enable-vdi) vdi="yes"
+  ;;
+  --disable-vvfat) vvfat="no"
+  ;;
+  --enable-vvfat) vvfat="yes"
+  ;;
+  --disable-qed) qed="no"
+  ;;
+  --enable-qed) qed="yes"
+  ;;
+  --disable-parallels) parallels="no"
+  ;;
+  --enable-parallels) parallels="yes"
+  ;;
+  --disable-sheepdog) sheepdog="no"
+  ;;
+  --enable-sheepdog) sheepdog="yes"
+  ;;
   --disable-vhost-user) vhost_user="no"
   ;;
   --enable-vhost-user)
@@ -XXX,XX +XXX,XX @@ disabled with --disable-FEATURE, default is enabled if available:
   qom-cast-debug  cast debugging support
   tools           build qemu-io, qemu-nbd and qemu-image tools
   vxhs            Veritas HyperScale vDisk backend support
+  bochs           bochs image format support
+  cloop           cloop image format support
+  dmg             dmg image format support
+  qcow1           qcow v1 image format support
+  vdi             vdi image format support
+  vvfat           vvfat image format support
+  qed             qed image format support
+  parallels       parallels image format support
+  sheepdog        sheepdog block driver support
   crypto-afalg    Linux AF_ALG crypto backend driver
   vhost-user      vhost-user support
   capstone        capstone disassembler support
@@ -XXX,XX +XXX,XX @@ echo "jemalloc support  $jemalloc"
 echo "avx2 optimization $avx2_opt"
 echo "replication support $replication"
 echo "VxHS block device $vxhs"
+echo "bochs support     $bochs"
+echo "cloop support     $cloop"
+echo "dmg support       $dmg"
+echo "qcow v1 support   $qcow1"
+echo "vdi support       $vdi"
+echo "vvfat support     $vvfat"
+echo "qed support       $qed"
+echo "parallels support $parallels"
+echo "sheepdog support  $sheepdog"
 echo "capstone          $capstone"
 echo "docker            $docker"
 echo "libpmem support   $libpmem"
@@ -XXX,XX +XXX,XX @@ if test "$libpmem" = "yes" ; then
   echo "CONFIG_LIBPMEM=y" >> $config_host_mak
 fi
 
+if test "$bochs" = "yes" ; then
+  echo "CONFIG_BOCHS=y" >> $config_host_mak
+fi
+if test "$cloop" = "yes" ; then
+  echo "CONFIG_CLOOP=y" >> $config_host_mak
+fi
+if test "$dmg" = "yes" ; then
+  echo "CONFIG_DMG=y" >> $config_host_mak
+fi
+if test "$qcow1" = "yes" ; then
+  echo "CONFIG_QCOW1=y" >> $config_host_mak
+fi
+if test "$vdi" = "yes" ; then
+  echo "CONFIG_VDI=y" >> $config_host_mak
+fi
+if test "$vvfat" = "yes" ; then
+  echo "CONFIG_VVFAT=y" >> $config_host_mak
+fi
+if test "$qed" = "yes" ; then
+  echo "CONFIG_QED=y" >> $config_host_mak
+fi
+if test "$parallels" = "yes" ; then
+  echo "CONFIG_PARALLELS=y" >> $config_host_mak
+fi
+if test "$sheepdog" = "yes" ; then
+  echo "CONFIG_SHEEPDOG=y" >> $config_host_mak
+fi
+
 if test "$tcg_interpreter" = "yes"; then
   QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
 elif test "$ARCH" = "sparc64" ; then
diff --git a/block/Makefile.objs b/block/Makefile.objs
index XXXXXXX..XXXXXXX 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -XXX,XX +XXX,XX @@
-block-obj-y += raw-format.o qcow.o vdi.o vmdk.o cloop.o bochs.o vpc.o vvfat.o dmg.o
+block-obj-y += raw-format.o vmdk.o vpc.o
+block-obj-$(CONFIG_QCOW1) += qcow.o
+block-obj-$(CONFIG_VDI) += vdi.o
+block-obj-$(CONFIG_CLOOP) += cloop.o
+block-obj-$(CONFIG_BOCHS) += bochs.o
+block-obj-$(CONFIG_VVFAT) += vvfat.o
+block-obj-$(CONFIG_DMG) += dmg.o
+
 block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-cache.o qcow2-bitmap.o
-block-obj-y += qed.o qed-l2-cache.o qed-table.o qed-cluster.o
-block-obj-y += qed-check.o
+block-obj-$(CONFIG_QED) += qed.o qed-l2-cache.o qed-table.o qed-cluster.o
+block-obj-$(CONFIG_QED) += qed-check.o
 block-obj-y += vhdx.o vhdx-endian.o vhdx-log.o
 block-obj-y += quorum.o
-block-obj-y += parallels.o blkdebug.o blkverify.o blkreplay.o
+block-obj-y += blkdebug.o blkverify.o blkreplay.o
+block-obj-$(CONFIG_PARALLELS) += parallels.o
 block-obj-y += blklogwrites.o
 block-obj-y += block-backend.o snapshot.o qapi.o
 block-obj-$(CONFIG_WIN32) += file-win32.o win32-aio.o
@@ -XXX,XX +XXX,XX @@ block-obj-y += null.o mirror.o commit.o io.o create.o
 block-obj-y += throttle-groups.o
 block-obj-$(CONFIG_LINUX) += nvme.o
 
-block-obj-y += nbd.o nbd-client.o sheepdog.o
+block-obj-y += nbd.o nbd-client.o
+block-obj-$(CONFIG_SHEEPDOG) += sheepdog.o
 block-obj-$(CONFIG_LIBISCSI) += iscsi.o
 block-obj-$(if $(CONFIG_LIBISCSI),y,n) += iscsi-opts.o
 block-obj-$(CONFIG_LIBNFS) += nfs.o
@@ -XXX,XX +XXX,XX @@ gluster.o-libs     := $(GLUSTERFS_LIBS)
 vxhs.o-libs        := $(VXHS_LIBS)
 ssh.o-cflags       := $(LIBSSH2_CFLAGS)
 ssh.o-libs         := $(LIBSSH2_LIBS)
-block-obj-$(if $(CONFIG_BZIP2),m,n) += dmg-bz2.o
+block-obj-dmg-bz2-$(CONFIG_BZIP2) += dmg-bz2.o
+block-obj-$(if $(CONFIG_DMG),m,n) += $(block-obj-dmg-bz2-y)
 dmg-bz2.o-libs     := $(BZIP2_LIBS)
 qcow.o-libs        := -lz
 linux-aio.o-libs   := -laio
-- 
2.19.1

From: Liam Merwick <Liam.Merwick@oracle.com>

In the assert checking the array dereference of JobVerbTable[verb]
in job_apply_verb() the check of the index, verb, allows an overrun
because an index equal to the array size is permitted.

Similarly, in the assert check of JobSTT[s0][s1] with index s1
in job_state_transition(), an off-by-one overrun is not flagged
either.

This is not a run-time issue as there are no callers actually
passing in the max value.

Signed-off-by: Liam Merwick <Liam.Merwick@oracle.com>
Reviewed-by: Darren Kenny <Darren.Kenny@oracle.com>
Reviewed-by: Mark Kanda <Mark.Kanda@oracle.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1541453919-25973-2-git-send-email-Liam.Merwick@oracle.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 job.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/job.c b/job.c
index XXXXXXX..XXXXXXX 100644
--- a/job.c
+++ b/job.c
@@ -XXX,XX +XXX,XX @@ bool job_is_internal(Job *job)
 static void job_state_transition(Job *job, JobStatus s1)
 {
     JobStatus s0 = job->status;
-    assert(s1 >= 0 && s1 <= JOB_STATUS__MAX);
+    assert(s1 >= 0 && s1 < JOB_STATUS__MAX);
     trace_job_state_transition(job, job->ret,
                                JobSTT[s0][s1] ? "allowed" : "disallowed",
                                JobStatus_str(s0), JobStatus_str(s1));
@@ -XXX,XX +XXX,XX @@ static void job_state_transition(Job *job, JobStatus s1)
 int job_apply_verb(Job *job, JobVerb verb, Error **errp)
 {
     JobStatus s0 = job->status;
-    assert(verb >= 0 && verb <= JOB_VERB__MAX);
+    assert(verb >= 0 && verb < JOB_VERB__MAX);
     trace_job_apply_verb(job, JobStatus_str(s0), JobVerb_str(verb),
                          JobVerbTable[verb][s0] ? "allowed" : "prohibited");
     if (JobVerbTable[verb][s0]) {
-- 
2.19.1

From: Liam Merwick <Liam.Merwick@oracle.com>

The dev_id returned by the call to blk_get_attached_dev_id() in
blk_root_get_parent_desc() can be NULL (an internal call to
object_get_canonical_path may have returned NULL).

Instead of just checking this case before before dereferencing,
adjust blk_get_attached_dev_id() to return the empty string if no
object path can be found (similar to the case when blk->dev is NULL
and an empty string is returned).

Signed-off-by: Liam Merwick <Liam.Merwick@oracle.com>
Message-id: 1541453919-25973-3-git-send-email-Liam.Merwick@oracle.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/block-backend.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index XXXXXXX..XXXXXXX 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -XXX,XX +XXX,XX @@ char *blk_get_attached_dev_id(BlockBackend *blk)
     } else if (dev->id) {
         return g_strdup(dev->id);
     }
-    return object_get_canonical_path(OBJECT(dev));
+
+    return object_get_canonical_path(OBJECT(dev)) ?: g_strdup("");
 }
 
 /*
-- 
2.19.1

From: Liam Merwick <Liam.Merwick@oracle.com>

The calls to find_mapping_for_cluster() may return NULL but it
isn't always checked for before dereferencing the value returned.
Additionally, add some asserts to cover cases where NULL can't
be returned but which might not be obvious at first glance.

Signed-off-by: Liam Merwick <Liam.Merwick@oracle.com>
Message-id: 1541453919-25973-5-git-send-email-Liam.Merwick@oracle.com
[mreitz: Dropped superfluous check of "mapping" following an assertion
         that it is not NULL, and fixed some indentation]
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/vvfat.c | 46 ++++++++++++++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 16 deletions(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index XXXXXXX..XXXXXXX 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -XXX,XX +XXX,XX @@ static inline void array_free(array_t* array)
 /* does not automatically grow */
 static inline void* array_get(array_t* array,unsigned int index) {
     assert(index < array->next);
+    assert(array->pointer);
     return array->pointer + index * array->item_size;
 }
 
-static inline int array_ensure_allocated(array_t* array, int index)
+static inline void array_ensure_allocated(array_t *array, int index)
 {
     if((index + 1) * array->item_size > array->size) {
         int new_size = (index + 32) * array->item_size;
         array->pointer = g_realloc(array->pointer, new_size);
-        if (!array->pointer)
-            return -1;
+        assert(array->pointer);
         memset(array->pointer + array->size, 0, new_size - array->size);
         array->size = new_size;
         array->next = index + 1;
     }
-
-    return 0;
 }
 
 static inline void* array_get_next(array_t* array) {
     unsigned int next = array->next;
 
-    if (array_ensure_allocated(array, next) < 0)
-        return NULL;
-
+    array_ensure_allocated(array, next);
     array->next = next + 1;
     return array_get(array, next);
 }
@@ -XXX,XX +XXX,XX @@ static int commit_direntries(BDRVVVFATState* s,
     direntry_t* direntry = array_get(&(s->directory), dir_index);
     uint32_t first_cluster = dir_index == 0 ? 0 : begin_of_direntry(direntry);
     mapping_t* mapping = find_mapping_for_cluster(s, first_cluster);
-
     int factor = 0x10 * s->sectors_per_cluster;
     int old_cluster_count, new_cluster_count;
-    int current_dir_index = mapping->info.dir.first_dir_index;
-    int first_dir_index = current_dir_index;
+    int current_dir_index;
+    int first_dir_index;
     int ret, i;
     uint32_t c;
 
-DLOG(fprintf(stderr, "commit_direntries for %s, parent_mapping_index %d\n", mapping->path, parent_mapping_index));
-
     assert(direntry);
     assert(mapping);
     assert(mapping->begin == first_cluster);
@@ -XXX,XX +XXX,XX @@ DLOG(fprintf(stderr, "commit_direntries for %s, parent_mapping_index %d\n", mapp
     assert(mapping->mode & MODE_DIRECTORY);
     assert(dir_index == 0 || is_directory(direntry));
 
+    DLOG(fprintf(stderr, "commit_direntries for %s, parent_mapping_index %d\n",
+                 mapping->path, parent_mapping_index));
+
+    current_dir_index = mapping->info.dir.first_dir_index;
+    first_dir_index = current_dir_index;
     mapping->info.dir.parent_mapping_index = parent_mapping_index;
 
     if (first_cluster == 0) {
@@ -XXX,XX +XXX,XX @@ DLOG(fprintf(stderr, "commit_direntries for %s, parent_mapping_index %d\n", mapp
         direntry = array_get(&(s->directory), first_dir_index + i);
         if (is_directory(direntry) && !is_dot(direntry)) {
             mapping = find_mapping_for_cluster(s, first_cluster);
+            if (mapping == NULL) {
+                return -1;
+            }
             assert(mapping->mode & MODE_DIRECTORY);
             ret = commit_direntries(s, first_dir_index + i,
                 array_index(&(s->mapping), mapping));
@@ -XXX,XX +XXX,XX @@ static int commit_one_file(BDRVVVFATState* s,
     assert(offset < size);
     assert((offset % s->cluster_size) == 0);
 
+    if (mapping == NULL) {
+        return -1;
+    }
+
     for (i = s->cluster_size; i < offset; i += s->cluster_size)
         c = modified_fat_get(s, c);
 
@@ -XXX,XX +XXX,XX @@ static int handle_renames_and_mkdirs(BDRVVVFATState* s)
         if (commit->action == ACTION_RENAME) {
             mapping_t* mapping = find_mapping_for_cluster(s,
                     commit->param.rename.cluster);
-            char* old_path = mapping->path;
+            char *old_path;
 
+            if (mapping == NULL) {
+                return -1;
+            }
+            old_path = mapping->path;
             assert(commit->path);
             mapping->path = commit->path;
             if (rename(old_path, mapping->path))
@@ -XXX,XX +XXX,XX @@ static int handle_renames_and_mkdirs(BDRVVVFATState* s)
                         direntry_t* d = direntry + i;
 
                         if (is_file(d) || (is_directory(d) && !is_dot(d))) {
+                            int l;
+                            char *new_path;
                             mapping_t* m = find_mapping_for_cluster(s,
                                     begin_of_direntry(d));
-                            int l = strlen(m->path);
-                            char* new_path = g_malloc(l + diff + 1);
+                            if (m == NULL) {
+                                return -1;
+                            }
+                            l = strlen(m->path);
+                            new_path = g_malloc(l + diff + 1);
 
                             assert(!strncmp(m->path, mapping->path, l2));
 
-- 
2.19.1

From: Liam Merwick <Liam.Merwick@oracle.com>

The commit for 0e4e4318eaa5 increments QCOW2_OL_MAX_BITNR but does not
add an array entry for QCOW2_OL_BITMAP_DIRECTORY_BITNR to metadata_ol_names[].
As a result, an array dereference of metadata_ol_names[8] in
qcow2_pre_write_overlap_check() could result in a read outside of the array bounds.

Fixes: 0e4e4318eaa5 ('qcow2: add overlap check for bitmap directory')

Cc: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Liam Merwick <Liam.Merwick@oracle.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 1541453919-25973-6-git-send-email-Liam.Merwick@oracle.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2-refcount.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index XXXXXXX..XXXXXXX 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -XXX,XX +XXX,XX @@ int qcow2_check_metadata_overlap(BlockDriverState *bs, int ign, int64_t offset,
 }
 
 static const char *metadata_ol_names[] = {
-    [QCOW2_OL_MAIN_HEADER_BITNR]    = "qcow2_header",
-    [QCOW2_OL_ACTIVE_L1_BITNR]      = "active L1 table",
-    [QCOW2_OL_ACTIVE_L2_BITNR]      = "active L2 table",
-    [QCOW2_OL_REFCOUNT_TABLE_BITNR] = "refcount table",
-    [QCOW2_OL_REFCOUNT_BLOCK_BITNR] = "refcount block",
-    [QCOW2_OL_SNAPSHOT_TABLE_BITNR] = "snapshot table",
-    [QCOW2_OL_INACTIVE_L1_BITNR]    = "inactive L1 table",
-    [QCOW2_OL_INACTIVE_L2_BITNR]    = "inactive L2 table",
+    [QCOW2_OL_MAIN_HEADER_BITNR]        = "qcow2_header",
+    [QCOW2_OL_ACTIVE_L1_BITNR]          = "active L1 table",
+    [QCOW2_OL_ACTIVE_L2_BITNR]          = "active L2 table",
+    [QCOW2_OL_REFCOUNT_TABLE_BITNR]     = "refcount table",
+    [QCOW2_OL_REFCOUNT_BLOCK_BITNR]     = "refcount block",
+    [QCOW2_OL_SNAPSHOT_TABLE_BITNR]     = "snapshot table",
+    [QCOW2_OL_INACTIVE_L1_BITNR]        = "inactive L1 table",
+    [QCOW2_OL_INACTIVE_L2_BITNR]        = "inactive L2 table",
+    [QCOW2_OL_BITMAP_DIRECTORY_BITNR]   = "bitmap directory",
 };
+QEMU_BUILD_BUG_ON(QCOW2_OL_MAX_BITNR != ARRAY_SIZE(metadata_ol_names));
 
 /*
  * First performs a check for metadata overlaps (through
-- 
2.19.1

The following changes since commit 4f59102571fce49af180cfc6d4cdd2b5df7bdb14:

Merge remote-tracking branch 'remotes/amarkovic/tags/mips-queue-oct-01-2019' into staging (2019-10-01 16:21:42 +0100)

are available in the Git repository at:

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

for you to fetch changes up to 7e693a0500455edab21754573c32b7146138cffd:

iotests: Remove Python 2 compatibility code (2019-10-04 11:59:16 +0200)

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

- Fix internal snapshots with typical -blockdev setups
- iotests: Require Python 3.6 or later

----------------------------------------------------------------
Kevin Wolf (4):
      block/snapshot: Restrict set of snapshot nodes
      iotests: Test internal snapshots with -blockdev
      iotests: Require Python 3.6 or later
      iotests: Remove Python 2 compatibility code

block/snapshot.c                 |  26 ++++--
 tests/qemu-iotests/044           |   3 -
 tests/qemu-iotests/163           |   3 -
 tests/qemu-iotests/267           | 168 ++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/267.out       | 182 +++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/check         |  13 ++-
 tests/qemu-iotests/common.filter |  11 ++-
 tests/qemu-iotests/group         |   1 +
 tests/qemu-iotests/iotests.py    |  13 +--
 9 files changed, 392 insertions(+), 28 deletions(-)
 create mode 100755 tests/qemu-iotests/267
 create mode 100644 tests/qemu-iotests/267.out

Nodes involved in internal snapshots were those that were returned by
bdrv_next(), inserted and not read-only. bdrv_next() in turn returns all
nodes that are either the root node of a BlockBackend or monitor-owned
nodes.

With the typical -drive use, this worked well enough. However, in the
typical -blockdev case, the user defines one node per option, making all
nodes monitor-owned nodes. This includes protocol nodes etc. which often
are not snapshottable, so "savevm" only returns an error.

Change the conditions so that internal snapshot still include all nodes
that have a BlockBackend attached (we definitely want to snapshot
anything attached to a guest device and probably also the built-in NBD
server; snapshotting block job BlockBackends is more of an accident, but
a preexisting one), but other monitor-owned nodes are only included if
they have no parents.

This makes internal snapshots usable again with typical -blockdev
configurations.

Cc: qemu-stable@nongnu.org
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Peter Krempa <pkrempa@redhat.com>
---
 block/snapshot.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/block/snapshot.c b/block/snapshot.c
index XXXXXXX..XXXXXXX 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -XXX,XX +XXX,XX @@
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qstring.h"
 #include "qemu/option.h"
+#include "sysemu/block-backend.h"
 
 QemuOptsList internal_snapshot_opts = {
     .name = "snapshot",
@@ -XXX,XX +XXX,XX @@ int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
     return ret;
 }
 
+static bool bdrv_all_snapshots_includes_bs(BlockDriverState *bs)
+{
+    if (!bdrv_is_inserted(bs) || bdrv_is_read_only(bs)) {
+        return false;
+    }
+
+    /* Include all nodes that are either in use by a BlockBackend, or that
+     * aren't attached to any node, but owned by the monitor. */
+    return bdrv_has_blk(bs) || QLIST_EMPTY(&bs->parents);
+}
 
 /* Group operations. All block drivers are involved.
  * These functions will properly handle dataplane (take aio_context_acquire
@@ -XXX,XX +XXX,XX @@ bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs)
         AioContext *ctx = bdrv_get_aio_context(bs);
 
         aio_context_acquire(ctx);
-        if (bdrv_is_inserted(bs) && !bdrv_is_read_only(bs)) {
+        if (bdrv_all_snapshots_includes_bs(bs)) {
             ok = bdrv_can_snapshot(bs);
         }
         aio_context_release(ctx);
@@ -XXX,XX +XXX,XX @@ int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bad_bs,
         AioContext *ctx = bdrv_get_aio_context(bs);
 
         aio_context_acquire(ctx);
-        if (bdrv_can_snapshot(bs) &&
-                bdrv_snapshot_find(bs, snapshot, name) >= 0) {
+        if (bdrv_all_snapshots_includes_bs(bs) &&
+            bdrv_snapshot_find(bs, snapshot, name) >= 0)
+        {
             ret = bdrv_snapshot_delete(bs, snapshot->id_str,
                                        snapshot->name, err);
         }
@@ -XXX,XX +XXX,XX @@ int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs,
         AioContext *ctx = bdrv_get_aio_context(bs);
 
         aio_context_acquire(ctx);
-        if (bdrv_can_snapshot(bs)) {
+        if (bdrv_all_snapshots_includes_bs(bs)) {
             ret = bdrv_snapshot_goto(bs, name, errp);
         }
         aio_context_release(ctx);
@@ -XXX,XX +XXX,XX @@ int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs)
         AioContext *ctx = bdrv_get_aio_context(bs);
 
         aio_context_acquire(ctx);
-        if (bdrv_can_snapshot(bs)) {
+        if (bdrv_all_snapshots_includes_bs(bs)) {
             err = bdrv_snapshot_find(bs, &sn, name);
         }
         aio_context_release(ctx);
@@ -XXX,XX +XXX,XX @@ int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn,
         if (bs == vm_state_bs) {
             sn->vm_state_size = vm_state_size;
             err = bdrv_snapshot_create(bs, sn);
-        } else if (bdrv_can_snapshot(bs)) {
+        } else if (bdrv_all_snapshots_includes_bs(bs)) {
             sn->vm_state_size = 0;
             err = bdrv_snapshot_create(bs, sn);
         }
@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_all_find_vmstate_bs(void)
         bool found;
 
         aio_context_acquire(ctx);
-        found = bdrv_can_snapshot(bs);
+        found = bdrv_all_snapshots_includes_bs(bs) && bdrv_can_snapshot(bs);
         aio_context_release(ctx);
 
         if (found) {
-- 
2.20.1

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Peter Krempa <pkrempa@redhat.com>
---
 tests/qemu-iotests/267           | 168 ++++++++++++++++++++++++++++
 tests/qemu-iotests/267.out       | 182 +++++++++++++++++++++++++++++++
 tests/qemu-iotests/common.filter |  11 +-
 tests/qemu-iotests/group         |   1 +
 4 files changed, 358 insertions(+), 4 deletions(-)
 create mode 100755 tests/qemu-iotests/267
 create mode 100644 tests/qemu-iotests/267.out

diff --git a/tests/qemu-iotests/267 b/tests/qemu-iotests/267
new file mode 100755
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/267
@@ -XXX,XX +XXX,XX @@
+#!/usr/bin/env bash
+#
+# Test which nodes are involved in internal snapshots
+#
+# Copyright (C) 2019 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=kwolf@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+status=1	# failure is the default!
+
+_cleanup()
+{
+    _cleanup_test_img
+    rm -f "$TEST_DIR/nbd"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+# Internal snapshots are (currently) impossible with refcount_bits=1
+_unsupported_imgopts 'refcount_bits=1[^0-9]'
+
+do_run_qemu()
+{
+    echo Testing: "$@"
+    (
+        if ! test -t 0; then
+            while read cmd; do
+                echo $cmd
+            done
+        fi
+        echo quit
+    ) | $QEMU -nographic -monitor stdio -nodefaults "$@"
+    echo
+}
+
+run_qemu()
+{
+    do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qemu | _filter_hmp |
+        _filter_generated_node_ids | _filter_imgfmt | _filter_vmstate_size
+}
+
+size=128M
+
+run_test()
+{
+    _make_test_img $size
+    printf "savevm snap0\ninfo snapshots\nloadvm snap0\n" | run_qemu "$@" | _filter_date
+}
+
+
+echo
+echo "=== No block devices at all ==="
+echo
+
+run_test
+
+echo
+echo "=== -drive if=none ==="
+echo
+
+run_test -drive driver=file,file="$TEST_IMG",if=none
+run_test -drive driver=$IMGFMT,file="$TEST_IMG",if=none
+run_test -drive driver=$IMGFMT,file="$TEST_IMG",if=none -device virtio-blk,drive=none0
+
+echo
+echo "=== -drive if=virtio ==="
+echo
+
+run_test -drive driver=file,file="$TEST_IMG",if=virtio
+run_test -drive driver=$IMGFMT,file="$TEST_IMG",if=virtio
+
+echo
+echo "=== Simple -blockdev ==="
+echo
+
+run_test -blockdev driver=file,filename="$TEST_IMG",node-name=file
+run_test -blockdev driver=file,filename="$TEST_IMG",node-name=file \
+         -blockdev driver=$IMGFMT,file=file,node-name=fmt
+run_test -blockdev driver=file,filename="$TEST_IMG",node-name=file \
+         -blockdev driver=raw,file=file,node-name=raw \
+         -blockdev driver=$IMGFMT,file=raw,node-name=fmt
+
+echo
+echo "=== -blockdev with a filter on top ==="
+echo
+
+run_test -blockdev driver=file,filename="$TEST_IMG",node-name=file \
+         -blockdev driver=$IMGFMT,file=file,node-name=fmt \
+         -blockdev driver=copy-on-read,file=fmt,node-name=filter
+
+echo
+echo "=== -blockdev with a backing file ==="
+echo
+
+TEST_IMG="$TEST_IMG.base" _make_test_img $size
+
+IMGOPTS="backing_file=$TEST_IMG.base" \
+run_test -blockdev driver=file,filename="$TEST_IMG.base",node-name=backing-file \
+         -blockdev driver=file,filename="$TEST_IMG",node-name=file \
+         -blockdev driver=$IMGFMT,file=file,backing=backing-file,node-name=fmt
+
+IMGOPTS="backing_file=$TEST_IMG.base" \
+run_test -blockdev driver=file,filename="$TEST_IMG.base",node-name=backing-file \
+         -blockdev driver=$IMGFMT,file=backing-file,node-name=backing-fmt \
+         -blockdev driver=file,filename="$TEST_IMG",node-name=file \
+         -blockdev driver=$IMGFMT,file=file,backing=backing-fmt,node-name=fmt
+
+# A snapshot should be present on the overlay, but not the backing file
+echo Internal snapshots on overlay:
+$QEMU_IMG snapshot -l "$TEST_IMG" | _filter_date | _filter_vmstate_size
+
+echo Internal snapshots on backing file:
+$QEMU_IMG snapshot -l "$TEST_IMG.base" | _filter_date | _filter_vmstate_size
+
+echo
+echo "=== -blockdev with NBD server on the backing file ==="
+echo
+
+IMGOPTS="backing_file=$TEST_IMG.base" _make_test_img $size
+cat <<EOF |
+nbd_server_start unix:$TEST_DIR/nbd
+nbd_server_add -w backing-fmt
+savevm snap0
+info snapshots
+loadvm snap0
+EOF
+run_qemu -blockdev driver=file,filename="$TEST_IMG.base",node-name=backing-file \
+         -blockdev driver=$IMGFMT,file=backing-file,node-name=backing-fmt \
+         -blockdev driver=file,filename="$TEST_IMG",node-name=file \
+         -blockdev driver=$IMGFMT,file=file,backing=backing-fmt,node-name=fmt |
+         _filter_date
+
+# This time, a snapshot should be created on both files
+echo Internal snapshots on overlay:
+$QEMU_IMG snapshot -l "$TEST_IMG" | _filter_date | _filter_vmstate_size
+
+echo Internal snapshots on backing file:
+$QEMU_IMG snapshot -l "$TEST_IMG.base" | _filter_date | _filter_vmstate_size
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/267.out b/tests/qemu-iotests/267.out
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/267.out
@@ -XXX,XX +XXX,XX @@
+QA output created by 267
+
+=== No block devices at all ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing:
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+Error: No block device can accept snapshots
+(qemu) info snapshots
+No available block device supports snapshots
+(qemu) loadvm snap0
+Error: No block device supports snapshots
+(qemu) quit
+
+
+=== -drive if=none ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -drive driver=file,file=TEST_DIR/t.IMGFMT,if=none
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+Error: Device 'none0' is writable but does not support snapshots
+(qemu) info snapshots
+No available block device supports snapshots
+(qemu) loadvm snap0
+Error: Device 'none0' is writable but does not support snapshots
+(qemu) quit
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -drive driver=IMGFMT,file=TEST_DIR/t.IMGFMT,if=none
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID        TAG                 VM SIZE                DATE       VM CLOCK
+--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -drive driver=IMGFMT,file=TEST_DIR/t.IMGFMT,if=none -device virtio-blk,drive=none0
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID        TAG                 VM SIZE                DATE       VM CLOCK
+--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+
+=== -drive if=virtio ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -drive driver=file,file=TEST_DIR/t.IMGFMT,if=virtio
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+Error: Device 'virtio0' is writable but does not support snapshots
+(qemu) info snapshots
+No available block device supports snapshots
+(qemu) loadvm snap0
+Error: Device 'virtio0' is writable but does not support snapshots
+(qemu) quit
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -drive driver=IMGFMT,file=TEST_DIR/t.IMGFMT,if=virtio
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID        TAG                 VM SIZE                DATE       VM CLOCK
+--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+
+=== Simple -blockdev ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+Error: Device '' is writable but does not support snapshots
+(qemu) info snapshots
+No available block device supports snapshots
+(qemu) loadvm snap0
+Error: Device '' is writable but does not support snapshots
+(qemu) quit
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=IMGFMT,file=file,node-name=fmt
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID        TAG                 VM SIZE                DATE       VM CLOCK
+--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=raw,file=file,node-name=raw -blockdev driver=IMGFMT,file=raw,node-name=fmt
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID        TAG                 VM SIZE                DATE       VM CLOCK
+--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+
+=== -blockdev with a filter on top ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=IMGFMT,file=file,node-name=fmt -blockdev driver=copy-on-read,file=fmt,node-name=filter
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID        TAG                 VM SIZE                DATE       VM CLOCK
+--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+
+=== -blockdev with a backing file ===
+
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT.base,node-name=backing-file -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=IMGFMT,file=file,backing=backing-file,node-name=fmt
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID        TAG                 VM SIZE                DATE       VM CLOCK
+--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT.base,node-name=backing-file -blockdev driver=IMGFMT,file=backing-file,node-name=backing-fmt -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=IMGFMT,file=file,backing=backing-fmt,node-name=fmt
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID        TAG                 VM SIZE                DATE       VM CLOCK
+--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+Internal snapshots on overlay:
+Snapshot list:
+ID        TAG                 VM SIZE                DATE       VM CLOCK
+1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+Internal snapshots on backing file:
+
+=== -blockdev with NBD server on the backing file ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT.base,node-name=backing-file -blockdev driver=IMGFMT,file=backing-file,node-name=backing-fmt -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=IMGFMT,file=file,backing=backing-fmt,node-name=fmt
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) nbd_server_start unix:TEST_DIR/nbd
+(qemu) nbd_server_add -w backing-fmt
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID        TAG                 VM SIZE                DATE       VM CLOCK
+--        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+Internal snapshots on overlay:
+Snapshot list:
+ID        TAG                 VM SIZE                DATE       VM CLOCK
+1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+Internal snapshots on backing file:
+Snapshot list:
+ID        TAG                 VM SIZE                DATE       VM CLOCK
+1         snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
+*** done
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 @@
 # standard filters
 #
 
-# ctime(3) dates
-#
 _filter_date()
 {
-    $SED \
-        -e 's/[A-Z][a-z][a-z] [A-z][a-z][a-z]  *[0-9][0-9]* [0-9][0-9]:[0-9][0-9]:[0-9][0-9] [0-9][0-9][0-9][0-9]$/DATE/'
+    $SED -re 's/[0-9]{4}-[0-9]{2}-[0-9]{2} [0-9]{2}:[0-9]{2}:[0-9]{2}/yyyy-mm-dd hh:mm:ss/'
+}
+
+_filter_vmstate_size()
+{
+    $SED -r -e 's/[0-9. ]{5} [KMGT]iB/     SIZE/' \
+            -e 's/[0-9. ]{5} B/   SIZE/'
 }
 
 _filter_generated_node_ids()
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -XXX,XX +XXX,XX @@
 263 rw quick
 265 rw auto quick
 266 rw quick
+267 rw auto quick snapshot
-- 
2.20.1

Running iotests is not required to build QEMU, so we can have stricter
version requirements for Python here and can make use of new features
and drop compatibility code earlier.

This makes qemu-iotests skip all Python tests if a Python version before
3.6 is used for the build.

Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/check | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -XXX,XX +XXX,XX @@ then
     export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper"
 fi
 
+python_usable=false
+if $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)'
+then
+    python_usable=true
+fi
+
 default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p')
 default_alias_machine=$($QEMU_PROG -machine help | \
    sed -n "/(alias of $default_machine)/ { s/ .*//p; q; }")
@@ -XXX,XX +XXX,XX @@ do
         start=$(_wallclock)
 
         if [ "$(head -n 1 "$source_iotests/$seq")" == "#!/usr/bin/env python" ]; then
-            run_command="$PYTHON $seq"
+            if $python_usable; then
+                run_command="$PYTHON $seq"
+            else
+                run_command="false"
+                echo "Unsupported Python version" > $seq.notrun
+            fi
         else
             run_command="./$seq"
         fi
-- 
2.20.1

Some scripts check the Python version number and have two code paths to
accomodate both Python 2 and 3. Remove the code specific to Python 2 and
assert the minimum version of 3.6 instead (check skips Python tests in
this case, so the assertion would only ever trigger if a Python script
is executed manually).

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/044        |  3 ---
 tests/qemu-iotests/163        |  3 ---
 tests/qemu-iotests/iotests.py | 13 +++----------
 3 files changed, 3 insertions(+), 16 deletions(-)

diff --git a/tests/qemu-iotests/044 b/tests/qemu-iotests/044
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/044
+++ b/tests/qemu-iotests/044
@@ -XXX,XX +XXX,XX @@ import struct
 import subprocess
 import sys
 
-if sys.version_info.major == 2:
-    range = xrange
-
 test_img = os.path.join(iotests.test_dir, 'test.img')
 
 class TestRefcountTableGrowth(iotests.QMPTestCase):
diff --git a/tests/qemu-iotests/163 b/tests/qemu-iotests/163
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/163
+++ b/tests/qemu-iotests/163
@@ -XXX,XX +XXX,XX @@
 import os, random, iotests, struct, qcow2, sys
 from iotests import qemu_img, qemu_io, image_size
 
-if sys.version_info.major == 2:
-    range = xrange
-
 test_img = os.path.join(iotests.test_dir, 'test.img')
 check_img = os.path.join(iotests.test_dir, 'check.img')
 
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -XXX,XX +XXX,XX @@ from collections import OrderedDict
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
 from qemu import qtest
 
+assert sys.version_info >= (3,6)
 
 # This will not work if arguments contain spaces but is necessary if we
 # want to support the override options that ./check supports.
@@ -XXX,XX +XXX,XX @@ def image_size(img):
     return json.loads(r)['virtual-size']
 
 def is_str(val):
-    if sys.version_info.major >= 3:
-        return isinstance(val, str)
-    else:
-        return isinstance(val, str) or isinstance(val, unicode)
+    return isinstance(val, str)
 
 test_dir_re = re.compile(r"%s" % test_dir)
 def filter_test_dir(msg):
@@ -XXX,XX +XXX,XX @@ def execute_test(test_function=None,
     else:
         # We need to filter out the time taken from the output so that
         # qemu-iotest can reliably diff the results against master output.
-        if sys.version_info.major >= 3:
-            output = io.StringIO()
-        else:
-            # io.StringIO is for unicode strings, which is not what
-            # 2.x's test runner emits.
-            output = io.BytesIO()
+        output = io.StringIO()
 
     logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
 
-- 
2.20.1