All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 00/37] Block layer patches
@ 2015-11-05 18:17 Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 01/37] block: Don't call blk_bs() twice in bdrv_lookup_bs() Kevin Wolf
                   ` (37 more replies)
  0 siblings, 38 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

The following changes since commit 8835b9df3bddf332c883c861d6a1defc12c4ebe9:

  Merge remote-tracking branch 'remotes/mdroth/tags/qga-pull-2015-11-04-tag' into staging (2015-11-05 10:52:35 +0000)

are available in the git repository at:


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

for you to fetch changes up to 22bdadd23b64af65ac2dd816848dbe2b1240a77a:

  Merge remote-tracking branch 'mreitz/tags/pull-block-for-kevin-2015-11-05' into queue-block (2015-11-05 18:01:37 +0100)

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

Block layer patches

----------------------------------------------------------------
Alberto Garcia (17):
      block: Don't call blk_bs() twice in bdrv_lookup_bs()
      block: check for existing device IDs in external_snapshot_prepare()
      block: rename BlockdevSnapshot to BlockdevSnapshotSync
      block: support passing 'backing': '' to 'blockdev-add'
      block: add a 'blockdev-snapshot' QMP command
      block: add tests for the 'blockdev-snapshot' command
      commit: reopen overlay_bs before base
      qemu-iotests: Test the reopening of overlay_bs in 'block-commit'
      throttle: Check for pending requests in throttle_group_unregister_bs()
      throttle: Use bs->throttle_state instead of bs->io_limits_enabled
      block: Disallow snapshots if the overlay doesn't support backing files
      block: Remove inner quotation marks in iotest 085
      block: test 'blockdev-snapshot' using a file BDS as the overlay
      mirror: block all operations on the target image during the job
      block: Add blk_get_refcnt()
      block: Add 'x-blockdev-del' QMP command
      iotests: Add tests for the x-blockdev-del command

Jeff Cody (2):
      qemu-iotests: fix cleanup of background processes
      qemu-iotests: fix -valgrind option for check

John Snow (2):
      qcow2: avoid misaligned 64bit bswap
      qemu-img: add check for zero-length job len

Kevin Wolf (2):
      qcow2: Fix qcow2_get_cluster_offset() for zero clusters
      Merge remote-tracking branch 'mreitz/tags/pull-block-for-kevin-2015-11-05' into queue-block

Max Reitz (15):
      block: Add blk_remove_bs()
      block: Make bdrv_states public
      block: Add functions for inheriting a BBRS
      blockdev: Add blockdev-open-tray
      blockdev: Add blockdev-close-tray
      blockdev: Add blockdev-remove-medium
      blockdev: Add blockdev-insert-medium
      blockdev: Implement eject with basic operations
      blockdev: Implement change with basic operations
      block: Inquire tray state before tray-moved events
      qmp: Introduce blockdev-change-medium
      hmp: Use blockdev-change-medium for change command
      blockdev: read-only-mode for blockdev-change-medium
      hmp: Add read-only-mode option to change command
      iotests: Add test for change-related QMP commands

 block.c                          |  22 +-
 block/block-backend.c            |  61 +++-
 block/commit.c                   |   8 +-
 block/mirror.c                   |   4 +
 block/qapi.c                     |   2 +-
 block/qcow2-cluster.c            |  15 +-
 block/qcow2-refcount.c           |  11 +-
 block/throttle-groups.c          |   7 +
 blockdev.c                       | 527 ++++++++++++++++++++--------
 hmp-commands.hx                  |  20 +-
 hmp.c                            |  47 ++-
 include/block/block_int.h        |   7 +-
 include/sysemu/block-backend.h   |   4 +
 include/sysemu/blockdev.h        |   2 -
 qapi-schema.json                 |  10 +-
 qapi/block-core.json             | 192 ++++++++++-
 qemu-img.c                       |   3 +-
 qmp-commands.hx                  | 317 ++++++++++++++++-
 qmp.c                            |   3 +-
 tests/qemu-iotests/039.out       |  30 +-
 tests/qemu-iotests/040           |  30 ++
 tests/qemu-iotests/040.out       |   4 +-
 tests/qemu-iotests/058           |  12 +-
 tests/qemu-iotests/061.out       |  12 +-
 tests/qemu-iotests/085           | 120 ++++++-
 tests/qemu-iotests/085.out       |  38 ++-
 tests/qemu-iotests/118           | 720 +++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/118.out       |   5 +
 tests/qemu-iotests/137.out       |   6 +-
 tests/qemu-iotests/139           | 414 ++++++++++++++++++++++
 tests/qemu-iotests/139.out       |   5 +
 tests/qemu-iotests/common        |   9 +-
 tests/qemu-iotests/common.config |  32 +-
 tests/qemu-iotests/common.qemu   |  18 +-
 tests/qemu-iotests/common.rc     |  18 +-
 tests/qemu-iotests/group         |   2 +
 36 files changed, 2486 insertions(+), 251 deletions(-)
 create mode 100755 tests/qemu-iotests/118
 create mode 100644 tests/qemu-iotests/118.out
 create mode 100644 tests/qemu-iotests/139
 create mode 100644 tests/qemu-iotests/139.out

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 01/37] block: Don't call blk_bs() twice in bdrv_lookup_bs()
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 02/37] block: Add blk_remove_bs() Kevin Wolf
                   ` (36 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Alberto Garcia <berto@igalia.com>

Signed-off-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index e9f40dc..eb8158a 100644
--- a/block.c
+++ b/block.c
@@ -2683,12 +2683,12 @@ BlockDriverState *bdrv_lookup_bs(const char *device,
         blk = blk_by_name(device);
 
         if (blk) {
-            if (!blk_bs(blk)) {
+            bs = blk_bs(blk);
+            if (!bs) {
                 error_setg(errp, "Device '%s' has no medium", device);
-                return NULL;
             }
 
-            return blk_bs(blk);
+            return bs;
         }
     }
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 02/37] block: Add blk_remove_bs()
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 01/37] block: Don't call blk_bs() twice in bdrv_lookup_bs() Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 03/37] block: Make bdrv_states public Kevin Wolf
                   ` (35 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Max Reitz <mreitz@redhat.com>

This function removes the BlockDriverState associated with the given
BlockBackend from that BB and sets the BDS pointer in the BB to NULL.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/block-backend.c          | 12 ++++++++++++
 include/sysemu/block-backend.h |  1 +
 2 files changed, 13 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index 19fdaae..878c448 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -334,6 +334,18 @@ void blk_hide_on_behalf_of_hmp_drive_del(BlockBackend *blk)
 }
 
 /*
+ * Disassociates the currently associated BlockDriverState from @blk.
+ */
+void blk_remove_bs(BlockBackend *blk)
+{
+    blk_update_root_state(blk);
+
+    blk->bs->blk = NULL;
+    bdrv_unref(blk->bs);
+    blk->bs = NULL;
+}
+
+/*
  * Associates a new BlockDriverState with @blk.
  */
 void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs)
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 9306a52..14a6d32 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -72,6 +72,7 @@ BlockBackend *blk_by_name(const char *name);
 BlockBackend *blk_next(BlockBackend *blk);
 
 BlockDriverState *blk_bs(BlockBackend *blk);
+void blk_remove_bs(BlockBackend *blk);
 void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs);
 
 void blk_hide_on_behalf_of_hmp_drive_del(BlockBackend *blk);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 03/37] block: Make bdrv_states public
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 01/37] block: Don't call blk_bs() twice in bdrv_lookup_bs() Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 02/37] block: Add blk_remove_bs() Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 04/37] block: Add functions for inheriting a BBRS Kevin Wolf
                   ` (34 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Max Reitz <mreitz@redhat.com>

When inserting a BDS tree into a BB, we will need to add the root BDS to
this list. Since we will want to do that in the blockdev-insert-medium
implementation in blockdev.c, we will need access to it there.

This patch is not exactly elegant, but bdrv_states will be removed in
the future anyway because we no longer need it since we have BBs.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c                   | 3 +--
 include/block/block_int.h | 2 ++
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index eb8158a..a99e6d8 100644
--- a/block.c
+++ b/block.c
@@ -73,8 +73,7 @@ struct BdrvDirtyBitmap {
 
 #define NOT_DONE 0x7fffffff /* used while emulated sync operation in progress */
 
-static QTAILQ_HEAD(, BlockDriverState) bdrv_states =
-    QTAILQ_HEAD_INITIALIZER(bdrv_states);
+struct BdrvStates bdrv_states = QTAILQ_HEAD_INITIALIZER(bdrv_states);
 
 static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states =
     QTAILQ_HEAD_INITIALIZER(graph_bdrv_states);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 3ceeb5a..6a3f64d 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -473,6 +473,8 @@ extern BlockDriver bdrv_file;
 extern BlockDriver bdrv_raw;
 extern BlockDriver bdrv_qcow2;
 
+extern QTAILQ_HEAD(BdrvStates, BlockDriverState) bdrv_states;
+
 /**
  * bdrv_setup_io_funcs:
  *
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 04/37] block: Add functions for inheriting a BBRS
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (2 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 03/37] block: Make bdrv_states public Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 05/37] blockdev: Add blockdev-open-tray Kevin Wolf
                   ` (33 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Max Reitz <mreitz@redhat.com>

In order to open a BDS which inherits a BB's root state,
blk_get_open_flags_from_root_state() is used to inquire the flags to be
passed to bdrv_open(), and blk_apply_root_state() is used to apply the
remaining state after the BDS has been opened.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/block-backend.c          | 27 +++++++++++++++++++++++++++
 include/sysemu/block-backend.h |  2 ++
 2 files changed, 29 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index 878c448..7d49539 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1239,6 +1239,33 @@ void blk_update_root_state(BlockBackend *blk)
     }
 }
 
+/*
+ * Applies the information in the root state to the given BlockDriverState. This
+ * does not include the flags which have to be specified for bdrv_open(), use
+ * blk_get_open_flags_from_root_state() to inquire them.
+ */
+void blk_apply_root_state(BlockBackend *blk, BlockDriverState *bs)
+{
+    bs->detect_zeroes = blk->root_state.detect_zeroes;
+    if (blk->root_state.throttle_group) {
+        bdrv_io_limits_enable(bs, blk->root_state.throttle_group);
+    }
+}
+
+/*
+ * Returns the flags to be used for bdrv_open() of a BlockDriverState which is
+ * supposed to inherit the root state.
+ */
+int blk_get_open_flags_from_root_state(BlockBackend *blk)
+{
+    int bs_flags;
+
+    bs_flags = blk->root_state.read_only ? 0 : BDRV_O_RDWR;
+    bs_flags |= blk->root_state.open_flags & ~BDRV_O_RDWR;
+
+    return bs_flags;
+}
+
 BlockBackendRootState *blk_get_root_state(BlockBackend *blk)
 {
     return &blk->root_state;
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 14a6d32..40e315b 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -167,6 +167,8 @@ void blk_io_unplug(BlockBackend *blk);
 BlockAcctStats *blk_get_stats(BlockBackend *blk);
 BlockBackendRootState *blk_get_root_state(BlockBackend *blk);
 void blk_update_root_state(BlockBackend *blk);
+void blk_apply_root_state(BlockBackend *blk, BlockDriverState *bs);
+int blk_get_open_flags_from_root_state(BlockBackend *blk);
 
 void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
                   BlockCompletionFunc *cb, void *opaque);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 05/37] blockdev: Add blockdev-open-tray
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (3 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 04/37] block: Add functions for inheriting a BBRS Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 06/37] blockdev: Add blockdev-close-tray Kevin Wolf
                   ` (32 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Max Reitz <mreitz@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c           | 36 ++++++++++++++++++++++++++++++++++++
 qapi/block-core.json | 32 ++++++++++++++++++++++++++++++++
 qmp-commands.hx      | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 116 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 8b8bfa9..be241eb 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2078,6 +2078,42 @@ out:
     aio_context_release(aio_context);
 }
 
+void qmp_blockdev_open_tray(const char *device, bool has_force, bool force,
+                            Error **errp)
+{
+    BlockBackend *blk;
+    bool locked;
+
+    if (!has_force) {
+        force = false;
+    }
+
+    blk = blk_by_name(device);
+    if (!blk) {
+        error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+                  "Device '%s' not found", device);
+        return;
+    }
+
+    if (!blk_dev_has_removable_media(blk)) {
+        error_setg(errp, "Device '%s' is not removable", device);
+        return;
+    }
+
+    if (blk_dev_is_tray_open(blk)) {
+        return;
+    }
+
+    locked = blk_dev_is_medium_locked(blk);
+    if (locked) {
+        blk_dev_eject_request(blk, force);
+    }
+
+    if (!locked || force) {
+        blk_dev_change_media_cb(blk, false);
+    }
+}
+
 /* throttling disk I/O limits */
 void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
                                int64_t bps_wr,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 425fdab..b65ab81 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1876,6 +1876,38 @@
 ##
 { 'command': 'blockdev-add', 'data': { 'options': 'BlockdevOptions' } }
 
+##
+# @blockdev-open-tray:
+#
+# Opens a block device's tray. If there is a block driver state tree inserted as
+# a medium, it will become inaccessible to the guest (but it will remain
+# associated to the block device, so closing the tray will make it accessible
+# again).
+#
+# If the tray was already open before, this will be a no-op.
+#
+# Once the tray opens, a DEVICE_TRAY_MOVED event is emitted. There are cases in
+# which no such event will be generated, these include:
+# - if the guest has locked the tray, @force is false and the guest does not
+#   respond to the eject request
+# - if the BlockBackend denoted by @device does not have a guest device attached
+#   to it
+# - if the guest device does not have an actual tray and is empty, for instance
+#   for floppy disk drives
+#
+# @device: block device name
+#
+# @force:  #optional if false (the default), an eject request will be sent to
+#          the guest if it has locked the tray (and the tray will not be opened
+#          immediately); if true, the tray will be opened regardless of whether
+#          it is locked
+#
+# Since: 2.5
+##
+{ 'command': 'blockdev-open-tray',
+  'data': { 'device': 'str',
+            '*force': 'bool' } }
+
 
 ##
 # @BlockErrorAction
diff --git a/qmp-commands.hx b/qmp-commands.hx
index d7cf0ff..0b796f9 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3936,6 +3936,54 @@ Example (2):
 EQMP
 
     {
+        .name       = "blockdev-open-tray",
+        .args_type  = "device:s,force:b?",
+        .mhandler.cmd_new = qmp_marshal_blockdev_open_tray,
+    },
+
+SQMP
+blockdev-open-tray
+------------------
+
+Opens a block device's tray. If there is a block driver state tree inserted as a
+medium, it will become inaccessible to the guest (but it will remain associated
+to the block device, so closing the tray will make it accessible again).
+
+If the tray was already open before, this will be a no-op.
+
+Once the tray opens, a DEVICE_TRAY_MOVED event is emitted. There are cases in
+which no such event will be generated, these include:
+- if the guest has locked the tray, @force is false and the guest does not
+  respond to the eject request
+- if the BlockBackend denoted by @device does not have a guest device attached
+  to it
+- if the guest device does not have an actual tray and is empty, for instance
+  for floppy disk drives
+
+Arguments:
+
+- "device": block device name (json-string)
+- "force": if false (the default), an eject request will be sent to the guest if
+           it has locked the tray (and the tray will not be opened immediately);
+           if true, the tray will be opened regardless of whether it is locked
+           (json-bool, optional)
+
+Example:
+
+-> { "execute": "blockdev-open-tray",
+     "arguments": { "device": "ide1-cd0" } }
+
+<- { "timestamp": { "seconds": 1418751016,
+                    "microseconds": 716996 },
+     "event": "DEVICE_TRAY_MOVED",
+     "data": { "device": "ide1-cd0",
+               "tray-open": true } }
+
+<- { "return": {} }
+
+EQMP
+
+    {
         .name       = "query-named-block-nodes",
         .args_type  = "",
         .mhandler.cmd_new = qmp_marshal_query_named_block_nodes,
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 06/37] blockdev: Add blockdev-close-tray
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (4 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 05/37] blockdev: Add blockdev-open-tray Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 07/37] blockdev: Add blockdev-remove-medium Kevin Wolf
                   ` (31 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Max Reitz <mreitz@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c           | 23 +++++++++++++++++++++++
 qapi/block-core.json | 16 ++++++++++++++++
 qmp-commands.hx      | 35 +++++++++++++++++++++++++++++++++++
 3 files changed, 74 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index be241eb..712c698 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2114,6 +2114,29 @@ void qmp_blockdev_open_tray(const char *device, bool has_force, bool force,
     }
 }
 
+void qmp_blockdev_close_tray(const char *device, Error **errp)
+{
+    BlockBackend *blk;
+
+    blk = blk_by_name(device);
+    if (!blk) {
+        error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+                  "Device '%s' not found", device);
+        return;
+    }
+
+    if (!blk_dev_has_removable_media(blk)) {
+        error_setg(errp, "Device '%s' is not removable", device);
+        return;
+    }
+
+    if (!blk_dev_is_tray_open(blk)) {
+        return;
+    }
+
+    blk_dev_change_media_cb(blk, true);
+}
+
 /* throttling disk I/O limits */
 void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
                                int64_t bps_wr,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index b65ab81..1cb719a 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1908,6 +1908,22 @@
   'data': { 'device': 'str',
             '*force': 'bool' } }
 
+##
+# @blockdev-close-tray:
+#
+# Closes a block device's tray. If there is a block driver state tree associated
+# with the block device (which is currently ejected), that tree will be loaded
+# as the medium.
+#
+# If the tray was already closed before, this will be a no-op.
+#
+# @device: block device name
+#
+# Since: 2.5
+##
+{ 'command': 'blockdev-close-tray',
+  'data': { 'device': 'str' } }
+
 
 ##
 # @BlockErrorAction
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 0b796f9..4b16d73 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3984,6 +3984,41 @@ Example:
 EQMP
 
     {
+        .name       = "blockdev-close-tray",
+        .args_type  = "device:s",
+        .mhandler.cmd_new = qmp_marshal_blockdev_close_tray,
+    },
+
+SQMP
+blockdev-close-tray
+-------------------
+
+Closes a block device's tray. If there is a block driver state tree associated
+with the block device (which is currently ejected), that tree will be loaded as
+the medium.
+
+If the tray was already closed before, this will be a no-op.
+
+Arguments:
+
+- "device": block device name (json-string)
+
+Example:
+
+-> { "execute": "blockdev-close-tray",
+     "arguments": { "device": "ide1-cd0" } }
+
+<- { "timestamp": { "seconds": 1418751345,
+                    "microseconds": 272147 },
+     "event": "DEVICE_TRAY_MOVED",
+     "data": { "device": "ide1-cd0",
+               "tray-open": false } }
+
+<- { "return": {} }
+
+EQMP
+
+    {
         .name       = "query-named-block-nodes",
         .args_type  = "",
         .mhandler.cmd_new = qmp_marshal_query_named_block_nodes,
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 07/37] blockdev: Add blockdev-remove-medium
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (5 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 06/37] blockdev: Add blockdev-close-tray Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 08/37] blockdev: Add blockdev-insert-medium Kevin Wolf
                   ` (30 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Max Reitz <mreitz@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c           | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 qapi/block-core.json | 16 ++++++++++++++++
 qmp-commands.hx      | 45 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 112 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 712c698..cd47727 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2137,6 +2137,57 @@ void qmp_blockdev_close_tray(const char *device, Error **errp)
     blk_dev_change_media_cb(blk, true);
 }
 
+void qmp_blockdev_remove_medium(const char *device, Error **errp)
+{
+    BlockBackend *blk;
+    BlockDriverState *bs;
+    AioContext *aio_context;
+    bool has_device;
+
+    blk = blk_by_name(device);
+    if (!blk) {
+        error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+                  "Device '%s' not found", device);
+        return;
+    }
+
+    /* For BBs without a device, we can exchange the BDS tree at will */
+    has_device = blk_get_attached_dev(blk);
+
+    if (has_device && !blk_dev_has_removable_media(blk)) {
+        error_setg(errp, "Device '%s' is not removable", device);
+        return;
+    }
+
+    if (has_device && !blk_dev_is_tray_open(blk)) {
+        error_setg(errp, "Tray of device '%s' is not open", device);
+        return;
+    }
+
+    bs = blk_bs(blk);
+    if (!bs) {
+        return;
+    }
+
+    aio_context = bdrv_get_aio_context(bs);
+    aio_context_acquire(aio_context);
+
+    if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_EJECT, errp)) {
+        goto out;
+    }
+
+    /* This follows the convention established by bdrv_make_anon() */
+    if (bs->device_list.tqe_prev) {
+        QTAILQ_REMOVE(&bdrv_states, bs, device_list);
+        bs->device_list.tqe_prev = NULL;
+    }
+
+    blk_remove_bs(blk);
+
+out:
+    aio_context_release(aio_context);
+}
+
 /* throttling disk I/O limits */
 void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
                                int64_t bps_wr,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 1cb719a..e19e82c 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1924,6 +1924,22 @@
 { 'command': 'blockdev-close-tray',
   'data': { 'device': 'str' } }
 
+##
+# @blockdev-remove-medium:
+#
+# Removes a medium (a block driver state tree) from a block device. That block
+# device's tray must currently be open (unless there is no attached guest
+# device).
+#
+# If the tray is open and there is no medium inserted, this will be a no-op.
+#
+# @device: block device name
+#
+# Since: 2.5
+##
+{ 'command': 'blockdev-remove-medium',
+  'data': { 'device': 'str' } }
+
 
 ##
 # @BlockErrorAction
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 4b16d73..6165398 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4019,6 +4019,51 @@ Example:
 EQMP
 
     {
+        .name       = "blockdev-remove-medium",
+        .args_type  = "device:s",
+        .mhandler.cmd_new = qmp_marshal_blockdev_remove_medium,
+    },
+
+SQMP
+blockdev-remove-medium
+----------------------
+
+Removes a medium (a block driver state tree) from a block device. That block
+device's tray must currently be open (unless there is no attached guest device).
+
+If the tray is open and there is no medium inserted, this will be a no-op.
+
+Arguments:
+
+- "device": block device name (json-string)
+
+Example:
+
+-> { "execute": "blockdev-remove-medium",
+     "arguments": { "device": "ide1-cd0" } }
+
+<- { "error": { "class": "GenericError",
+                "desc": "Tray of device 'ide1-cd0' is not open" } }
+
+-> { "execute": "blockdev-open-tray",
+     "arguments": { "device": "ide1-cd0" } }
+
+<- { "timestamp": { "seconds": 1418751627,
+                    "microseconds": 549958 },
+     "event": "DEVICE_TRAY_MOVED",
+     "data": { "device": "ide1-cd0",
+               "tray-open": true } }
+
+<- { "return": {} }
+
+-> { "execute": "blockdev-remove-medium",
+     "arguments": { "device": "ide1-cd0" } }
+
+<- { "return": {} }
+
+EQMP
+
+    {
         .name       = "query-named-block-nodes",
         .args_type  = "",
         .mhandler.cmd_new = qmp_marshal_query_named_block_nodes,
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 08/37] blockdev: Add blockdev-insert-medium
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (6 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 07/37] blockdev: Add blockdev-remove-medium Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 09/37] blockdev: Implement eject with basic operations Kevin Wolf
                   ` (29 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Max Reitz <mreitz@redhat.com>

And a helper function for that, which directly takes a pointer to the
BDS to be inserted instead of its node-name (which will be used for
implementing 'change' using blockdev-insert-medium).

Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c           | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 qapi/block-core.json | 17 ++++++++++++++++
 qmp-commands.hx      | 37 ++++++++++++++++++++++++++++++++++
 3 files changed, 110 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index cd47727..3126f8f 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2188,6 +2188,62 @@ out:
     aio_context_release(aio_context);
 }
 
+static void qmp_blockdev_insert_anon_medium(const char *device,
+                                            BlockDriverState *bs, Error **errp)
+{
+    BlockBackend *blk;
+    bool has_device;
+
+    blk = blk_by_name(device);
+    if (!blk) {
+        error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+                  "Device '%s' not found", device);
+        return;
+    }
+
+    /* For BBs without a device, we can exchange the BDS tree at will */
+    has_device = blk_get_attached_dev(blk);
+
+    if (has_device && !blk_dev_has_removable_media(blk)) {
+        error_setg(errp, "Device '%s' is not removable", device);
+        return;
+    }
+
+    if (has_device && !blk_dev_is_tray_open(blk)) {
+        error_setg(errp, "Tray of device '%s' is not open", device);
+        return;
+    }
+
+    if (blk_bs(blk)) {
+        error_setg(errp, "There already is a medium in device '%s'", device);
+        return;
+    }
+
+    blk_insert_bs(blk, bs);
+
+    QTAILQ_INSERT_TAIL(&bdrv_states, bs, device_list);
+}
+
+void qmp_blockdev_insert_medium(const char *device, const char *node_name,
+                                Error **errp)
+{
+    BlockDriverState *bs;
+
+    bs = bdrv_find_node(node_name);
+    if (!bs) {
+        error_setg(errp, "Node '%s' not found", node_name);
+        return;
+    }
+
+    if (bs->blk) {
+        error_setg(errp, "Node '%s' is already in use by '%s'", node_name,
+                   blk_name(bs->blk));
+        return;
+    }
+
+    qmp_blockdev_insert_anon_medium(device, bs, errp);
+}
+
 /* throttling disk I/O limits */
 void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
                                int64_t bps_wr,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index e19e82c..5c4fc72 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1940,6 +1940,23 @@
 { 'command': 'blockdev-remove-medium',
   'data': { 'device': 'str' } }
 
+##
+# @blockdev-insert-medium:
+#
+# Inserts a medium (a block driver state tree) into a block device. That block
+# device's tray must currently be open (unless there is no attached guest
+# device) and there must be no medium inserted already.
+#
+# @device:    block device name
+#
+# @node-name: name of a node in the block driver state graph
+#
+# Since: 2.5
+##
+{ 'command': 'blockdev-insert-medium',
+  'data': { 'device': 'str',
+            'node-name': 'str'} }
+
 
 ##
 # @BlockErrorAction
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 6165398..1dfa809 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4064,6 +4064,43 @@ Example:
 EQMP
 
     {
+        .name       = "blockdev-insert-medium",
+        .args_type  = "device:s,node-name:s",
+        .mhandler.cmd_new = qmp_marshal_blockdev_insert_medium,
+    },
+
+SQMP
+blockdev-insert-medium
+----------------------
+
+Inserts a medium (a block driver state tree) into a block device. That block
+device's tray must currently be open (unless there is no attached guest device)
+and there must be no medium inserted already.
+
+Arguments:
+
+- "device": block device name (json-string)
+- "node-name": root node of the BDS tree to insert into the block device
+
+Example:
+
+-> { "execute": "blockdev-add",
+     "arguments": { "options": { "node-name": "node0",
+                                 "driver": "raw",
+                                 "file": { "driver": "file",
+                                           "filename": "fedora.iso" } } } }
+
+<- { "return": {} }
+
+-> { "execute": "blockdev-insert-medium",
+     "arguments": { "device": "ide1-cd0",
+                    "node-name": "node0" } }
+
+<- { "return": {} }
+
+EQMP
+
+    {
         .name       = "query-named-block-nodes",
         .args_type  = "",
         .mhandler.cmd_new = qmp_marshal_query_named_block_nodes,
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 09/37] blockdev: Implement eject with basic operations
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (7 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 08/37] blockdev: Add blockdev-insert-medium Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 10/37] blockdev: Implement change " Kevin Wolf
                   ` (28 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Max Reitz <mreitz@redhat.com>

Implement 'eject' by calling blockdev-open-tray and
blockdev-remove-medium.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 3126f8f..8c86a8d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1970,16 +1970,15 @@ out:
 
 void qmp_eject(const char *device, bool has_force, bool force, Error **errp)
 {
-    BlockBackend *blk;
+    Error *local_err = NULL;
 
-    blk = blk_by_name(device);
-    if (!blk) {
-        error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
-                  "Device '%s' not found", device);
+    qmp_blockdev_open_tray(device, has_force, force, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
         return;
     }
 
-    eject_device(blk, force, errp);
+    qmp_blockdev_remove_medium(device, errp);
 }
 
 void qmp_block_passwd(bool has_device, const char *device,
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 10/37] blockdev: Implement change with basic operations
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (8 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 09/37] blockdev: Implement eject with basic operations Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 11/37] block: Inquire tray state before tray-moved events Kevin Wolf
                   ` (27 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Max Reitz <mreitz@redhat.com>

Implement 'change' on block devices by calling blockdev-open-tray,
blockdev-remove-medium, blockdev-insert-medium (a variation of that
which does not need a node-name) and blockdev-close-tray.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c | 178 +++++++++++++++++++++++--------------------------------------
 1 file changed, 68 insertions(+), 110 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 8c86a8d..df93173 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1930,44 +1930,6 @@ exit:
     }
 }
 
-
-static void eject_device(BlockBackend *blk, int force, Error **errp)
-{
-    BlockDriverState *bs = blk_bs(blk);
-    AioContext *aio_context;
-
-    if (!bs) {
-        /* No medium inserted, so there is nothing to do */
-        return;
-    }
-
-    aio_context = bdrv_get_aio_context(bs);
-    aio_context_acquire(aio_context);
-
-    if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_EJECT, errp)) {
-        goto out;
-    }
-    if (!blk_dev_has_removable_media(blk)) {
-        error_setg(errp, "Device '%s' is not removable",
-                   bdrv_get_device_name(bs));
-        goto out;
-    }
-
-    if (blk_dev_is_medium_locked(blk) && !blk_dev_is_tray_open(blk)) {
-        blk_dev_eject_request(blk, force);
-        if (!force) {
-            error_setg(errp, "Device '%s' is locked",
-                       bdrv_get_device_name(bs));
-            goto out;
-        }
-    }
-
-    bdrv_close(bs);
-
-out:
-    aio_context_release(aio_context);
-}
-
 void qmp_eject(const char *device, bool has_force, bool force, Error **errp)
 {
     Error *local_err = NULL;
@@ -2005,78 +1967,6 @@ void qmp_block_passwd(bool has_device, const char *device,
     aio_context_release(aio_context);
 }
 
-/* Assumes AioContext is held */
-static void qmp_bdrv_open_encrypted(BlockDriverState **pbs,
-                                    const char *filename,
-                                    int bdrv_flags, const char *format,
-                                    const char *password, Error **errp)
-{
-    Error *local_err = NULL;
-    QDict *options = NULL;
-    int ret;
-
-    if (format) {
-        options = qdict_new();
-        qdict_put(options, "driver", qstring_from_str(format));
-    }
-
-    ret = bdrv_open(pbs, filename, NULL, options, bdrv_flags, &local_err);
-    if (ret < 0) {
-        error_propagate(errp, local_err);
-        return;
-    }
-
-    bdrv_add_key(*pbs, password, errp);
-}
-
-void qmp_change_blockdev(const char *device, const char *filename,
-                         const char *format, Error **errp)
-{
-    BlockBackend *blk;
-    BlockDriverState *bs;
-    AioContext *aio_context;
-    int bdrv_flags;
-    bool new_bs;
-    Error *err = NULL;
-
-    blk = blk_by_name(device);
-    if (!blk) {
-        error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
-                  "Device '%s' not found", device);
-        return;
-    }
-    bs = blk_bs(blk);
-    new_bs = !bs;
-
-    aio_context = blk_get_aio_context(blk);
-    aio_context_acquire(aio_context);
-
-    eject_device(blk, 0, &err);
-    if (err) {
-        error_propagate(errp, err);
-        goto out;
-    }
-
-    bdrv_flags = blk_is_read_only(blk) ? 0 : BDRV_O_RDWR;
-    bdrv_flags |= blk_get_root_state(blk)->open_flags & ~BDRV_O_RDWR;
-
-    qmp_bdrv_open_encrypted(&bs, filename, bdrv_flags, format, NULL, &err);
-    if (err) {
-        error_propagate(errp, err);
-        goto out;
-    }
-
-    if (new_bs) {
-        blk_insert_bs(blk, bs);
-        /* Has been sent automatically by bdrv_open() if blk_bs(blk) was not
-         * NULL */
-        blk_dev_change_media_cb(blk, true);
-    }
-
-out:
-    aio_context_release(aio_context);
-}
-
 void qmp_blockdev_open_tray(const char *device, bool has_force, bool force,
                             Error **errp)
 {
@@ -2243,6 +2133,74 @@ void qmp_blockdev_insert_medium(const char *device, const char *node_name,
     qmp_blockdev_insert_anon_medium(device, bs, errp);
 }
 
+void qmp_change_blockdev(const char *device, const char *filename,
+                         const char *format, Error **errp)
+{
+    BlockBackend *blk;
+    BlockDriverState *medium_bs = NULL;
+    int bdrv_flags, ret;
+    QDict *options = NULL;
+    Error *err = NULL;
+
+    blk = blk_by_name(device);
+    if (!blk) {
+        error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+                  "Device '%s' not found", device);
+        goto fail;
+    }
+
+    if (blk_bs(blk)) {
+        blk_update_root_state(blk);
+    }
+
+    bdrv_flags = blk_get_open_flags_from_root_state(blk);
+
+    if (format) {
+        options = qdict_new();
+        qdict_put(options, "driver", qstring_from_str(format));
+    }
+
+    assert(!medium_bs);
+    ret = bdrv_open(&medium_bs, filename, NULL, options, bdrv_flags, errp);
+    if (ret < 0) {
+        goto fail;
+    }
+
+    blk_apply_root_state(blk, medium_bs);
+
+    bdrv_add_key(medium_bs, NULL, &err);
+    if (err) {
+        error_propagate(errp, err);
+        goto fail;
+    }
+
+    qmp_blockdev_open_tray(device, false, false, &err);
+    if (err) {
+        error_propagate(errp, err);
+        goto fail;
+    }
+
+    qmp_blockdev_remove_medium(device, &err);
+    if (err) {
+        error_propagate(errp, err);
+        goto fail;
+    }
+
+    qmp_blockdev_insert_anon_medium(device, medium_bs, &err);
+    if (err) {
+        error_propagate(errp, err);
+        goto fail;
+    }
+
+    qmp_blockdev_close_tray(device, errp);
+
+fail:
+    /* If the medium has been inserted, the device has its own reference, so
+     * ours must be relinquished; and if it has not been inserted successfully,
+     * the reference must be relinquished anyway */
+    bdrv_unref(medium_bs);
+}
+
 /* throttling disk I/O limits */
 void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
                                int64_t bps_wr,
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 11/37] block: Inquire tray state before tray-moved events
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (9 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 10/37] blockdev: Implement change " Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 12/37] qmp: Introduce blockdev-change-medium Kevin Wolf
                   ` (26 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Max Reitz <mreitz@redhat.com>

blk_dev_change_media_cb() is called for all potential tray movements;
however, it is possible to request closing the tray but nothing actually
happening (on a floppy disk drive without a medium).

Thus, the actual tray status should be inquired before sending a
tray-moved event (and an event should be sent whenever the status
changed).

Checking @load is now superfluous; it was necessary because it was
possible to change a medium without having explicitly opened the tray
and closed it again (or it might have been possible, at least). This is
no longer possible, though.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/block-backend.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 7d49539..1ac6982 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -429,18 +429,15 @@ void blk_set_dev_ops(BlockBackend *blk, const BlockDevOps *ops,
 void blk_dev_change_media_cb(BlockBackend *blk, bool load)
 {
     if (blk->dev_ops && blk->dev_ops->change_media_cb) {
-        bool tray_was_closed = !blk_dev_is_tray_open(blk);
+        bool tray_was_open, tray_is_open;
 
+        tray_was_open = blk_dev_is_tray_open(blk);
         blk->dev_ops->change_media_cb(blk->dev_opaque, load);
-        if (tray_was_closed) {
-            /* tray open */
-            qapi_event_send_device_tray_moved(blk_name(blk),
-                                              true, &error_abort);
-        }
-        if (load) {
-            /* tray close */
-            qapi_event_send_device_tray_moved(blk_name(blk),
-                                              false, &error_abort);
+        tray_is_open = blk_dev_is_tray_open(blk);
+
+        if (tray_was_open != tray_is_open) {
+            qapi_event_send_device_tray_moved(blk_name(blk), tray_is_open,
+                                              &error_abort);
         }
     }
 }
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 12/37] qmp: Introduce blockdev-change-medium
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (10 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 11/37] block: Inquire tray state before tray-moved events Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 13/37] hmp: Use blockdev-change-medium for change command Kevin Wolf
                   ` (25 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Max Reitz <mreitz@redhat.com>

Introduce a new QMP command 'blockdev-change-medium' which is intended
to replace the 'change' command for block devices. The existing function
qmp_change_blockdev() is accordingly renamed to
qmp_blockdev_change_medium().

Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c                |  7 ++++---
 include/sysemu/blockdev.h |  2 --
 qapi-schema.json          |  6 ++++--
 qapi/block-core.json      | 23 +++++++++++++++++++++++
 qmp-commands.hx           | 31 +++++++++++++++++++++++++++++++
 qmp.c                     |  2 +-
 6 files changed, 63 insertions(+), 8 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index df93173..dec6169 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2133,8 +2133,9 @@ void qmp_blockdev_insert_medium(const char *device, const char *node_name,
     qmp_blockdev_insert_anon_medium(device, bs, errp);
 }
 
-void qmp_change_blockdev(const char *device, const char *filename,
-                         const char *format, Error **errp)
+void qmp_blockdev_change_medium(const char *device, const char *filename,
+                                bool has_format, const char *format,
+                                Error **errp)
 {
     BlockBackend *blk;
     BlockDriverState *medium_bs = NULL;
@@ -2155,7 +2156,7 @@ void qmp_change_blockdev(const char *device, const char *filename,
 
     bdrv_flags = blk_get_open_flags_from_root_state(blk);
 
-    if (format) {
+    if (has_format) {
         options = qdict_new();
         qdict_put(options, "driver", qstring_from_str(format));
     }
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index a00be94..b06a060 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -63,8 +63,6 @@ DriveInfo *drive_new(QemuOpts *arg, BlockInterfaceType block_default_type);
 
 /* device-hotplug */
 
-void qmp_change_blockdev(const char *device, const char *filename,
-                         const char *format, Error **errp);
 void hmp_commit(Monitor *mon, const QDict *qdict);
 void hmp_drive_del(Monitor *mon, const QDict *qdict);
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 702b7b5..058b8ec 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1842,8 +1842,10 @@
 #          device's password.  The behavior of reads and writes to the block
 #          device between when these calls are executed is undefined.
 #
-# Notes:  It is strongly recommended that this interface is not used especially
-#         for changing block devices.
+# Notes:  This interface is deprecated, and it is strongly recommended that you
+#         avoid using it.  For changing block devices, use
+#         blockdev-change-medium; for changing VNC parameters, use
+#         change-vnc-password.
 #
 # Since: 0.14.0
 ##
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 5c4fc72..e9fa649 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1959,6 +1959,29 @@
 
 
 ##
+# @blockdev-change-medium:
+#
+# Changes the medium inserted into a block device by ejecting the current medium
+# and loading a new image file which is inserted as the new medium (this command
+# combines blockdev-open-tray, blockdev-remove-medium, blockdev-insert-medium
+# and blockdev-close-tray).
+#
+# @device:          block device name
+#
+# @filename:        filename of the new image to be loaded
+#
+# @format:          #optional, format to open the new image with (defaults to
+#                   the probed format)
+#
+# Since: 2.5
+##
+{ 'command': 'blockdev-change-medium',
+  'data': { 'device': 'str',
+            'filename': 'str',
+            '*format': 'str' } }
+
+
+##
 # @BlockErrorAction
 #
 # An enumeration of action that has been taken when a DISK I/O occurs
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 1dfa809..d6e4d7e 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4163,6 +4163,37 @@ Example:
 EQMP
 
     {
+        .name       = "blockdev-change-medium",
+        .args_type  = "device:B,filename:F,format:s?",
+        .mhandler.cmd_new = qmp_marshal_blockdev_change_medium,
+    },
+
+SQMP
+blockdev-change-medium
+----------------------
+
+Changes the medium inserted into a block device by ejecting the current medium
+and loading a new image file which is inserted as the new medium.
+
+Arguments:
+
+- "device": device name (json-string)
+- "filename": filename of the new image (json-string)
+- "format": format of the new image (json-string, optional)
+
+Examples:
+
+1. Change a removable medium
+
+-> { "execute": "blockdev-change-medium",
+             "arguments": { "device": "ide1-cd0",
+                            "filename": "/srv/images/Fedora-12-x86_64-DVD.iso",
+                            "format": "raw" } }
+<- { "return": {} }
+
+EQMP
+
+    {
         .name       = "query-memdev",
         .args_type  = "",
         .mhandler.cmd_new = qmp_marshal_query_memdev,
diff --git a/qmp.c b/qmp.c
index ff54e5a..4e44f98 100644
--- a/qmp.c
+++ b/qmp.c
@@ -414,7 +414,7 @@ void qmp_change(const char *device, const char *target,
     if (strcmp(device, "vnc") == 0) {
         qmp_change_vnc(target, has_arg, arg, errp);
     } else {
-        qmp_change_blockdev(device, target, arg, errp);
+        qmp_blockdev_change_medium(device, target, has_arg, arg, errp);
     }
 }
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 13/37] hmp: Use blockdev-change-medium for change command
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (11 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 12/37] qmp: Introduce blockdev-change-medium Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 14/37] blockdev: read-only-mode for blockdev-change-medium Kevin Wolf
                   ` (24 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Max Reitz <mreitz@redhat.com>

Use separate code paths for the two overloaded functions of the 'change'
HMP command, and invoke the 'blockdev-change-medium' QMP command if used
on a block device (by calling qmp_blockdev_change_medium()).

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hmp.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/hmp.c b/hmp.c
index a15d00c..5e5358f 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1338,22 +1338,25 @@ void hmp_change(Monitor *mon, const QDict *qdict)
     const char *arg = qdict_get_try_str(qdict, "arg");
     Error *err = NULL;
 
-    if (strcmp(device, "vnc") == 0 &&
-            (strcmp(target, "passwd") == 0 ||
-             strcmp(target, "password") == 0)) {
-        if (!arg) {
-            monitor_read_password(mon, hmp_change_read_arg, NULL);
+    if (strcmp(device, "vnc") == 0) {
+        if (strcmp(target, "passwd") == 0 ||
+            strcmp(target, "password") == 0) {
+            if (!arg) {
+                monitor_read_password(mon, hmp_change_read_arg, NULL);
+                return;
+            }
+        }
+        qmp_change("vnc", target, !!arg, arg, &err);
+    } else {
+        qmp_blockdev_change_medium(device, target, !!arg, arg, &err);
+        if (err &&
+            error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) {
+            error_free(err);
+            monitor_read_block_device_key(mon, device, NULL, NULL);
             return;
         }
     }
 
-    qmp_change(device, target, !!arg, arg, &err);
-    if (err &&
-        error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) {
-        error_free(err);
-        monitor_read_block_device_key(mon, device, NULL, NULL);
-        return;
-    }
     hmp_handle_error(mon, &err);
 }
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 14/37] blockdev: read-only-mode for blockdev-change-medium
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (12 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 13/37] hmp: Use blockdev-change-medium for change command Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 15/37] hmp: Add read-only-mode option to change command Kevin Wolf
                   ` (23 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Max Reitz <mreitz@redhat.com>

Add an option to qmp_blockdev_change_medium() which allows changing the
read-only status of the block device whose medium is changed.

Some drives do not have a inherently fixed read-only status; for
instance, floppy disks can be set read-only or writable independently of
the drive. Some users may find it useful to be able to therefore change
the read-only status of a block device when changing the medium.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c           | 22 ++++++++++++++++++++++
 hmp.c                |  2 +-
 qapi/block-core.json | 24 +++++++++++++++++++++++-
 qmp-commands.hx      | 24 +++++++++++++++++++++++-
 qmp.c                |  3 ++-
 5 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index dec6169..005fcc5 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2135,6 +2135,8 @@ void qmp_blockdev_insert_medium(const char *device, const char *node_name,
 
 void qmp_blockdev_change_medium(const char *device, const char *filename,
                                 bool has_format, const char *format,
+                                bool has_read_only,
+                                BlockdevChangeReadOnlyMode read_only,
                                 Error **errp)
 {
     BlockBackend *blk;
@@ -2156,6 +2158,26 @@ void qmp_blockdev_change_medium(const char *device, const char *filename,
 
     bdrv_flags = blk_get_open_flags_from_root_state(blk);
 
+    if (!has_read_only) {
+        read_only = BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN;
+    }
+
+    switch (read_only) {
+    case BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN:
+        break;
+
+    case BLOCKDEV_CHANGE_READ_ONLY_MODE_READ_ONLY:
+        bdrv_flags &= ~BDRV_O_RDWR;
+        break;
+
+    case BLOCKDEV_CHANGE_READ_ONLY_MODE_READ_WRITE:
+        bdrv_flags |= BDRV_O_RDWR;
+        break;
+
+    default:
+        abort();
+    }
+
     if (has_format) {
         options = qdict_new();
         qdict_put(options, "driver", qstring_from_str(format));
diff --git a/hmp.c b/hmp.c
index 5e5358f..efa07a2 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1348,7 +1348,7 @@ void hmp_change(Monitor *mon, const QDict *qdict)
         }
         qmp_change("vnc", target, !!arg, arg, &err);
     } else {
-        qmp_blockdev_change_medium(device, target, !!arg, arg, &err);
+        qmp_blockdev_change_medium(device, target, !!arg, arg, false, 0, &err);
         if (err &&
             error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) {
             error_free(err);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index e9fa649..fa08ba9 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1959,6 +1959,24 @@
 
 
 ##
+# @BlockdevChangeReadOnlyMode:
+#
+# Specifies the new read-only mode of a block device subject to the
+# @blockdev-change-medium command.
+#
+# @retain:      Retains the current read-only mode
+#
+# @read-only:   Makes the device read-only
+#
+# @read-write:  Makes the device writable
+#
+# Since: 2.3
+##
+{ 'enum': 'BlockdevChangeReadOnlyMode',
+  'data': ['retain', 'read-only', 'read-write'] }
+
+
+##
 # @blockdev-change-medium:
 #
 # Changes the medium inserted into a block device by ejecting the current medium
@@ -1973,12 +1991,16 @@
 # @format:          #optional, format to open the new image with (defaults to
 #                   the probed format)
 #
+# @read-only-mode:  #optional, change the read-only mode of the device; defaults
+#                   to 'retain'
+#
 # Since: 2.5
 ##
 { 'command': 'blockdev-change-medium',
   'data': { 'device': 'str',
             'filename': 'str',
-            '*format': 'str' } }
+            '*format': 'str',
+            '*read-only-mode': 'BlockdevChangeReadOnlyMode' } }
 
 
 ##
diff --git a/qmp-commands.hx b/qmp-commands.hx
index d6e4d7e..707e0bd 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4164,7 +4164,7 @@ EQMP
 
     {
         .name       = "blockdev-change-medium",
-        .args_type  = "device:B,filename:F,format:s?",
+        .args_type  = "device:B,filename:F,format:s?,read-only-mode:s?",
         .mhandler.cmd_new = qmp_marshal_blockdev_change_medium,
     },
 
@@ -4180,6 +4180,8 @@ Arguments:
 - "device": device name (json-string)
 - "filename": filename of the new image (json-string)
 - "format": format of the new image (json-string, optional)
+- "read-only-mode": new read-only mode (json-string, optional)
+          - Possible values: "retain" (default), "read-only", "read-write"
 
 Examples:
 
@@ -4191,6 +4193,26 @@ Examples:
                             "format": "raw" } }
 <- { "return": {} }
 
+2. Load a read-only medium into a writable drive
+
+-> { "execute": "blockdev-change-medium",
+             "arguments": { "device": "isa-fd0",
+                            "filename": "/srv/images/ro.img",
+                            "format": "raw",
+                            "read-only-mode": "retain" } }
+
+<- { "error":
+     { "class": "GenericError",
+       "desc": "Could not open '/srv/images/ro.img': Permission denied" } }
+
+-> { "execute": "blockdev-change-medium",
+             "arguments": { "device": "isa-fd0",
+                            "filename": "/srv/images/ro.img",
+                            "format": "raw",
+                            "read-only-mode": "read-only" } }
+
+<- { "return": {} }
+
 EQMP
 
     {
diff --git a/qmp.c b/qmp.c
index 4e44f98..ddc63ea 100644
--- a/qmp.c
+++ b/qmp.c
@@ -414,7 +414,8 @@ void qmp_change(const char *device, const char *target,
     if (strcmp(device, "vnc") == 0) {
         qmp_change_vnc(target, has_arg, arg, errp);
     } else {
-        qmp_blockdev_change_medium(device, target, has_arg, arg, errp);
+        qmp_blockdev_change_medium(device, target, has_arg, arg, false, 0,
+                                   errp);
     }
 }
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 15/37] hmp: Add read-only-mode option to change command
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (13 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 14/37] blockdev: read-only-mode for blockdev-change-medium Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 16/37] iotests: Add test for change-related QMP commands Kevin Wolf
                   ` (22 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Max Reitz <mreitz@redhat.com>

Expose the new read-only-mode option of 'blockdev-change-medium' for the
'change' HMP command.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hmp-commands.hx | 20 +++++++++++++++++---
 hmp.c           | 22 +++++++++++++++++++++-
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 3a4ae39..814ea86 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -194,8 +194,8 @@ ETEXI
 
     {
         .name       = "change",
-        .args_type  = "device:B,target:F,arg:s?",
-        .params     = "device filename [format]",
+        .args_type  = "device:B,target:F,arg:s?,read-only-mode:s?",
+        .params     = "device filename [format [read-only-mode]]",
         .help       = "change a removable medium, optional format",
         .mhandler.cmd = hmp_change,
     },
@@ -206,7 +206,7 @@ STEXI
 Change the configuration of a device.
 
 @table @option
-@item change @var{diskdevice} @var{filename} [@var{format}]
+@item change @var{diskdevice} @var{filename} [@var{format} [@var{read-only-mode}]]
 Change the medium for a removable disk device to point to @var{filename}. eg
 
 @example
@@ -215,6 +215,20 @@ Change the medium for a removable disk device to point to @var{filename}. eg
 
 @var{format} is optional.
 
+@var{read-only-mode} may be used to change the read-only status of the device.
+It accepts the following values:
+
+@table @var
+@item retain
+Retains the current status; this is the default.
+
+@item read-only
+Makes the device read-only.
+
+@item read-write
+Makes the device writable.
+@end table
+
 @item change vnc @var{display},@var{options}
 Change the configuration of the VNC server. The valid syntax for @var{display}
 and @var{options} are described at @ref{sec_invocation}. eg
diff --git a/hmp.c b/hmp.c
index efa07a2..8166fd2 100644
--- a/hmp.c
+++ b/hmp.c
@@ -27,6 +27,7 @@
 #include "qapi/opts-visitor.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/string-output-visitor.h"
+#include "qapi/util.h"
 #include "qapi-visit.h"
 #include "ui/console.h"
 #include "block/qapi.h"
@@ -1336,9 +1337,16 @@ void hmp_change(Monitor *mon, const QDict *qdict)
     const char *device = qdict_get_str(qdict, "device");
     const char *target = qdict_get_str(qdict, "target");
     const char *arg = qdict_get_try_str(qdict, "arg");
+    const char *read_only = qdict_get_try_str(qdict, "read-only-mode");
+    BlockdevChangeReadOnlyMode read_only_mode = 0;
     Error *err = NULL;
 
     if (strcmp(device, "vnc") == 0) {
+        if (read_only) {
+            monitor_printf(mon,
+                           "Parameter 'read-only-mode' is invalid for VNC\n");
+            return;
+        }
         if (strcmp(target, "passwd") == 0 ||
             strcmp(target, "password") == 0) {
             if (!arg) {
@@ -1348,7 +1356,19 @@ void hmp_change(Monitor *mon, const QDict *qdict)
         }
         qmp_change("vnc", target, !!arg, arg, &err);
     } else {
-        qmp_blockdev_change_medium(device, target, !!arg, arg, false, 0, &err);
+        if (read_only) {
+            read_only_mode =
+                qapi_enum_parse(BlockdevChangeReadOnlyMode_lookup,
+                                read_only, BLOCKDEV_CHANGE_READ_ONLY_MODE_MAX,
+                                BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN, &err);
+            if (err) {
+                hmp_handle_error(mon, &err);
+                return;
+            }
+        }
+
+        qmp_blockdev_change_medium(device, target, !!arg, arg,
+                                   !!read_only, read_only_mode, &err);
         if (err &&
             error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) {
             error_free(err);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 16/37] iotests: Add test for change-related QMP commands
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (14 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 15/37] hmp: Add read-only-mode option to change command Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 17/37] block: check for existing device IDs in external_snapshot_prepare() Kevin Wolf
                   ` (21 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Max Reitz <mreitz@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/118     | 720 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/118.out |   5 +
 tests/qemu-iotests/group   |   1 +
 3 files changed, 726 insertions(+)
 create mode 100755 tests/qemu-iotests/118
 create mode 100644 tests/qemu-iotests/118.out

diff --git a/tests/qemu-iotests/118 b/tests/qemu-iotests/118
new file mode 100755
index 0000000..a2bcd54
--- /dev/null
+++ b/tests/qemu-iotests/118
@@ -0,0 +1,720 @@
+#!/usr/bin/env python
+#
+# Test case for the QMP 'change' command and all other associated
+# commands
+#
+# Copyright (C) 2015 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/>.
+#
+
+import os
+import stat
+import time
+import iotests
+from iotests import qemu_img
+
+old_img = os.path.join(iotests.test_dir, 'test0.img')
+new_img = os.path.join(iotests.test_dir, 'test1.img')
+
+class ChangeBaseClass(iotests.QMPTestCase):
+    has_opened = False
+    has_closed = False
+
+    def process_events(self):
+        for event in self.vm.get_qmp_events(wait=False):
+            if (event['event'] == 'DEVICE_TRAY_MOVED' and
+                event['data']['device'] == 'drive0'):
+                if event['data']['tray-open'] == False:
+                    self.has_closed = True
+                else:
+                    self.has_opened = True
+
+    def wait_for_open(self):
+        timeout = time.clock() + 3
+        while not self.has_opened and time.clock() < timeout:
+            self.process_events()
+        if not self.has_opened:
+            self.fail('Timeout while waiting for the tray to open')
+
+    def wait_for_close(self):
+        timeout = time.clock() + 3
+        while not self.has_closed and time.clock() < timeout:
+            self.process_events()
+        if not self.has_opened:
+            self.fail('Timeout while waiting for the tray to close')
+
+class GeneralChangeTestsBaseClass(ChangeBaseClass):
+    def test_change(self):
+        result = self.vm.qmp('change', device='drive0', target=new_img,
+                                       arg=iotests.imgfmt)
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_for_open()
+        self.wait_for_close()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
+
+    def test_blockdev_change_medium(self):
+        result = self.vm.qmp('blockdev-change-medium', device='drive0',
+                                                       filename=new_img,
+                                                       format=iotests.imgfmt)
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_for_open()
+        self.wait_for_close()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
+
+    def test_eject(self):
+        result = self.vm.qmp('eject', device='drive0', force=True)
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_for_open()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', True)
+        self.assert_qmp_absent(result, 'return[0]/inserted')
+
+    def test_tray_eject_change(self):
+        result = self.vm.qmp('eject', device='drive0', force=True)
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_for_open()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', True)
+        self.assert_qmp_absent(result, 'return[0]/inserted')
+
+        result = self.vm.qmp('blockdev-change-medium', device='drive0',
+                                                       filename=new_img,
+                                                       format=iotests.imgfmt)
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_for_close()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
+
+    def test_tray_open_close(self):
+        result = self.vm.qmp('blockdev-open-tray', device='drive0', force=True)
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_for_open()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', True)
+        if self.was_empty == True:
+            self.assert_qmp_absent(result, 'return[0]/inserted')
+        else:
+            self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img)
+
+        result = self.vm.qmp('blockdev-close-tray', device='drive0')
+        self.assert_qmp(result, 'return', {})
+
+        if self.has_real_tray or not self.was_empty:
+            self.wait_for_close()
+
+        result = self.vm.qmp('query-block')
+        if self.has_real_tray or not self.was_empty:
+            self.assert_qmp(result, 'return[0]/tray_open', False)
+        else:
+            self.assert_qmp(result, 'return[0]/tray_open', True)
+        if self.was_empty == True:
+            self.assert_qmp_absent(result, 'return[0]/inserted')
+        else:
+            self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img)
+
+    def test_tray_eject_close(self):
+        result = self.vm.qmp('eject', device='drive0', force=True)
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_for_open()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', True)
+        self.assert_qmp_absent(result, 'return[0]/inserted')
+
+        result = self.vm.qmp('blockdev-close-tray', device='drive0')
+        self.assert_qmp(result, 'return', {})
+
+        if self.has_real_tray:
+            self.wait_for_close()
+
+        result = self.vm.qmp('query-block')
+        if self.has_real_tray:
+            self.assert_qmp(result, 'return[0]/tray_open', False)
+        else:
+            self.assert_qmp(result, 'return[0]/tray_open', True)
+        self.assert_qmp_absent(result, 'return[0]/inserted')
+
+    def test_tray_open_change(self):
+        result = self.vm.qmp('blockdev-open-tray', device='drive0', force=True)
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_for_open()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', True)
+        if self.was_empty == True:
+            self.assert_qmp_absent(result, 'return[0]/inserted')
+        else:
+            self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img)
+
+        result = self.vm.qmp('blockdev-change-medium', device='drive0',
+                                                       filename=new_img,
+                                                       format=iotests.imgfmt)
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_for_close()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
+
+    def test_cycle(self):
+        result = self.vm.qmp('blockdev-add',
+                             options={'node-name': 'new',
+                                      'driver': iotests.imgfmt,
+                                      'file': {'filename': new_img,
+                                               'driver': 'file'}})
+        self.assert_qmp(result, 'return', {})
+
+        result = self.vm.qmp('blockdev-open-tray', device='drive0', force=True)
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_for_open()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', True)
+        if self.was_empty == True:
+            self.assert_qmp_absent(result, 'return[0]/inserted')
+        else:
+            self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img)
+
+        result = self.vm.qmp('blockdev-remove-medium', device='drive0')
+        self.assert_qmp(result, 'return', {})
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', True)
+        self.assert_qmp_absent(result, 'return[0]/inserted')
+
+        result = self.vm.qmp('blockdev-insert-medium', device='drive0',
+                                                       node_name='new')
+        self.assert_qmp(result, 'return', {})
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', True)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
+
+        result = self.vm.qmp('blockdev-close-tray', device='drive0')
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_for_close()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
+
+    def test_close_on_closed(self):
+        result = self.vm.qmp('blockdev-close-tray', device='drive0')
+        # Should be a no-op
+        self.assert_qmp(result, 'return', {})
+        self.assertEquals(self.vm.get_qmp_events(wait=False), [])
+
+    def test_remove_on_closed(self):
+        if self.has_opened:
+            # Empty floppy drive
+            return
+
+        result = self.vm.qmp('blockdev-remove-medium', device='drive0')
+        self.assert_qmp(result, 'error/class', 'GenericError')
+
+    def test_insert_on_closed(self):
+        if self.has_opened:
+            # Empty floppy drive
+            return
+
+        result = self.vm.qmp('blockdev-add',
+                             options={'node-name': 'new',
+                                      'driver': iotests.imgfmt,
+                                      'file': {'filename': new_img,
+                                               'driver': 'file'}})
+        self.assert_qmp(result, 'return', {})
+
+        result = self.vm.qmp('blockdev-insert-medium', device='drive0',
+                                                       node_name='new')
+        self.assert_qmp(result, 'error/class', 'GenericError')
+
+class TestInitiallyFilled(GeneralChangeTestsBaseClass):
+    was_empty = False
+
+    def setUp(self, media, interface):
+        qemu_img('create', '-f', iotests.imgfmt, old_img, '1440k')
+        qemu_img('create', '-f', iotests.imgfmt, new_img, '1440k')
+        self.vm = iotests.VM().add_drive(old_img, 'media=%s' % media, interface)
+        self.vm.launch()
+
+    def tearDown(self):
+        self.vm.shutdown()
+        os.remove(old_img)
+        os.remove(new_img)
+
+    def test_insert_on_filled(self):
+        result = self.vm.qmp('blockdev-add',
+                             options={'node-name': 'new',
+                                      'driver': iotests.imgfmt,
+                                      'file': {'filename': new_img,
+                                               'driver': 'file'}})
+        self.assert_qmp(result, 'return', {})
+
+        result = self.vm.qmp('blockdev-open-tray', device='drive0')
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_for_open()
+
+        result = self.vm.qmp('blockdev-insert-medium', device='drive0',
+                                                       node_name='new')
+        self.assert_qmp(result, 'error/class', 'GenericError')
+
+class TestInitiallyEmpty(GeneralChangeTestsBaseClass):
+    was_empty = True
+
+    def setUp(self, media, interface):
+        qemu_img('create', '-f', iotests.imgfmt, new_img, '1440k')
+        self.vm = iotests.VM().add_drive(None, 'media=%s' % media, interface)
+        self.vm.launch()
+
+    def tearDown(self):
+        self.vm.shutdown()
+        os.remove(new_img)
+
+    def test_remove_on_empty(self):
+        result = self.vm.qmp('blockdev-open-tray', device='drive0')
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_for_open()
+
+        result = self.vm.qmp('blockdev-remove-medium', device='drive0')
+        # Should be a no-op
+        self.assert_qmp(result, 'return', {})
+
+class TestCDInitiallyFilled(TestInitiallyFilled):
+    TestInitiallyFilled = TestInitiallyFilled
+    has_real_tray = True
+
+    def setUp(self):
+        self.TestInitiallyFilled.setUp(self, 'cdrom', 'ide')
+
+class TestCDInitiallyEmpty(TestInitiallyEmpty):
+    TestInitiallyEmpty = TestInitiallyEmpty
+    has_real_tray = True
+
+    def setUp(self):
+        self.TestInitiallyEmpty.setUp(self, 'cdrom', 'ide')
+
+class TestFloppyInitiallyFilled(TestInitiallyFilled):
+    TestInitiallyFilled = TestInitiallyFilled
+    has_real_tray = False
+
+    def setUp(self):
+        self.TestInitiallyFilled.setUp(self, 'disk', 'floppy')
+
+class TestFloppyInitiallyEmpty(TestInitiallyEmpty):
+    TestInitiallyEmpty = TestInitiallyEmpty
+    has_real_tray = False
+
+    def setUp(self):
+        self.TestInitiallyEmpty.setUp(self, 'disk', 'floppy')
+        # FDDs not having a real tray and there not being a medium inside the
+        # tray at startup means the tray will be considered open
+        self.has_opened = True
+
+class TestChangeReadOnly(ChangeBaseClass):
+    def setUp(self):
+        qemu_img('create', '-f', iotests.imgfmt, old_img, '1440k')
+        qemu_img('create', '-f', iotests.imgfmt, new_img, '1440k')
+        self.vm = iotests.VM()
+
+    def tearDown(self):
+        self.vm.shutdown()
+        os.chmod(old_img, 0666)
+        os.chmod(new_img, 0666)
+        os.remove(old_img)
+        os.remove(new_img)
+
+    def test_ro_ro_retain(self):
+        os.chmod(old_img, 0444)
+        os.chmod(new_img, 0444)
+        self.vm.add_drive(old_img, 'media=disk,read-only=on', 'floppy')
+        self.vm.launch()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/ro', True)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img)
+
+        result = self.vm.qmp('blockdev-change-medium', device='drive0',
+                                                       filename=new_img,
+                                                       format=iotests.imgfmt,
+                                                       read_only_mode='retain')
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_for_open()
+        self.wait_for_close()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/ro', True)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
+
+    def test_ro_rw_retain(self):
+        os.chmod(old_img, 0444)
+        self.vm.add_drive(old_img, 'media=disk,read-only=on', 'floppy')
+        self.vm.launch()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/ro', True)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img)
+
+        result = self.vm.qmp('blockdev-change-medium', device='drive0',
+                                                       filename=new_img,
+                                                       format=iotests.imgfmt,
+                                                       read_only_mode='retain')
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_for_open()
+        self.wait_for_close()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/ro', True)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
+
+    def test_rw_ro_retain(self):
+        os.chmod(new_img, 0444)
+        self.vm.add_drive(old_img, 'media=disk', 'floppy')
+        self.vm.launch()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/ro', False)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img)
+
+        result = self.vm.qmp('blockdev-change-medium', device='drive0',
+                                                       filename=new_img,
+                                                       format=iotests.imgfmt,
+                                                       read_only_mode='retain')
+        self.assert_qmp(result, 'error/class', 'GenericError')
+
+        self.assertEquals(self.vm.get_qmp_events(wait=False), [])
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/ro', False)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img)
+
+    def test_ro_rw(self):
+        os.chmod(old_img, 0444)
+        self.vm.add_drive(old_img, 'media=disk,read-only=on', 'floppy')
+        self.vm.launch()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/ro', True)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img)
+
+        result = self.vm.qmp('blockdev-change-medium',
+                             device='drive0',
+                             filename=new_img,
+                             format=iotests.imgfmt,
+                             read_only_mode='read-write')
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_for_open()
+        self.wait_for_close()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/ro', False)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
+
+    def test_rw_ro(self):
+        os.chmod(new_img, 0444)
+        self.vm.add_drive(old_img, 'media=disk', 'floppy')
+        self.vm.launch()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/ro', False)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img)
+
+        result = self.vm.qmp('blockdev-change-medium',
+                             device='drive0',
+                             filename=new_img,
+                             format=iotests.imgfmt,
+                             read_only_mode='read-only')
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_for_open()
+        self.wait_for_close()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/ro', True)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
+
+    def test_make_rw_ro(self):
+        self.vm.add_drive(old_img, 'media=disk', 'floppy')
+        self.vm.launch()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/ro', False)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img)
+
+        result = self.vm.qmp('blockdev-change-medium',
+                             device='drive0',
+                             filename=new_img,
+                             format=iotests.imgfmt,
+                             read_only_mode='read-only')
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_for_open()
+        self.wait_for_close()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/ro', True)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
+
+    def test_make_ro_rw(self):
+        os.chmod(new_img, 0444)
+        self.vm.add_drive(old_img, 'media=disk', 'floppy')
+        self.vm.launch()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/ro', False)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img)
+
+        result = self.vm.qmp('blockdev-change-medium',
+                             device='drive0',
+                             filename=new_img,
+                             format=iotests.imgfmt,
+                             read_only_mode='read-write')
+        self.assert_qmp(result, 'error/class', 'GenericError')
+
+        self.assertEquals(self.vm.get_qmp_events(wait=False), [])
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/ro', False)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img)
+
+    def test_make_rw_ro_by_retain(self):
+        os.chmod(old_img, 0444)
+        self.vm.add_drive(old_img, 'media=disk,read-only=on', 'floppy')
+        self.vm.launch()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/ro', True)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img)
+
+        result = self.vm.qmp('blockdev-change-medium', device='drive0',
+                                                       filename=new_img,
+                                                       format=iotests.imgfmt,
+                                                       read_only_mode='retain')
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_for_open()
+        self.wait_for_close()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/ro', True)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
+
+    def test_make_ro_rw_by_retain(self):
+        os.chmod(new_img, 0444)
+        self.vm.add_drive(old_img, 'media=disk', 'floppy')
+        self.vm.launch()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/ro', False)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img)
+
+        result = self.vm.qmp('blockdev-change-medium', device='drive0',
+                                                       filename=new_img,
+                                                       format=iotests.imgfmt,
+                                                       read_only_mode='retain')
+        self.assert_qmp(result, 'error/class', 'GenericError')
+
+        self.assertEquals(self.vm.get_qmp_events(wait=False), [])
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/ro', False)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img)
+
+    def test_rw_ro_cycle(self):
+        os.chmod(new_img, 0444)
+        self.vm.add_drive(old_img, 'media=disk', 'floppy')
+        self.vm.launch()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/ro', False)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img)
+
+        result = self.vm.qmp('blockdev-add',
+                             options={'node-name': 'new',
+                                      'driver': iotests.imgfmt,
+                                      'read-only': True,
+                                      'file': {'filename': new_img,
+                                               'driver': 'file'}})
+        self.assert_qmp(result, 'return', {})
+
+        result = self.vm.qmp('blockdev-open-tray', device='drive0', force=True)
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_for_open()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', True)
+        self.assert_qmp(result, 'return[0]/inserted/ro', False)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img)
+
+        result = self.vm.qmp('blockdev-remove-medium', device='drive0')
+        self.assert_qmp(result, 'return', {})
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', True)
+        self.assert_qmp_absent(result, 'return[0]/inserted')
+
+        result = self.vm.qmp('blockdev-insert-medium', device='drive0',
+                                                       node_name='new')
+        self.assert_qmp(result, 'return', {})
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', True)
+        self.assert_qmp(result, 'return[0]/inserted/ro', True)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
+
+        result = self.vm.qmp('blockdev-close-tray', device='drive0')
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_for_close()
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/ro', True)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
+
+GeneralChangeTestsBaseClass = None
+TestInitiallyFilled = None
+TestInitiallyEmpty = None
+
+
+class TestBlockJobsAfterCycle(ChangeBaseClass):
+    def setUp(self):
+        qemu_img('create', '-f', iotests.imgfmt, old_img, '1M')
+
+        self.vm = iotests.VM()
+        self.vm.launch()
+
+        result = self.vm.qmp('blockdev-add',
+                             options={'id': 'drive0',
+                                      'driver': 'null-co'})
+        self.assert_qmp(result, 'return', {})
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/image/format', 'null-co')
+
+        # For device-less BBs, calling blockdev-open-tray or blockdev-close-tray
+        # is not necessary
+        result = self.vm.qmp('blockdev-remove-medium', device='drive0')
+        self.assert_qmp(result, 'return', {})
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp_absent(result, 'return[0]/inserted')
+
+        result = self.vm.qmp('blockdev-add',
+                             options={'node-name': 'node0',
+                                      'driver': iotests.imgfmt,
+                                      'file': {'filename': old_img,
+                                               'driver': 'file'}})
+        self.assert_qmp(result, 'return', {})
+
+        result = self.vm.qmp('blockdev-insert-medium', device='drive0',
+                                                       node_name='node0')
+        self.assert_qmp(result, 'return', {})
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img)
+
+    def tearDown(self):
+        self.vm.shutdown()
+        os.remove(old_img)
+        try:
+            os.remove(new_img)
+        except OSError:
+            pass
+
+    def test_snapshot_and_commit(self):
+        # We need backing file support
+        if iotests.imgfmt != 'qcow2' and iotests.imgfmt != 'qed':
+            return
+
+        result = self.vm.qmp('blockdev-snapshot-sync', device='drive0',
+                                                       snapshot_file=new_img,
+                                                       format=iotests.imgfmt)
+        self.assert_qmp(result, 'return', {})
+
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
+        self.assert_qmp(result,
+                        'return[0]/inserted/image/backing-image/filename',
+                        old_img)
+
+        result = self.vm.qmp('block-commit', device='drive0')
+        self.assert_qmp(result, 'return', {})
+
+        self.vm.event_wait(name='BLOCK_JOB_READY')
+
+        result = self.vm.qmp('query-block-jobs')
+        self.assert_qmp(result, 'return[0]/device', 'drive0')
+
+        result = self.vm.qmp('block-job-complete', device='drive0')
+        self.assert_qmp(result, 'return', {})
+
+        self.vm.event_wait(name='BLOCK_JOB_COMPLETED')
+
+
+if __name__ == '__main__':
+    if iotests.qemu_default_machine != 'pc':
+        # We need floppy and IDE CD-ROM
+        iotests.notrun('not suitable for this machine type: %s' %
+                       iotests.qemu_default_machine)
+    iotests.main()
diff --git a/tests/qemu-iotests/118.out b/tests/qemu-iotests/118.out
new file mode 100644
index 0000000..6a91713
--- /dev/null
+++ b/tests/qemu-iotests/118.out
@@ -0,0 +1,5 @@
+...........................................................
+----------------------------------------------------------------------
+Ran 59 tests
+
+OK
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 30c784e..a6a61ae 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -122,6 +122,7 @@
 114 rw auto quick
 115 rw auto
 116 rw auto quick
+118 rw auto
 119 rw auto quick
 120 rw auto quick
 121 rw auto
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 17/37] block: check for existing device IDs in external_snapshot_prepare()
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (15 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 16/37] iotests: Add test for change-related QMP commands Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 18/37] block: rename BlockdevSnapshot to BlockdevSnapshotSync Kevin Wolf
                   ` (20 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Alberto Garcia <berto@igalia.com>

The 'snapshot-node-name' parameter of blockdev-snapshot-sync allows
setting the node name of the image that is going to be created.

Before creating the image, external_snapshot_prepare() checks that the
name is not already being used. The check is however incomplete since
it only considers existing node names, but node names must not clash
with device IDs either because they share the same namespace.

If the user attempts to create a snapshot using the name of an
existing device for the 'snapshot-node-name' parameter the operation
will eventually fail, but only after the new image has been created.

This patch replaces bdrv_find_node() with bdrv_lookup_bs() to extend
the check to existing device IDs, and thus detect possible name
clashes before the new image is created.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 005fcc5..2c2bf03 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1569,8 +1569,9 @@ static void external_snapshot_prepare(BlkTransactionState *common,
         return;
     }
 
-    if (has_snapshot_node_name && bdrv_find_node(snapshot_node_name)) {
-        error_setg(errp, "New snapshot node name already existing");
+    if (has_snapshot_node_name &&
+        bdrv_lookup_bs(snapshot_node_name, snapshot_node_name, NULL)) {
+        error_setg(errp, "New snapshot node name already in use");
         return;
     }
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 18/37] block: rename BlockdevSnapshot to BlockdevSnapshotSync
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (16 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 17/37] block: check for existing device IDs in external_snapshot_prepare() Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 19/37] block: support passing 'backing': '' to 'blockdev-add' Kevin Wolf
                   ` (19 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Alberto Garcia <berto@igalia.com>

We will introduce the 'blockdev-snapshot' command that will require
its own struct for the parameters, so we need to rename this one in
order to avoid name clashes.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c           | 2 +-
 qapi-schema.json     | 2 +-
 qapi/block-core.json | 8 ++++----
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 2c2bf03..2237aaf 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1158,7 +1158,7 @@ void qmp_blockdev_snapshot_sync(bool has_device, const char *device,
                                 bool has_format, const char *format,
                                 bool has_mode, NewImageMode mode, Error **errp)
 {
-    BlockdevSnapshot snapshot = {
+    BlockdevSnapshotSync snapshot = {
         .has_device = has_device,
         .device = (char *) device,
         .has_node_name = has_node_name,
diff --git a/qapi-schema.json b/qapi-schema.json
index 058b8ec..60827a4 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1534,7 +1534,7 @@
 ##
 { 'union': 'TransactionAction',
   'data': {
-       'blockdev-snapshot-sync': 'BlockdevSnapshot',
+       'blockdev-snapshot-sync': 'BlockdevSnapshotSync',
        'drive-backup': 'DriveBackup',
        'blockdev-backup': 'BlockdevBackup',
        'abort': 'Abort',
diff --git a/qapi/block-core.json b/qapi/block-core.json
index fa08ba9..f592adc 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -682,7 +682,7 @@
   'data': [ 'existing', 'absolute-paths' ] }
 
 ##
-# @BlockdevSnapshot
+# @BlockdevSnapshotSync
 #
 # Either @device or @node-name must be set but not both.
 #
@@ -699,7 +699,7 @@
 # @mode: #optional whether and how QEMU should create a new image, default is
 #        'absolute-paths'.
 ##
-{ 'struct': 'BlockdevSnapshot',
+{ 'struct': 'BlockdevSnapshotSync',
   'data': { '*device': 'str', '*node-name': 'str',
             'snapshot-file': 'str', '*snapshot-node-name': 'str',
             '*format': 'str', '*mode': 'NewImageMode' } }
@@ -790,7 +790,7 @@
 #
 # Generates a synchronous snapshot of a block device.
 #
-# For the arguments, see the documentation of BlockdevSnapshot.
+# For the arguments, see the documentation of BlockdevSnapshotSync.
 #
 # Returns: nothing on success
 #          If @device is not a valid block device, DeviceNotFound
@@ -798,7 +798,7 @@
 # Since 0.14.0
 ##
 { 'command': 'blockdev-snapshot-sync',
-  'data': 'BlockdevSnapshot' }
+  'data': 'BlockdevSnapshotSync' }
 
 ##
 # @change-backing-file
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 19/37] block: support passing 'backing': '' to 'blockdev-add'
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (17 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 18/37] block: rename BlockdevSnapshot to BlockdevSnapshotSync Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 20/37] block: add a 'blockdev-snapshot' QMP command Kevin Wolf
                   ` (18 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Alberto Garcia <berto@igalia.com>

Passing an empty string allows opening an image but not its backing
file. This was already described in the API documentation, only the
implementation was missing.

This is useful for creating snapshots using images opened with
blockdev-add, since they are not supposed to have a backing image
before the operation.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/block.c b/block.c
index a99e6d8..3493501 100644
--- a/block.c
+++ b/block.c
@@ -1393,6 +1393,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
     BlockDriverState *bs;
     BlockDriver *drv = NULL;
     const char *drvname;
+    const char *backing;
     Error *local_err = NULL;
     int snapshot_flags = 0;
 
@@ -1460,6 +1461,12 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
 
     assert(drvname || !(flags & BDRV_O_PROTOCOL));
 
+    backing = qdict_get_try_str(options, "backing");
+    if (backing && *backing == '\0') {
+        flags |= BDRV_O_NO_BACKING;
+        qdict_del(options, "backing");
+    }
+
     bs->open_flags = flags;
     bs->options = options;
     options = qdict_clone_shallow(options);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 20/37] block: add a 'blockdev-snapshot' QMP command
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (18 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 19/37] block: support passing 'backing': '' to 'blockdev-add' Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 21/37] block: add tests for the 'blockdev-snapshot' command Kevin Wolf
                   ` (17 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Alberto Garcia <berto@igalia.com>

One of the limitations of the 'blockdev-snapshot-sync' command is that
it does not allow passing BlockdevOptions to the newly created
snapshots, so they are always opened using the default values.

Extending the command to allow passing options is not a practical
solution because there is overlap between those options and some of
the existing parameters of the command.

This patch introduces a new 'blockdev-snapshot' command with a simpler
interface: it just takes two references to existing block devices that
will be used as the source and target for the snapshot.

Since the main difference between the two commands is that one of them
creates and opens the target image, while the other uses an already
opened one, the bulk of the implementation is shared.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c           | 165 ++++++++++++++++++++++++++++++++-------------------
 qapi-schema.json     |   2 +
 qapi/block-core.json |  28 +++++++++
 qmp-commands.hx      |  38 ++++++++++++
 4 files changed, 172 insertions(+), 61 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 2237aaf..3a5183d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1175,6 +1175,18 @@ void qmp_blockdev_snapshot_sync(bool has_device, const char *device,
                        &snapshot, errp);
 }
 
+void qmp_blockdev_snapshot(const char *node, const char *overlay,
+                           Error **errp)
+{
+    BlockdevSnapshot snapshot_data = {
+        .node = (char *) node,
+        .overlay = (char *) overlay
+    };
+
+    blockdev_do_action(TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT,
+                       &snapshot_data, errp);
+}
+
 void qmp_blockdev_snapshot_internal_sync(const char *device,
                                          const char *name,
                                          Error **errp)
@@ -1520,58 +1532,48 @@ typedef struct ExternalSnapshotState {
 static void external_snapshot_prepare(BlkTransactionState *common,
                                       Error **errp)
 {
-    int flags, ret;
-    QDict *options;
+    int flags = 0, ret;
+    QDict *options = NULL;
     Error *local_err = NULL;
-    bool has_device = false;
+    /* Device and node name of the image to generate the snapshot from */
     const char *device;
-    bool has_node_name = false;
     const char *node_name;
-    bool has_snapshot_node_name = false;
-    const char *snapshot_node_name;
+    /* Reference to the new image (for 'blockdev-snapshot') */
+    const char *snapshot_ref;
+    /* File name of the new image (for 'blockdev-snapshot-sync') */
     const char *new_image_file;
-    const char *format = "qcow2";
-    enum NewImageMode mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
     ExternalSnapshotState *state =
                              DO_UPCAST(ExternalSnapshotState, common, common);
     TransactionAction *action = common->action;
 
-    /* get parameters */
-    g_assert(action->type == TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC);
-
-    has_device = action->u.blockdev_snapshot_sync->has_device;
-    device = action->u.blockdev_snapshot_sync->device;
-    has_node_name = action->u.blockdev_snapshot_sync->has_node_name;
-    node_name = action->u.blockdev_snapshot_sync->node_name;
-    has_snapshot_node_name =
-        action->u.blockdev_snapshot_sync->has_snapshot_node_name;
-    snapshot_node_name = action->u.blockdev_snapshot_sync->snapshot_node_name;
-
-    new_image_file = action->u.blockdev_snapshot_sync->snapshot_file;
-    if (action->u.blockdev_snapshot_sync->has_format) {
-        format = action->u.blockdev_snapshot_sync->format;
-    }
-    if (action->u.blockdev_snapshot_sync->has_mode) {
-        mode = action->u.blockdev_snapshot_sync->mode;
+    /* 'blockdev-snapshot' and 'blockdev-snapshot-sync' have similar
+     * purpose but a different set of parameters */
+    switch (action->type) {
+    case TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT:
+        {
+            BlockdevSnapshot *s = action->u.blockdev_snapshot;
+            device = s->node;
+            node_name = s->node;
+            new_image_file = NULL;
+            snapshot_ref = s->overlay;
+        }
+        break;
+    case TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC:
+        {
+            BlockdevSnapshotSync *s = action->u.blockdev_snapshot_sync;
+            device = s->has_device ? s->device : NULL;
+            node_name = s->has_node_name ? s->node_name : NULL;
+            new_image_file = s->snapshot_file;
+            snapshot_ref = NULL;
+        }
+        break;
+    default:
+        g_assert_not_reached();
     }
 
     /* start processing */
-    state->old_bs = bdrv_lookup_bs(has_device ? device : NULL,
-                                   has_node_name ? node_name : NULL,
-                                   &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        return;
-    }
-
-    if (has_node_name && !has_snapshot_node_name) {
-        error_setg(errp, "New snapshot node name missing");
-        return;
-    }
-
-    if (has_snapshot_node_name &&
-        bdrv_lookup_bs(snapshot_node_name, snapshot_node_name, NULL)) {
-        error_setg(errp, "New snapshot node name already in use");
+    state->old_bs = bdrv_lookup_bs(device, node_name, errp);
+    if (!state->old_bs) {
         return;
     }
 
@@ -1602,35 +1604,70 @@ static void external_snapshot_prepare(BlkTransactionState *common,
         return;
     }
 
-    flags = state->old_bs->open_flags;
+    if (action->type == TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC) {
+        BlockdevSnapshotSync *s = action->u.blockdev_snapshot_sync;
+        const char *format = s->has_format ? s->format : "qcow2";
+        enum NewImageMode mode;
+        const char *snapshot_node_name =
+            s->has_snapshot_node_name ? s->snapshot_node_name : NULL;
 
-    /* create new image w/backing file */
-    if (mode != NEW_IMAGE_MODE_EXISTING) {
-        bdrv_img_create(new_image_file, format,
-                        state->old_bs->filename,
-                        state->old_bs->drv->format_name,
-                        NULL, -1, flags, &local_err, false);
-        if (local_err) {
-            error_propagate(errp, local_err);
+        if (node_name && !snapshot_node_name) {
+            error_setg(errp, "New snapshot node name missing");
             return;
         }
-    }
 
-    options = qdict_new();
-    if (has_snapshot_node_name) {
-        qdict_put(options, "node-name",
-                  qstring_from_str(snapshot_node_name));
+        if (snapshot_node_name &&
+            bdrv_lookup_bs(snapshot_node_name, snapshot_node_name, NULL)) {
+            error_setg(errp, "New snapshot node name already in use");
+            return;
+        }
+
+        flags = state->old_bs->open_flags;
+
+        /* create new image w/backing file */
+        mode = s->has_mode ? s->mode : NEW_IMAGE_MODE_ABSOLUTE_PATHS;
+        if (mode != NEW_IMAGE_MODE_EXISTING) {
+            bdrv_img_create(new_image_file, format,
+                            state->old_bs->filename,
+                            state->old_bs->drv->format_name,
+                            NULL, -1, flags, &local_err, false);
+            if (local_err) {
+                error_propagate(errp, local_err);
+                return;
+            }
+        }
+
+        options = qdict_new();
+        if (s->has_snapshot_node_name) {
+            qdict_put(options, "node-name",
+                      qstring_from_str(snapshot_node_name));
+        }
+        qdict_put(options, "driver", qstring_from_str(format));
+
+        flags |= BDRV_O_NO_BACKING;
     }
-    qdict_put(options, "driver", qstring_from_str(format));
 
-    /* TODO Inherit bs->options or only take explicit options with an
-     * extended QMP command? */
     assert(state->new_bs == NULL);
-    ret = bdrv_open(&state->new_bs, new_image_file, NULL, options,
-                    flags | BDRV_O_NO_BACKING, &local_err);
+    ret = bdrv_open(&state->new_bs, new_image_file, snapshot_ref, options,
+                    flags, errp);
     /* We will manually add the backing_hd field to the bs later */
     if (ret != 0) {
-        error_propagate(errp, local_err);
+        return;
+    }
+
+    if (state->new_bs->blk != NULL) {
+        error_setg(errp, "The snapshot is already in use by %s",
+                   blk_name(state->new_bs->blk));
+        return;
+    }
+
+    if (bdrv_op_is_blocked(state->new_bs, BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT,
+                           errp)) {
+        return;
+    }
+
+    if (state->new_bs->backing != NULL) {
+        error_setg(errp, "The snapshot already has a backing image");
     }
 }
 
@@ -1833,6 +1870,12 @@ static void abort_commit(BlkTransactionState *common)
 }
 
 static const BdrvActionOps actions[] = {
+    [TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT] = {
+        .instance_size = sizeof(ExternalSnapshotState),
+        .prepare  = external_snapshot_prepare,
+        .commit   = external_snapshot_commit,
+        .abort = external_snapshot_abort,
+    },
     [TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC] = {
         .instance_size = sizeof(ExternalSnapshotState),
         .prepare  = external_snapshot_prepare,
diff --git a/qapi-schema.json b/qapi-schema.json
index 60827a4..2d8fa7c 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1531,9 +1531,11 @@
 # abort since 1.6
 # blockdev-snapshot-internal-sync since 1.7
 # blockdev-backup since 2.3
+# blockdev-snapshot since 2.5
 ##
 { 'union': 'TransactionAction',
   'data': {
+       'blockdev-snapshot': 'BlockdevSnapshot',
        'blockdev-snapshot-sync': 'BlockdevSnapshotSync',
        'drive-backup': 'DriveBackup',
        'blockdev-backup': 'BlockdevBackup',
diff --git a/qapi/block-core.json b/qapi/block-core.json
index f592adc..083d2cd 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -705,6 +705,21 @@
             '*format': 'str', '*mode': 'NewImageMode' } }
 
 ##
+# @BlockdevSnapshot
+#
+# @node: device or node name that will have a snapshot created.
+#
+# @overlay: reference to the existing block device that will become
+#           the overlay of @node, as part of creating the snapshot.
+#           It must not have a current backing file (this can be
+#           achieved by passing "backing": "" to blockdev-add).
+#
+# Since 2.5
+##
+{ 'struct': 'BlockdevSnapshot',
+  'data': { 'node': 'str', 'overlay': 'str' } }
+
+##
 # @DriveBackup
 #
 # @device: the name of the device which should be copied.
@@ -800,6 +815,19 @@
 { 'command': 'blockdev-snapshot-sync',
   'data': 'BlockdevSnapshotSync' }
 
+
+##
+# @blockdev-snapshot
+#
+# Generates a snapshot of a block device.
+#
+# For the arguments, see the documentation of BlockdevSnapshot.
+#
+# Since 2.5
+##
+{ 'command': 'blockdev-snapshot',
+  'data': 'BlockdevSnapshot' }
+
 ##
 # @change-backing-file
 #
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 707e0bd..bd4d38f 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1476,6 +1476,44 @@ Example:
 EQMP
 
     {
+        .name       = "blockdev-snapshot",
+        .args_type  = "node:s,overlay:s",
+        .mhandler.cmd_new = qmp_marshal_blockdev_snapshot,
+    },
+
+SQMP
+blockdev-snapshot
+-----------------
+Since 2.5
+
+Create a snapshot, by installing 'node' as the backing image of
+'overlay'. Additionally, if 'node' is associated with a block
+device, the block device changes to using 'overlay' as its new active
+image.
+
+Arguments:
+
+- "node": device that will have a snapshot created (json-string)
+- "overlay": device that will have 'node' as its backing image (json-string)
+
+Example:
+
+-> { "execute": "blockdev-add",
+                "arguments": { "options": { "driver": "qcow2",
+                                            "node-name": "node1534",
+                                            "file": { "driver": "file",
+                                                      "filename": "hd1.qcow2" },
+                                            "backing": "" } } }
+
+<- { "return": {} }
+
+-> { "execute": "blockdev-snapshot", "arguments": { "node": "ide-hd0",
+                                                    "overlay": "node1534" } }
+<- { "return": {} }
+
+EQMP
+
+    {
         .name       = "blockdev-snapshot-internal-sync",
         .args_type  = "device:B,name:s",
         .mhandler.cmd_new = qmp_marshal_blockdev_snapshot_internal_sync,
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 21/37] block: add tests for the 'blockdev-snapshot' command
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (19 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 20/37] block: add a 'blockdev-snapshot' QMP command Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 22/37] commit: reopen overlay_bs before base Kevin Wolf
                   ` (16 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Alberto Garcia <berto@igalia.com>

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/085     | 102 ++++++++++++++++++++++++++++++++++++++++++---
 tests/qemu-iotests/085.out |  34 ++++++++++++++-
 2 files changed, 128 insertions(+), 8 deletions(-)

diff --git a/tests/qemu-iotests/085 b/tests/qemu-iotests/085
index 56cd6f8..9484117 100755
--- a/tests/qemu-iotests/085
+++ b/tests/qemu-iotests/085
@@ -7,6 +7,7 @@
 # snapshots are performed.
 #
 # Copyright (C) 2014 Red Hat, Inc.
+# Copyright (C) 2015 Igalia, S.L.
 #
 # 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
@@ -34,17 +35,17 @@ status=1	# failure is the default!
 snapshot_virt0="snapshot-v0.qcow2"
 snapshot_virt1="snapshot-v1.qcow2"
 
-MAX_SNAPSHOTS=10
+SNAPSHOTS=10
 
 _cleanup()
 {
     _cleanup_qemu
-    for i in $(seq 1 ${MAX_SNAPSHOTS})
+    for i in $(seq 1 ${SNAPSHOTS})
     do
         rm -f "${TEST_DIR}/${i}-${snapshot_virt0}"
         rm -f "${TEST_DIR}/${i}-${snapshot_virt1}"
     done
-	_cleanup_test_img
+    rm -f "${TEST_IMG}.1" "${TEST_IMG}.2"
 
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -85,18 +86,50 @@ function create_group_snapshot()
     _send_qemu_cmd $h "${cmd}" "return"
 }
 
+# ${1}: unique identifier for the snapshot filename
+# ${2}: true: open backing images; false: don't open them (default)
+function add_snapshot_image()
+{
+    if [ "${2}" = "true" ]; then
+        extra_params=""
+    else
+        extra_params="'backing': '', "
+    fi
+    base_image="${TEST_DIR}/$((${1}-1))-${snapshot_virt0}"
+    snapshot_file="${TEST_DIR}/${1}-${snapshot_virt0}"
+    _make_test_img -b "${base_image}" "$size"
+    mv "${TEST_IMG}" "${snapshot_file}"
+    cmd="{ 'execute': 'blockdev-add', 'arguments':
+           { 'options':
+             { 'driver': 'qcow2', 'node-name': 'snap_"${1}"', "${extra_params}"
+               'file':
+               { 'driver': 'file', 'filename': '"${snapshot_file}"' } } } }"
+    _send_qemu_cmd $h "${cmd}" "return"
+}
+
+# ${1}: unique identifier for the snapshot filename
+# ${2}: expected response, defaults to 'return'
+function blockdev_snapshot()
+{
+    cmd="{ 'execute': 'blockdev-snapshot',
+                      'arguments': { 'node': 'virtio0',
+                                     'overlay':'snap_"${1}"' } }"
+    _send_qemu_cmd $h "${cmd}" "${2:-return}"
+}
+
 size=128M
 
 _make_test_img $size
-mv "${TEST_IMG}" "${TEST_IMG}.orig"
+mv "${TEST_IMG}" "${TEST_IMG}.1"
 _make_test_img $size
+mv "${TEST_IMG}" "${TEST_IMG}.2"
 
 echo
 echo === Running QEMU ===
 echo
 
 qemu_comm_method="qmp"
-_launch_qemu -drive file="${TEST_IMG}.orig",if=virtio -drive file="${TEST_IMG}",if=virtio
+_launch_qemu -drive file="${TEST_IMG}.1",if=virtio -drive file="${TEST_IMG}.2",if=virtio
 h=$QEMU_HANDLE
 
 echo
@@ -105,6 +138,8 @@ echo
 
 _send_qemu_cmd $h "{ 'execute': 'qmp_capabilities' }" "return"
 
+# Tests for the blockdev-snapshot-sync command
+
 echo
 echo === Create a single snapshot on virtio0 ===
 echo
@@ -132,11 +167,66 @@ echo
 echo === Create several transactional group snapshots ===
 echo
 
-for i in $(seq 2 ${MAX_SNAPSHOTS})
+for i in $(seq 2 ${SNAPSHOTS})
 do
     create_group_snapshot ${i}
 done
 
+# Tests for the blockdev-snapshot command
+
+echo
+echo === Create a couple of snapshots using blockdev-snapshot ===
+echo
+
+SNAPSHOTS=$((${SNAPSHOTS}+1))
+add_snapshot_image ${SNAPSHOTS}
+blockdev_snapshot ${SNAPSHOTS}
+
+SNAPSHOTS=$((${SNAPSHOTS}+1))
+add_snapshot_image ${SNAPSHOTS}
+blockdev_snapshot ${SNAPSHOTS}
+
+echo
+echo === Invalid command - snapshot node used as active layer ===
+echo
+
+blockdev_snapshot ${SNAPSHOTS} error
+
+_send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot',
+                     'arguments': { 'node':'virtio0',
+                                    'overlay':'virtio0' }
+                   }" "error"
+
+_send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot',
+                     'arguments': { 'node':'virtio0',
+                                    'overlay':'virtio1' }
+                   }" "error"
+
+echo
+echo === Invalid command - snapshot node used as backing hd ===
+echo
+
+blockdev_snapshot $((${SNAPSHOTS}-1)) error
+
+echo
+echo === Invalid command - snapshot node has a backing image ===
+echo
+
+SNAPSHOTS=$((${SNAPSHOTS}+1))
+add_snapshot_image ${SNAPSHOTS} true
+blockdev_snapshot ${SNAPSHOTS} error
+
+echo
+echo === Invalid command - The node does not exist ===
+echo
+
+blockdev_snapshot $((${SNAPSHOTS}+1)) error
+
+_send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot',
+                     'arguments': { 'node':'nodevice',
+                                    'overlay':'snap_"${SNAPSHOTS}"' }
+                   }" "error"
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/085.out b/tests/qemu-iotests/085.out
index a6cf19e..52292ea 100644
--- a/tests/qemu-iotests/085.out
+++ b/tests/qemu-iotests/085.out
@@ -11,7 +11,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 
 === Create a single snapshot on virtio0 ===
 
-Formatting 'TEST_DIR/1-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file=TEST_DIR/t.qcow2.orig backing_fmt=qcow2 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/1-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file=TEST_DIR/t.qcow2.1 backing_fmt=qcow2 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16
 {"return": {}}
 
 === Invalid command - missing device and nodename ===
@@ -26,7 +26,7 @@ Formatting 'TEST_DIR/1-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file
 === Create several transactional group snapshots ===
 
 Formatting 'TEST_DIR/2-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file=TEST_DIR/1-snapshot-v0.qcow2 backing_fmt=qcow2 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16
-Formatting 'TEST_DIR/2-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file=TEST_DIR/t.qcow2 backing_fmt=qcow2 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/2-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file=TEST_DIR/t.qcow2.2 backing_fmt=qcow2 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16
 {"return": {}}
 Formatting 'TEST_DIR/3-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file=TEST_DIR/2-snapshot-v0.qcow2 backing_fmt=qcow2 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16
 Formatting 'TEST_DIR/3-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file=TEST_DIR/2-snapshot-v1.qcow2 backing_fmt=qcow2 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16
@@ -52,4 +52,34 @@ Formatting 'TEST_DIR/9-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file
 Formatting 'TEST_DIR/10-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file=TEST_DIR/9-snapshot-v0.qcow2 backing_fmt=qcow2 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16
 Formatting 'TEST_DIR/10-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file=TEST_DIR/9-snapshot-v1.qcow2 backing_fmt=qcow2 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16
 {"return": {}}
+
+=== Create a couple of snapshots using blockdev-snapshot ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/10-snapshot-v0.IMGFMT
+{"return": {}}
+{"return": {}}
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/11-snapshot-v0.IMGFMT
+{"return": {}}
+{"return": {}}
+
+=== Invalid command - snapshot node used as active layer ===
+
+{"error": {"class": "GenericError", "desc": "The snapshot is already in use by virtio0"}}
+{"error": {"class": "GenericError", "desc": "The snapshot is already in use by virtio0"}}
+{"error": {"class": "GenericError", "desc": "The snapshot is already in use by virtio1"}}
+
+=== Invalid command - snapshot node used as backing hd ===
+
+{"error": {"class": "GenericError", "desc": "Node 'snap_11' is busy: node is used as backing hd of 'virtio0'"}}
+
+=== Invalid command - snapshot node has a backing image ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/12-snapshot-v0.IMGFMT
+{"return": {}}
+{"error": {"class": "GenericError", "desc": "The snapshot already has a backing image"}}
+
+=== Invalid command - The node does not exist ===
+
+{"error": {"class": "GenericError", "desc": "Cannot find device=snap_14 nor node_name=snap_14"}}
+{"error": {"class": "GenericError", "desc": "Cannot find device=nodevice nor node_name=nodevice"}}
 *** done
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 22/37] commit: reopen overlay_bs before base
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (20 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 21/37] block: add tests for the 'blockdev-snapshot' command Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 23/37] qemu-iotests: Test the reopening of overlay_bs in 'block-commit' Kevin Wolf
                   ` (15 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Alberto Garcia <berto@igalia.com>

'block-commit' needs write access to two different nodes of the chain:

- 'base', because that's where the data is written to.
- the overlay of 'top', because it needs to update the backing file
  string to point to 'base' after the operation.

Both images have to be opened in read-write mode, and commit_start()
takes care of reopening them if necessary.

With the current implementation, however, when overlay_bs is reopened
in read-write mode it has the side effect of making 'base' read-only
again, eventually making 'block-commit' fail.

This needs to be fixed in bdrv_reopen(), but until we get to that it
can be worked around simply by swapping the order of base and
overlay_bs in the reopen queue.

In order to reproduce this bug, overlay_bs needs to be initially in
read-only mode. That is: the 'top' parameter of 'block-commit' cannot
be the active layer nor its immediate backing chain.

Cc: qemu-stable@nongnu.org
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/commit.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/commit.c b/block/commit.c
index fdebe87..a5d02aa 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -236,14 +236,14 @@ void commit_start(BlockDriverState *bs, BlockDriverState *base,
     orig_overlay_flags = bdrv_get_flags(overlay_bs);
 
     /* convert base & overlay_bs to r/w, if necessary */
-    if (!(orig_base_flags & BDRV_O_RDWR)) {
-        reopen_queue = bdrv_reopen_queue(reopen_queue, base, NULL,
-                                         orig_base_flags | BDRV_O_RDWR);
-    }
     if (!(orig_overlay_flags & BDRV_O_RDWR)) {
         reopen_queue = bdrv_reopen_queue(reopen_queue, overlay_bs, NULL,
                                          orig_overlay_flags | BDRV_O_RDWR);
     }
+    if (!(orig_base_flags & BDRV_O_RDWR)) {
+        reopen_queue = bdrv_reopen_queue(reopen_queue, base, NULL,
+                                         orig_base_flags | BDRV_O_RDWR);
+    }
     if (reopen_queue) {
         bdrv_reopen_multiple(reopen_queue, &local_err);
         if (local_err != NULL) {
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 23/37] qemu-iotests: Test the reopening of overlay_bs in 'block-commit'
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (21 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 22/37] commit: reopen overlay_bs before base Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 24/37] qcow2: avoid misaligned 64bit bswap Kevin Wolf
                   ` (14 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Alberto Garcia <berto@igalia.com>

The 'block-commit' command needs the overlay image of 'top' to
be opened in read-write mode in order to update the backing file
string. If 'top' is not the active layer or its backing file then its
overlay needs to be reopened during the block job.

This is a test case for that scenario.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/040     | 30 ++++++++++++++++++++++++++++++
 tests/qemu-iotests/040.out |  4 ++--
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index ea2f98e..5bdaf3d 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -41,6 +41,7 @@ class ImageCommitTestCase(iotests.QMPTestCase):
         while not completed:
             for event in self.vm.get_qmp_events(wait=True):
                 if event['event'] == 'BLOCK_JOB_COMPLETED':
+                    self.assert_qmp_absent(event, 'data/error')
                     self.assert_qmp(event, 'data/type', 'commit')
                     self.assert_qmp(event, 'data/device', 'drive0')
                     self.assert_qmp(event, 'data/offset', event['data']['len'])
@@ -251,5 +252,34 @@ class TestSetSpeed(ImageCommitTestCase):
 class TestActiveZeroLengthImage(TestSingleDrive):
     image_len = 0
 
+class TestReopenOverlay(ImageCommitTestCase):
+    image_len = 1024 * 1024
+    img0 = os.path.join(iotests.test_dir, '0.img')
+    img1 = os.path.join(iotests.test_dir, '1.img')
+    img2 = os.path.join(iotests.test_dir, '2.img')
+    img3 = os.path.join(iotests.test_dir, '3.img')
+
+    def setUp(self):
+        iotests.create_image(self.img0, self.image_len)
+        qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % self.img0, self.img1)
+        qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % self.img1, self.img2)
+        qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % self.img2, self.img3)
+        qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0xab 0 128K', self.img1)
+        self.vm = iotests.VM().add_drive(self.img3)
+        self.vm.launch()
+
+    def tearDown(self):
+        self.vm.shutdown()
+        os.remove(self.img0)
+        os.remove(self.img1)
+        os.remove(self.img2)
+        os.remove(self.img3)
+
+    # This tests what happens when the overlay image of the 'top' node
+    # needs to be reopened in read-write mode in order to update the
+    # backing image string.
+    def test_reopen_overlay(self):
+        self.run_commit_test(self.img1, self.img0)
+
 if __name__ == '__main__':
     iotests.main(supported_fmts=['qcow2', 'qed'])
diff --git a/tests/qemu-iotests/040.out b/tests/qemu-iotests/040.out
index 42314e9..4fd1c2d 100644
--- a/tests/qemu-iotests/040.out
+++ b/tests/qemu-iotests/040.out
@@ -1,5 +1,5 @@
-........................
+.........................
 ----------------------------------------------------------------------
-Ran 24 tests
+Ran 25 tests
 
 OK
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 24/37] qcow2: avoid misaligned 64bit bswap
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (22 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 23/37] qemu-iotests: Test the reopening of overlay_bs in 'block-commit' Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 25/37] qemu-img: add check for zero-length job len Kevin Wolf
                   ` (13 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: John Snow <jsnow@redhat.com>

If we create a buffer directly on the stack by using 12 bytes, there's
no guarantee the 64bit value we want to swap will be aligned, which
could cause errors with undefined behavior.

Spotted with clang -fsanitize=undefined and observed in iotests 15, 26,
44, 115 and 121.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-refcount.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 4b81c8d..6e0e5bd 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -560,13 +560,16 @@ static int alloc_refcount_block(BlockDriverState *bs,
     }
 
     /* Hook up the new refcount table in the qcow2 header */
-    uint8_t data[12];
-    cpu_to_be64w((uint64_t*)data, table_offset);
-    cpu_to_be32w((uint32_t*)(data + 8), table_clusters);
+    struct QEMU_PACKED {
+        uint64_t d64;
+        uint32_t d32;
+    } data;
+    cpu_to_be64w(&data.d64, table_offset);
+    cpu_to_be32w(&data.d32, table_clusters);
     BLKDBG_EVENT(bs->file, BLKDBG_REFBLOCK_ALLOC_SWITCH_TABLE);
     ret = bdrv_pwrite_sync(bs->file->bs,
                            offsetof(QCowHeader, refcount_table_offset),
-                           data, sizeof(data));
+                           &data, sizeof(data));
     if (ret < 0) {
         goto fail_table;
     }
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 25/37] qemu-img: add check for zero-length job len
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (23 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 24/37] qcow2: avoid misaligned 64bit bswap Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 26/37] throttle: Check for pending requests in throttle_group_unregister_bs() Kevin Wolf
                   ` (12 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: John Snow <jsnow@redhat.com>

The mirror job doesn't update its total length until
it has already started running, so we should translate
a zero-length job-len as meaning 0%.

Otherwise, we may get divide-by-zero faults.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-img.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index 3025776..9831db7 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -656,7 +656,8 @@ static void run_block_job(BlockJob *job, Error **errp)
 
     do {
         aio_poll(aio_context, true);
-        qemu_progress_print((float)job->offset / job->len * 100.f, 0);
+        qemu_progress_print(job->len ?
+                            ((float)job->offset / job->len * 100.f) : 0.0f, 0);
     } while (!job->ready);
 
     block_job_complete_sync(job, errp);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 26/37] throttle: Check for pending requests in throttle_group_unregister_bs()
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (24 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 25/37] qemu-img: add check for zero-length job len Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 27/37] throttle: Use bs->throttle_state instead of bs->io_limits_enabled Kevin Wolf
                   ` (11 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Alberto Garcia <berto@igalia.com>

throttle_group_unregister_bs() removes a BlockDriverState from its
throttling group and destroys the timers. This means that there must
be no pending throttled requests at that point (because it would be
impossible to complete them), so the caller has to drain them first.

At the moment throttle_group_unregister_bs() is only called from
bdrv_io_limits_disable(), which already takes care of draining the
requests, so there's nothing to worry about, but this patch makes
this invariant explicit in the documentation and adds the relevant
assertions.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/throttle-groups.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index 3419af7..13b5baa 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -437,6 +437,9 @@ void throttle_group_register_bs(BlockDriverState *bs, const char *groupname)
  * list, destroying the timers and setting the throttle_state pointer
  * to NULL.
  *
+ * The BlockDriverState must not have pending throttled requests, so
+ * the caller has to drain them first.
+ *
  * The group will be destroyed if it's empty after this operation.
  *
  * @bs: the BlockDriverState to remove
@@ -446,6 +449,10 @@ void throttle_group_unregister_bs(BlockDriverState *bs)
     ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts);
     int i;
 
+    assert(bs->pending_reqs[0] == 0 && bs->pending_reqs[1] == 0);
+    assert(qemu_co_queue_empty(&bs->throttled_reqs[0]));
+    assert(qemu_co_queue_empty(&bs->throttled_reqs[1]));
+
     qemu_mutex_lock(&tg->lock);
     for (i = 0; i < 2; i++) {
         if (tg->tokens[i] == bs) {
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 27/37] throttle: Use bs->throttle_state instead of bs->io_limits_enabled
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (25 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 26/37] throttle: Check for pending requests in throttle_group_unregister_bs() Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 28/37] block: Disallow snapshots if the overlay doesn't support backing files Kevin Wolf
                   ` (10 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Alberto Garcia <berto@igalia.com>

There are two ways to check for I/O limits in a BlockDriverState:

- bs->throttle_state: if this pointer is not NULL, it means that this
  BDS is member of a throttling group, its ThrottleTimers structure
  has been initialized and its I/O limits are ready to be applied.

- bs->io_limits_enabled: if true it means that the throttle_state
  pointer is valid _and_ the limits are currently enabled.

The latter is used in several places to check whether a BDS has I/O
limits configured, but what it really checks is whether requests
are being throttled or not. For example, io_limits_enabled can be
temporarily set to false in cases like bdrv_read_unthrottled() without
otherwise touching the throtting configuration of that BDS.

This patch replaces bs->io_limits_enabled with bs->throttle_state in
all cases where what we really want to check is the existence of I/O
limits, not whether they are currently enabled or not.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c                   | 6 +++---
 block/qapi.c              | 2 +-
 blockdev.c                | 4 ++--
 include/block/block_int.h | 5 ++++-
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/block.c b/block.c
index 3493501..cffac75 100644
--- a/block.c
+++ b/block.c
@@ -1907,7 +1907,7 @@ void bdrv_close(BlockDriverState *bs)
     }
 
     /* Disable I/O limits and drain all pending throttled requests */
-    if (bs->io_limits_enabled) {
+    if (bs->throttle_state) {
         bdrv_io_limits_disable(bs);
     }
 
@@ -3712,7 +3712,7 @@ void bdrv_detach_aio_context(BlockDriverState *bs)
         baf->detach_aio_context(baf->opaque);
     }
 
-    if (bs->io_limits_enabled) {
+    if (bs->throttle_state) {
         throttle_timers_detach_aio_context(&bs->throttle_timers);
     }
     if (bs->drv->bdrv_detach_aio_context) {
@@ -3748,7 +3748,7 @@ void bdrv_attach_aio_context(BlockDriverState *bs,
     if (bs->drv->bdrv_attach_aio_context) {
         bs->drv->bdrv_attach_aio_context(bs, new_context);
     }
-    if (bs->io_limits_enabled) {
+    if (bs->throttle_state) {
         throttle_timers_attach_aio_context(&bs->throttle_timers, new_context);
     }
 
diff --git a/block/qapi.c b/block/qapi.c
index ec0f513..89d4274 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -64,7 +64,7 @@ BlockDeviceInfo *bdrv_block_device_info(BlockDriverState *bs, Error **errp)
     info->backing_file_depth = bdrv_get_backing_file_depth(bs);
     info->detect_zeroes = bs->detect_zeroes;
 
-    if (bs->io_limits_enabled) {
+    if (bs->throttle_state) {
         ThrottleConfig cfg;
 
         throttle_group_get_config(bs, &cfg);
diff --git a/blockdev.c b/blockdev.c
index 3a5183d..2bf18a7 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2351,14 +2351,14 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
     if (throttle_enabled(&cfg)) {
         /* Enable I/O limits if they're not enabled yet, otherwise
          * just update the throttling group. */
-        if (!bs->io_limits_enabled) {
+        if (!bs->throttle_state) {
             bdrv_io_limits_enable(bs, has_group ? group : device);
         } else if (has_group) {
             bdrv_io_limits_update_group(bs, group);
         }
         /* Set the new throttling configuration */
         bdrv_set_io_limits(bs, &cfg);
-    } else if (bs->io_limits_enabled) {
+    } else if (bs->throttle_state) {
         /* If all throttling settings are set to 0, disable I/O limits */
         bdrv_io_limits_disable(bs);
     }
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 6a3f64d..603145a 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -390,7 +390,10 @@ struct BlockDriverState {
     /* number of in-flight serialising requests */
     unsigned int serialising_in_flight;
 
-    /* I/O throttling */
+    /* I/O throttling.
+     * throttle_state tells us if this BDS has I/O limits configured.
+     * io_limits_enabled tells us if they are currently being
+     * enforced, but it can be temporarily set to false */
     CoQueue      throttled_reqs[2];
     bool         io_limits_enabled;
     /* The following fields are protected by the ThrottleGroup lock.
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 28/37] block: Disallow snapshots if the overlay doesn't support backing files
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (26 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 27/37] throttle: Use bs->throttle_state instead of bs->io_limits_enabled Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 29/37] block: Remove inner quotation marks in iotest 085 Kevin Wolf
                   ` (9 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Alberto Garcia <berto@igalia.com>

This addresses scenarios like this one:

  { 'execute': 'blockdev-add', 'arguments':
    { 'options': { 'driver': 'qcow2',
                   'node-name': 'new0',
                   'file': { 'driver': 'file',
                             'filename': 'new.qcow2',
                             'node-name': 'file0' } } } }

  { 'execute': 'blockdev-snapshot', 'arguments':
    { 'node': 'virtio0',
      'overlay': 'file0' } }

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 2bf18a7..8749d78 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1668,6 +1668,11 @@ static void external_snapshot_prepare(BlkTransactionState *common,
 
     if (state->new_bs->backing != NULL) {
         error_setg(errp, "The snapshot already has a backing image");
+        return;
+    }
+
+    if (!state->new_bs->drv->supports_backing) {
+        error_setg(errp, "The snapshot does not support backing images");
     }
 }
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 29/37] block: Remove inner quotation marks in iotest 085
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (27 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 28/37] block: Disallow snapshots if the overlay doesn't support backing files Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 30/37] block: test 'blockdev-snapshot' using a file BDS as the overlay Kevin Wolf
                   ` (8 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Alberto Garcia <berto@igalia.com>

This patch removes the inner quotation marks in all cases like this:

   cmd=" ... "${variable}" ... "

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/085 | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/tests/qemu-iotests/085 b/tests/qemu-iotests/085
index 9484117..80e547d 100755
--- a/tests/qemu-iotests/085
+++ b/tests/qemu-iotests/085
@@ -65,7 +65,7 @@ function create_single_snapshot()
 {
     cmd="{ 'execute': 'blockdev-snapshot-sync',
                       'arguments': { 'device': 'virtio0',
-                                     'snapshot-file':'"${TEST_DIR}/${1}-${snapshot_virt0}"',
+                                     'snapshot-file':'${TEST_DIR}/${1}-${snapshot_virt0}',
                                      'format': 'qcow2' } }"
     _send_qemu_cmd $h "${cmd}" "return"
 }
@@ -77,10 +77,10 @@ function create_group_snapshot()
            {'actions': [
                { 'type': 'blockdev-snapshot-sync', 'data' :
                    { 'device': 'virtio0',
-                      'snapshot-file': '"${TEST_DIR}/${1}-${snapshot_virt0}"' } },
+                      'snapshot-file': '${TEST_DIR}/${1}-${snapshot_virt0}' } },
                { 'type': 'blockdev-snapshot-sync', 'data' :
                    { 'device': 'virtio1',
-                       'snapshot-file': '"${TEST_DIR}/${1}-${snapshot_virt1}"' } } ]
+                       'snapshot-file': '${TEST_DIR}/${1}-${snapshot_virt1}' } } ]
              } }"
 
     _send_qemu_cmd $h "${cmd}" "return"
@@ -101,9 +101,9 @@ function add_snapshot_image()
     mv "${TEST_IMG}" "${snapshot_file}"
     cmd="{ 'execute': 'blockdev-add', 'arguments':
            { 'options':
-             { 'driver': 'qcow2', 'node-name': 'snap_"${1}"', "${extra_params}"
+             { 'driver': 'qcow2', 'node-name': 'snap_${1}', ${extra_params}
                'file':
-               { 'driver': 'file', 'filename': '"${snapshot_file}"' } } } }"
+               { 'driver': 'file', 'filename': '${snapshot_file}' } } } }"
     _send_qemu_cmd $h "${cmd}" "return"
 }
 
@@ -113,7 +113,7 @@ function blockdev_snapshot()
 {
     cmd="{ 'execute': 'blockdev-snapshot',
                       'arguments': { 'node': 'virtio0',
-                                     'overlay':'snap_"${1}"' } }"
+                                     'overlay':'snap_${1}' } }"
     _send_qemu_cmd $h "${cmd}" "${2:-return}"
 }
 
@@ -152,7 +152,7 @@ echo === Invalid command - missing device and nodename ===
 echo
 
 _send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot-sync',
-                         'arguments': { 'snapshot-file':'"${TEST_DIR}/1-${snapshot_virt0}"',
+                         'arguments': { 'snapshot-file':'${TEST_DIR}/1-${snapshot_virt0}',
                                      'format': 'qcow2' } }" "error"
 
 echo
@@ -224,7 +224,7 @@ blockdev_snapshot $((${SNAPSHOTS}+1)) error
 
 _send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot',
                      'arguments': { 'node':'nodevice',
-                                    'overlay':'snap_"${SNAPSHOTS}"' }
+                                    'overlay':'snap_${SNAPSHOTS}' }
                    }" "error"
 
 # success, all done
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 30/37] block: test 'blockdev-snapshot' using a file BDS as the overlay
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (28 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 29/37] block: Remove inner quotation marks in iotest 085 Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:17 ` [Qemu-devel] [PULL 31/37] qemu-iotests: fix cleanup of background processes Kevin Wolf
                   ` (7 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Alberto Garcia <berto@igalia.com>

This test checks that it is not possible to create a snapshot if the
requested overlay node is a BDS which does not support backing images.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/085     | 12 +++++++++++-
 tests/qemu-iotests/085.out |  4 ++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/085 b/tests/qemu-iotests/085
index 80e547d..aa77eca 100755
--- a/tests/qemu-iotests/085
+++ b/tests/qemu-iotests/085
@@ -103,7 +103,8 @@ function add_snapshot_image()
            { 'options':
              { 'driver': 'qcow2', 'node-name': 'snap_${1}', ${extra_params}
                'file':
-               { 'driver': 'file', 'filename': '${snapshot_file}' } } } }"
+               { 'driver': 'file', 'filename': '${snapshot_file}',
+                 'node-name': 'file_${1}' } } } }"
     _send_qemu_cmd $h "${cmd}" "return"
 }
 
@@ -187,6 +188,15 @@ add_snapshot_image ${SNAPSHOTS}
 blockdev_snapshot ${SNAPSHOTS}
 
 echo
+echo === Invalid command - cannot create a snapshot using a file BDS ===
+echo
+
+_send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot',
+                     'arguments': { 'node':'virtio0',
+                                    'overlay':'file_${SNAPSHOTS}' }
+                   }" "error"
+
+echo
 echo === Invalid command - snapshot node used as active layer ===
 echo
 
diff --git a/tests/qemu-iotests/085.out b/tests/qemu-iotests/085.out
index 52292ea..01c78d6 100644
--- a/tests/qemu-iotests/085.out
+++ b/tests/qemu-iotests/085.out
@@ -62,6 +62,10 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/
 {"return": {}}
 {"return": {}}
 
+=== Invalid command - cannot create a snapshot using a file BDS ===
+
+{"error": {"class": "GenericError", "desc": "The snapshot does not support backing images"}}
+
 === Invalid command - snapshot node used as active layer ===
 
 {"error": {"class": "GenericError", "desc": "The snapshot is already in use by virtio0"}}
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 31/37] qemu-iotests: fix cleanup of background processes
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (29 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 30/37] block: test 'blockdev-snapshot' using a file BDS as the overlay Kevin Wolf
@ 2015-11-05 18:17 ` Kevin Wolf
  2015-11-05 18:18 ` [Qemu-devel] [PULL 32/37] qemu-iotests: fix -valgrind option for check Kevin Wolf
                   ` (6 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:17 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Jeff Cody <jcody@redhat.com>

Commit 934659c switched the iotests to run qemu and qemu-nbd from a bash
subshell, in order to catch segfaults.  Unfortunately, this means the
process PID cannot be captured via '$!'. We stopped killing qemu and
qemu-nbd processes, leaving a lot of orphaned, running qemu processes
after executing iotests.

Since the process is using exec in the subshell, the PID is the
same as the subshell PID.

Track these PIDs for cleanup using pidfiles in the $TEST_DIR. Only
track the qemu PID, however, if requested - not all usage requires
killing the process.

Reported-by: John Snow <jsnow@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
Message-id: 9e4f958b3895b7259b98d845bb46f000ba362869.1446232490.git.jcody@redhat.com
[mreitz@redhat.com: Replaced '! -z "..."' by '-n "..."']
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/058           | 12 ++++++++----
 tests/qemu-iotests/common.config | 14 ++++++++++++--
 tests/qemu-iotests/common.qemu   | 18 ++++++++++++------
 tests/qemu-iotests/common.rc     |  8 +++++---
 4 files changed, 37 insertions(+), 15 deletions(-)

diff --git a/tests/qemu-iotests/058 b/tests/qemu-iotests/058
index f2bdd0b..63a6598 100755
--- a/tests/qemu-iotests/058
+++ b/tests/qemu-iotests/058
@@ -32,11 +32,17 @@ status=1	# failure is the default!
 
 nbd_unix_socket=$TEST_DIR/test_qemu_nbd_socket
 nbd_snapshot_img="nbd:unix:$nbd_unix_socket"
+rm -f "${TEST_DIR}/qemu-nbd.pid"
 
 _cleanup_nbd()
 {
-    if [ -n "$NBD_SNAPSHOT_PID" ]; then
-        kill "$NBD_SNAPSHOT_PID"
+    local NBD_SNAPSHOT_PID
+    if [ -f "${TEST_DIR}/qemu-nbd.pid" ]; then
+        read NBD_SNAPSHOT_PID < "${TEST_DIR}/qemu-nbd.pid"
+        rm -f "${TEST_DIR}/qemu-nbd.pid"
+        if [ -n "$NBD_SNAPSHOT_PID" ]; then
+            kill "$NBD_SNAPSHOT_PID"
+        fi
     fi
     rm -f "$nbd_unix_socket"
 }
@@ -60,7 +66,6 @@ _export_nbd_snapshot()
 {
     _cleanup_nbd
     $QEMU_NBD -v -t -k "$nbd_unix_socket" "$TEST_IMG" -l $1 &
-    NBD_SNAPSHOT_PID=$!
     _wait_for_nbd
 }
 
@@ -68,7 +73,6 @@ _export_nbd_snapshot1()
 {
     _cleanup_nbd
     $QEMU_NBD -v -t -k "$nbd_unix_socket" "$TEST_IMG" -l snapshot.name=$1 &
-    NBD_SNAPSHOT_PID=$!
     _wait_for_nbd
 }
 
diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
index 596bb2b..be99730 100644
--- a/tests/qemu-iotests/common.config
+++ b/tests/qemu-iotests/common.config
@@ -44,6 +44,8 @@ export HOST_OPTIONS=${HOST_OPTIONS:=local.config}
 export CHECK_OPTIONS=${CHECK_OPTIONS:="-g auto"}
 export PWD=`pwd`
 
+export _QEMU_HANDLE=0
+
 # $1 = prog to look for, $2* = default pathnames if not found in $PATH
 set_prog_path()
 {
@@ -105,7 +107,12 @@ fi
 
 _qemu_wrapper()
 {
-    (exec "$QEMU_PROG" $QEMU_OPTIONS "$@")
+    (
+        if [ -n "${QEMU_NEED_PID}" ]; then
+            echo $BASHPID > "${TEST_DIR}/qemu-${_QEMU_HANDLE}.pid"
+        fi
+        exec "$QEMU_PROG" $QEMU_OPTIONS "$@"
+    )
 }
 
 _qemu_img_wrapper()
@@ -120,7 +127,10 @@ _qemu_io_wrapper()
 
 _qemu_nbd_wrapper()
 {
-    (exec "$QEMU_NBD_PROG" $QEMU_NBD_OPTIONS "$@")
+    (
+        echo $BASHPID > "${TEST_DIR}/qemu-nbd.pid"
+        exec "$QEMU_NBD_PROG" $QEMU_NBD_OPTIONS "$@"
+    )
 }
 
 export QEMU=_qemu_wrapper
diff --git a/tests/qemu-iotests/common.qemu b/tests/qemu-iotests/common.qemu
index e3faa53..8bf3969 100644
--- a/tests/qemu-iotests/common.qemu
+++ b/tests/qemu-iotests/common.qemu
@@ -30,8 +30,6 @@ QEMU_COMM_TIMEOUT=10
 QEMU_FIFO_IN="${TEST_DIR}/qmp-in-$$"
 QEMU_FIFO_OUT="${TEST_DIR}/qmp-out-$$"
 
-QEMU_PID=
-_QEMU_HANDLE=0
 QEMU_HANDLE=0
 
 # If bash version is >= 4.1, these will be overwritten and dynamic
@@ -153,11 +151,11 @@ function _launch_qemu()
     mkfifo "${fifo_out}"
     mkfifo "${fifo_in}"
 
+    QEMU_NEED_PID='y'\
     ${QEMU} -nographic -serial none ${comm} -machine accel=qtest "${@}" \
                                                                 >"${fifo_out}" \
                                                                 2>&1 \
                                                                 <"${fifo_in}" &
-    QEMU_PID[${_QEMU_HANDLE}]=$!
 
     if [[ "${BASH_VERSINFO[0]}" -ge "5" ||
         ("${BASH_VERSINFO[0]}" -ge "4"  &&  "${BASH_VERSINFO[1]}" -ge "1") ]]
@@ -196,10 +194,18 @@ function _cleanup_qemu()
     # QEMU_PID[], QEMU_IN[], QEMU_OUT[] all use same indices
     for i in "${!QEMU_OUT[@]}"
     do
-        if [ -z "${wait}" ]; then
-            kill -KILL ${QEMU_PID[$i]} 2>/dev/null
+        local QEMU_PID
+        if [ -f "${TEST_DIR}/qemu-${i}.pid" ]; then
+            read QEMU_PID < "${TEST_DIR}/qemu-${i}.pid"
+            rm -f "${TEST_DIR}/qemu-${i}.pid"
+            if [ -z "${wait}" ] && [ -n "${QEMU_PID}" ]; then
+                kill -KILL ${QEMU_PID} 2>/dev/null
+            fi
+            if [ -n "${QEMU_PID}" ]; then
+                wait ${QEMU_PID} 2>/dev/null # silent kill
+            fi
         fi
-        wait ${QEMU_PID[$i]} 2>/dev/null # silent kill
+
         if [ -n "${wait}" ]; then
             cat <&${QEMU_OUT[$i]} | _filter_testdir | _filter_qemu \
                                   | _filter_qemu_io | _filter_qmp
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 28e4bea..4878e99 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -154,7 +154,6 @@ _make_test_img()
     # Start an NBD server on the image file, which is what we'll be talking to
     if [ $IMGPROTO = "nbd" ]; then
         eval "$QEMU_NBD -v -t -b 127.0.0.1 -p 10810 -f $IMGFMT  $TEST_IMG_FILE &"
-        QEMU_NBD_PID=$!
         sleep 1 # FIXME: qemu-nbd needs to be listening before we continue
     fi
 }
@@ -175,8 +174,11 @@ _cleanup_test_img()
     case "$IMGPROTO" in
 
         nbd)
-            if [ -n "$QEMU_NBD_PID" ]; then
-                kill $QEMU_NBD_PID
+            if [ -f "${TEST_DIR}/qemu-nbd.pid" ]; then
+                local QEMU_NBD_PID
+                read QEMU_NBD_PID < "${TEST_DIR}/qemu-nbd.pid"
+                kill ${QEMU_NBD_PID}
+                rm -f "${TEST_DIR}/qemu-nbd.pid"
             fi
             rm -f "$TEST_IMG_FILE"
             ;;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 32/37] qemu-iotests: fix -valgrind option for check
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (30 preceding siblings ...)
  2015-11-05 18:17 ` [Qemu-devel] [PULL 31/37] qemu-iotests: fix cleanup of background processes Kevin Wolf
@ 2015-11-05 18:18 ` Kevin Wolf
  2015-11-05 18:18 ` [Qemu-devel] [PULL 33/37] mirror: block all operations on the target image during the job Kevin Wolf
                   ` (5 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:18 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Jeff Cody <jcody@redhat.com>

Commit 934659c switched the iotests to run qemu-io from a bash subshell,
in order to catch segfaults.  This method is incompatible with the
current valgrind_qemu_io() bash function.

Move the valgrind usage into the exec subshell in _qemu_io_wrapper(),
while making sure the original return value is passed back to the
caller.

Update test output for tests 039, 061, and 137 as it looks for the
specific subshell command when the process is terminated.

Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
Message-id: 0066fd85d26ca641a1c25135ff2479b7985701cf.1446232490.git.jcody@redhat.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/039.out       | 30 +++++++++++++++++++++++++-----
 tests/qemu-iotests/061.out       | 12 ++++++++++--
 tests/qemu-iotests/137.out       |  6 +++++-
 tests/qemu-iotests/common        |  9 ++-------
 tests/qemu-iotests/common.config | 18 +++++++++++++++++-
 tests/qemu-iotests/common.rc     | 10 ----------
 6 files changed, 59 insertions(+), 26 deletions(-)

diff --git a/tests/qemu-iotests/039.out b/tests/qemu-iotests/039.out
index 03a31c5..32c8846 100644
--- a/tests/qemu-iotests/039.out
+++ b/tests/qemu-iotests/039.out
@@ -11,7 +11,11 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 wrote 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-./common.config: Killed                  ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" )
+./common.config: Killed                  ( if [ "${VALGRIND_QEMU}" == "y" ]; then
+    exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+else
+    exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+fi )
 incompatible_features     0x1
 ERROR cluster 5 refcount=0 reference=1
 ERROR OFLAG_COPIED data cluster: l2_entry=8000000000050000 refcount=0
@@ -46,7 +50,11 @@ read 512/512 bytes at offset 0
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 wrote 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-./common.config: Killed                  ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" )
+./common.config: Killed                  ( if [ "${VALGRIND_QEMU}" == "y" ]; then
+    exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+else
+    exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+fi )
 incompatible_features     0x1
 ERROR cluster 5 refcount=0 reference=1
 Rebuilding refcount structure
@@ -60,7 +68,11 @@ incompatible_features     0x0
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 wrote 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-./common.config: Killed                  ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" )
+./common.config: Killed                  ( if [ "${VALGRIND_QEMU}" == "y" ]; then
+    exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+else
+    exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+fi )
 incompatible_features     0x0
 No errors were found on the image.
 
@@ -79,7 +91,11 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 wrote 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-./common.config: Killed                  ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" )
+./common.config: Killed                  ( if [ "${VALGRIND_QEMU}" == "y" ]; then
+    exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+else
+    exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+fi )
 incompatible_features     0x1
 ERROR cluster 5 refcount=0 reference=1
 ERROR OFLAG_COPIED data cluster: l2_entry=8000000000050000 refcount=0
@@ -89,7 +105,11 @@ Data may be corrupted, or further writes to the image may corrupt it.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 wrote 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-./common.config: Killed                  ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" )
+./common.config: Killed                  ( if [ "${VALGRIND_QEMU}" == "y" ]; then
+    exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+else
+    exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+fi )
 incompatible_features     0x0
 No errors were found on the image.
 *** done
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
index b16bea9..f2598a8 100644
--- a/tests/qemu-iotests/061.out
+++ b/tests/qemu-iotests/061.out
@@ -57,7 +57,11 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 wrote 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-./common.config: Killed                  ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" )
+./common.config: Killed                  ( if [ "${VALGRIND_QEMU}" == "y" ]; then
+    exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+else
+    exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+fi )
 magic                     0x514649fb
 version                   3
 backing_file_offset       0x0
@@ -215,7 +219,11 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 wrote 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-./common.config: Killed                  ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" )
+./common.config: Killed                  ( if [ "${VALGRIND_QEMU}" == "y" ]; then
+    exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+else
+    exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+fi )
 magic                     0x514649fb
 version                   3
 backing_file_offset       0x0
diff --git a/tests/qemu-iotests/137.out b/tests/qemu-iotests/137.out
index cf55a41..88c702c 100644
--- a/tests/qemu-iotests/137.out
+++ b/tests/qemu-iotests/137.out
@@ -31,7 +31,11 @@ Cache clean interval too big
 Unsupported value 'blubb' for qcow2 option 'overlap-check'. Allowed are any of the following: none, constant, cached, all
 wrote 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-./common.config: Killed                  ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" )
+./common.config: Killed                  ( if [ "${VALGRIND_QEMU}" == "y" ]; then
+    exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+else
+    exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@";
+fi )
 incompatible_features     0x0
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 wrote 65536/65536 bytes at offset 0
diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
index 25c351b..ff84f4b 100644
--- a/tests/qemu-iotests/common
+++ b/tests/qemu-iotests/common
@@ -41,7 +41,6 @@ sortme=false
 expunge=true
 have_test_arg=false
 randomize=false
-valgrind=false
 cachemode=false
 rm -f $tmp.list $tmp.tmp $tmp.sed
 
@@ -53,6 +52,7 @@ export CACHEMODE="writeback"
 export QEMU_IO_OPTIONS=""
 export CACHEMODE_IS_DEFAULT=true
 export QEMU_OPTIONS="-nodefaults"
+export VALGRIND_QEMU=
 
 for r
 do
@@ -278,7 +278,7 @@ testlist options
             ;;
 
         -valgrind)
-            valgrind=true
+            VALGRIND_QEMU='y'
             xpand=false
             ;;
 
@@ -436,8 +436,3 @@ fi
 if [ "$IMGPROTO" = "nbd" ] ; then
     [ "$QEMU_NBD" = "" ] && _fatal "qemu-nbd not found"
 fi
-
-if $valgrind; then
-    export REAL_QEMU_IO="$QEMU_IO_PROG"
-    export QEMU_IO_PROG=valgrind_qemu_io
-fi
diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
index be99730..3ed51b8 100644
--- a/tests/qemu-iotests/common.config
+++ b/tests/qemu-iotests/common.config
@@ -122,7 +122,23 @@ _qemu_img_wrapper()
 
 _qemu_io_wrapper()
 {
-    (exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@")
+    local VALGRIND_LOGFILE=/tmp/$$.valgrind
+    local RETVAL
+    (
+        if [ "${VALGRIND_QEMU}" == "y" ]; then
+            exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"
+        else
+            exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@"
+        fi
+    )
+    RETVAL=$?
+    if [ "${VALGRIND_QEMU}" == "y" ]; then
+        if [ $RETVAL == 99 ]; then
+            cat "${VALGRIND_LOGFILE}"
+        fi
+        rm -f "${VALGRIND_LOGFILE}"
+    fi
+    (exit $RETVAL)
 }
 
 _qemu_nbd_wrapper()
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 4878e99..d9913f8 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -70,16 +70,6 @@ else
     TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT
 fi
 
-function valgrind_qemu_io()
-{
-    valgrind --log-file=/tmp/$$.valgrind --error-exitcode=99 $REAL_QEMU_IO "$@"
-    if [ $? != 0 ]; then
-        cat /tmp/$$.valgrind
-    fi
-    rm -f /tmp/$$.valgrind
-}
-
-
 _optstr_add()
 {
     if [ -n "$1" ]; then
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 33/37] mirror: block all operations on the target image during the job
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (31 preceding siblings ...)
  2015-11-05 18:18 ` [Qemu-devel] [PULL 32/37] qemu-iotests: fix -valgrind option for check Kevin Wolf
@ 2015-11-05 18:18 ` Kevin Wolf
  2015-11-05 18:18 ` [Qemu-devel] [PULL 34/37] block: Add blk_get_refcnt() Kevin Wolf
                   ` (4 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:18 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Alberto Garcia <berto@igalia.com>

There's nothing preventing the target image from being used by other
operations during the 'drive-mirror' job, so we should block them all
until the job is done.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 82b88fd04cde918a08a6f9a4ab85626d7fd7b502.1446475331.git.berto@igalia.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/mirror.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/block/mirror.c b/block/mirror.c
index b1252a1..60f1cb5 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -384,6 +384,7 @@ static void mirror_exit(BlockJob *job, void *opaque)
         aio_context_release(replace_aio_context);
     }
     g_free(s->replaces);
+    bdrv_op_unblock_all(s->target, s->common.blocker);
     bdrv_unref(s->target);
     block_job_completed(&s->common, data->ret);
     g_free(data);
@@ -744,6 +745,9 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
         block_job_release(bs);
         return;
     }
+
+    bdrv_op_block_all(s->target, s->common.blocker);
+
     bdrv_set_enable_write_cache(s->target, true);
     if (s->target->blk) {
         blk_set_on_error(s->target->blk, on_target_error, on_target_error);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 34/37] block: Add blk_get_refcnt()
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (32 preceding siblings ...)
  2015-11-05 18:18 ` [Qemu-devel] [PULL 33/37] mirror: block all operations on the target image during the job Kevin Wolf
@ 2015-11-05 18:18 ` Kevin Wolf
  2015-11-05 18:18 ` [Qemu-devel] [PULL 35/37] block: Add 'x-blockdev-del' QMP command Kevin Wolf
                   ` (3 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:18 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Alberto Garcia <berto@igalia.com>

This function returns the reference count of a given BlockBackend.
For convenience, it returns 0 if the BlockBackend pointer is NULL.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: dfdd8a17dbe3288842840636d2cfe5bb895abcb0.1446475331.git.berto@igalia.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/block-backend.c          | 5 +++++
 include/sysemu/block-backend.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index 1ac6982..6f9309f 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -189,6 +189,11 @@ static void drive_info_del(DriveInfo *dinfo)
     g_free(dinfo);
 }
 
+int blk_get_refcnt(BlockBackend *blk)
+{
+    return blk ? blk->refcnt : 0;
+}
+
 /*
  * Increment @blk's reference count.
  * @blk must not be null.
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 40e315b..f4a68e2 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -65,6 +65,7 @@ BlockBackend *blk_new_with_bs(const char *name, Error **errp);
 BlockBackend *blk_new_open(const char *name, const char *filename,
                            const char *reference, QDict *options, int flags,
                            Error **errp);
+int blk_get_refcnt(BlockBackend *blk);
 void blk_ref(BlockBackend *blk);
 void blk_unref(BlockBackend *blk);
 const char *blk_name(BlockBackend *blk);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 35/37] block: Add 'x-blockdev-del' QMP command
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (33 preceding siblings ...)
  2015-11-05 18:18 ` [Qemu-devel] [PULL 34/37] block: Add blk_get_refcnt() Kevin Wolf
@ 2015-11-05 18:18 ` Kevin Wolf
  2015-11-05 18:18 ` [Qemu-devel] [PULL 36/37] iotests: Add tests for the x-blockdev-del command Kevin Wolf
                   ` (2 subsequent siblings)
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:18 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Alberto Garcia <berto@igalia.com>

This command is still experimental, hence the name.

This is the companion to 'blockdev-add'. It allows deleting a
BlockBackend with its associated BlockDriverState tree, or a
BlockDriverState that is not attached to any backend.

In either case, the command fails if the reference count is greater
than 1 or the BlockDriverState has any parents.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 6cfc148c77aca1da942b094d811bfa3fcf7ac7bb.1446475331.git.berto@igalia.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 blockdev.c           | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 qapi/block-core.json | 32 +++++++++++++++++++++++--
 qmp-commands.hx      | 61 ++++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 155 insertions(+), 4 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 8749d78..31a4d89 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3469,6 +3469,72 @@ fail:
     qmp_output_visitor_cleanup(ov);
 }
 
+void qmp_x_blockdev_del(bool has_id, const char *id,
+                        bool has_node_name, const char *node_name, Error **errp)
+{
+    AioContext *aio_context;
+    BlockBackend *blk;
+    BlockDriverState *bs;
+
+    if (has_id && has_node_name) {
+        error_setg(errp, "Only one of id and node-name must be specified");
+        return;
+    } else if (!has_id && !has_node_name) {
+        error_setg(errp, "No block device specified");
+        return;
+    }
+
+    if (has_id) {
+        blk = blk_by_name(id);
+        if (!blk) {
+            error_setg(errp, "Cannot find block backend %s", id);
+            return;
+        }
+        if (blk_get_refcnt(blk) > 1) {
+            error_setg(errp, "Block backend %s is in use", id);
+            return;
+        }
+        bs = blk_bs(blk);
+        aio_context = blk_get_aio_context(blk);
+    } else {
+        bs = bdrv_find_node(node_name);
+        if (!bs) {
+            error_setg(errp, "Cannot find node %s", node_name);
+            return;
+        }
+        blk = bs->blk;
+        if (blk) {
+            error_setg(errp, "Node %s is in use by %s",
+                       node_name, blk_name(blk));
+            return;
+        }
+        aio_context = bdrv_get_aio_context(bs);
+    }
+
+    aio_context_acquire(aio_context);
+
+    if (bs) {
+        if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, errp)) {
+            goto out;
+        }
+
+        if (bs->refcnt > 1 || !QLIST_EMPTY(&bs->parents)) {
+            error_setg(errp, "Block device %s is in use",
+                       bdrv_get_device_or_node_name(bs));
+            goto out;
+        }
+    }
+
+    if (blk) {
+        blk_unref(blk);
+    } else {
+        bdrv_unref(bs);
+    }
+
+out:
+    aio_context_release(aio_context);
+}
+
 BlockJobInfoList *qmp_query_block_jobs(Error **errp)
 {
     BlockJobInfoList *head = NULL, **p_next = &head;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 083d2cd..470e86c 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1895,8 +1895,8 @@
 # level and no BlockBackend will be created.
 #
 # This command is still a work in progress.  It doesn't support all
-# block drivers, it lacks a matching blockdev-del, and more.  Stay
-# away from it unless you want to help with its development.
+# block drivers among other things.  Stay away from it unless you want
+# to help with its development.
 #
 # @options: block device options for the new device
 #
@@ -1905,6 +1905,34 @@
 { 'command': 'blockdev-add', 'data': { 'options': 'BlockdevOptions' } }
 
 ##
+# @x-blockdev-del:
+#
+# Deletes a block device that has been added using blockdev-add.
+# The selected device can be either a block backend or a graph node.
+#
+# In the former case the backend will be destroyed, along with its
+# inserted medium if there's any. The command will fail if the backend
+# or its medium are in use.
+#
+# In the latter case the node will be destroyed. The command will fail
+# if the node is attached to a block backend or is otherwise being
+# used.
+#
+# One of @id or @node-name must be specified, but not both.
+#
+# This command is still a work in progress and is considered
+# experimental. Stay away from it unless you want to help with its
+# development.
+#
+# @id: #optional Name of the block backend device to delete.
+#
+# @node-name: #optional Name of the graph node to delete.
+#
+# Since: 2.5
+##
+{ 'command': 'x-blockdev-del', 'data': { '*id': 'str', '*node-name': 'str' } }
+
+##
 # @blockdev-open-tray:
 #
 # Opens a block device's tray. If there is a block driver state tree inserted as
diff --git a/qmp-commands.hx b/qmp-commands.hx
index bd4d38f..658e7da 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3927,8 +3927,8 @@ blockdev-add
 Add a block device.
 
 This command is still a work in progress.  It doesn't support all
-block drivers, it lacks a matching blockdev-del, and more.  Stay away
-from it unless you want to help with its development.
+block drivers among other things.  Stay away from it unless you want
+to help with its development.
 
 Arguments:
 
@@ -3974,6 +3974,63 @@ Example (2):
 EQMP
 
     {
+        .name       = "x-blockdev-del",
+        .args_type  = "id:s?,node-name:s?",
+        .mhandler.cmd_new = qmp_marshal_x_blockdev_del,
+    },
+
+SQMP
+x-blockdev-del
+------------
+Since 2.5
+
+Deletes a block device thas has been added using blockdev-add.
+The selected device can be either a block backend or a graph node.
+
+In the former case the backend will be destroyed, along with its
+inserted medium if there's any. The command will fail if the backend
+or its medium are in use.
+
+In the latter case the node will be destroyed. The command will fail
+if the node is attached to a block backend or is otherwise being
+used.
+
+One of "id" or "node-name" must be specified, but not both.
+
+This command is still a work in progress and is considered
+experimental. Stay away from it unless you want to help with its
+development.
+
+Arguments:
+
+- "id": Name of the block backend device to delete (json-string, optional)
+- "node-name": Name of the graph node to delete (json-string, optional)
+
+Example:
+
+-> { "execute": "blockdev-add",
+     "arguments": {
+         "options": {
+             "driver": "qcow2",
+             "id": "drive0",
+             "file": {
+                 "driver": "file",
+                 "filename": "test.qcow2"
+             }
+         }
+     }
+   }
+
+<- { "return": {} }
+
+-> { "execute": "x-blockdev-del",
+     "arguments": { "id": "drive0" }
+   }
+<- { "return": {} }
+
+EQMP
+
+    {
         .name       = "blockdev-open-tray",
         .args_type  = "device:s,force:b?",
         .mhandler.cmd_new = qmp_marshal_blockdev_open_tray,
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 36/37] iotests: Add tests for the x-blockdev-del command
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (34 preceding siblings ...)
  2015-11-05 18:18 ` [Qemu-devel] [PULL 35/37] block: Add 'x-blockdev-del' QMP command Kevin Wolf
@ 2015-11-05 18:18 ` Kevin Wolf
  2015-11-05 18:18 ` [Qemu-devel] [PULL 37/37] qcow2: Fix qcow2_get_cluster_offset() for zero clusters Kevin Wolf
  2015-11-05 19:01 ` [Qemu-devel] [PULL 00/37] Block layer patches Peter Maydell
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:18 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

From: Alberto Garcia <berto@igalia.com>

Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-id: 57c3b0d4d0c73ddadd19e5bded9492c359cc4568.1446475331.git.berto@igalia.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/139     | 414 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/139.out |   5 +
 tests/qemu-iotests/group   |   1 +
 3 files changed, 420 insertions(+)
 create mode 100644 tests/qemu-iotests/139
 create mode 100644 tests/qemu-iotests/139.out

diff --git a/tests/qemu-iotests/139 b/tests/qemu-iotests/139
new file mode 100644
index 0000000..b5470f7
--- /dev/null
+++ b/tests/qemu-iotests/139
@@ -0,0 +1,414 @@
+#!/usr/bin/env python
+#
+# Test cases for the QMP 'x-blockdev-del' command
+#
+# Copyright (C) 2015 Igalia, S.L.
+# Author: Alberto Garcia <berto@igalia.com>
+#
+# 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/>.
+#
+
+import os
+import iotests
+import time
+
+base_img = os.path.join(iotests.test_dir, 'base.img')
+new_img = os.path.join(iotests.test_dir, 'new.img')
+
+class TestBlockdevDel(iotests.QMPTestCase):
+
+    def setUp(self):
+        iotests.qemu_img('create', '-f', iotests.imgfmt, base_img, '1M')
+        self.vm = iotests.VM()
+        self.vm.launch()
+
+    def tearDown(self):
+        self.vm.shutdown()
+        os.remove(base_img)
+        if os.path.isfile(new_img):
+            os.remove(new_img)
+
+    # Check whether a BlockBackend exists
+    def checkBlockBackend(self, backend, node, must_exist = True):
+        result = self.vm.qmp('query-block')
+        backends = filter(lambda x: x['device'] == backend, result['return'])
+        self.assertLessEqual(len(backends), 1)
+        self.assertEqual(must_exist, len(backends) == 1)
+        if must_exist:
+            if node:
+                self.assertEqual(backends[0]['inserted']['node-name'], node)
+            else:
+                self.assertFalse(backends[0].has_key('inserted'))
+
+    # Check whether a BlockDriverState exists
+    def checkBlockDriverState(self, node, must_exist = True):
+        result = self.vm.qmp('query-named-block-nodes')
+        nodes = filter(lambda x: x['node-name'] == node, result['return'])
+        self.assertLessEqual(len(nodes), 1)
+        self.assertEqual(must_exist, len(nodes) == 1)
+
+    # Add a new BlockBackend (with its attached BlockDriverState)
+    def addBlockBackend(self, backend, node):
+        file_node = '%s_file' % node
+        self.checkBlockBackend(backend, node, False)
+        self.checkBlockDriverState(node, False)
+        self.checkBlockDriverState(file_node, False)
+        opts = {'driver': iotests.imgfmt,
+                'id': backend,
+                'node-name': node,
+                'file': {'driver': 'file',
+                         'node-name': file_node,
+                         'filename': base_img}}
+        result = self.vm.qmp('blockdev-add', conv_keys = False, options = opts)
+        self.assert_qmp(result, 'return', {})
+        self.checkBlockBackend(backend, node)
+        self.checkBlockDriverState(node)
+        self.checkBlockDriverState(file_node)
+
+    # Add a BlockDriverState without a BlockBackend
+    def addBlockDriverState(self, node):
+        file_node = '%s_file' % node
+        self.checkBlockDriverState(node, False)
+        self.checkBlockDriverState(file_node, False)
+        opts = {'driver': iotests.imgfmt,
+                'node-name': node,
+                'file': {'driver': 'file',
+                         'node-name': file_node,
+                         'filename': base_img}}
+        result = self.vm.qmp('blockdev-add', conv_keys = False, options = opts)
+        self.assert_qmp(result, 'return', {})
+        self.checkBlockDriverState(node)
+        self.checkBlockDriverState(file_node)
+
+    # Add a BlockDriverState that will be used as overlay for the base_img BDS
+    def addBlockDriverStateOverlay(self, node):
+        self.checkBlockDriverState(node, False)
+        iotests.qemu_img('create', '-f', iotests.imgfmt,
+                         '-b', base_img, new_img, '1M')
+        opts = {'driver': iotests.imgfmt,
+                'node-name': node,
+                'backing': '',
+                'file': {'driver': 'file',
+                         'filename': new_img}}
+        result = self.vm.qmp('blockdev-add', conv_keys = False, options = opts)
+        self.assert_qmp(result, 'return', {})
+        self.checkBlockDriverState(node)
+
+    # Delete a BlockBackend
+    def delBlockBackend(self, backend, node, expect_error = False,
+                        destroys_media = True):
+        self.checkBlockBackend(backend, node)
+        if node:
+            self.checkBlockDriverState(node)
+        result = self.vm.qmp('x-blockdev-del', id = backend)
+        if expect_error:
+            self.assert_qmp(result, 'error/class', 'GenericError')
+            if node:
+                self.checkBlockDriverState(node)
+        else:
+            self.assert_qmp(result, 'return', {})
+            if node:
+                self.checkBlockDriverState(node, not destroys_media)
+        self.checkBlockBackend(backend, node, must_exist = expect_error)
+
+    # Delete a BlockDriverState
+    def delBlockDriverState(self, node, expect_error = False):
+        self.checkBlockDriverState(node)
+        result = self.vm.qmp('x-blockdev-del', node_name = node)
+        if expect_error:
+            self.assert_qmp(result, 'error/class', 'GenericError')
+        else:
+            self.assert_qmp(result, 'return', {})
+        self.checkBlockDriverState(node, expect_error)
+
+    # Add a device model
+    def addDeviceModel(self, device, backend):
+        result = self.vm.qmp('device_add', id = device,
+                             driver = 'virtio-blk-pci', drive = backend)
+        self.assert_qmp(result, 'return', {})
+
+    # Delete a device model
+    def delDeviceModel(self, device):
+        result = self.vm.qmp('device_del', id = device)
+        self.assert_qmp(result, 'return', {})
+
+        result = self.vm.qmp('system_reset')
+        self.assert_qmp(result, 'return', {})
+
+        device_path = '/machine/peripheral/%s/virtio-backend' % device
+        event = self.vm.event_wait(name="DEVICE_DELETED",
+                                   match={'data': {'path': device_path}})
+        self.assertNotEqual(event, None)
+
+        event = self.vm.event_wait(name="DEVICE_DELETED",
+                                   match={'data': {'device': device}})
+        self.assertNotEqual(event, None)
+
+    # Remove a BlockDriverState
+    def ejectDrive(self, backend, node, expect_error = False,
+                   destroys_media = True):
+        self.checkBlockBackend(backend, node)
+        self.checkBlockDriverState(node)
+        result = self.vm.qmp('eject', device = backend)
+        if expect_error:
+            self.assert_qmp(result, 'error/class', 'GenericError')
+            self.checkBlockDriverState(node)
+            self.checkBlockBackend(backend, node)
+        else:
+            self.assert_qmp(result, 'return', {})
+            self.checkBlockDriverState(node, not destroys_media)
+            self.checkBlockBackend(backend, None)
+
+    # Insert a BlockDriverState
+    def insertDrive(self, backend, node):
+        self.checkBlockBackend(backend, None)
+        self.checkBlockDriverState(node)
+        result = self.vm.qmp('blockdev-insert-medium',
+                             device = backend, node_name = node)
+        self.assert_qmp(result, 'return', {})
+        self.checkBlockBackend(backend, node)
+        self.checkBlockDriverState(node)
+
+    # Create a snapshot using 'blockdev-snapshot-sync'
+    def createSnapshotSync(self, node, overlay):
+        self.checkBlockDriverState(node)
+        self.checkBlockDriverState(overlay, False)
+        opts = {'node-name': node,
+                'snapshot-file': new_img,
+                'snapshot-node-name': overlay,
+                'format': iotests.imgfmt}
+        result = self.vm.qmp('blockdev-snapshot-sync', conv_keys=False, **opts)
+        self.assert_qmp(result, 'return', {})
+        self.checkBlockDriverState(node)
+        self.checkBlockDriverState(overlay)
+
+    # Create a snapshot using 'blockdev-snapshot'
+    def createSnapshot(self, node, overlay):
+        self.checkBlockDriverState(node)
+        self.checkBlockDriverState(overlay)
+        result = self.vm.qmp('blockdev-snapshot',
+                             node = node, overlay = overlay)
+        self.assert_qmp(result, 'return', {})
+        self.checkBlockDriverState(node)
+        self.checkBlockDriverState(overlay)
+
+    # Create a mirror
+    def createMirror(self, backend, node, new_node):
+        self.checkBlockBackend(backend, node)
+        self.checkBlockDriverState(new_node, False)
+        opts = {'device': backend,
+                'target': new_img,
+                'node-name': new_node,
+                'sync': 'top',
+                'format': iotests.imgfmt}
+        result = self.vm.qmp('drive-mirror', conv_keys=False, **opts)
+        self.assert_qmp(result, 'return', {})
+        self.checkBlockBackend(backend, node)
+        self.checkBlockDriverState(new_node)
+
+    # Complete an existing block job
+    def completeBlockJob(self, backend, node_before, node_after):
+        self.checkBlockBackend(backend, node_before)
+        result = self.vm.qmp('block-job-complete', device=backend)
+        self.assert_qmp(result, 'return', {})
+        self.wait_until_completed(backend)
+        self.checkBlockBackend(backend, node_after)
+
+    # Add a BlkDebug node
+    # Note that the purpose of this is to test the x-blockdev-del
+    # sanity checks, not to create a usable blkdebug drive
+    def addBlkDebug(self, debug, node):
+        self.checkBlockDriverState(node, False)
+        self.checkBlockDriverState(debug, False)
+        image = {'driver': iotests.imgfmt,
+                 'node-name': node,
+                 'file': {'driver': 'file',
+                          'filename': base_img}}
+        opts = {'driver': 'blkdebug',
+                'node-name': debug,
+                'image': image}
+        result = self.vm.qmp('blockdev-add', conv_keys = False, options = opts)
+        self.assert_qmp(result, 'return', {})
+        self.checkBlockDriverState(node)
+        self.checkBlockDriverState(debug)
+
+    # Add a BlkVerify node
+    # Note that the purpose of this is to test the x-blockdev-del
+    # sanity checks, not to create a usable blkverify drive
+    def addBlkVerify(self, blkverify, test, raw):
+        self.checkBlockDriverState(test, False)
+        self.checkBlockDriverState(raw, False)
+        self.checkBlockDriverState(blkverify, False)
+        iotests.qemu_img('create', '-f', iotests.imgfmt, new_img, '1M')
+        node_0 = {'driver': iotests.imgfmt,
+                  'node-name': test,
+                  'file': {'driver': 'file',
+                           'filename': base_img}}
+        node_1 = {'driver': iotests.imgfmt,
+                  'node-name': raw,
+                  'file': {'driver': 'file',
+                           'filename': new_img}}
+        opts = {'driver': 'blkverify',
+                'node-name': blkverify,
+                'test': node_0,
+                'raw': node_1}
+        result = self.vm.qmp('blockdev-add', conv_keys = False, options = opts)
+        self.assert_qmp(result, 'return', {})
+        self.checkBlockDriverState(test)
+        self.checkBlockDriverState(raw)
+        self.checkBlockDriverState(blkverify)
+
+    # Add a Quorum node
+    def addQuorum(self, quorum, child0, child1):
+        self.checkBlockDriverState(child0, False)
+        self.checkBlockDriverState(child1, False)
+        self.checkBlockDriverState(quorum, False)
+        iotests.qemu_img('create', '-f', iotests.imgfmt, new_img, '1M')
+        child_0 = {'driver': iotests.imgfmt,
+                   'node-name': child0,
+                   'file': {'driver': 'file',
+                            'filename': base_img}}
+        child_1 = {'driver': iotests.imgfmt,
+                   'node-name': child1,
+                   'file': {'driver': 'file',
+                            'filename': new_img}}
+        opts = {'driver': 'quorum',
+                'node-name': quorum,
+                'vote-threshold': 1,
+                'children': [ child_0, child_1 ]}
+        result = self.vm.qmp('blockdev-add', conv_keys = False, options = opts)
+        self.assert_qmp(result, 'return', {})
+        self.checkBlockDriverState(child0)
+        self.checkBlockDriverState(child1)
+        self.checkBlockDriverState(quorum)
+
+    ########################
+    # The tests start here #
+    ########################
+
+    def testWrongParameters(self):
+        self.addBlockBackend('drive0', 'node0')
+        result = self.vm.qmp('x-blockdev-del')
+        self.assert_qmp(result, 'error/class', 'GenericError')
+        result = self.vm.qmp('x-blockdev-del', id='drive0', node_name='node0')
+        self.assert_qmp(result, 'error/class', 'GenericError')
+        self.delBlockBackend('drive0', 'node0')
+
+    def testBlockBackend(self):
+        self.addBlockBackend('drive0', 'node0')
+        # You cannot delete a BDS that is attached to a backend
+        self.delBlockDriverState('node0', expect_error = True)
+        self.delBlockBackend('drive0', 'node0')
+
+    def testBlockDriverState(self):
+        self.addBlockDriverState('node0')
+        # You cannot delete a file BDS directly
+        self.delBlockDriverState('node0_file', expect_error = True)
+        self.delBlockDriverState('node0')
+
+    def testEject(self):
+        self.addBlockBackend('drive0', 'node0')
+        self.ejectDrive('drive0', 'node0')
+        self.delBlockBackend('drive0', None)
+
+    def testDeviceModel(self):
+        self.addBlockBackend('drive0', 'node0')
+        self.addDeviceModel('device0', 'drive0')
+        self.ejectDrive('drive0', 'node0', expect_error = True)
+        self.delBlockBackend('drive0', 'node0', expect_error = True)
+        self.delDeviceModel('device0')
+        self.delBlockBackend('drive0', 'node0')
+
+    def testAttachMedia(self):
+        # This creates a BlockBackend and removes its media
+        self.addBlockBackend('drive0', 'node0')
+        self.ejectDrive('drive0', 'node0')
+        # This creates a new BlockDriverState and inserts it into the backend
+        self.addBlockDriverState('node1')
+        self.insertDrive('drive0', 'node1')
+        # The backend can't be removed: the new BDS has an extra reference
+        self.delBlockBackend('drive0', 'node1', expect_error = True)
+        self.delBlockDriverState('node1', expect_error = True)
+        # The BDS still exists after being ejected, but now it can be removed
+        self.ejectDrive('drive0', 'node1', destroys_media = False)
+        self.delBlockDriverState('node1')
+        self.delBlockBackend('drive0', None)
+
+    def testSnapshotSync(self):
+        self.addBlockBackend('drive0', 'node0')
+        self.createSnapshotSync('node0', 'overlay0')
+        # This fails because node0 is now being used as a backing image
+        self.delBlockDriverState('node0', expect_error = True)
+        # This succeeds because overlay0 only has the backend reference
+        self.delBlockBackend('drive0', 'overlay0')
+        self.checkBlockDriverState('node0', False)
+
+    def testSnapshot(self):
+        self.addBlockBackend('drive0', 'node0')
+        self.addBlockDriverStateOverlay('overlay0')
+        self.createSnapshot('node0', 'overlay0')
+        self.delBlockBackend('drive0', 'overlay0', expect_error = True)
+        self.delBlockDriverState('node0', expect_error = True)
+        self.delBlockDriverState('overlay0', expect_error = True)
+        self.ejectDrive('drive0', 'overlay0', destroys_media = False)
+        self.delBlockBackend('drive0', None)
+        self.delBlockDriverState('node0', expect_error = True)
+        self.delBlockDriverState('overlay0')
+        self.checkBlockDriverState('node0', False)
+
+    def testMirror(self):
+        self.addBlockBackend('drive0', 'node0')
+        self.createMirror('drive0', 'node0', 'mirror0')
+        # The block job prevents removing the device
+        self.delBlockBackend('drive0', 'node0', expect_error = True)
+        self.delBlockDriverState('node0', expect_error = True)
+        self.delBlockDriverState('mirror0', expect_error = True)
+        self.wait_ready('drive0')
+        self.completeBlockJob('drive0', 'node0', 'mirror0')
+        self.assert_no_active_block_jobs()
+        self.checkBlockDriverState('node0', False)
+        # This succeeds because the backend now points to mirror0
+        self.delBlockBackend('drive0', 'mirror0')
+
+    def testBlkDebug(self):
+        self.addBlkDebug('debug0', 'node0')
+        # 'node0' is used by the blkdebug node
+        self.delBlockDriverState('node0', expect_error = True)
+        # But we can remove the blkdebug node directly
+        self.delBlockDriverState('debug0')
+        self.checkBlockDriverState('node0', False)
+
+    def testBlkVerify(self):
+        self.addBlkVerify('verify0', 'node0', 'node1')
+        # We cannot remove the children of a blkverify device
+        self.delBlockDriverState('node0', expect_error = True)
+        self.delBlockDriverState('node1', expect_error = True)
+        # But we can remove the blkverify node directly
+        self.delBlockDriverState('verify0')
+        self.checkBlockDriverState('node0', False)
+        self.checkBlockDriverState('node1', False)
+
+    def testQuorum(self):
+        self.addQuorum('quorum0', 'node0', 'node1')
+        # We cannot remove the children of a Quorum device
+        self.delBlockDriverState('node0', expect_error = True)
+        self.delBlockDriverState('node1', expect_error = True)
+        # But we can remove the Quorum node directly
+        self.delBlockDriverState('quorum0')
+        self.checkBlockDriverState('node0', False)
+        self.checkBlockDriverState('node1', False)
+
+
+if __name__ == '__main__':
+    iotests.main(supported_fmts=["qcow2"])
diff --git a/tests/qemu-iotests/139.out b/tests/qemu-iotests/139.out
new file mode 100644
index 0000000..281b69e
--- /dev/null
+++ b/tests/qemu-iotests/139.out
@@ -0,0 +1,5 @@
+............
+----------------------------------------------------------------------
+Ran 12 tests
+
+OK
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index a6a61ae..c69265d 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -138,3 +138,4 @@
 135 rw auto
 137 rw auto
 138 rw auto quick
+139 rw auto quick
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [Qemu-devel] [PULL 37/37] qcow2: Fix qcow2_get_cluster_offset() for zero clusters
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (35 preceding siblings ...)
  2015-11-05 18:18 ` [Qemu-devel] [PULL 36/37] iotests: Add tests for the x-blockdev-del command Kevin Wolf
@ 2015-11-05 18:18 ` Kevin Wolf
  2015-11-05 19:01 ` [Qemu-devel] [PULL 00/37] Block layer patches Peter Maydell
  37 siblings, 0 replies; 41+ messages in thread
From: Kevin Wolf @ 2015-11-05 18:18 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, qemu-devel

When searching for contiguous zero clusters, we only need to check the
cluster type. Before this patch, an increasing offset (L2E_OFFSET_MASK)
was expected, so that the function never returned more than a single
zero cluster in practice. This patch fixes it to actually return as many
contiguous zero clusters as it can.

Cc: qemu-stable@nongnu.org
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-id: 1446657384-5907-1-git-send-email-kwolf@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2-cluster.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 67be0ce..24a60e2 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -312,7 +312,7 @@ static int count_contiguous_clusters(int nb_clusters, int cluster_size,
     if (!offset)
         return 0;
 
-    assert(qcow2_get_cluster_type(first_entry) != QCOW2_CLUSTER_COMPRESSED);
+    assert(qcow2_get_cluster_type(first_entry) == QCOW2_CLUSTER_NORMAL);
 
     for (i = 0; i < nb_clusters; i++) {
         uint64_t l2_entry = be64_to_cpu(l2_table[i]) & mask;
@@ -324,14 +324,16 @@ static int count_contiguous_clusters(int nb_clusters, int cluster_size,
 	return i;
 }
 
-static int count_contiguous_free_clusters(int nb_clusters, uint64_t *l2_table)
+static int count_contiguous_clusters_by_type(int nb_clusters,
+                                             uint64_t *l2_table,
+                                             int wanted_type)
 {
     int i;
 
     for (i = 0; i < nb_clusters; i++) {
         int type = qcow2_get_cluster_type(be64_to_cpu(l2_table[i]));
 
-        if (type != QCOW2_CLUSTER_UNALLOCATED) {
+        if (type != wanted_type) {
             break;
         }
     }
@@ -554,13 +556,14 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset,
             ret = -EIO;
             goto fail;
         }
-        c = count_contiguous_clusters(nb_clusters, s->cluster_size,
-                &l2_table[l2_index], QCOW_OFLAG_ZERO);
+        c = count_contiguous_clusters_by_type(nb_clusters, &l2_table[l2_index],
+                                              QCOW2_CLUSTER_ZERO);
         *cluster_offset = 0;
         break;
     case QCOW2_CLUSTER_UNALLOCATED:
         /* how many empty clusters ? */
-        c = count_contiguous_free_clusters(nb_clusters, &l2_table[l2_index]);
+        c = count_contiguous_clusters_by_type(nb_clusters, &l2_table[l2_index],
+                                              QCOW2_CLUSTER_UNALLOCATED);
         *cluster_offset = 0;
         break;
     case QCOW2_CLUSTER_NORMAL:
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PULL 00/37] Block layer patches
  2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
                   ` (36 preceding siblings ...)
  2015-11-05 18:18 ` [Qemu-devel] [PULL 37/37] qcow2: Fix qcow2_get_cluster_offset() for zero clusters Kevin Wolf
@ 2015-11-05 19:01 ` Peter Maydell
  2015-11-06  8:17   ` Kevin Wolf
  37 siblings, 1 reply; 41+ messages in thread
From: Peter Maydell @ 2015-11-05 19:01 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: QEMU Developers, Qemu-block

On 5 November 2015 at 18:17, Kevin Wolf <kwolf@redhat.com> wrote:
> The following changes since commit 8835b9df3bddf332c883c861d6a1defc12c4ebe9:
>
>   Merge remote-tracking branch 'remotes/mdroth/tags/qga-pull-2015-11-04-tag' into staging (2015-11-05 10:52:35 +0000)
>
> are available in the git repository at:
>
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to 22bdadd23b64af65ac2dd816848dbe2b1240a77a:
>
>   Merge remote-tracking branch 'mreitz/tags/pull-block-for-kevin-2015-11-05' into queue-block (2015-11-05 18:01:37 +0100)
>
> ----------------------------------------------------------------
>
> Block layer patches

Fails to build on OSX, I'm afraid:
Undefined symbols for architecture x86_64:
  "_qmp_change_blockdev", referenced from:
      -[QemuCocoaAppController changeDeviceMedia:] in cocoa.o

thanks
-- PMM

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PULL 00/37] Block layer patches
  2015-11-05 19:01 ` [Qemu-devel] [PULL 00/37] Block layer patches Peter Maydell
@ 2015-11-06  8:17   ` Kevin Wolf
  2015-11-06 15:10     ` Max Reitz
  0 siblings, 1 reply; 41+ messages in thread
From: Kevin Wolf @ 2015-11-06  8:17 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers, Qemu-block, mreitz

Am 05.11.2015 um 20:01 hat Peter Maydell geschrieben:
> On 5 November 2015 at 18:17, Kevin Wolf <kwolf@redhat.com> wrote:
> > The following changes since commit 8835b9df3bddf332c883c861d6a1defc12c4ebe9:
> >
> >   Merge remote-tracking branch 'remotes/mdroth/tags/qga-pull-2015-11-04-tag' into staging (2015-11-05 10:52:35 +0000)
> >
> > are available in the git repository at:
> >
> >
> >   git://repo.or.cz/qemu/kevin.git tags/for-upstream
> >
> > for you to fetch changes up to 22bdadd23b64af65ac2dd816848dbe2b1240a77a:
> >
> >   Merge remote-tracking branch 'mreitz/tags/pull-block-for-kevin-2015-11-05' into queue-block (2015-11-05 18:01:37 +0100)
> >
> > ----------------------------------------------------------------
> >
> > Block layer patches
> 
> Fails to build on OSX, I'm afraid:
> Undefined symbols for architecture x86_64:
>   "_qmp_change_blockdev", referenced from:
>       -[QemuCocoaAppController changeDeviceMedia:] in cocoa.o

Max, I think this is yours.

Kevin

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [Qemu-devel] [PULL 00/37] Block layer patches
  2015-11-06  8:17   ` Kevin Wolf
@ 2015-11-06 15:10     ` Max Reitz
  0 siblings, 0 replies; 41+ messages in thread
From: Max Reitz @ 2015-11-06 15:10 UTC (permalink / raw)
  To: Kevin Wolf, Peter Maydell; +Cc: QEMU Developers, Qemu-block

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

On 06.11.2015 09:17, Kevin Wolf wrote:
> Am 05.11.2015 um 20:01 hat Peter Maydell geschrieben:
>> On 5 November 2015 at 18:17, Kevin Wolf <kwolf@redhat.com> wrote:
>>> The following changes since commit 8835b9df3bddf332c883c861d6a1defc12c4ebe9:
>>>
>>>   Merge remote-tracking branch 'remotes/mdroth/tags/qga-pull-2015-11-04-tag' into staging (2015-11-05 10:52:35 +0000)
>>>
>>> are available in the git repository at:
>>>
>>>
>>>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>>>
>>> for you to fetch changes up to 22bdadd23b64af65ac2dd816848dbe2b1240a77a:
>>>
>>>   Merge remote-tracking branch 'mreitz/tags/pull-block-for-kevin-2015-11-05' into queue-block (2015-11-05 18:01:37 +0100)
>>>
>>> ----------------------------------------------------------------
>>>
>>> Block layer patches
>>
>> Fails to build on OSX, I'm afraid:
>> Undefined symbols for architecture x86_64:
>>   "_qmp_change_blockdev", referenced from:
>>       -[QemuCocoaAppController changeDeviceMedia:] in cocoa.o
> 
> Max, I think this is yours.

Indeed, will fix. Sorry.

Max



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

^ permalink raw reply	[flat|nested] 41+ messages in thread

end of thread, other threads:[~2015-11-06 15:10 UTC | newest]

Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-05 18:17 [Qemu-devel] [PULL 00/37] Block layer patches Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 01/37] block: Don't call blk_bs() twice in bdrv_lookup_bs() Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 02/37] block: Add blk_remove_bs() Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 03/37] block: Make bdrv_states public Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 04/37] block: Add functions for inheriting a BBRS Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 05/37] blockdev: Add blockdev-open-tray Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 06/37] blockdev: Add blockdev-close-tray Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 07/37] blockdev: Add blockdev-remove-medium Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 08/37] blockdev: Add blockdev-insert-medium Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 09/37] blockdev: Implement eject with basic operations Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 10/37] blockdev: Implement change " Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 11/37] block: Inquire tray state before tray-moved events Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 12/37] qmp: Introduce blockdev-change-medium Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 13/37] hmp: Use blockdev-change-medium for change command Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 14/37] blockdev: read-only-mode for blockdev-change-medium Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 15/37] hmp: Add read-only-mode option to change command Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 16/37] iotests: Add test for change-related QMP commands Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 17/37] block: check for existing device IDs in external_snapshot_prepare() Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 18/37] block: rename BlockdevSnapshot to BlockdevSnapshotSync Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 19/37] block: support passing 'backing': '' to 'blockdev-add' Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 20/37] block: add a 'blockdev-snapshot' QMP command Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 21/37] block: add tests for the 'blockdev-snapshot' command Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 22/37] commit: reopen overlay_bs before base Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 23/37] qemu-iotests: Test the reopening of overlay_bs in 'block-commit' Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 24/37] qcow2: avoid misaligned 64bit bswap Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 25/37] qemu-img: add check for zero-length job len Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 26/37] throttle: Check for pending requests in throttle_group_unregister_bs() Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 27/37] throttle: Use bs->throttle_state instead of bs->io_limits_enabled Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 28/37] block: Disallow snapshots if the overlay doesn't support backing files Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 29/37] block: Remove inner quotation marks in iotest 085 Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 30/37] block: test 'blockdev-snapshot' using a file BDS as the overlay Kevin Wolf
2015-11-05 18:17 ` [Qemu-devel] [PULL 31/37] qemu-iotests: fix cleanup of background processes Kevin Wolf
2015-11-05 18:18 ` [Qemu-devel] [PULL 32/37] qemu-iotests: fix -valgrind option for check Kevin Wolf
2015-11-05 18:18 ` [Qemu-devel] [PULL 33/37] mirror: block all operations on the target image during the job Kevin Wolf
2015-11-05 18:18 ` [Qemu-devel] [PULL 34/37] block: Add blk_get_refcnt() Kevin Wolf
2015-11-05 18:18 ` [Qemu-devel] [PULL 35/37] block: Add 'x-blockdev-del' QMP command Kevin Wolf
2015-11-05 18:18 ` [Qemu-devel] [PULL 36/37] iotests: Add tests for the x-blockdev-del command Kevin Wolf
2015-11-05 18:18 ` [Qemu-devel] [PULL 37/37] qcow2: Fix qcow2_get_cluster_offset() for zero clusters Kevin Wolf
2015-11-05 19:01 ` [Qemu-devel] [PULL 00/37] Block layer patches Peter Maydell
2015-11-06  8:17   ` Kevin Wolf
2015-11-06 15:10     ` Max Reitz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.