[Qemu-devel] [PATCH v7 00/20] block: Image locking series for 2.8

[Qemu-devel] [PATCH v7 00/20] block: Image locking series for 2.8

[Fam Zheng]

v7: - Rebase.
    - Address comments from Kevin and Max.
    - Option rename: "exclusive" -> "auto". [Kevin]
    - Option placement: "root BDS" -> "node".
      It's still a bit controversy where the option should go, but so far it
      seems per node is the safest.
    - Assert in bdrv_open_common that lock flags don't conflict. [Kevin]
    - Skip qemu_dup patches as they are already merged.
    - Fix a wrong assertion in raw_reopen_to_unlock.
    - Put "static int" to the same line in raw-posix.c. [Kevin]
    - Remove superfluous blank lines.
    - Add comment in raw posix's sophisticated reopen code. [Kevin]
    - .bdrv_lockf is kept on the contrary to separate .bdrv_lock and
      .bdrv_unlock, because having the BdrvLockfCmd in the interface makes
      implementing the reopen logic as is done in this series much easier.
    - Drop the unnecessary mirror target locking patch. [Max]
    - Add a patch to let 130 work with image locking.

Fam Zheng (20):
  block: Add flag bits for image locking
  qapi: Add lock-mode in blockdev-add options
  block: Add and parse "lock-mode" option for image locking
  block: Introduce image file locking
  osdep: Add qemu_lock_fd and qemu_unlock_fd
  raw-posix: Add image locking support
  qemu-io: Add "-L" option for BDRV_O_NO_LOCK
  qemu-img: Add "-L" option to sub commands
  qemu-img: Update documentation of "-L" option
  qemu-nbd: Add "--no-lock/-L" option
  block: Don't lock drive-backup target image in none mode
  qemu-iotests: 046: Move version detection out from verify_io
  qemu-iotests: Wait for QEMU processes before checking image in 091
  qemu-iotests: 030: Disable image locking when checking test image
  iotests: 087: Disable image locking in cases where file is shared
  iotests: 130: Check image info locklessly
  iotests: Disable image locking in 085
  tests: Use null-co:// instead of /dev/null
  block: Turn on image locking by default
  qemu-iotests: Add test case 153 for image locking

 block.c                    |  80 +++++++++
 block/raw-posix.c          | 298 ++++++++++++++++++++++++++++++-
 blockdev.c                 |  14 ++
 include/block/block.h      |  14 +-
 include/block/block_int.h  |   5 +
 include/qemu/osdep.h       |   2 +
 qapi/block-core.json       |  19 +-
 qemu-img-cmds.hx           |  44 ++---
 qemu-img.c                 |  92 ++++++++--
 qemu-img.texi              |   3 +
 qemu-io.c                  |  24 ++-
 qemu-nbd.c                 |   7 +-
 qemu-nbd.texi              |   2 +
 qemu-options.hx            |   1 +
 tests/drive_del-test.c     |   2 +-
 tests/nvme-test.c          |   2 +-
 tests/qemu-iotests/030     |   2 +-
 tests/qemu-iotests/046     |  22 +--
 tests/qemu-iotests/085     |   3 +-
 tests/qemu-iotests/087     |   6 +
 tests/qemu-iotests/091     |   3 +
 tests/qemu-iotests/091.out |   1 +
 tests/qemu-iotests/130     |   4 +-
 tests/qemu-iotests/130.out |   4 +-
 tests/qemu-iotests/153     | 197 +++++++++++++++++++++
 tests/qemu-iotests/153.out | 426 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/group   |   1 +
 tests/usb-hcd-uhci-test.c  |   2 +-
 tests/usb-hcd-xhci-test.c  |   2 +-
 tests/virtio-blk-test.c    |   2 +-
 tests/virtio-scsi-test.c   |   4 +-
 util/osdep.c               |  29 +++
 32 files changed, 1246 insertions(+), 71 deletions(-)
 create mode 100755 tests/qemu-iotests/153
 create mode 100644 tests/qemu-iotests/153.out

-- 
2.7.4

[Qemu-devel] [PATCH v7 01/20] block: Add flag bits for image locking

[Fam Zheng]

Later the block layer will automatically lock the images to avoid unexpected
concurrent accesses to the same image, which will easily corrupt the metadata
or user data, unless in some very special cases, like migration.

The exceptional cases like shared storage migration and testing should
set BDRV_O_SHARED_LOCK or BDRV_O_NO_LOCK to advise an appropriate
locking mode.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 include/block/block.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/block/block.h b/include/block/block.h
index 11c162d..4f8450b 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -97,6 +97,8 @@ typedef struct HDGeometry {
                                       select an appropriate protocol driver,
                                       ignoring the format layer */
 #define BDRV_O_NO_IO       0x10000 /* don't initialize for I/O */
+#define BDRV_O_NO_LOCK     0x20000 /* don't lock image file */
+#define BDRV_O_SHARED_LOCK 0x40000 /* lock the image file in shared mode */
 
 #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_NO_FLUSH)
 
-- 
2.7.4

[Qemu-devel] [PATCH v7 02/20] qapi: Add lock-mode in blockdev-add options

[Fam Zheng]

To allow overriding the default locking behavior when opening the image.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 qapi/block-core.json | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 5e2d7d7..d1eb197 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2151,6 +2151,20 @@
             '*debug-level': 'int' } }
 
 ##
+# @BlockdevLockMode
+#
+# Describes how QEMU should lock the image.
+#
+# @off:       Disabled
+# @shared:    Use shared lock for both RO and RW images.
+# @auto:      Use exclusive lock for RW images, and shared lock for RO images.
+#
+# Since: 2.7
+##
+{ 'enum': 'BlockdevLockMode',
+  'data': [ 'off', 'shared', 'auto' ] }
+
+##
 # @BlockdevOptions
 #
 # Options for creating a block device.  Many options are available for all
@@ -2172,6 +2186,8 @@
 # @detect-zeroes: #optional detect and optimize zero writes (Since 2.1)
 #                 (default: off)
 #
+# @lock-mode: #optional how to lock the image. (default: auto) (Since 2.7)
+#
 # Remaining options are determined by the block driver.
 #
 # Since: 1.7
@@ -2185,7 +2201,8 @@
             '*cache': 'BlockdevCacheOptions',
             '*aio': 'BlockdevAioOptions',
             '*read-only': 'bool',
-            '*detect-zeroes': 'BlockdevDetectZeroesOptions' },
+            '*detect-zeroes': 'BlockdevDetectZeroesOptions',
+            '*lock-mode': 'BlockdevLockMode' },
   'discriminator': 'driver',
   'data': {
       'archipelago':'BlockdevOptionsArchipelago',
-- 
2.7.4

[Qemu-devel] [PATCH v7 03/20] block: Add and parse "lock-mode" option for image locking

[Fam Zheng]

Respect the locking mode from CLI or QMP, and set the open flags
accordingly.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c               | 21 +++++++++++++++++++++
 blockdev.c            |  9 +++++++++
 include/block/block.h |  1 +
 qemu-options.hx       |  1 +
 4 files changed, 32 insertions(+)

diff --git a/block.c b/block.c
index 30d64e6..5f76f60 100644
--- a/block.c
+++ b/block.c
@@ -705,6 +705,7 @@ static void bdrv_inherited_options(int *child_flags, QDict *child_options,
      * the parent. */
     qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_DIRECT);
     qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_NO_FLUSH);
+    qdict_copy_default(child_options, parent_options, BDRV_OPT_LOCK_MODE);
 
     /* Our block drivers take care to send flushes and respect unmap policy,
      * so we can default to enable both on lower layers regardless of the
@@ -757,6 +758,7 @@ static void bdrv_backing_options(int *child_flags, QDict *child_options,
      * which is only applied on the top level (BlockBackend) */
     qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_DIRECT);
     qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_NO_FLUSH);
+    qdict_copy_default(child_options, parent_options, BDRV_OPT_LOCK_MODE);
 
     /* backing files always opened read-only */
     flags &= ~(BDRV_O_RDWR | BDRV_O_COPY_ON_READ);
@@ -880,6 +882,11 @@ static QemuOptsList bdrv_runtime_opts = {
             .name = BDRV_OPT_CACHE_NO_FLUSH,
             .type = QEMU_OPT_BOOL,
             .help = "Ignore flush requests",
+        },{
+            .name = BDRV_OPT_LOCK_MODE,
+            .type = QEMU_OPT_STRING,
+            .help = "how to lock the image (auto, shared, off. "
+                    "default: auto)",
         },
         { /* end of list */ }
     },
@@ -897,6 +904,7 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
     const char *filename;
     const char *driver_name = NULL;
     const char *node_name = NULL;
+    const char *lock_mode = NULL;
     QemuOpts *opts;
     BlockDriver *drv;
     Error *local_err = NULL;
@@ -940,6 +948,19 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
         goto fail_opts;
     }
 
+    lock_mode = qemu_opt_get(opts, BDRV_OPT_LOCK_MODE) ? : "off";
+    if (!strcmp(lock_mode, "auto")) {
+        /* Default */
+    } else if (!strcmp(lock_mode, "shared")) {
+        bs->open_flags |= BDRV_O_SHARED_LOCK;
+    } else if (!strcmp(lock_mode, "off")) {
+        bs->open_flags |= BDRV_O_NO_LOCK;
+    } else {
+        error_setg(errp, "invalid lock mode");
+        ret = -EINVAL;
+        goto fail_opts;
+    }
+
     bs->read_only = !(bs->open_flags & BDRV_O_RDWR);
 
     if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv, bs->read_only)) {
diff --git a/blockdev.c b/blockdev.c
index 2161400..a9032b9 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -545,6 +545,10 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
         qdict_put(bs_opts, "driver", qstring_from_str(buf));
     }
 
+    if ((buf = qemu_opt_get(opts, BDRV_OPT_LOCK_MODE)) != NULL) {
+        qdict_put(bs_opts, BDRV_OPT_LOCK_MODE, qstring_from_str(buf));
+    }
+
     on_write_error = BLOCKDEV_ON_ERROR_ENOSPC;
     if ((buf = qemu_opt_get(opts, "werror")) != NULL) {
         on_write_error = parse_block_error_action(buf, 0, &error);
@@ -4265,6 +4269,11 @@ QemuOptsList qemu_common_drive_opts = {
             .type = QEMU_OPT_BOOL,
             .help = "whether to account for failed I/O operations "
                     "in the statistics",
+        },{
+            .name = BDRV_OPT_LOCK_MODE,
+            .type = QEMU_OPT_STRING,
+            .help = "how to lock the image (auto, shared, off. "
+                    "default: auto)",
         },
         { /* end of list */ }
     },
diff --git a/include/block/block.h b/include/block/block.h
index 4f8450b..a14101b 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -108,6 +108,7 @@ typedef struct HDGeometry {
 #define BDRV_OPT_CACHE_WB       "cache.writeback"
 #define BDRV_OPT_CACHE_DIRECT   "cache.direct"
 #define BDRV_OPT_CACHE_NO_FLUSH "cache.no-flush"
+#define BDRV_OPT_LOCK_MODE      "lock-mode"
 
 
 #define BDRV_SECTOR_BITS   9
diff --git a/qemu-options.hx b/qemu-options.hx
index a71aaf8..0f67b64 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -521,6 +521,7 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive,
     "       [,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n"
     "       [,readonly=on|off][,copy-on-read=on|off]\n"
     "       [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n"
+    "       [,lock-mode=auto|shared|off]\n"
     "       [[,bps=b]|[[,bps_rd=r][,bps_wr=w]]]\n"
     "       [[,iops=i]|[[,iops_rd=r][,iops_wr=w]]]\n"
     "       [[,bps_max=bm]|[[,bps_rd_max=rm][,bps_wr_max=wm]]]\n"
-- 
2.7.4

[Qemu-devel] [PATCH v7 04/20] block: Introduce image file locking

[Fam Zheng]

Block drivers can implement this new operation .bdrv_lockf to actually lock the
image in the protocol specific way.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c                   | 59 +++++++++++++++++++++++++++++++++++++++++++++++
 include/block/block.h     | 11 ++++++++-
 include/block/block_int.h |  5 ++++
 3 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 5f76f60..55a393f 100644
--- a/block.c
+++ b/block.c
@@ -859,6 +859,50 @@ out:
     g_free(gen_node_name);
 }
 
+BdrvLockfCmd bdrv_get_locking_cmd(int flags)
+{
+    if (flags & BDRV_O_NO_LOCK) {
+        return BDRV_LOCKF_UNLOCK;
+    } else if (flags & BDRV_O_SHARED_LOCK) {
+        return BDRV_LOCKF_SHARED;
+    } else if (flags & BDRV_O_RDWR) {
+        return BDRV_LOCKF_EXCLUSIVE;
+    } else {
+        return BDRV_LOCKF_SHARED;
+    }
+}
+
+static int bdrv_lock_unlock_image_do(BlockDriverState *bs, BdrvLockfCmd cmd)
+{
+    int ret;
+
+    if (bs->cur_lock == cmd) {
+        return 0;
+    } else if (!bs->drv) {
+        return -ENOMEDIUM;
+    } else if (!bs->drv->bdrv_lockf) {
+        return 0;
+    }
+    ret = bs->drv->bdrv_lockf(bs, cmd);
+    if (ret == -ENOTSUP) {
+        /* Handle it the same way as !bs->drv->bdrv_lockf */
+        ret = 0;
+    } else if (ret == 0) {
+        bs->cur_lock = cmd;
+    }
+    return ret;
+}
+
+static int bdrv_lock_image(BlockDriverState *bs)
+{
+    return bdrv_lock_unlock_image_do(bs, bdrv_get_locking_cmd(bs->open_flags));
+}
+
+static int bdrv_unlock_image(BlockDriverState *bs)
+{
+    return bdrv_lock_unlock_image_do(bs, BDRV_LOCKF_UNLOCK);
+}
+
 static QemuOptsList bdrv_runtime_opts = {
     .name = "bdrv_common",
     .head = QTAILQ_HEAD_INITIALIZER(bdrv_runtime_opts.head),
@@ -1025,6 +1069,16 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
         goto free_and_fail;
     }
 
+    assert(!((open_flags & BDRV_O_NO_LOCK) &&
+             (open_flags & BDRV_O_SHARED_LOCK)));
+    if (!(open_flags & (BDRV_O_NO_LOCK | BDRV_O_INACTIVE))) {
+        ret = bdrv_lock_image(bs);
+        if (ret) {
+            error_setg(errp, "Failed to lock image");
+            goto free_and_fail;
+        }
+    }
+
     ret = refresh_total_sectors(bs, bs->total_sectors);
     if (ret < 0) {
         error_setg_errno(errp, -ret, "Could not refresh total sector count");
@@ -2182,6 +2236,7 @@ static void bdrv_close(BlockDriverState *bs)
     if (bs->drv) {
         BdrvChild *child, *next;
 
+        bdrv_unlock_image(bs);
         bs->drv->bdrv_close(bs);
         bs->drv = NULL;
 
@@ -3101,6 +3156,9 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
         error_setg_errno(errp, -ret, "Could not refresh total sector count");
         return;
     }
+    if (!(bs->open_flags & BDRV_O_NO_LOCK)) {
+        bdrv_lock_image(bs);
+    }
 }
 
 void bdrv_invalidate_cache_all(Error **errp)
@@ -3143,6 +3201,7 @@ static int bdrv_inactivate_recurse(BlockDriverState *bs,
     }
 
     if (setting_flag) {
+        ret = bdrv_unlock_image(bs);
         bs->open_flags |= BDRV_O_INACTIVE;
     }
     return 0;
diff --git a/include/block/block.h b/include/block/block.h
index a14101b..9ceabcd 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -187,6 +187,15 @@ typedef enum BlockOpType {
     BLOCK_OP_TYPE_MAX,
 } BlockOpType;
 
+typedef enum {
+    /* The values are ordered so that lower number implies higher restriction.
+     * Starting from 1 to make 0 an invalid value.
+     * */
+    BDRV_LOCKF_EXCLUSIVE = 1,
+    BDRV_LOCKF_SHARED,
+    BDRV_LOCKF_UNLOCK,
+} BdrvLockfCmd;
+
 void bdrv_info_print(Monitor *mon, const QObject *data);
 void bdrv_info(Monitor *mon, QObject **ret_data);
 void bdrv_stats_print(Monitor *mon, const QObject *data);
@@ -274,7 +283,7 @@ int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top,
 BlockDriverState *bdrv_find_overlay(BlockDriverState *active,
                                     BlockDriverState *bs);
 BlockDriverState *bdrv_find_base(BlockDriverState *bs);
-
+BdrvLockfCmd bdrv_get_locking_cmd(int flags);
 
 typedef struct BdrvCheckResult {
     int corruptions;
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 47665be..448fa65 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -319,6 +319,10 @@ struct BlockDriver {
                            Error **errp);
     void (*bdrv_del_child)(BlockDriverState *parent, BdrvChild *child,
                            Error **errp);
+    /**
+     * Lock/unlock the image.
+     */
+    int (*bdrv_lockf)(BlockDriverState *bs, BdrvLockfCmd cmd);
 
     QLIST_ENTRY(BlockDriver) list;
 };
@@ -528,6 +532,7 @@ struct BlockDriverState {
     unsigned io_plug_disabled;
 
     int quiesce_counter;
+    BdrvLockfCmd cur_lock;
 };
 
 struct BlockBackendRootState {
-- 
2.7.4

[Qemu-devel] [PATCH v7 05/20] osdep: Add qemu_lock_fd and qemu_unlock_fd

[Fam Zheng]

They are wrappers of POSIX fcntl "file private locking".

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 include/qemu/osdep.h |  2 ++
 util/osdep.c         | 29 +++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 9e9fa61..f773f49 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -286,6 +286,8 @@ int qemu_close(int fd);
 #ifndef _WIN32
 int qemu_dup(int fd);
 #endif
+int qemu_lock_fd(int fd, int64_t start, int64_t len, bool exclusive);
+int qemu_unlock_fd(int fd, int64_t start, int64_t len);
 
 #if defined(__HAIKU__) && defined(__i386__)
 #define FMT_pid "%ld"
diff --git a/util/osdep.c b/util/osdep.c
index 06fb1cf..b85a490 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -140,6 +140,35 @@ static int qemu_parse_fdset(const char *param)
 {
     return qemu_parse_fd(param);
 }
+
+static int qemu_lock_fcntl(int fd, int64_t start, int64_t len, int fl_type)
+{
+#ifdef F_OFD_SETLK
+    int ret;
+    struct flock fl = {
+        .l_whence = SEEK_SET,
+        .l_start  = start,
+        .l_len    = len,
+        .l_type   = fl_type,
+    };
+    do {
+        ret = fcntl(fd, F_OFD_SETLK, &fl);
+    } while (ret == -1 && errno == EINTR);
+    return ret == -1 ? -errno : 0;
+#else
+    return -ENOTSUP;
+#endif
+}
+
+int qemu_lock_fd(int fd, int64_t start, int64_t len, bool exclusive)
+{
+    return qemu_lock_fcntl(fd, start, len, exclusive ? F_WRLCK : F_RDLCK);
+}
+
+int qemu_unlock_fd(int fd, int64_t start, int64_t len)
+{
+    return qemu_lock_fcntl(fd, start, len, F_UNLCK);
+}
 #endif
 
 /*
-- 
2.7.4

[Qemu-devel] [PATCH v7 06/20] raw-posix: Add image locking support

[Fam Zheng]

virtlockd in libvirt locks the first byte, we lock byte 1 to avoid
the intervene.

Both file and host device protocols are covered.

The complication is with reopen. We have three different locking states,
namely "unlocked", "shared locked" and "exclusively locked".

When we reopen, the new fd may need a new locking mode. Moving away to or from
exclusive is a bit tricky because we cannot do it atomically. This patch solves
it by dup() s->fd to s->lock_fd and avoid close(), so that there isn't a racy
window where we drop the lock on one fd before acquiring the exclusive lock on
the other.

To make the logic easier to manage, and allow better reuse, the code is
internally organized by state transition table (old_lock -> new_lock).

Signed-off-by: Fam Zheng <famz@redhat.com>

---

v7: Fix raw_reopen_to_unlock wrong assertion.
---
 block/raw-posix.c | 298 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 297 insertions(+), 1 deletion(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 6ed7547..371667d 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -133,6 +133,7 @@ do { \
 
 typedef struct BDRVRawState {
     int fd;
+    int lock_fd;
     int type;
     int open_flags;
     size_t buf_align;
@@ -149,6 +150,7 @@ typedef struct BDRVRawState {
 
 typedef struct BDRVRawReopenState {
     int fd;
+    int lock_fd;
     int open_flags;
 } BDRVRawReopenState;
 
@@ -367,6 +369,38 @@ static void raw_parse_flags(int bdrv_flags, int *open_flags)
     }
 }
 
+
+static int raw_lockf_fd(int fd, BdrvLockfCmd cmd)
+{
+    assert(fd >= 0);
+    /* Locking byte 1 avoids interfereing with virtlockd. */
+    switch (cmd) {
+    case BDRV_LOCKF_EXCLUSIVE:
+        return qemu_lock_fd(fd, 1, 1, true);
+    case BDRV_LOCKF_SHARED:
+        return qemu_lock_fd(fd, 1, 1, false);
+    case BDRV_LOCKF_UNLOCK:
+        return qemu_unlock_fd(fd, 1, 1);
+    default:
+        abort();
+    }
+}
+
+static int raw_lockf(BlockDriverState *bs, BdrvLockfCmd cmd)
+{
+
+    BDRVRawState *s = bs->opaque;
+
+    if (s->lock_fd < 0) {
+        s->lock_fd = qemu_dup(s->fd);
+        if (s->lock_fd < 0) {
+            return s->lock_fd;
+        }
+    }
+
+    return raw_lockf_fd(s->lock_fd, cmd);
+}
+
 #ifdef CONFIG_LINUX_AIO
 static bool raw_use_aio(int bdrv_flags)
 {
@@ -433,6 +467,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
     raw_parse_flags(bdrv_flags, &s->open_flags);
 
     s->fd = -1;
+    s->lock_fd = -1;
     fd = qemu_open(filename, s->open_flags, 0644);
     if (fd < 0) {
         ret = -errno;
@@ -529,6 +564,253 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
     return raw_open_common(bs, options, flags, 0, errp);
 }
 
+typedef enum {
+    RAW_REOPEN_PREPARE,
+    RAW_REOPEN_COMMIT,
+    RAW_REOPEN_ABORT
+} RawReopenOperation;
+
+typedef int (*RawReopenFunc)(BDRVReopenState *state,
+                             RawReopenOperation op,
+                             BdrvLockfCmd old_lock,
+                             BdrvLockfCmd new_lock,
+                             Error **errp);
+
+static int
+raw_reopen_identical(BDRVReopenState *state,
+                     RawReopenOperation op,
+                     BdrvLockfCmd old_lock,
+                     BdrvLockfCmd new_lock,
+                     Error **errp)
+{
+    assert(old_lock == new_lock);
+    return 0;
+}
+
+static int
+raw_reopen_from_unlock(BDRVReopenState *state,
+                       RawReopenOperation op,
+                       BdrvLockfCmd old_lock,
+                       BdrvLockfCmd new_lock,
+                       Error **errp)
+{
+    BDRVRawReopenState *raw_s = state->opaque;
+    int ret = 0;
+
+    assert(old_lock != new_lock);
+    assert(old_lock == BDRV_LOCKF_UNLOCK);
+    switch (op) {
+    case RAW_REOPEN_PREPARE:
+        ret = raw_lockf_fd(raw_s->lock_fd, new_lock);
+        if (ret) {
+            error_setg_errno(errp, -ret, "Failed to lock new fd");
+        }
+        break;
+    case RAW_REOPEN_COMMIT:
+    case RAW_REOPEN_ABORT:
+        break;
+    }
+
+    return ret;
+}
+
+static int
+raw_reopen_to_unlock(BDRVReopenState *state,
+                     RawReopenOperation op,
+                     BdrvLockfCmd old_lock,
+                     BdrvLockfCmd new_lock,
+                     Error **errp)
+{
+    BDRVRawState *s = state->bs->opaque;
+    int ret = 0;
+
+    assert(old_lock != new_lock);
+    assert(new_lock == BDRV_LOCKF_UNLOCK);
+    switch (op) {
+    case RAW_REOPEN_PREPARE:
+        break;
+    case RAW_REOPEN_COMMIT:
+        if (s->lock_fd >= 0) {
+            qemu_close(s->lock_fd);
+            s->lock_fd = -1;
+        }
+        break;
+    case RAW_REOPEN_ABORT:
+        break;
+    }
+
+    return ret;
+}
+
+static int
+raw_reopen_upgrade(BDRVReopenState *state,
+                   RawReopenOperation op,
+                   BdrvLockfCmd old_lock,
+                   BdrvLockfCmd new_lock,
+                   Error **errp)
+{
+    BDRVRawReopenState *raw_s = state->opaque;
+    BDRVRawState *s = state->bs->opaque;
+    int ret = 0, ret2;
+
+    assert(old_lock == BDRV_LOCKF_SHARED);
+    assert(new_lock == BDRV_LOCKF_EXCLUSIVE);
+    switch (op) {
+    case RAW_REOPEN_PREPARE:
+        ret = raw_lockf_fd(raw_s->lock_fd, BDRV_LOCKF_SHARED);
+        if (ret) {
+            error_setg_errno(errp, -ret, "Failed to lock new fd (shared)");
+            break;
+        }
+        ret = raw_lockf_fd(s->lock_fd, BDRV_LOCKF_UNLOCK);
+        if (ret) {
+            error_setg_errno(errp, -ret, "Failed to unlock old fd");
+            goto restore;
+        }
+        ret = raw_lockf_fd(raw_s->lock_fd, BDRV_LOCKF_EXCLUSIVE);
+        if (ret) {
+            error_setg_errno(errp, -ret, "Failed to lock new fd (exclusive)");
+            goto restore;
+        }
+        break;
+    case RAW_REOPEN_COMMIT:
+        break;
+    case RAW_REOPEN_ABORT:
+        raw_lockf_fd(raw_s->lock_fd, BDRV_LOCKF_SHARED);
+        ret = raw_lockf_fd(s->lock_fd, BDRV_LOCKF_SHARED);
+        if (ret) {
+            error_report("Failed to restore lock on old fd");
+        }
+        break;
+    }
+
+    return ret;
+restore:
+    ret2 = raw_lockf_fd(s->lock_fd, BDRV_LOCKF_SHARED);
+    if (ret2) {
+        error_report("Failed to restore old lock");
+    }
+    return ret;
+}
+
+static int
+raw_reopen_downgrade(BDRVReopenState *state,
+                     RawReopenOperation op,
+                     BdrvLockfCmd old_lock,
+                     BdrvLockfCmd new_lock,
+                     Error **errp)
+{
+    BDRVRawReopenState *raw_s = state->opaque;
+    BDRVRawState *s = state->bs->opaque;
+    int ret = 0;
+
+    assert(old_lock == BDRV_LOCKF_EXCLUSIVE);
+    assert(new_lock == BDRV_LOCKF_SHARED);
+    switch (op) {
+    case RAW_REOPEN_PREPARE:
+        break;
+    case RAW_REOPEN_COMMIT:
+        ret = raw_lockf_fd(s->lock_fd, BDRV_LOCKF_SHARED);
+        if (ret) {
+            error_report("Failed to downgrade old lock");
+            break;
+        }
+        ret = raw_lockf_fd(raw_s->lock_fd, BDRV_LOCKF_SHARED);
+        if (ret) {
+            error_report("Failed to lock new fd");
+            break;
+        }
+        break;
+    case RAW_REOPEN_ABORT:
+        break;
+    }
+
+    return ret;
+}
+
+/**
+ * Transactionally moving between three possible locking states is tricky and
+ * must be done carefully. That is mostly because downgrading an exclusive lock
+ * to shared or unlocked is not guaranteed to be revertable. As a result, in
+ * such cases we have to defer the downgraing to "commit", given that no revert
+ * will happen after that point, and that downgrading a lock should never fail.
+ *
+ * On the other hand, upgrading a lock (e.g. from unlocked or shared to
+ * exclusive lock) must happen in "prepare" because it may fail.
+ *
+ * Manage the operation matrix with this state transition table to make
+ * fulfulling above conditions easier.
+ */
+static const struct RawReopenFuncRecord {
+    BdrvLockfCmd old_lock;
+    BdrvLockfCmd new_lock;
+    RawReopenFunc func;
+    bool need_lock_fd;
+} reopen_functions[] = {
+    {BDRV_LOCKF_UNLOCK, BDRV_LOCKF_UNLOCK, raw_reopen_identical, false},
+    {BDRV_LOCKF_UNLOCK, BDRV_LOCKF_SHARED, raw_reopen_from_unlock, true},
+    {BDRV_LOCKF_UNLOCK, BDRV_LOCKF_EXCLUSIVE, raw_reopen_from_unlock, true},
+    {BDRV_LOCKF_SHARED, BDRV_LOCKF_UNLOCK, raw_reopen_to_unlock, false},
+    {BDRV_LOCKF_SHARED, BDRV_LOCKF_SHARED, raw_reopen_identical, false},
+    {BDRV_LOCKF_SHARED, BDRV_LOCKF_EXCLUSIVE, raw_reopen_upgrade, true},
+    {BDRV_LOCKF_EXCLUSIVE, BDRV_LOCKF_UNLOCK, raw_reopen_to_unlock, false},
+    {BDRV_LOCKF_EXCLUSIVE, BDRV_LOCKF_SHARED, raw_reopen_downgrade, true},
+    {BDRV_LOCKF_EXCLUSIVE, BDRV_LOCKF_EXCLUSIVE, raw_reopen_identical, false},
+};
+
+static int raw_reopen_handle_lock(BDRVReopenState *state,
+                                  RawReopenOperation op,
+                                  Error **errp)
+{
+    BDRVRawReopenState *raw_s = state->opaque;
+    BDRVRawState *s = state->bs->opaque;
+    BdrvLockfCmd old_lock, new_lock;
+    const struct RawReopenFuncRecord *rec;
+    int ret;
+
+    old_lock = bdrv_get_locking_cmd(bdrv_get_flags(state->bs));
+    new_lock = bdrv_get_locking_cmd(state->flags);
+
+    for (rec = &reopen_functions[0];
+         rec < &reopen_functions[ARRAY_SIZE(reopen_functions)];
+         rec++) {
+        if (rec->old_lock == old_lock && rec->new_lock == new_lock) {
+            break;
+        }
+    }
+    assert(rec != &reopen_functions[ARRAY_SIZE(reopen_functions)]);
+
+    switch (op) {
+    case RAW_REOPEN_PREPARE:
+        if (rec->need_lock_fd) {
+            ret = qemu_dup(raw_s->fd);
+            if (ret < 0) {
+                error_setg_errno(errp, -ret, "Failed to dup new fd");
+                return ret;
+            }
+            raw_s->lock_fd = ret;
+        }
+        return rec->func(state, op, old_lock, new_lock, errp);
+    case RAW_REOPEN_COMMIT:
+        rec->func(state, op, old_lock, new_lock, errp);
+        if (rec->need_lock_fd) {
+            if (s->lock_fd >= 0) {
+                qemu_close(s->lock_fd);
+            }
+            s->lock_fd = raw_s->lock_fd;
+        }
+        break;
+    case RAW_REOPEN_ABORT:
+        rec->func(state, op, old_lock, new_lock, errp);
+        if (rec->need_lock_fd && raw_s->lock_fd >= 0) {
+            qemu_close(raw_s->lock_fd);
+            raw_s->lock_fd = -1;
+        }
+        break;
+    }
+    return 0;
+}
+
 static int raw_reopen_prepare(BDRVReopenState *state,
                               BlockReopenQueue *queue, Error **errp)
 {
@@ -607,6 +889,10 @@ static int raw_reopen_prepare(BDRVReopenState *state,
         }
     }
 
+    if (!ret) {
+        ret = raw_reopen_handle_lock(state, RAW_REOPEN_PREPARE, errp);
+    }
+
     return ret;
 }
 
@@ -617,6 +903,8 @@ static void raw_reopen_commit(BDRVReopenState *state)
 
     s->open_flags = raw_s->open_flags;
 
+    raw_reopen_handle_lock(state, RAW_REOPEN_COMMIT, NULL);
+
     qemu_close(s->fd);
     s->fd = raw_s->fd;
 
@@ -634,6 +922,8 @@ static void raw_reopen_abort(BDRVReopenState *state)
         return;
     }
 
+    raw_reopen_handle_lock(state, RAW_REOPEN_ABORT, NULL);
+
     if (raw_s->fd >= 0) {
         qemu_close(raw_s->fd);
         raw_s->fd = -1;
@@ -1321,6 +1611,10 @@ 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;
+    }
 }
 
 static int raw_truncate(BlockDriverState *bs, int64_t offset)
@@ -1874,7 +2168,7 @@ BlockDriver bdrv_file = {
     .bdrv_get_info = raw_get_info,
     .bdrv_get_allocated_file_size
                         = raw_get_allocated_file_size,
-
+    .bdrv_lockf = raw_lockf,
     .create_opts = &raw_create_opts,
 };
 
@@ -2324,6 +2618,8 @@ static BlockDriver bdrv_host_device = {
 #ifdef __linux__
     .bdrv_aio_ioctl     = hdev_aio_ioctl,
 #endif
+
+    .bdrv_lockf = raw_lockf,
 };
 
 #if defined(__linux__) || defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
-- 
2.7.4

[Qemu-devel] [PATCH v7 07/20] qemu-io: Add "-L" option for BDRV_O_NO_LOCK

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 qemu-io.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/qemu-io.c b/qemu-io.c
index db129ea..5c14eba 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -108,6 +108,7 @@ static void open_help(void)
 " -r, -- open file read-only\n"
 " -s, -- use snapshot file\n"
 " -n, -- disable host cache, short for -t none\n"
+" -L, -- disable image locking\n"
 " -k, -- use kernel AIO implementation (on Linux only)\n"
 " -t, -- use the given cache mode for the image\n"
 " -d, -- use the given discard mode for the image\n"
@@ -124,7 +125,7 @@ static const cmdinfo_t open_cmd = {
     .argmin     = 1,
     .argmax     = -1,
     .flags      = CMD_NOFILE_OK,
-    .args       = "[-rsnk] [-t cache] [-d discard] [-o options] [path]",
+    .args       = "[-rsnLk] [-t cache] [-d discard] [-o options] [path]",
     .oneline    = "open the file specified by path",
     .help       = open_help,
 };
@@ -143,12 +144,13 @@ static int open_f(BlockBackend *blk, int argc, char **argv)
 {
     int flags = BDRV_O_UNMAP;
     int readonly = 0;
+    bool nolock = false;
     bool writethrough = true;
     int c;
     QemuOpts *qopts;
     QDict *opts;
 
-    while ((c = getopt(argc, argv, "snro:kt:d:")) != -1) {
+    while ((c = getopt(argc, argv, "snrLo:kt:d:")) != -1) {
         switch (c) {
         case 's':
             flags |= BDRV_O_SNAPSHOT;
@@ -177,6 +179,9 @@ static int open_f(BlockBackend *blk, int argc, char **argv)
                 return 0;
             }
             break;
+        case 'L':
+            nolock = true;
+            break;
         case 'o':
             if (imageOpts) {
                 printf("--image-opts and 'open -o' are mutually exclusive\n");
@@ -198,6 +203,10 @@ static int open_f(BlockBackend *blk, int argc, char **argv)
         flags |= BDRV_O_RDWR;
     }
 
+    if (nolock) {
+        flags |= BDRV_O_NO_LOCK;
+    }
+
     if (imageOpts && (optind == argc - 1)) {
         if (!qemu_opts_parse_noisily(&empty_opts, argv[optind], false)) {
             qemu_opts_reset(&empty_opts);
@@ -436,13 +445,15 @@ static QemuOptsList file_opts = {
 int main(int argc, char **argv)
 {
     int readonly = 0;
-    const char *sopt = "hVc:d:f:rsnmkt:T:";
+    const char *sopt = "hVc:d:f:rLsnmkt:T:";
+    bool nolock = false;
     const struct option lopt[] = {
         { "help", no_argument, NULL, 'h' },
         { "version", no_argument, NULL, 'V' },
         { "cmd", required_argument, NULL, 'c' },
         { "format", required_argument, NULL, 'f' },
         { "read-only", no_argument, NULL, 'r' },
+        { "no-lock", no_argument, NULL, 'L' },
         { "snapshot", no_argument, NULL, 's' },
         { "nocache", no_argument, NULL, 'n' },
         { "misalign", no_argument, NULL, 'm' },
@@ -501,6 +512,9 @@ int main(int argc, char **argv)
         case 'r':
             readonly = 1;
             break;
+        case 'L':
+            nolock = true;
+            break;
         case 'm':
             qemuio_misalign = true;
             break;
@@ -586,6 +600,10 @@ int main(int argc, char **argv)
         flags |= BDRV_O_RDWR;
     }
 
+    if (nolock) {
+        flags |= BDRV_O_NO_LOCK;
+    }
+
     if ((argc - optind) == 1) {
         if (imageOpts) {
             QemuOpts *qopts = NULL;
-- 
2.7.4

[Qemu-devel] [PATCH v7 08/20] qemu-img: Add "-L" option to sub commands

[Fam Zheng]

If specified, BDRV_O_NO_LOCK flag will be set when opening the image.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 qemu-img.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 72 insertions(+), 19 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 2e40e1f..1e2a30c 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -608,6 +608,7 @@ static int img_check(int argc, char **argv)
     ImageCheck *check;
     bool quiet = false;
     bool image_opts = false;
+    bool nolock = false;
 
     fmt = NULL;
     output = NULL;
@@ -624,7 +625,7 @@ static int img_check(int argc, char **argv)
             {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
             {0, 0, 0, 0}
         };
-        c = getopt_long(argc, argv, "hf:r:T:q",
+        c = getopt_long(argc, argv, "hf:r:T:qL",
                         long_options, &option_index);
         if (c == -1) {
             break;
@@ -658,6 +659,9 @@ static int img_check(int argc, char **argv)
         case 'q':
             quiet = true;
             break;
+        case 'L':
+            nolock = true;
+            break;
         case OPTION_OBJECT: {
             QemuOpts *opts;
             opts = qemu_opts_parse_noisily(&qemu_object_opts,
@@ -691,6 +695,7 @@ static int img_check(int argc, char **argv)
         return 1;
     }
 
+    flags |= nolock ? BDRV_O_NO_LOCK : 0;
     ret = bdrv_parse_cache_mode(cache, &flags, &writethrough);
     if (ret < 0) {
         error_report("Invalid source cache option: %s", cache);
@@ -811,6 +816,7 @@ static int img_commit(int argc, char **argv)
     Error *local_err = NULL;
     CommonBlockJobCBInfo cbi;
     bool image_opts = false;
+    bool nolock = false;
 
     fmt = NULL;
     cache = BDRV_DEFAULT_CACHE;
@@ -822,7 +828,7 @@ static int img_commit(int argc, char **argv)
             {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
             {0, 0, 0, 0}
         };
-        c = getopt_long(argc, argv, "f:ht:b:dpq",
+        c = getopt_long(argc, argv, "f:ht:b:dpqL",
                         long_options, NULL);
         if (c == -1) {
             break;
@@ -852,6 +858,9 @@ static int img_commit(int argc, char **argv)
         case 'q':
             quiet = true;
             break;
+        case 'L':
+            nolock = true;
+            break;
         case OPTION_OBJECT: {
             QemuOpts *opts;
             opts = qemu_opts_parse_noisily(&qemu_object_opts,
@@ -883,6 +892,7 @@ static int img_commit(int argc, char **argv)
     }
 
     flags = BDRV_O_RDWR | BDRV_O_UNMAP;
+    flags |= nolock ? BDRV_O_NO_LOCK : 0;
     ret = bdrv_parse_cache_mode(cache, &flags, &writethrough);
     if (ret < 0) {
         error_report("Invalid cache option: %s", cache);
@@ -1141,6 +1151,7 @@ static int img_compare(int argc, char **argv)
     int c, pnum;
     uint64_t progress_base;
     bool image_opts = false;
+    bool nolock = false;
 
     cache = BDRV_DEFAULT_CACHE;
     for (;;) {
@@ -1150,7 +1161,7 @@ static int img_compare(int argc, char **argv)
             {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
             {0, 0, 0, 0}
         };
-        c = getopt_long(argc, argv, "hf:F:T:pqs",
+        c = getopt_long(argc, argv, "hf:F:T:pqsL",
                         long_options, NULL);
         if (c == -1) {
             break;
@@ -1178,6 +1189,9 @@ static int img_compare(int argc, char **argv)
         case 's':
             strict = true;
             break;
+        case 'L':
+            nolock = true;
+            break;
         case OPTION_OBJECT: {
             QemuOpts *opts;
             opts = qemu_opts_parse_noisily(&qemu_object_opts,
@@ -1215,7 +1229,7 @@ static int img_compare(int argc, char **argv)
     /* Initialize before goto out */
     qemu_progress_init(progress, 2.0);
 
-    flags = 0;
+    flags = nolock ? BDRV_O_NO_LOCK : 0;
     ret = bdrv_parse_cache_mode(cache, &flags, &writethrough);
     if (ret < 0) {
         error_report("Invalid source cache option: %s", cache);
@@ -1764,6 +1778,7 @@ static int img_convert(int argc, char **argv)
     QemuOpts *sn_opts = NULL;
     ImgConvertState state;
     bool image_opts = false;
+    bool nolock = false;
 
     fmt = NULL;
     out_fmt = "raw";
@@ -1779,7 +1794,7 @@ static int img_convert(int argc, char **argv)
             {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
             {0, 0, 0, 0}
         };
-        c = getopt_long(argc, argv, "hf:O:B:ce6o:s:l:S:pt:T:qn",
+        c = getopt_long(argc, argv, "hf:O:B:ce6o:s:l:S:pt:T:qnL",
                         long_options, NULL);
         if (c == -1) {
             break;
@@ -1868,6 +1883,9 @@ static int img_convert(int argc, char **argv)
         case 'q':
             quiet = true;
             break;
+        case 'L':
+            nolock = true;
+            break;
         case 'n':
             skip_create = 1;
             break;
@@ -1916,7 +1934,7 @@ static int img_convert(int argc, char **argv)
         goto out;
     }
 
-    src_flags = 0;
+    src_flags = nolock ? BDRV_O_NO_LOCK : 0;
     ret = bdrv_parse_cache_mode(src_cache, &src_flags, &src_writethrough);
     if (ret < 0) {
         error_report("Invalid source cache option: %s", src_cache);
@@ -2066,6 +2084,7 @@ static int img_convert(int argc, char **argv)
     }
 
     flags = min_sparse ? (BDRV_O_RDWR | BDRV_O_UNMAP) : BDRV_O_RDWR;
+    flags |= nolock ? BDRV_O_NO_LOCK : 0;
     ret = bdrv_parse_cache_mode(cache, &flags, &writethrough);
     if (ret < 0) {
         error_report("Invalid cache option: %s", cache);
@@ -2246,12 +2265,14 @@ static gboolean str_equal_func(gconstpointer a, gconstpointer b)
 static ImageInfoList *collect_image_info_list(bool image_opts,
                                               const char *filename,
                                               const char *fmt,
-                                              bool chain)
+                                              bool chain,
+                                              bool nolock)
 {
     ImageInfoList *head = NULL;
     ImageInfoList **last = &head;
     GHashTable *filenames;
     Error *err = NULL;
+    int flags;
 
     filenames = g_hash_table_new_full(g_str_hash, str_equal_func, NULL, NULL);
 
@@ -2268,8 +2289,9 @@ static ImageInfoList *collect_image_info_list(bool image_opts,
         }
         g_hash_table_insert(filenames, (gpointer)filename, NULL);
 
-        blk = img_open(image_opts, filename, fmt,
-                       BDRV_O_NO_BACKING | BDRV_O_NO_IO, false, false);
+        flags = BDRV_O_NO_BACKING | BDRV_O_NO_IO;
+        flags |= nolock ? BDRV_O_NO_LOCK : 0;
+        blk = img_open(image_opts, filename, fmt, flags, false, false);
         if (!blk) {
             goto err;
         }
@@ -2321,6 +2343,7 @@ static int img_info(int argc, char **argv)
     const char *filename, *fmt, *output;
     ImageInfoList *list;
     bool image_opts = false;
+    bool nolock = false;
 
     fmt = NULL;
     output = NULL;
@@ -2335,7 +2358,7 @@ static int img_info(int argc, char **argv)
             {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
             {0, 0, 0, 0}
         };
-        c = getopt_long(argc, argv, "f:h",
+        c = getopt_long(argc, argv, "f:hL",
                         long_options, &option_index);
         if (c == -1) {
             break;
@@ -2348,6 +2371,9 @@ static int img_info(int argc, char **argv)
         case 'f':
             fmt = optarg;
             break;
+        case 'L':
+            nolock = true;
+            break;
         case OPTION_OUTPUT:
             output = optarg;
             break;
@@ -2387,7 +2413,7 @@ static int img_info(int argc, char **argv)
         return 1;
     }
 
-    list = collect_image_info_list(image_opts, filename, fmt, chain);
+    list = collect_image_info_list(image_opts, filename, fmt, chain, nolock);
     if (!list) {
         return 1;
     }
@@ -2533,6 +2559,8 @@ static int img_map(int argc, char **argv)
     MapEntry curr = { .length = 0 }, next;
     int ret = 0;
     bool image_opts = false;
+    bool nolock = false;
+    int flags;
 
     fmt = NULL;
     output = NULL;
@@ -2546,7 +2574,7 @@ static int img_map(int argc, char **argv)
             {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
             {0, 0, 0, 0}
         };
-        c = getopt_long(argc, argv, "f:h",
+        c = getopt_long(argc, argv, "f:hL",
                         long_options, &option_index);
         if (c == -1) {
             break;
@@ -2559,6 +2587,9 @@ static int img_map(int argc, char **argv)
         case 'f':
             fmt = optarg;
             break;
+        case 'L':
+            nolock = true;
+            break;
         case OPTION_OUTPUT:
             output = optarg;
             break;
@@ -2595,7 +2626,8 @@ static int img_map(int argc, char **argv)
         return 1;
     }
 
-    blk = img_open(image_opts, filename, fmt, 0, false, false);
+    flags = nolock ? BDRV_O_NO_LOCK : 0;
+    blk = img_open(image_opts, filename, fmt, flags, false, false);
     if (!blk) {
         return 1;
     }
@@ -2658,6 +2690,7 @@ static int img_snapshot(int argc, char **argv)
     bool quiet = false;
     Error *err = NULL;
     bool image_opts = false;
+    bool nolock = false;
 
     bdrv_oflags = BDRV_O_RDWR;
     /* Parse commandline parameters */
@@ -2668,7 +2701,7 @@ static int img_snapshot(int argc, char **argv)
             {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
             {0, 0, 0, 0}
         };
-        c = getopt_long(argc, argv, "la:c:d:hq",
+        c = getopt_long(argc, argv, "la:c:d:hqL",
                         long_options, NULL);
         if (c == -1) {
             break;
@@ -2713,6 +2746,9 @@ static int img_snapshot(int argc, char **argv)
         case 'q':
             quiet = true;
             break;
+        case 'L':
+            nolock = true;
+            break;
         case OPTION_OBJECT: {
             QemuOpts *opts;
             opts = qemu_opts_parse_noisily(&qemu_object_opts,
@@ -2738,6 +2774,7 @@ static int img_snapshot(int argc, char **argv)
         return 1;
     }
 
+    bdrv_oflags |= nolock ? BDRV_O_NO_LOCK : 0;
     /* Open the image */
     blk = img_open(image_opts, filename, NULL, bdrv_oflags, false, quiet);
     if (!blk) {
@@ -2807,6 +2844,7 @@ static int img_rebase(int argc, char **argv)
     bool quiet = false;
     Error *local_err = NULL;
     bool image_opts = false;
+    bool nolock = false;
 
     /* Parse commandline parameters */
     fmt = NULL;
@@ -2821,7 +2859,7 @@ static int img_rebase(int argc, char **argv)
             {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
             {0, 0, 0, 0}
         };
-        c = getopt_long(argc, argv, "hf:F:b:upt:T:q",
+        c = getopt_long(argc, argv, "hf:F:b:upt:T:qL",
                         long_options, NULL);
         if (c == -1) {
             break;
@@ -2852,6 +2890,9 @@ static int img_rebase(int argc, char **argv)
         case 'T':
             src_cache = optarg;
             break;
+        case 'L':
+            nolock = true;
+            break;
         case 'q':
             quiet = true;
             break;
@@ -2891,6 +2932,7 @@ static int img_rebase(int argc, char **argv)
     qemu_progress_print(0, 100);
 
     flags = BDRV_O_RDWR | (unsafe ? BDRV_O_NO_BACKING : 0);
+    flags |= nolock ? BDRV_O_NO_LOCK : 0;
     ret = bdrv_parse_cache_mode(cache, &flags, &writethrough);
     if (ret < 0) {
         error_report("Invalid cache option: %s", cache);
@@ -3152,6 +3194,8 @@ static int img_resize(int argc, char **argv)
     bool quiet = false;
     BlockBackend *blk = NULL;
     QemuOpts *param;
+    int flags;
+    bool nolock = false;
 
     static QemuOptsList resize_options = {
         .name = "resize_options",
@@ -3186,7 +3230,7 @@ static int img_resize(int argc, char **argv)
             {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
             {0, 0, 0, 0}
         };
-        c = getopt_long(argc, argv, "f:hq",
+        c = getopt_long(argc, argv, "f:hqL",
                         long_options, NULL);
         if (c == -1) {
             break;
@@ -3202,6 +3246,9 @@ static int img_resize(int argc, char **argv)
         case 'q':
             quiet = true;
             break;
+        case 'L':
+            nolock = true;
+            break;
         case OPTION_OBJECT: {
             QemuOpts *opts;
             opts = qemu_opts_parse_noisily(&qemu_object_opts,
@@ -3253,8 +3300,9 @@ static int img_resize(int argc, char **argv)
     n = qemu_opt_get_size(param, BLOCK_OPT_SIZE, 0);
     qemu_opts_del(param);
 
-    blk = img_open(image_opts, filename, fmt,
-                   BDRV_O_RDWR, false, quiet);
+    flags = BDRV_O_RDWR;
+    flags |= nolock ? BDRV_O_NO_LOCK : 0;
+    blk = img_open(image_opts, filename, fmt, flags, false, quiet);
     if (!blk) {
         ret = -1;
         goto out;
@@ -3315,6 +3363,7 @@ static int img_amend(int argc, char **argv)
     BlockBackend *blk = NULL;
     BlockDriverState *bs = NULL;
     bool image_opts = false;
+    bool nolock = false;
 
     cache = BDRV_DEFAULT_CACHE;
     for (;;) {
@@ -3324,7 +3373,7 @@ static int img_amend(int argc, char **argv)
             {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
             {0, 0, 0, 0}
         };
-        c = getopt_long(argc, argv, "ho:f:t:pq",
+        c = getopt_long(argc, argv, "ho:f:t:pqL",
                         long_options, NULL);
         if (c == -1) {
             break;
@@ -3361,6 +3410,9 @@ static int img_amend(int argc, char **argv)
             case 'q':
                 quiet = true;
                 break;
+            case 'L':
+                nolock = true;
+                break;
             case OPTION_OBJECT:
                 opts = qemu_opts_parse_noisily(&qemu_object_opts,
                                                optarg, true);
@@ -3406,6 +3458,7 @@ static int img_amend(int argc, char **argv)
     }
 
     flags = BDRV_O_RDWR;
+    flags |= nolock ? BDRV_O_NO_LOCK : 0;
     ret = bdrv_parse_cache_mode(cache, &flags, &writethrough);
     if (ret < 0) {
         error_report("Invalid cache option: %s", cache);
-- 
2.7.4

[Qemu-devel] [PATCH v7 09/20] qemu-img: Update documentation of "-L" option

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 qemu-img-cmds.hx | 44 ++++++++++++++++++++++----------------------
 qemu-img.c       |  1 +
 qemu-img.texi    |  3 +++
 3 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 7e95b2d..11bfa96 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -16,68 +16,68 @@ STEXI
 ETEXI
 
 DEF("check", img_check,
-    "check [-q] [--object objectdef] [--image-opts] [-f fmt] [--output=ofmt] [-r [leaks | all]] [-T src_cache] filename")
+    "check [-q] [-L] [--object objectdef] [--image-opts] [-f fmt] [--output=ofmt] [-r [leaks | all]] [-T src_cache] filename")
 STEXI
-@item check [--object @var{objectdef}] [--image-opts] [-q] [-f @var{fmt}] [--output=@var{ofmt}] [-r [leaks | all]] [-T @var{src_cache}] @var{filename}
+@item check [--object @var{objectdef}] [--image-opts] [-q] [-L] [-f @var{fmt}] [--output=@var{ofmt}] [-r [leaks | all]] [-T @var{src_cache}] @var{filename}
 ETEXI
 
 DEF("create", img_create,
-    "create [-q] [--object objectdef] [--image-opts] [-f fmt] [-o options] filename [size]")
+    "create [-q] [-L] [--object objectdef] [--image-opts] [-f fmt] [-o options] filename [size]")
 STEXI
-@item create [--object @var{objectdef}] [--image-opts] [-q] [-f @var{fmt}] [-o @var{options}] @var{filename} [@var{size}]
+@item create [--object @var{objectdef}] [--image-opts] [-q] [-L] [-f @var{fmt}] [-o @var{options}] @var{filename} [@var{size}]
 ETEXI
 
 DEF("commit", img_commit,
-    "commit [-q] [--object objectdef] [--image-opts] [-f fmt] [-t cache] [-b base] [-d] [-p] filename")
+    "commit [-q] [-L] [--object objectdef] [--image-opts] [-f fmt] [-t cache] [-b base] [-d] [-p] filename")
 STEXI
-@item commit [--object @var{objectdef}] [--image-opts] [-q] [-f @var{fmt}] [-t @var{cache}] [-b @var{base}] [-d] [-p] @var{filename}
+@item commit [--object @var{objectdef}] [--image-opts] [-q] [-L] [-f @var{fmt}] [-t @var{cache}] [-b @var{base}] [-d] [-p] @var{filename}
 ETEXI
 
 DEF("compare", img_compare,
-    "compare [--object objectdef] [--image-opts] [-f fmt] [-F fmt] [-T src_cache] [-p] [-q] [-s] filename1 filename2")
+    "compare [--object objectdef] [--image-opts] [-f fmt] [-F fmt] [-T src_cache] [-p] [-q] [-L] [-s] filename1 filename2")
 STEXI
-@item compare [--object @var{objectdef}] [--image-opts] [-f @var{fmt}] [-F @var{fmt}] [-T @var{src_cache}] [-p] [-q] [-s] @var{filename1} @var{filename2}
+@item compare [--object @var{objectdef}] [--image-opts] [-f @var{fmt}] [-F @var{fmt}] [-T @var{src_cache}] [-p] [-q] [-L] [-s] @var{filename1} @var{filename2}
 ETEXI
 
 DEF("convert", img_convert,
-    "convert [--object objectdef] [--image-opts] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] filename [filename2 [...]] output_filename")
+    "convert [--object objectdef] [--image-opts] [-c] [-p] [-q] [-L] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] filename [filename2 [...]] output_filename")
 STEXI
-@item convert [--object @var{objectdef}] [--image-opts] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
+@item convert [--object @var{objectdef}] [--image-opts] [-c] [-p] [-q] [-L] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
 ETEXI
 
 DEF("info", img_info,
-    "info [--object objectdef] [--image-opts] [-f fmt] [--output=ofmt] [--backing-chain] filename")
+    "info [--object objectdef] [--image-opts] [-f fmt] [-L] [--output=ofmt] [--backing-chain] filename")
 STEXI
-@item info [--object @var{objectdef}] [--image-opts] [-f @var{fmt}] [--output=@var{ofmt}] [--backing-chain] @var{filename}
+@item info [--object @var{objectdef}] [--image-opts] [-f @var{fmt}] [-L] [--output=@var{ofmt}] [--backing-chain] @var{filename}
 ETEXI
 
 DEF("map", img_map,
-    "map [--object objectdef] [--image-opts] [-f fmt] [--output=ofmt] filename")
+    "map [--object objectdef] [--image-opts] [-f fmt] [-L] [--output=ofmt] filename")
 STEXI
-@item map [--object @var{objectdef}] [--image-opts] [-f @var{fmt}] [--output=@var{ofmt}] @var{filename}
+@item map [--object @var{objectdef}] [--image-opts] [-f @var{fmt}] [-L] [--output=@var{ofmt}] @var{filename}
 ETEXI
 
 DEF("snapshot", img_snapshot,
-    "snapshot [--object objectdef] [--image-opts] [-q] [-l | -a snapshot | -c snapshot | -d snapshot] filename")
+    "snapshot [--object objectdef] [--image-opts] [-q] [-L] [-l | -a snapshot | -c snapshot | -d snapshot] filename")
 STEXI
-@item snapshot [--object @var{objectdef}] [--image-opts] [-q] [-l | -a @var{snapshot} | -c @var{snapshot} | -d @var{snapshot}] @var{filename}
+@item snapshot [--object @var{objectdef}] [--image-opts] [-q] [-L] [-l | -a @var{snapshot} | -c @var{snapshot} | -d @var{snapshot}] @var{filename}
 ETEXI
 
 DEF("rebase", img_rebase,
-    "rebase [--object objectdef] [--image-opts] [-q] [-f fmt] [-t cache] [-T src_cache] [-p] [-u] -b backing_file [-F backing_fmt] filename")
+    "rebase [--object objectdef] [--image-opts] [-q] [-L] [-f fmt] [-t cache] [-T src_cache] [-p] [-u] -b backing_file [-F backing_fmt] filename")
 STEXI
-@item rebase [--object @var{objectdef}] [--image-opts] [-q] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-p] [-u] -b @var{backing_file} [-F @var{backing_fmt}] @var{filename}
+@item rebase [--object @var{objectdef}] [--image-opts] [-q] [-L] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-p] [-u] -b @var{backing_file} [-F @var{backing_fmt}] @var{filename}
 ETEXI
 
 DEF("resize", img_resize,
-    "resize [--object objectdef] [--image-opts] [-q] filename [+ | -]size")
+    "resize [--object objectdef] [--image-opts] [-q] [-L] filename [+ | -]size")
 STEXI
-@item resize [--object @var{objectdef}] [--image-opts] [-q] @var{filename} [+ | -]@var{size}
+@item resize [--object @var{objectdef}] [--image-opts] [-q] [-L] @var{filename} [+ | -]@var{size}
 ETEXI
 
 DEF("amend", img_amend,
-    "amend [--object objectdef] [--image-opts] [-p] [-q] [-f fmt] [-t cache] -o options filename")
+    "amend [--object objectdef] [--image-opts] [-p] [-q] [-L] [-f fmt] [-t cache] -o options filename")
 STEXI
-@item amend [--object @var{objectdef}] [--image-opts] [-p] [-q] [-f @var{fmt}] [-t @var{cache}] -o @var{options} @var{filename}
+@item amend [--object @var{objectdef}] [--image-opts] [-p] [-q] [-L] [-f @var{fmt}] [-t @var{cache}] -o @var{options} @var{filename}
 @end table
 ETEXI
diff --git a/qemu-img.c b/qemu-img.c
index 1e2a30c..4176a8b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -141,6 +141,7 @@ static void QEMU_NORETURN help(void)
            "  '-h' with or without a command shows this help and lists the supported formats\n"
            "  '-p' show progress of command (only certain commands)\n"
            "  '-q' use Quiet mode - do not print any output (except errors)\n"
+           "  '-L' don't lock the image\n"
            "  '-S' indicates the consecutive number of bytes (defaults to 4k) that must\n"
            "       contain only zeros for qemu-img to create a sparse image during\n"
            "       conversion. If the number of bytes is 0, the source will not be scanned for\n"
diff --git a/qemu-img.texi b/qemu-img.texi
index 449a19c..b3ceb79 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -88,6 +88,9 @@ progress is reported when the process receives a @code{SIGUSR1} signal.
 @item -q
 Quiet mode - do not print any output (except errors). There's no progress bar
 in case both @var{-q} and @var{-p} options are used.
+@item -L
+disables image locking. The image will not be locked, other processes can
+access and lock this image while we are using it.
 @item -S @var{size}
 indicates the consecutive number of bytes that must contain only zeros
 for qemu-img to create a sparse image during conversion. This value is rounded
-- 
2.7.4

[Qemu-devel] [PATCH v7 10/20] qemu-nbd: Add "--no-lock/-L" option

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 qemu-nbd.c    | 7 ++++++-
 qemu-nbd.texi | 2 ++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index e3571c2..145156c 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -102,6 +102,7 @@ static void usage(const char *name)
 "Block device options:\n"
 "  -f, --format=FORMAT       set image format (raw, qcow2, ...)\n"
 "  -r, --read-only           export read-only\n"
+"  -L, --no-lock             disable image locking\n"
 "  -s, --snapshot            use FILE as an external snapshot, create a temporary\n"
 "                            file with backing_file=FILE, redirect the write to\n"
 "                            the temporary one\n"
@@ -474,7 +475,7 @@ int main(int argc, char **argv)
     off_t fd_size;
     QemuOpts *sn_opts = NULL;
     const char *sn_id_or_name = NULL;
-    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:x:T:";
+    const char *sopt = "hVb:o:p:rsnLP:c:dvk:e:f:tl:x:T:";
     struct option lopt[] = {
         { "help", no_argument, NULL, 'h' },
         { "version", no_argument, NULL, 'V' },
@@ -483,6 +484,7 @@ int main(int argc, char **argv)
         { "socket", required_argument, NULL, 'k' },
         { "offset", required_argument, NULL, 'o' },
         { "read-only", no_argument, NULL, 'r' },
+        { "no-lock", no_argument, NULL, 'L' },
         { "partition", required_argument, NULL, 'P' },
         { "connect", required_argument, NULL, 'c' },
         { "disconnect", no_argument, NULL, 'd' },
@@ -638,6 +640,9 @@ int main(int argc, char **argv)
             nbdflags |= NBD_FLAG_READ_ONLY;
             flags &= ~BDRV_O_RDWR;
             break;
+        case 'L':
+            flags |= BDRV_O_NO_LOCK;
+            break;
         case 'P':
             partition = strtol(optarg, &end, 0);
             if (*end) {
diff --git a/qemu-nbd.texi b/qemu-nbd.texi
index 91ebf04..5936b37 100644
--- a/qemu-nbd.texi
+++ b/qemu-nbd.texi
@@ -43,6 +43,8 @@ Force the use of the block driver for format @var{fmt} instead of
 auto-detecting
 @item -r, --read-only
 Export the disk as read-only
+@item -L, --no-lock
+Disable image locking
 @item -P, --partition=@var{num}
 Only expose partition @var{num}
 @item -s, --snapshot
-- 
2.7.4

[Qemu-devel] [PATCH v7 11/20] block: Don't lock drive-backup target image in none mode

[Fam Zheng]

As a very special case, in sync=none mode, the source is the backing image of
the target, which will be RO opened again. This won't work with image locking
because the first open could be exclusive.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 blockdev.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index a9032b9..55b0fde 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3233,6 +3233,11 @@ static void do_drive_backup(const char *job_id, const char *device,
         }
     }
     if (sync == MIRROR_SYNC_MODE_NONE) {
+        /* XXX: bs will be open second time as the backing file of target,
+         * disable image locking. Once block layer allows sharing backing BDS,
+         * change below to BDRV_O_NO_BACKING and assign it after bdrv_open().
+         **/
+        flags |= BDRV_O_NO_LOCK;
         source = bs;
     }
 
-- 
2.7.4

[Qemu-devel] [PATCH v7 12/20] qemu-iotests: 046: Move version detection out from verify_io

[Fam Zheng]

So the image lock won't complain.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/qemu-iotests/046 | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/tests/qemu-iotests/046 b/tests/qemu-iotests/046
index e528b67..f15ccbf 100755
--- a/tests/qemu-iotests/046
+++ b/tests/qemu-iotests/046
@@ -192,15 +192,7 @@ echo "== Verify image content =="
 
 function verify_io()
 {
-    if ($QEMU_IMG info -f "$IMGFMT" "$TEST_IMG" | grep "compat: 0.10" > /dev/null); then
-        # For v2 images, discarded clusters are read from the backing file
-        # Keep the variable empty so that the backing file value can be used as
-        # the default below
-        discarded=
-    else
-        # Discarded clusters are zeroed for v3 or later
-        discarded=0
-    fi
+    discarded=$1
 
     echo read -P 0 0 0x10000
 
@@ -261,7 +253,17 @@ function verify_io()
     echo read -P 17  0x11c000 0x4000
 }
 
-verify_io | $QEMU_IO "$TEST_IMG" | _filter_qemu_io
+if ($QEMU_IMG info -f "$IMGFMT" "$TEST_IMG" | grep "compat: 0.10" > /dev/null); then
+    # For v2 images, discarded clusters are read from the backing file
+    # Keep the variable empty so that the backing file value can be used as
+    # the default below
+    discarded=
+else
+    # Discarded clusters are zeroed for v3 or later
+    discarded=0
+fi
+
+verify_io $discarded | $QEMU_IO "$TEST_IMG" | _filter_qemu_io
 
 _check_test_img
 
-- 
2.7.4

[Qemu-devel] [PATCH v7 13/20] qemu-iotests: Wait for QEMU processes before checking image in 091

[Fam Zheng]

We should wait for the QEMU process to terminate and close the image
before we check the data.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/091     | 3 +++
 tests/qemu-iotests/091.out | 1 +
 2 files changed, 4 insertions(+)

diff --git a/tests/qemu-iotests/091 b/tests/qemu-iotests/091
index 32bbd56..32aa5c3 100755
--- a/tests/qemu-iotests/091
+++ b/tests/qemu-iotests/091
@@ -95,6 +95,9 @@ echo "vm2: qemu process running successfully"
 echo "vm2: flush io, and quit"
 _send_qemu_cmd $h2 'qemu-io disk flush' "(qemu)"
 _send_qemu_cmd $h2 'quit' ""
+echo "vm1: quit"
+_send_qemu_cmd $h1 'quit' ""
+wait
 
 echo "Check image pattern"
 ${QEMU_IO} -c "read -P 0x22 0 4M" "${TEST_IMG}" | _filter_testdir | _filter_qemu_io
diff --git a/tests/qemu-iotests/091.out b/tests/qemu-iotests/091.out
index 5017f8c..6658ca8 100644
--- a/tests/qemu-iotests/091.out
+++ b/tests/qemu-iotests/091.out
@@ -18,6 +18,7 @@ vm1: live migration completed
 vm2: qemu-io disk write complete
 vm2: qemu process running successfully
 vm2: flush io, and quit
+vm1: quit
 Check image pattern
 read 4194304/4194304 bytes at offset 0
 4 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-- 
2.7.4

[Qemu-devel] [PATCH v7 14/20] qemu-iotests: 030: Disable image locking when checking test image

[Fam Zheng]

The VM is running, qemu-io would fail the lock acquisition.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/030 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index 3ac2443..fa996ef 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -95,7 +95,7 @@ class TestSingleDrive(iotests.QMPTestCase):
         self.assert_no_active_block_jobs()
 
         # The image map is empty before the operation
-        empty_map = qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img)
+        empty_map = qemu_io('-L', '-f', iotests.imgfmt, '-c', 'map', test_img)
 
         # This is a no-op: no data should ever be copied from the base image
         result = self.vm.qmp('block-stream', device='drive0', base=mid_img)
-- 
2.7.4

[Qemu-devel] [PATCH v7 15/20] iotests: 087: Disable image locking in cases where file is shared

[Fam Zheng]

Otherwise the error handling we are expecting will be masked by the
preceding image locking check, and is going to be indistinguishable
because the error messages are all the same.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/087 | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/qemu-iotests/087 b/tests/qemu-iotests/087
index e7bca37..b640cf6 100755
--- a/tests/qemu-iotests/087
+++ b/tests/qemu-iotests/087
@@ -85,6 +85,7 @@ run_qemu <<EOF
         "driver": "$IMGFMT",
         "id": "disk",
         "node-name": "test-node",
+        "lock-mode": "off",
         "file": {
             "driver": "file",
             "filename": "$TEST_IMG"
@@ -97,6 +98,7 @@ run_qemu <<EOF
       "options": {
         "driver": "$IMGFMT",
         "id": "disk",
+        "lock-mode": "off",
         "file": {
             "driver": "file",
             "filename": "$TEST_IMG"
@@ -109,6 +111,7 @@ run_qemu <<EOF
       "options": {
         "driver": "$IMGFMT",
         "id": "test-node",
+        "lock-mode": "off",
         "file": {
             "driver": "file",
             "filename": "$TEST_IMG"
@@ -122,6 +125,7 @@ run_qemu <<EOF
         "driver": "$IMGFMT",
         "id": "disk2",
         "node-name": "disk",
+        "lock-mode": "off",
         "file": {
             "driver": "file",
             "filename": "$TEST_IMG"
@@ -135,6 +139,7 @@ run_qemu <<EOF
         "driver": "$IMGFMT",
         "id": "disk2",
         "node-name": "test-node",
+        "lock-mode": "off",
         "file": {
             "driver": "file",
             "filename": "$TEST_IMG"
@@ -148,6 +153,7 @@ run_qemu <<EOF
         "driver": "$IMGFMT",
         "id": "disk3",
         "node-name": "disk3",
+        "lock-mode": "off",
         "file": {
             "driver": "file",
             "filename": "$TEST_IMG"
-- 
2.7.4

[Qemu-devel] [PATCH v7 16/20] iotests: 130: Check image info locklessly

[Fam Zheng]

By the time _img_info is run, QEMU process's resources may still be on
its way being cleaned up, asynchronously, even though the process itself
is already gone after the "kill -KILL" and "wait" commands in
_cleanup_qemu.

Change the last HMP command to 'q' to ensure the locks are released.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/qemu-iotests/130     | 4 ++--
 tests/qemu-iotests/130.out | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/130 b/tests/qemu-iotests/130
index ecc8a5b..f14a04f 100755
--- a/tests/qemu-iotests/130
+++ b/tests/qemu-iotests/130
@@ -60,7 +60,7 @@ echo
 # Test that a backing file isn't written
 _launch_qemu -drive id=testdisk,file="$TEST_IMG",backing.file.filename="$TEST_IMG.base"
 _send_qemu_cmd $QEMU_HANDLE "commit testdisk" "(qemu)"
-_send_qemu_cmd $QEMU_HANDLE '' '(qemu)'
+_send_qemu_cmd $QEMU_HANDLE 'q' '(qemu)'
 _cleanup_qemu
 _img_info | _filter_img_info
 
@@ -69,7 +69,7 @@ _img_info | _filter_img_info
 _make_test_img -F raw -b "$TEST_IMG.orig" 64M
 _launch_qemu -drive id=testdisk,file="$TEST_IMG",backing.file.filename="$TEST_IMG.base",backing.driver=$IMGFMT
 _send_qemu_cmd $QEMU_HANDLE "commit testdisk" "(qemu)"
-_send_qemu_cmd $QEMU_HANDLE '' '(qemu)'
+_send_qemu_cmd $QEMU_HANDLE 'q' '(qemu)'
 _cleanup_qemu
 _img_info | _filter_img_info
 
diff --git a/tests/qemu-iotests/130.out b/tests/qemu-iotests/130.out
index ae95b50..2422329 100644
--- a/tests/qemu-iotests/130.out
+++ b/tests/qemu-iotests/130.out
@@ -10,14 +10,14 @@ virtual size: 64M (67108864 bytes)
 
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) c^[[K^[[Dco^[[K^[[D^[[Dcom^[[K^[[D^[[D^[[Dcomm^[[K^[[D^[[D^[[D^[[Dcommi^[[K^[[D^[[D^[[D^[[D^[[Dcommit^[[K^[[D^[[D^[[D^[[D^[[D^[[Dcommit ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit t^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit te^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit tes^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit test^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit testd^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit testdi^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit testdis^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit testdisk^[[K
-(qemu) 
+(qemu) q^[[K
 image: TEST_DIR/t.IMGFMT
 file format: IMGFMT
 virtual size: 64M (67108864 bytes)
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.orig backing_fmt=raw
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) c^[[K^[[Dco^[[K^[[D^[[Dcom^[[K^[[D^[[D^[[Dcomm^[[K^[[D^[[D^[[D^[[Dcommi^[[K^[[D^[[D^[[D^[[D^[[Dcommit^[[K^[[D^[[D^[[D^[[D^[[D^[[Dcommit ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit t^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit te^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit tes^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit test^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit testd^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit testdi^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit testdis^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit testdisk^[[K
-(qemu) 
+(qemu) q^[[K
 image: TEST_DIR/t.IMGFMT
 file format: IMGFMT
 virtual size: 64M (67108864 bytes)
-- 
2.7.4

[Qemu-devel] [PATCH v7 17/20] iotests: Disable image locking in 085

[Fam Zheng]

The cases is about live snapshot features. Disable image locking because
otherwise a few tests are going to fail because we reuse the same images
at blockdev-add.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/qemu-iotests/085 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/085 b/tests/qemu-iotests/085
index aa77eca..6109981 100755
--- a/tests/qemu-iotests/085
+++ b/tests/qemu-iotests/085
@@ -102,6 +102,7 @@ function add_snapshot_image()
     cmd="{ 'execute': 'blockdev-add', 'arguments':
            { 'options':
              { 'driver': 'qcow2', 'node-name': 'snap_${1}', ${extra_params}
+               'read-only': true,
                'file':
                { 'driver': 'file', 'filename': '${snapshot_file}',
                  'node-name': 'file_${1}' } } } }"
@@ -130,7 +131,7 @@ echo === Running QEMU ===
 echo
 
 qemu_comm_method="qmp"
-_launch_qemu -drive file="${TEST_IMG}.1",if=virtio -drive file="${TEST_IMG}.2",if=virtio
+_launch_qemu -drive file="${TEST_IMG}.1",if=virtio,lock-mode=off -drive file="${TEST_IMG}.2",if=virtio,lock-mode=off
 h=$QEMU_HANDLE
 
 echo
-- 
2.7.4

[Qemu-devel] [PATCH v7 18/20] tests: Use null-co:// instead of /dev/null

[Fam Zheng]

With image locking, opening /dev/null can fail when multiple tests run
in parallel (make -j2, for example). Use null-co:// as the null protocol
doesn't do image locking.

While it's arguable we could special-case /dev/null, /dev/zero,
/dev/urandom etc in raw-posix driver, it is not really necessary because
user can always specify lock-mode=off when it is appropriate. So let's
write sensible testing code too.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 tests/drive_del-test.c    | 2 +-
 tests/nvme-test.c         | 2 +-
 tests/usb-hcd-uhci-test.c | 2 +-
 tests/usb-hcd-xhci-test.c | 2 +-
 tests/virtio-blk-test.c   | 2 +-
 tests/virtio-scsi-test.c  | 4 ++--
 6 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/drive_del-test.c b/tests/drive_del-test.c
index 121b9c9..2175139 100644
--- a/tests/drive_del-test.c
+++ b/tests/drive_del-test.c
@@ -92,7 +92,7 @@ static void test_after_failed_device_add(void)
 static void test_drive_del_device_del(void)
 {
     /* Start with a drive used by a device that unplugs instantaneously */
-    qtest_start("-drive if=none,id=drive0,file=/dev/null,format=raw"
+    qtest_start("-drive if=none,id=drive0,file=null-co://,format=raw"
                 " -device virtio-scsi-pci"
                 " -device scsi-hd,drive=drive0,id=dev0");
 
diff --git a/tests/nvme-test.c b/tests/nvme-test.c
index c8bece4..7674a44 100644
--- a/tests/nvme-test.c
+++ b/tests/nvme-test.c
@@ -22,7 +22,7 @@ int main(int argc, char **argv)
     g_test_init(&argc, &argv, NULL);
     qtest_add_func("/nvme/nop", nop);
 
-    qtest_start("-drive id=drv0,if=none,file=/dev/null,format=raw "
+    qtest_start("-drive id=drv0,if=none,file=null-co://,format=raw "
                 "-device nvme,drive=drv0,serial=foo");
     ret = g_test_run();
 
diff --git a/tests/usb-hcd-uhci-test.c b/tests/usb-hcd-uhci-test.c
index 5cd59ad..3684503 100644
--- a/tests/usb-hcd-uhci-test.c
+++ b/tests/usb-hcd-uhci-test.c
@@ -85,7 +85,7 @@ int main(int argc, char **argv)
     qtest_add_func("/uhci/pci/hotplug/usb-storage", test_usb_storage_hotplug);
 
     qtest_start("-device piix3-usb-uhci,id=uhci,addr=1d.0"
-                " -drive id=drive0,if=none,file=/dev/null,format=raw"
+                " -drive id=drive0,if=none,file=null-co://,format=raw"
                 " -device usb-tablet,bus=uhci.0,port=1");
     ret = g_test_run();
     qtest_end();
diff --git a/tests/usb-hcd-xhci-test.c b/tests/usb-hcd-xhci-test.c
index 22513e9..031764d 100644
--- a/tests/usb-hcd-xhci-test.c
+++ b/tests/usb-hcd-xhci-test.c
@@ -89,7 +89,7 @@ int main(int argc, char **argv)
     qtest_add_func("/xhci/pci/hotplug/usb-uas", test_usb_uas_hotplug);
 
     qtest_start("-device nec-usb-xhci,id=xhci"
-                " -drive id=drive0,if=none,file=/dev/null,format=raw");
+                " -drive id=drive0,if=none,file=null-co://,format=raw");
     ret = g_test_run();
     qtest_end();
 
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 811cf75..5dba567 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -66,7 +66,7 @@ static QPCIBus *pci_test_start(void)
     tmp_path = drive_create();
 
     cmdline = g_strdup_printf("-drive if=none,id=drive0,file=%s,format=raw "
-                        "-drive if=none,id=drive1,file=/dev/null,format=raw "
+                        "-drive if=none,id=drive1,file=null-co://,format=raw "
                         "-device virtio-blk-pci,id=drv0,drive=drive0,"
                         "addr=%x.%x",
                         tmp_path, PCI_SLOT, PCI_FN);
diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c
index f1489e6..3c2f5df 100644
--- a/tests/virtio-scsi-test.c
+++ b/tests/virtio-scsi-test.c
@@ -40,7 +40,7 @@ static void qvirtio_scsi_start(const char *extra_opts)
     char *cmdline;
 
     cmdline = g_strdup_printf(
-                "-drive id=drv0,if=none,file=/dev/null,format=raw "
+                "-drive id=drv0,if=none,file=null-co://,format=raw "
                 "-device virtio-scsi-pci,id=vs0 "
                 "-device scsi-hd,bus=vs0.0,drive=drv0 %s",
                 extra_opts ? : "");
@@ -192,7 +192,7 @@ static void hotplug(void)
 {
     QDict *response;
 
-    qvirtio_scsi_start("-drive id=drv1,if=none,file=/dev/null,format=raw");
+    qvirtio_scsi_start("-drive id=drv1,if=none,file=null-co://,format=raw");
     response = qmp("{\"execute\": \"device_add\","
                    " \"arguments\": {"
                    "   \"driver\": \"scsi-hd\","
-- 
2.7.4

[Qemu-devel] [PATCH v7 19/20] block: Turn on image locking by default

[Fam Zheng]

Now that test cases are covered, we can turn it on.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 55a393f..ee9f3dc 100644
--- a/block.c
+++ b/block.c
@@ -992,7 +992,7 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
         goto fail_opts;
     }
 
-    lock_mode = qemu_opt_get(opts, BDRV_OPT_LOCK_MODE) ? : "off";
+    lock_mode = qemu_opt_get(opts, BDRV_OPT_LOCK_MODE) ? : "auto";
     if (!strcmp(lock_mode, "auto")) {
         /* Default */
     } else if (!strcmp(lock_mode, "shared")) {
-- 
2.7.4

[Qemu-devel] [PATCH v7 20/20] qemu-iotests: Add test case 153 for image locking

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/153     | 197 +++++++++++++++++++++
 tests/qemu-iotests/153.out | 426 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 624 insertions(+)
 create mode 100755 tests/qemu-iotests/153
 create mode 100644 tests/qemu-iotests/153.out

diff --git a/tests/qemu-iotests/153 b/tests/qemu-iotests/153
new file mode 100755
index 0000000..68770ec
--- /dev/null
+++ b/tests/qemu-iotests/153
@@ -0,0 +1,197 @@
+#!/bin/bash
+#
+# Test image locking
+#
+# Copyright (C) 2016 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=famz@redhat.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+tmp=/tmp/$$
+status=1	# failure is the default!
+
+_cleanup()
+{
+    _cleanup_test_img
+    rm -f "${TEST_IMG}.base"
+    rm -f "${TEST_IMG}.convert"
+    rm -f "${TEST_IMG}.a"
+    rm -f "${TEST_IMG}.b"
+    rm -f "${TEST_IMG}.lnk"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.qemu
+
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+size=32M
+
+_run_cmd()
+{
+    echo
+    (echo "$@"; "$@" 2>&1 1>/dev/null) | _filter_testdir
+}
+
+function _do_run_qemu()
+{
+    (
+        if ! test -t 0; then
+            while read cmd; do
+                echo $cmd
+            done
+        fi
+        echo quit
+    ) | $QEMU -nographic -monitor stdio -serial none "$@" 1>/dev/null
+}
+
+function _run_qemu_with_images()
+{
+    _do_run_qemu \
+        $(for i in $@; do echo "-drive if=none,file=$i"; done) 2>&1 \
+        | _filter_testdir | _filter_qemu
+}
+
+for opts1 in "" "readonly=on" "lock-mode=off" "lock-mode=shared"; do
+    echo
+    echo "== Creating base image =="
+    TEST_IMG="${TEST_IMG}.base" _make_test_img $size
+
+    echo
+    echo "== Creating test image =="
+    $QEMU_IMG create -f $IMGFMT "${TEST_IMG}" -b ${TEST_IMG}.base | _filter_img_create
+
+    echo
+    echo "== Launching QEMU $opts1 =="
+    _launch_qemu -drive file="${TEST_IMG}",if=none,$opts1
+    h=$QEMU_HANDLE
+
+    for opts2 in "" "lock-mode=auto" "lock-mode=shared" \
+                 "lock-mode=off" "lock-mode=auto,readonly=on"; do
+        echo
+        echo "== Launching another QEMU $opts2 =="
+        echo "quit" | \
+            $QEMU -nographic -monitor stdio \
+            -drive file="${TEST_IMG}",if=none,$opts2 2>&1 1>/dev/null | \
+            _filter_testdir | _filter_qemu
+    done
+
+    for L in "" "-L"; do
+
+        echo
+        echo "== Running utility commands $(test -n "$L" && echo "(lock disabled)") =="
+        _run_cmd $QEMU_IO $L -c "read 0 512" "${TEST_IMG}"
+        _run_cmd $QEMU_IO $L -r -c "read 0 512" "${TEST_IMG}"
+        _run_cmd $QEMU_IMG info        $L "${TEST_IMG}"
+        _run_cmd $QEMU_IMG check       $L "${TEST_IMG}"
+        _run_cmd $QEMU_IMG compare     $L "${TEST_IMG}" "${TEST_IMG}"
+        _run_cmd $QEMU_IMG map         $L "${TEST_IMG}"
+        _run_cmd $QEMU_IMG amend -o "" $L "${TEST_IMG}"
+        _run_cmd $QEMU_IMG commit      $L "${TEST_IMG}"
+        _run_cmd $QEMU_IMG resize      $L "${TEST_IMG}" $size
+        _run_cmd $QEMU_IMG rebase      $L "${TEST_IMG}" -b "${TEST_IMG}.base"
+        _run_cmd $QEMU_IMG snapshot -l $L "${TEST_IMG}"
+        _run_cmd $QEMU_IMG convert     $L "${TEST_IMG}" "${TEST_IMG}.convert"
+    done
+    _send_qemu_cmd $h "{ 'execute': 'quit', }" ""
+    echo
+    echo "Round done"
+    _cleanup_qemu
+done
+
+function _enum_opts()
+{
+    echo lock-mode={shared,auto,off},readonly={on,off}
+}
+
+for opt1 in $(_enum_opts); do
+    for opt2 in $(_enum_opts); do
+        echo
+        echo "== Two devices with the same image ($opt1 - $opt2) =="
+        _run_qemu_with_images "${TEST_IMG},$opt1" "${TEST_IMG},$opt2"
+    done
+done
+
+(
+    $QEMU_IMG create -f qcow2 "${TEST_IMG}.a" -b "${TEST_IMG}"
+    $QEMU_IMG create -f qcow2 "${TEST_IMG}.b" -b "${TEST_IMG}"
+    $QEMU_IMG create -f qcow2 "${TEST_IMG}.c" -b "${TEST_IMG}.b"
+) | _filter_img_create
+
+echo
+echo "== Two devices sharing the same file in backing chain =="
+_run_qemu_with_images "${TEST_IMG}.a" "${TEST_IMG}.b"
+_run_qemu_with_images "${TEST_IMG}.a" "${TEST_IMG}.c"
+
+echo
+echo "== Backing image also as an active device =="
+_run_qemu_with_images "${TEST_IMG}.a" "${TEST_IMG}"
+
+echo
+echo "== Backing image also as an active device (ro) =="
+_run_qemu_with_images "${TEST_IMG}.a" "${TEST_IMG},readonly=on"
+
+echo
+echo "== Backing image also as an active device (shared) =="
+_run_qemu_with_images "${TEST_IMG}.a" "${TEST_IMG},lock-mode=shared"
+
+echo
+echo "== Symbolic link =="
+rm -f "${TEST_IMG}.lnk" &>/dev/null
+ln -s ${TEST_IMG} "${TEST_IMG}.lnk" || echo "Failed to create link"
+_run_qemu_with_images "${TEST_IMG}.lnk" "${TEST_IMG}"
+
+echo
+echo "== Test that close one BDS doesn't unlock others =="
+_launch_qemu -drive file="${TEST_IMG}",if=none,id=d0
+
+_send_qemu_cmd $QEMU_HANDLE \
+    "{ 'execute': 'qmp_capabilities' }" \
+    'return'
+
+_send_qemu_cmd $QEMU_HANDLE \
+    "{ 'execute': 'human-monitor-command',
+       'arguments': { 'command-line': 'drive_add 0 if=none,id=d1,file=${TEST_IMG}' } }" \
+    "Failed to lock image"
+
+_send_qemu_cmd $QEMU_HANDLE \
+    "{ 'execute': 'human-monitor-command',
+       'arguments': { 'command-line': 'drive_add 0 if=none,id=d1,file=${TEST_IMG},lock-mode=off' } }" \
+    "OK"
+
+_send_qemu_cmd $QEMU_HANDLE \
+    "{ 'execute': 'human-monitor-command',
+       'arguments': { 'command-line': 'drive_del d1' } }" \
+    ""
+
+_run_cmd $QEMU_IMG info "${TEST_IMG}"
+
+_cleanup_qemu
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/153.out b/tests/qemu-iotests/153.out
new file mode 100644
index 0000000..377d9e2
--- /dev/null
+++ b/tests/qemu-iotests/153.out
@@ -0,0 +1,426 @@
+QA output created by 153
+
+== Creating base image ==
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=33554432
+
+== Creating test image ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33554432 backing_file=TEST_DIR/t.IMGFMT.base
+
+== Launching QEMU  ==
+
+== Launching another QEMU  ==
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=none,: Failed to lock image
+
+== Launching another QEMU lock-mode=auto ==
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=none,lock-mode=auto: Failed to lock image
+
+== Launching another QEMU lock-mode=shared ==
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=none,lock-mode=shared: Failed to lock image
+
+== Launching another QEMU lock-mode=off ==
+
+== Launching another QEMU lock-mode=auto,readonly=on ==
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=none,lock-mode=auto,readonly=on: Failed to lock image
+
+== Running utility commands  ==
+
+_qemu_io_wrapper -c read 0 512 TEST_DIR/t.qcow2
+can't open device TEST_DIR/t.qcow2: Failed to lock image
+no file open, try 'help open'
+
+_qemu_io_wrapper -r -c read 0 512 TEST_DIR/t.qcow2
+can't open device TEST_DIR/t.qcow2: Failed to lock image
+no file open, try 'help open'
+
+_qemu_img_wrapper info TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image
+
+_qemu_img_wrapper check TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image
+
+_qemu_img_wrapper compare TEST_DIR/t.qcow2 TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image
+
+_qemu_img_wrapper map TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image
+
+_qemu_img_wrapper amend -o  TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image
+
+_qemu_img_wrapper commit TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image
+
+_qemu_img_wrapper resize TEST_DIR/t.qcow2 32M
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image
+
+_qemu_img_wrapper rebase TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image
+
+_qemu_img_wrapper snapshot -l TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image
+
+_qemu_img_wrapper convert TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.convert
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image
+
+== Running utility commands (lock disabled) ==
+
+_qemu_io_wrapper -L -c read 0 512 TEST_DIR/t.qcow2
+
+_qemu_io_wrapper -L -r -c read 0 512 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper info -L TEST_DIR/t.qcow2
+
+_qemu_img_wrapper check -L TEST_DIR/t.qcow2
+
+_qemu_img_wrapper compare -L TEST_DIR/t.qcow2 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper map -L TEST_DIR/t.qcow2
+
+_qemu_img_wrapper amend -o  -L TEST_DIR/t.qcow2
+
+_qemu_img_wrapper commit -L TEST_DIR/t.qcow2
+
+_qemu_img_wrapper resize -L TEST_DIR/t.qcow2 32M
+
+_qemu_img_wrapper rebase -L TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base
+
+_qemu_img_wrapper snapshot -l -L TEST_DIR/t.qcow2
+
+_qemu_img_wrapper convert -L TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.convert
+
+Round done
+
+== Creating base image ==
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=33554432
+
+== Creating test image ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33554432 backing_file=TEST_DIR/t.IMGFMT.base
+
+== Launching QEMU readonly=on ==
+
+== Launching another QEMU  ==
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=none,: Failed to lock image
+
+== Launching another QEMU lock-mode=auto ==
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=none,lock-mode=auto: Failed to lock image
+
+== Launching another QEMU lock-mode=shared ==
+
+== Launching another QEMU lock-mode=off ==
+
+== Launching another QEMU lock-mode=auto,readonly=on ==
+
+== Running utility commands  ==
+
+_qemu_io_wrapper -c read 0 512 TEST_DIR/t.qcow2
+can't open device TEST_DIR/t.qcow2: Failed to lock image
+no file open, try 'help open'
+
+_qemu_io_wrapper -r -c read 0 512 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper info TEST_DIR/t.qcow2
+
+_qemu_img_wrapper check TEST_DIR/t.qcow2
+
+_qemu_img_wrapper compare TEST_DIR/t.qcow2 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper map TEST_DIR/t.qcow2
+
+_qemu_img_wrapper amend -o  TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image
+
+_qemu_img_wrapper commit TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image
+
+_qemu_img_wrapper resize TEST_DIR/t.qcow2 32M
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image
+
+_qemu_img_wrapper rebase TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image
+
+_qemu_img_wrapper snapshot -l TEST_DIR/t.qcow2
+
+_qemu_img_wrapper convert TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.convert
+
+== Running utility commands (lock disabled) ==
+
+_qemu_io_wrapper -L -c read 0 512 TEST_DIR/t.qcow2
+
+_qemu_io_wrapper -L -r -c read 0 512 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper info -L TEST_DIR/t.qcow2
+
+_qemu_img_wrapper check -L TEST_DIR/t.qcow2
+
+_qemu_img_wrapper compare -L TEST_DIR/t.qcow2 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper map -L TEST_DIR/t.qcow2
+
+_qemu_img_wrapper amend -o  -L TEST_DIR/t.qcow2
+
+_qemu_img_wrapper commit -L TEST_DIR/t.qcow2
+
+_qemu_img_wrapper resize -L TEST_DIR/t.qcow2 32M
+
+_qemu_img_wrapper rebase -L TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base
+
+_qemu_img_wrapper snapshot -l -L TEST_DIR/t.qcow2
+
+_qemu_img_wrapper convert -L TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.convert
+
+Round done
+
+== Creating base image ==
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=33554432
+
+== Creating test image ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33554432 backing_file=TEST_DIR/t.IMGFMT.base
+
+== Launching QEMU lock-mode=off ==
+
+== Launching another QEMU  ==
+
+== Launching another QEMU lock-mode=auto ==
+
+== Launching another QEMU lock-mode=shared ==
+
+== Launching another QEMU lock-mode=off ==
+
+== Launching another QEMU lock-mode=auto,readonly=on ==
+
+== Running utility commands  ==
+
+_qemu_io_wrapper -c read 0 512 TEST_DIR/t.qcow2
+
+_qemu_io_wrapper -r -c read 0 512 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper info TEST_DIR/t.qcow2
+
+_qemu_img_wrapper check TEST_DIR/t.qcow2
+
+_qemu_img_wrapper compare TEST_DIR/t.qcow2 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper map TEST_DIR/t.qcow2
+
+_qemu_img_wrapper amend -o  TEST_DIR/t.qcow2
+
+_qemu_img_wrapper commit TEST_DIR/t.qcow2
+
+_qemu_img_wrapper resize TEST_DIR/t.qcow2 32M
+
+_qemu_img_wrapper rebase TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base
+
+_qemu_img_wrapper snapshot -l TEST_DIR/t.qcow2
+
+_qemu_img_wrapper convert TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.convert
+
+== Running utility commands (lock disabled) ==
+
+_qemu_io_wrapper -L -c read 0 512 TEST_DIR/t.qcow2
+
+_qemu_io_wrapper -L -r -c read 0 512 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper info -L TEST_DIR/t.qcow2
+
+_qemu_img_wrapper check -L TEST_DIR/t.qcow2
+
+_qemu_img_wrapper compare -L TEST_DIR/t.qcow2 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper map -L TEST_DIR/t.qcow2
+
+_qemu_img_wrapper amend -o  -L TEST_DIR/t.qcow2
+
+_qemu_img_wrapper commit -L TEST_DIR/t.qcow2
+
+_qemu_img_wrapper resize -L TEST_DIR/t.qcow2 32M
+
+_qemu_img_wrapper rebase -L TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base
+
+_qemu_img_wrapper snapshot -l -L TEST_DIR/t.qcow2
+
+_qemu_img_wrapper convert -L TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.convert
+
+Round done
+
+== Creating base image ==
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=33554432
+
+== Creating test image ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33554432 backing_file=TEST_DIR/t.IMGFMT.base
+
+== Launching QEMU lock-mode=shared ==
+
+== Launching another QEMU  ==
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=none,: Failed to lock image
+
+== Launching another QEMU lock-mode=auto ==
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=none,lock-mode=auto: Failed to lock image
+
+== Launching another QEMU lock-mode=shared ==
+
+== Launching another QEMU lock-mode=off ==
+
+== Launching another QEMU lock-mode=auto,readonly=on ==
+
+== Running utility commands  ==
+
+_qemu_io_wrapper -c read 0 512 TEST_DIR/t.qcow2
+can't open device TEST_DIR/t.qcow2: Failed to lock image
+no file open, try 'help open'
+
+_qemu_io_wrapper -r -c read 0 512 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper info TEST_DIR/t.qcow2
+
+_qemu_img_wrapper check TEST_DIR/t.qcow2
+
+_qemu_img_wrapper compare TEST_DIR/t.qcow2 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper map TEST_DIR/t.qcow2
+
+_qemu_img_wrapper amend -o  TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image
+
+_qemu_img_wrapper commit TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image
+
+_qemu_img_wrapper resize TEST_DIR/t.qcow2 32M
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image
+
+_qemu_img_wrapper rebase TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image
+
+_qemu_img_wrapper snapshot -l TEST_DIR/t.qcow2
+
+_qemu_img_wrapper convert TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.convert
+
+== Running utility commands (lock disabled) ==
+
+_qemu_io_wrapper -L -c read 0 512 TEST_DIR/t.qcow2
+
+_qemu_io_wrapper -L -r -c read 0 512 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper info -L TEST_DIR/t.qcow2
+
+_qemu_img_wrapper check -L TEST_DIR/t.qcow2
+
+_qemu_img_wrapper compare -L TEST_DIR/t.qcow2 TEST_DIR/t.qcow2
+
+_qemu_img_wrapper map -L TEST_DIR/t.qcow2
+
+_qemu_img_wrapper amend -o  -L TEST_DIR/t.qcow2
+
+_qemu_img_wrapper commit -L TEST_DIR/t.qcow2
+
+_qemu_img_wrapper resize -L TEST_DIR/t.qcow2 32M
+
+_qemu_img_wrapper rebase -L TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base
+
+_qemu_img_wrapper snapshot -l -L TEST_DIR/t.qcow2
+
+_qemu_img_wrapper convert -L TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.convert
+
+Round done
+
+== Two devices with the same image (lock-mode=shared,readonly=on - lock-mode=shared,readonly=on) ==
+
+== Two devices with the same image (lock-mode=shared,readonly=on - lock-mode=shared,readonly=off) ==
+
+== Two devices with the same image (lock-mode=shared,readonly=on - lock-mode=auto,readonly=on) ==
+
+== Two devices with the same image (lock-mode=shared,readonly=on - lock-mode=auto,readonly=off) ==
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,lock-mode=auto,readonly=off: Failed to lock image
+
+== Two devices with the same image (lock-mode=shared,readonly=on - lock-mode=off,readonly=on) ==
+
+== Two devices with the same image (lock-mode=shared,readonly=on - lock-mode=off,readonly=off) ==
+
+== Two devices with the same image (lock-mode=shared,readonly=off - lock-mode=shared,readonly=on) ==
+
+== Two devices with the same image (lock-mode=shared,readonly=off - lock-mode=shared,readonly=off) ==
+
+== Two devices with the same image (lock-mode=shared,readonly=off - lock-mode=auto,readonly=on) ==
+
+== Two devices with the same image (lock-mode=shared,readonly=off - lock-mode=auto,readonly=off) ==
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,lock-mode=auto,readonly=off: Failed to lock image
+
+== Two devices with the same image (lock-mode=shared,readonly=off - lock-mode=off,readonly=on) ==
+
+== Two devices with the same image (lock-mode=shared,readonly=off - lock-mode=off,readonly=off) ==
+
+== Two devices with the same image (lock-mode=auto,readonly=on - lock-mode=shared,readonly=on) ==
+
+== Two devices with the same image (lock-mode=auto,readonly=on - lock-mode=shared,readonly=off) ==
+
+== Two devices with the same image (lock-mode=auto,readonly=on - lock-mode=auto,readonly=on) ==
+
+== Two devices with the same image (lock-mode=auto,readonly=on - lock-mode=auto,readonly=off) ==
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,lock-mode=auto,readonly=off: Failed to lock image
+
+== Two devices with the same image (lock-mode=auto,readonly=on - lock-mode=off,readonly=on) ==
+
+== Two devices with the same image (lock-mode=auto,readonly=on - lock-mode=off,readonly=off) ==
+
+== Two devices with the same image (lock-mode=auto,readonly=off - lock-mode=shared,readonly=on) ==
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,lock-mode=shared,readonly=on: Failed to lock image
+
+== Two devices with the same image (lock-mode=auto,readonly=off - lock-mode=shared,readonly=off) ==
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,lock-mode=shared,readonly=off: Failed to lock image
+
+== Two devices with the same image (lock-mode=auto,readonly=off - lock-mode=auto,readonly=on) ==
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,lock-mode=auto,readonly=on: Failed to lock image
+
+== Two devices with the same image (lock-mode=auto,readonly=off - lock-mode=auto,readonly=off) ==
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,lock-mode=auto,readonly=off: Failed to lock image
+
+== Two devices with the same image (lock-mode=auto,readonly=off - lock-mode=off,readonly=on) ==
+
+== Two devices with the same image (lock-mode=auto,readonly=off - lock-mode=off,readonly=off) ==
+
+== Two devices with the same image (lock-mode=off,readonly=on - lock-mode=shared,readonly=on) ==
+
+== Two devices with the same image (lock-mode=off,readonly=on - lock-mode=shared,readonly=off) ==
+
+== Two devices with the same image (lock-mode=off,readonly=on - lock-mode=auto,readonly=on) ==
+
+== Two devices with the same image (lock-mode=off,readonly=on - lock-mode=auto,readonly=off) ==
+
+== Two devices with the same image (lock-mode=off,readonly=on - lock-mode=off,readonly=on) ==
+
+== Two devices with the same image (lock-mode=off,readonly=on - lock-mode=off,readonly=off) ==
+
+== Two devices with the same image (lock-mode=off,readonly=off - lock-mode=shared,readonly=on) ==
+
+== Two devices with the same image (lock-mode=off,readonly=off - lock-mode=shared,readonly=off) ==
+
+== Two devices with the same image (lock-mode=off,readonly=off - lock-mode=auto,readonly=on) ==
+
+== Two devices with the same image (lock-mode=off,readonly=off - lock-mode=auto,readonly=off) ==
+
+== Two devices with the same image (lock-mode=off,readonly=off - lock-mode=off,readonly=on) ==
+
+== Two devices with the same image (lock-mode=off,readonly=off - lock-mode=off,readonly=off) ==
+Formatting 'TEST_DIR/t.IMGFMT.a', fmt=IMGFMT size=33554432 backing_file=TEST_DIR/t.IMGFMT
+Formatting 'TEST_DIR/t.IMGFMT.b', fmt=IMGFMT size=33554432 backing_file=TEST_DIR/t.IMGFMT
+Formatting 'TEST_DIR/t.IMGFMT.c', fmt=IMGFMT size=33554432 backing_file=TEST_DIR/t.IMGFMT.b
+
+== Two devices sharing the same file in backing chain ==
+
+== Backing image also as an active device ==
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2: Failed to lock image
+
+== Backing image also as an active device (ro) ==
+
+== Backing image also as an active device (shared) ==
+
+== Symbolic link ==
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2: Failed to lock image
+
+== Test that close one BDS doesn't unlock others ==
+{"return": {}}
+{"return": "Failed to lock imagern"}
+{"return": "OKrn"}
+
+_qemu_img_wrapper info TEST_DIR/t.qcow2
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to lock image
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 3a3973e..81ccc07 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -153,6 +153,7 @@
 149 rw auto sudo
 150 rw auto quick
 152 rw auto quick
+153 rw auto quick
 154 rw auto backing quick
 155 rw auto
 156 rw auto quick
-- 
2.7.4

Re: [Qemu-devel] [PATCH v7 00/20] block: Image locking series for 2.8

[no-reply]

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Message-id: 1470662013-19785-1-git-send-email-famz@redhat.com
Type: series
Subject: [Qemu-devel] [PATCH v7 00/20] block: Image locking series for 2.8

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git show --no-patch --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/1470662013-19785-1-git-send-email-famz@redhat.com -> patchew/1470662013-19785-1-git-send-email-famz@redhat.com
Switched to a new branch 'test'
0a93007 qemu-iotests: Add test case 153 for image locking
3fe871c block: Turn on image locking by default
95e318e tests: Use null-co:// instead of /dev/null
ffd5c0a iotests: Disable image locking in 085
ff4e648 iotests: 130: Check image info locklessly
23213a9 iotests: 087: Disable image locking in cases where file is shared
a98e4f0 qemu-iotests: 030: Disable image locking when checking test image
1b052c5 qemu-iotests: Wait for QEMU processes before checking image in 091
74b6e02 qemu-iotests: 046: Move version detection out from verify_io
75b6dac block: Don't lock drive-backup target image in none mode
cc08d0b qemu-nbd: Add "--no-lock/-L" option
d996114 qemu-img: Update documentation of "-L" option
5cd0c72 qemu-img: Add "-L" option to sub commands
dc1e924 qemu-io: Add "-L" option for BDRV_O_NO_LOCK
6104ab5 raw-posix: Add image locking support
cfd1487 osdep: Add qemu_lock_fd and qemu_unlock_fd
f455bc2 block: Introduce image file locking
b2af979 block: Add and parse "lock-mode" option for image locking
483596d qapi: Add lock-mode in blockdev-add options
88916dc block: Add flag bits for image locking

=== OUTPUT BEGIN ===
Checking PATCH 1/20: block: Add flag bits for image locking...
Checking PATCH 2/20: qapi: Add lock-mode in blockdev-add options...
Checking PATCH 3/20: block: Add and parse "lock-mode" option for image locking...
ERROR: do not use assignment in if condition
#80: FILE: blockdev.c:548:
+    if ((buf = qemu_opt_get(opts, BDRV_OPT_LOCK_MODE)) != NULL) {

total: 1 errors, 0 warnings, 86 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 4/20: block: Introduce image file locking...
Checking PATCH 5/20: osdep: Add qemu_lock_fd and qemu_unlock_fd...
Checking PATCH 6/20: raw-posix: Add image locking support...
Checking PATCH 7/20: qemu-io: Add "-L" option for BDRV_O_NO_LOCK...
Checking PATCH 8/20: qemu-img: Add "-L" option to sub commands...
Checking PATCH 9/20: qemu-img: Update documentation of "-L" option...
Checking PATCH 10/20: qemu-nbd: Add "--no-lock/-L" option...
Checking PATCH 11/20: block: Don't lock drive-backup target image in none mode...
Checking PATCH 12/20: qemu-iotests: 046: Move version detection out from verify_io...
Checking PATCH 13/20: qemu-iotests: Wait for QEMU processes before checking image in 091...
Checking PATCH 14/20: qemu-iotests: 030: Disable image locking when checking test image...
Checking PATCH 15/20: iotests: 087: Disable image locking in cases where file is shared...
Checking PATCH 16/20: iotests: 130: Check image info locklessly...
ERROR: Invalid UTF-8, patch and commit message should be encoded in UTF-8
#47: FILE: tests/qemu-iotests/130.out:13:
+(qemu) q^[[K
         ^

ERROR: Invalid UTF-8, patch and commit message should be encoded in UTF-8
#55: FILE: tests/qemu-iotests/130.out:20:
+(qemu) q^[[K
         ^

total: 2 errors, 0 warnings, 32 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 17/20: iotests: Disable image locking in 085...
Checking PATCH 18/20: tests: Use null-co:// instead of /dev/null...
Checking PATCH 19/20: block: Turn on image locking by default...
Checking PATCH 20/20: qemu-iotests: Add test case 153 for image locking...
=== OUTPUT END ===

Test command exited with code: 1

Re: [Qemu-devel] [PATCH v7 00/20] block: Image locking series for 2.8

[Fam Zheng]

On Mon, 08/08 06:59, no-reply@ec2-52-6-146-230.compute-1.amazonaws.com wrote:
> Checking PATCH 3/20: block: Add and parse "lock-mode" option for image locking...
> ERROR: do not use assignment in if condition
> #80: FILE: blockdev.c:548:
> +    if ((buf = qemu_opt_get(opts, BDRV_OPT_LOCK_MODE)) != NULL) {
> 
> total: 1 errors, 0 warnings, 86 lines checked

I intentionally ignored this to be consistent with the several other occasions
in this function.

> 
> Your patch has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> Checking PATCH 4/20: block: Introduce image file locking...
> Checking PATCH 5/20: osdep: Add qemu_lock_fd and qemu_unlock_fd...
> Checking PATCH 6/20: raw-posix: Add image locking support...
> Checking PATCH 7/20: qemu-io: Add "-L" option for BDRV_O_NO_LOCK...
> Checking PATCH 8/20: qemu-img: Add "-L" option to sub commands...
> Checking PATCH 9/20: qemu-img: Update documentation of "-L" option...
> Checking PATCH 10/20: qemu-nbd: Add "--no-lock/-L" option...
> Checking PATCH 11/20: block: Don't lock drive-backup target image in none mode...
> Checking PATCH 12/20: qemu-iotests: 046: Move version detection out from verify_io...
> Checking PATCH 13/20: qemu-iotests: Wait for QEMU processes before checking image in 091...
> Checking PATCH 14/20: qemu-iotests: 030: Disable image locking when checking test image...
> Checking PATCH 15/20: iotests: 087: Disable image locking in cases where file is shared...
> Checking PATCH 16/20: iotests: 130: Check image info locklessly...
> ERROR: Invalid UTF-8, patch and commit message should be encoded in UTF-8
> #47: FILE: tests/qemu-iotests/130.out:13:
> +(qemu) q^[[K
>          ^
> 
> ERROR: Invalid UTF-8, patch and commit message should be encoded in UTF-8
> #55: FILE: tests/qemu-iotests/130.out:20:
> +(qemu) q^[[K
>          ^

And this one seems harmless.

Fam

Re: [Qemu-devel] [PATCH v7 00/20] block: Image locking series for 2.8

[Fam Zheng]

On Mon, 08/08 21:13, Fam Zheng wrote:
> v7: - Rebase.
>     - Address comments from Kevin and Max.
>     - Option rename: "exclusive" -> "auto". [Kevin]
>     - Option placement: "root BDS" -> "node".
>       It's still a bit controversy where the option should go, but so far it
>       seems per node is the safest.
>     - Assert in bdrv_open_common that lock flags don't conflict. [Kevin]
>     - Skip qemu_dup patches as they are already merged.
>     - Fix a wrong assertion in raw_reopen_to_unlock.
>     - Put "static int" to the same line in raw-posix.c. [Kevin]
>     - Remove superfluous blank lines.
>     - Add comment in raw posix's sophisticated reopen code. [Kevin]
>     - .bdrv_lockf is kept on the contrary to separate .bdrv_lock and
>       .bdrv_unlock, because having the BdrvLockfCmd in the interface makes
>       implementing the reopen logic as is done in this series much easier.
>     - Drop the unnecessary mirror target locking patch. [Max]
>     - Add a patch to let 130 work with image locking.

Ping?

Re: [Qemu-devel] [PATCH v7 02/20] qapi: Add lock-mode in blockdev-add options

[Kevin Wolf]

Am 08.08.2016 um 15:13 hat Fam Zheng geschrieben:
> To allow overriding the default locking behavior when opening the image.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  qapi/block-core.json | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 5e2d7d7..d1eb197 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2151,6 +2151,20 @@
>              '*debug-level': 'int' } }
>  
>  ##
> +# @BlockdevLockMode
> +#
> +# Describes how QEMU should lock the image.
> +#
> +# @off:       Disabled
> +# @shared:    Use shared lock for both RO and RW images.
> +# @auto:      Use exclusive lock for RW images, and shared lock for RO images.
> +#
> +# Since: 2.7
> +##
> +{ 'enum': 'BlockdevLockMode',
> +  'data': [ 'off', 'shared', 'auto' ] }

You decided to drop 'exclusive'? Didn't we agree last time that we
should have both a real 'exclusive' and 'auto'?

Kevin

Re: [Qemu-devel] [PATCH v7 03/20] block: Add and parse "lock-mode" option for image locking

[Kevin Wolf]

Am 08.08.2016 um 15:13 hat Fam Zheng geschrieben:
> Respect the locking mode from CLI or QMP, and set the open flags
> accordingly.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block.c               | 21 +++++++++++++++++++++
>  blockdev.c            |  9 +++++++++
>  include/block/block.h |  1 +
>  qemu-options.hx       |  1 +
>  4 files changed, 32 insertions(+)
> 
> diff --git a/block.c b/block.c
> index 30d64e6..5f76f60 100644
> --- a/block.c
> +++ b/block.c
> @@ -705,6 +705,7 @@ static void bdrv_inherited_options(int *child_flags, QDict *child_options,
>       * the parent. */
>      qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_DIRECT);
>      qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_NO_FLUSH);
> +    qdict_copy_default(child_options, parent_options, BDRV_OPT_LOCK_MODE);
>  
>      /* Our block drivers take care to send flushes and respect unmap policy,
>       * so we can default to enable both on lower layers regardless of the
> @@ -757,6 +758,7 @@ static void bdrv_backing_options(int *child_flags, QDict *child_options,
>       * which is only applied on the top level (BlockBackend) */
>      qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_DIRECT);
>      qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_NO_FLUSH);
> +    qdict_copy_default(child_options, parent_options, BDRV_OPT_LOCK_MODE);
>  
>      /* backing files always opened read-only */
>      flags &= ~(BDRV_O_RDWR | BDRV_O_COPY_ON_READ);
> @@ -880,6 +882,11 @@ static QemuOptsList bdrv_runtime_opts = {
>              .name = BDRV_OPT_CACHE_NO_FLUSH,
>              .type = QEMU_OPT_BOOL,
>              .help = "Ignore flush requests",
> +        },{
> +            .name = BDRV_OPT_LOCK_MODE,
> +            .type = QEMU_OPT_STRING,
> +            .help = "how to lock the image (auto, shared, off. "
> +                    "default: auto)",
>          },
>          { /* end of list */ }
>      },
> @@ -897,6 +904,7 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
>      const char *filename;
>      const char *driver_name = NULL;
>      const char *node_name = NULL;
> +    const char *lock_mode = NULL;
>      QemuOpts *opts;
>      BlockDriver *drv;
>      Error *local_err = NULL;
> @@ -940,6 +948,19 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
>          goto fail_opts;
>      }
>  
> +    lock_mode = qemu_opt_get(opts, BDRV_OPT_LOCK_MODE) ? : "off";

Am I missing some other place that overrides this or does it make the
default "off" rather than "auto"?

> +    if (!strcmp(lock_mode, "auto")) {
> +        /* Default */
> +    } else if (!strcmp(lock_mode, "shared")) {
> +        bs->open_flags |= BDRV_O_SHARED_LOCK;
> +    } else if (!strcmp(lock_mode, "off")) {
> +        bs->open_flags |= BDRV_O_NO_LOCK;
> +    } else {
> +        error_setg(errp, "invalid lock mode");
> +        ret = -EINVAL;
> +        goto fail_opts;
> +    }
> +
>      bs->read_only = !(bs->open_flags & BDRV_O_RDWR);

The other thing is that we didn't really finish the discussion whether
leaving the configuration on the node level to the user is a good idea
when the restrictions that the user can influence rather come from the
top level (the guest OS) and only propagate down the BDS tree.

Aren't we requiring that the user understands the internal workings of
qemu in order to infer which nodes need to be locked in which mode,
rather than just saying "my guest is okay with sharing this disk, you
can figure out what this means for locking"?

Kevin

Re: [Qemu-devel] [PATCH v7 03/20] block: Add and parse "lock-mode" option for image locking

[Fam Zheng]

On Tue, 09/06 18:33, Kevin Wolf wrote:
> > +    lock_mode = qemu_opt_get(opts, BDRV_OPT_LOCK_MODE) ? : "off";
> 
> Am I missing some other place that overrides this or does it make the
> default "off" rather than "auto"?

It is in patch 19, once all tests are fixed (this patch cannot be reordered
after tests fixing because it involves specifying the lock-mode option.)

Re: [Qemu-devel] [PATCH v7 02/20] qapi: Add lock-mode in blockdev-add options

[Fam Zheng]

On Tue, 09/06 18:18, Kevin Wolf wrote:
> Am 08.08.2016 um 15:13 hat Fam Zheng geschrieben:
> > To allow overriding the default locking behavior when opening the image.
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >  qapi/block-core.json | 19 ++++++++++++++++++-
> >  1 file changed, 18 insertions(+), 1 deletion(-)
> > 
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index 5e2d7d7..d1eb197 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -2151,6 +2151,20 @@
> >              '*debug-level': 'int' } }
> >  
> >  ##
> > +# @BlockdevLockMode
> > +#
> > +# Describes how QEMU should lock the image.
> > +#
> > +# @off:       Disabled
> > +# @shared:    Use shared lock for both RO and RW images.
> > +# @auto:      Use exclusive lock for RW images, and shared lock for RO images.
> > +#
> > +# Since: 2.7
> > +##
> > +{ 'enum': 'BlockdevLockMode',
> > +  'data': [ 'off', 'shared', 'auto' ] }
> 
> You decided to drop 'exclusive'? Didn't we agree last time that we
> should have both a real 'exclusive' and 'auto'?

I must have misunderstood it. I can add exclusive back.

Fam

Re: [Qemu-devel] [PATCH v7 02/20] qapi: Add lock-mode in blockdev-add options

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1506 bytes --]

On 08/08/2016 08:13 AM, Fam Zheng wrote:
> To allow overriding the default locking behavior when opening the image.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  qapi/block-core.json | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 5e2d7d7..d1eb197 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2151,6 +2151,20 @@
>              '*debug-level': 'int' } }
>  
>  ##
> +# @BlockdevLockMode
> +#
> +# Describes how QEMU should lock the image.
> +#
> +# @off:       Disabled
> +# @shared:    Use shared lock for both RO and RW images.
> +# @auto:      Use exclusive lock for RW images, and shared lock for RO images.
> +#
> +# Since: 2.7

Just a reminder to update this to 2.8 (probably throughout the series).

> @@ -2185,7 +2201,8 @@
>              '*cache': 'BlockdevCacheOptions',
>              '*aio': 'BlockdevAioOptions',
>              '*read-only': 'bool',
> -            '*detect-zeroes': 'BlockdevDetectZeroesOptions' },
> +            '*detect-zeroes': 'BlockdevDetectZeroesOptions',
> +            '*lock-mode': 'BlockdevLockMode' },
>    'discriminator': 'driver',
>    'data': {
>        'archipelago':'BlockdevOptionsArchipelago',
> 

Will this need (yet another) rebase on top of Kevin's blockdev-add work?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v7 02/20] qapi: Add lock-mode in blockdev-add options

[Fam Zheng]

On Thu, 09/22 09:58, Eric Blake wrote:
> On 08/08/2016 08:13 AM, Fam Zheng wrote:
> > To allow overriding the default locking behavior when opening the image.
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >  qapi/block-core.json | 19 ++++++++++++++++++-
> >  1 file changed, 18 insertions(+), 1 deletion(-)
> > 
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index 5e2d7d7..d1eb197 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -2151,6 +2151,20 @@
> >              '*debug-level': 'int' } }
> >  
> >  ##
> > +# @BlockdevLockMode
> > +#
> > +# Describes how QEMU should lock the image.
> > +#
> > +# @off:       Disabled
> > +# @shared:    Use shared lock for both RO and RW images.
> > +# @auto:      Use exclusive lock for RW images, and shared lock for RO images.
> > +#
> > +# Since: 2.7
> 
> Just a reminder to update this to 2.8 (probably throughout the series).

Good point, thanks!

> 
> > @@ -2185,7 +2201,8 @@
> >              '*cache': 'BlockdevCacheOptions',
> >              '*aio': 'BlockdevAioOptions',
> >              '*read-only': 'bool',
> > -            '*detect-zeroes': 'BlockdevDetectZeroesOptions' },
> > +            '*detect-zeroes': 'BlockdevDetectZeroesOptions',
> > +            '*lock-mode': 'BlockdevLockMode' },
> >    'discriminator': 'driver',
> >    'data': {
> >        'archipelago':'BlockdevOptionsArchipelago',
> > 
> 
> Will this need (yet another) rebase on top of Kevin's blockdev-add work?

Yes, I think so..

Fam