[Qemu-devel] [PULL 00/33] Block layer patches

[Qemu-devel] [PULL 00/33] Block layer patches

[Kevin Wolf]

The following changes since commit 430da7a81d356e368ccd88dcca60f38da9aa5b9a:

  Merge remote-tracking branch 'remotes/riku/tags/pull-linux-user-20160915' into staging (2016-09-22 15:39:54 +0100)

are available in the git repository at:


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

for you to fetch changes up to 5dccdc1b1e0c57746e5f90c69f697029a3686b5f:

  block: Remove BB interface from blockdev-add/del (2016-09-22 18:02:15 +0200)

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

----------------------------------------------------------------
Alberto Garcia (7):
      block: Remove bdrv_is_snapshot
      block: Set BDRV_O_ALLOW_RDWR and snapshot_options before storing the flags
      block: Update bs->open_flags earlier in bdrv_open_common()
      block: Add "read-only" to the options QDict
      block: Don't queue the same BDS twice in bdrv_reopen_queue_child()
      commit: Add 'base' to the reopen queue before 'overlay_bs'
      block: rename "read-only" to BDRV_OPT_READ_ONLY

Daniel P. Berrange (1):
      qcow2: fix encryption during cow of sectors

Denis V. Lunev (1):
      tests: allow to specify list of formats to test for check-block.sh

Kevin Wolf (24):
      hmp: Remove dead code in hmp_qemu_io()
      block: Fix 'since' for compressed Drive/BlockdevBackup
      block: Add blk_by_dev()
      qdev-monitor: Factor out find_device_state()
      qdev-monitor: Add blk_by_qdev_id()
      block: Accept device model name for blockdev-open/close-tray
      block: Accept device model name for x-blockdev-insert-medium
      block: Accept device model name for x-blockdev-remove-medium
      block: Accept device model name for eject
      block: Accept device model name for blockdev-change-medium
      block: Accept device model name for block_set_io_throttle
      qemu-iotests/118: Test media change with qdev name
      qemu-iotests/041: Avoid blockdev-add with id
      qemu-iotests/067: Avoid blockdev-add with id
      qemu-iotests/071: Avoid blockdev-add with id
      qemu-iotests/081: Avoid blockdev-add with id
      qemu-iotests/087: Avoid blockdev-add with id
      qemu-iotests/117: Avoid blockdev-add with id
      qemu-iotests/118: Avoid blockdev-add with id
      qemu-iotests/124: Avoid blockdev-add with id
      qemu-iotests/139: Avoid blockdev-add with id
      block: Avoid printing NULL string in error messages
      qemu-iotests/141: Avoid blockdev-add with id
      block: Remove BB interface from blockdev-add/del

 block.c                        |  89 ++++++++----
 block/block-backend.c          |  19 +++
 block/commit.c                 |   8 +-
 block/qcow2-cluster.c          |   2 +-
 block/vvfat.c                  |   3 +-
 blockdev.c                     | 309 ++++++++++++++++++++---------------------
 docs/qmp-commands.txt          |  78 ++++++-----
 hmp.c                          |  21 ++-
 include/block/block.h          |   2 +-
 include/sysemu/block-backend.h |   2 +
 qapi/block-core.json           |  78 +++++------
 qapi/block.json                |   9 +-
 qdev-monitor.c                 |  34 ++++-
 qmp.c                          |   4 +-
 tests/check-block.sh           |  13 +-
 tests/qemu-iotests/041         |  71 +++++-----
 tests/qemu-iotests/067         |   6 +-
 tests/qemu-iotests/067.out     | 211 +++++++++++++++++-----------
 tests/qemu-iotests/071         |   8 +-
 tests/qemu-iotests/081         |   2 +-
 tests/qemu-iotests/085.out     |   6 +-
 tests/qemu-iotests/087         |  62 +--------
 tests/qemu-iotests/087.out     |   8 +-
 tests/qemu-iotests/117         |   4 +-
 tests/qemu-iotests/118         |  91 +++++++++---
 tests/qemu-iotests/124         |  17 ++-
 tests/qemu-iotests/139         | 178 ++++++++----------------
 tests/qemu-iotests/139.out     |   4 +-
 tests/qemu-iotests/141         |  24 ++--
 tests/qemu-iotests/141.out     |  24 ++--
 tests/qemu-iotests/158         |  80 +++++++++++
 tests/qemu-iotests/158.out     |  36 +++++
 tests/qemu-iotests/group       |   1 +
 tests/qemu-iotests/iotests.py  |   5 +
 34 files changed, 843 insertions(+), 666 deletions(-)
 create mode 100755 tests/qemu-iotests/158
 create mode 100644 tests/qemu-iotests/158.out

[Qemu-devel] [PULL 01/33] qcow2: fix encryption during cow of sectors

[Kevin Wolf]

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

Broken in previous commit:

  commit aaa4d20b4972bb1a811ce929502e6741835d584e
  Author: Kevin Wolf <kwolf@redhat.com>
  Date:   Wed Jun 1 15:21:05 2016 +0200

      qcow2: Make copy_sectors() byte based

The copy_sectors() code was originally using the 'sector'
parameter for encryption, which was passed in by the caller
from the QCowL2Meta.offset field (aka the guest logical
offset).

After the change, the code is using 'cluster_offset' which
was passed in from QCow2L2Meta.alloc_offset field (aka the
host physical offset).

This would cause the data to be encrypted using an incorrect
initialization vector which will in turn cause later reads
to return garbage.

Although current qcow2 built-in encryption is blocked from
usage in the emulator, one could still hit this if writing
to the file via qemu-{img,io,nbd} commands.

Cc: qemu-stable@nongnu.org
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-cluster.c      |  2 +-
 tests/qemu-iotests/158     | 80 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/158.out | 36 +++++++++++++++++++++
 tests/qemu-iotests/group   |  1 +
 4 files changed, 118 insertions(+), 1 deletion(-)
 create mode 100755 tests/qemu-iotests/158
 create mode 100644 tests/qemu-iotests/158.out

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 9ab445d..61d1ffd 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -429,7 +429,7 @@ static int coroutine_fn do_perform_cow(BlockDriverState *bs,
 
     if (bs->encrypted) {
         Error *err = NULL;
-        int64_t sector = (cluster_offset + offset_in_cluster)
+        int64_t sector = (src_cluster_offset + offset_in_cluster)
                          >> BDRV_SECTOR_BITS;
         assert(s->cipher);
         assert((offset_in_cluster & ~BDRV_SECTOR_MASK) == 0);
diff --git a/tests/qemu-iotests/158 b/tests/qemu-iotests/158
new file mode 100755
index 0000000..a6cdd6d
--- /dev/null
+++ b/tests/qemu-iotests/158
@@ -0,0 +1,80 @@
+#!/bin/bash
+#
+# Test encrypted read/write using backing files
+#
+# 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/>.
+#
+
+# creator
+owner=berrange@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+status=1	# failure is the default!
+
+_cleanup()
+{
+	_cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2
+_supported_proto generic
+_supported_os Linux
+
+
+size=128M
+TEST_IMG_BASE=$TEST_IMG.base
+
+TEST_IMG_SAVE=$TEST_IMG
+TEST_IMG=$TEST_IMG_BASE
+echo "== create base =="
+IMGOPTS="encryption=on" _make_test_img $size
+TEST_IMG=$TEST_IMG_SAVE
+
+echo
+echo "== writing whole image =="
+echo "astrochicken" | $QEMU_IO -c "write -P 0xa 0 $size" "$TEST_IMG_BASE" | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== verify pattern =="
+echo "astrochicken" | $QEMU_IO -c "read -P 0xa 0 $size" "$TEST_IMG_BASE" | _filter_qemu_io | _filter_testdir
+
+echo "== create overlay =="
+IMGOPTS="encryption=on" _make_test_img -b "$TEST_IMG_BASE" $size
+
+echo
+echo "== writing part of a cluster =="
+echo "astrochicken" | $QEMU_IO -c "write -P 0xe 0 1024" "$TEST_IMG" | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== verify pattern =="
+echo "astrochicken" | $QEMU_IO -c "read -P 0xe 0 1024" "$TEST_IMG" | _filter_qemu_io | _filter_testdir
+echo
+echo "== verify pattern =="
+echo "astrochicken" | $QEMU_IO -c "read -P 0xa 1024 64512" "$TEST_IMG" | _filter_qemu_io | _filter_testdir
+
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/158.out b/tests/qemu-iotests/158.out
new file mode 100644
index 0000000..b3f37e2
--- /dev/null
+++ b/tests/qemu-iotests/158.out
@@ -0,0 +1,36 @@
+QA output created by 158
+== create base ==
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728 encryption=on
+
+== writing whole image ==
+Disk image 'TEST_DIR/t.qcow2.base' is encrypted.
+password:
+wrote 134217728/134217728 bytes at offset 0
+128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verify pattern ==
+Disk image 'TEST_DIR/t.qcow2.base' is encrypted.
+password:
+read 134217728/134217728 bytes at offset 0
+128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+== create overlay ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base encryption=on
+
+== writing part of a cluster ==
+Disk image 'TEST_DIR/t.qcow2' is encrypted.
+password:
+wrote 1024/1024 bytes at offset 0
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verify pattern ==
+Disk image 'TEST_DIR/t.qcow2' is encrypted.
+password:
+read 1024/1024 bytes at offset 0
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verify pattern ==
+Disk image 'TEST_DIR/t.qcow2' is encrypted.
+password:
+read 64512/64512 bytes at offset 1024
+63 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index a57fc92..7eb1770 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -157,6 +157,7 @@
 155 rw auto
 156 rw auto quick
 157 auto
+158 rw auto quick
 159 rw auto quick
 160 rw auto quick
 162 auto quick
-- 
1.8.3.1

[Qemu-devel] [PULL 02/33] hmp: Remove dead code in hmp_qemu_io()

[Kevin Wolf]

blk can never be NULL, drop the check. This fixes a Coverity warning.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hmp.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/hmp.c b/hmp.c
index ad33b44..0a16aef 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1924,6 +1924,7 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict)
 {
     BlockBackend *blk;
     BlockBackend *local_blk = NULL;
+    AioContext *aio_context;
     const char* device = qdict_get_str(qdict, "device");
     const char* command = qdict_get_str(qdict, "command");
     Error *err = NULL;
@@ -1939,17 +1940,12 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict)
         }
     }
 
-    if (blk) {
-        AioContext *aio_context = blk_get_aio_context(blk);
-        aio_context_acquire(aio_context);
+    aio_context = blk_get_aio_context(blk);
+    aio_context_acquire(aio_context);
 
-        qemuio_command(blk, command);
+    qemuio_command(blk, command);
 
-        aio_context_release(aio_context);
-    } else {
-        error_set(&err, ERROR_CLASS_DEVICE_NOT_FOUND,
-                  "Device '%s' not found", device);
-    }
+    aio_context_release(aio_context);
 
 fail:
     blk_unref(local_blk);
-- 
1.8.3.1

[Qemu-devel] [PULL 03/33] tests: allow to specify list of formats to test for check-block.sh

[Kevin Wolf]

From: "Denis V. Lunev" <den@openvz.org>

This would make code better and allow to test specific format.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/check-block.sh | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/tests/check-block.sh b/tests/check-block.sh
index a37797a..c3de378 100755
--- a/tests/check-block.sh
+++ b/tests/check-block.sh
@@ -1,5 +1,10 @@
 #!/bin/sh
 
+FORMAT_LIST="raw qcow2 qed vmdk vpc"
+if [ "$#" -ne 0 ]; then
+    FORMAT_LIST="$@"
+fi
+
 export QEMU_PROG="$(pwd)/x86_64-softmmu/qemu-system-x86_64"
 export QEMU_IMG_PROG="$(pwd)/qemu-img"
 export QEMU_IO_PROG="$(pwd)/qemu-io"
@@ -12,10 +17,8 @@ fi
 cd tests/qemu-iotests
 
 ret=0
-./check -T -nocache -raw || ret=1
-./check -T -nocache -qcow2 || ret=1
-./check -T -nocache -qed|| ret=1
-./check -T -nocache -vmdk|| ret=1
-./check -T -nocache -vpc || ret=1
+for FMT in $FORMAT_LIST ; do
+    ./check -T -nocache -$FMT || ret=1
+done
 
 exit $ret
-- 
1.8.3.1

[Qemu-devel] [PULL 04/33] block: Remove bdrv_is_snapshot

[Kevin Wolf]

From: Alberto Garcia <berto@igalia.com>

This is unnecessary and has been unused since 5433c24f0f9306c82ad9bcc.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c               | 5 -----
 include/block/block.h | 1 -
 2 files changed, 6 deletions(-)

diff --git a/block.c b/block.c
index afaff93..d448520 100644
--- a/block.c
+++ b/block.c
@@ -3013,11 +3013,6 @@ bool bdrv_debug_is_suspended(BlockDriverState *bs, const char *tag)
     return false;
 }
 
-int bdrv_is_snapshot(BlockDriverState *bs)
-{
-    return !!(bs->open_flags & BDRV_O_SNAPSHOT);
-}
-
 /* backing_file can either be relative, or absolute, or a protocol.  If it is
  * relative, it must be relative to the chain.  So, passing in bs->filename
  * from a BDS as backing_file should not be done, as that may be relative to
diff --git a/include/block/block.h b/include/block/block.h
index ffecebf..f374e1a 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -414,7 +414,6 @@ void bdrv_get_full_backing_filename_from_filename(const char *backed,
                                                   const char *backing,
                                                   char *dest, size_t sz,
                                                   Error **errp);
-int bdrv_is_snapshot(BlockDriverState *bs);
 
 int path_has_protocol(const char *path);
 int path_is_absolute(const char *path);
-- 
1.8.3.1

[Qemu-devel] [PULL 05/33] block: Set BDRV_O_ALLOW_RDWR and snapshot_options before storing the flags

[Kevin Wolf]

From: Alberto Garcia <berto@igalia.com>

If an image is opened with snapshot=on, its flags are modified by
bdrv_backing_options() and then bs->open_flags is updated accordingly.
This last step is unnecessary if we calculate the new flags before
setting bs->open_flags.

Soon we'll introduce the "read-only" option, and then we'll need to
be able to modify its value in the QDict when snapshot=on. This is
more cumbersome if bs->options is already set. This patch simplifies
that. Other than that, there are no semantic changes. Although it
might seem that bs->options can have a different value now because
it is stored after calling bdrv_backing_options(), this call doesn't
actually modify them in this scenario.

The code that sets BDRV_O_ALLOW_RDWR is also moved for the same
reason.

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

diff --git a/block.c b/block.c
index d448520..324dbe7 100644
--- a/block.c
+++ b/block.c
@@ -1675,6 +1675,17 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
         goto fail;
     }
 
+    if (flags & BDRV_O_RDWR) {
+        flags |= BDRV_O_ALLOW_RDWR;
+    }
+
+    if (flags & BDRV_O_SNAPSHOT) {
+        snapshot_options = qdict_new();
+        bdrv_temp_snapshot_options(&snapshot_flags, snapshot_options,
+                                   flags, options);
+        bdrv_backing_options(&flags, options, flags, options);
+    }
+
     bs->open_flags = flags;
     bs->options = options;
     options = qdict_clone_shallow(options);
@@ -1699,18 +1710,6 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
 
     /* Open image file without format layer */
     if ((flags & BDRV_O_PROTOCOL) == 0) {
-        if (flags & BDRV_O_RDWR) {
-            flags |= BDRV_O_ALLOW_RDWR;
-        }
-        if (flags & BDRV_O_SNAPSHOT) {
-            snapshot_options = qdict_new();
-            bdrv_temp_snapshot_options(&snapshot_flags, snapshot_options,
-                                       flags, options);
-            bdrv_backing_options(&flags, options, flags, options);
-        }
-
-        bs->open_flags = flags;
-
         file = bdrv_open_child(filename, options, "file", bs,
                                &child_file, true, &local_err);
         if (local_err) {
-- 
1.8.3.1

[Qemu-devel] [PULL 06/33] block: Update bs->open_flags earlier in bdrv_open_common()

[Kevin Wolf]

From: Alberto Garcia <berto@igalia.com>

We're only doing this immediately before opening the image, but
bs->open_flags is used earlier in the function. At the moment this is
not causing problems because none of the checked flags are modified by
update_flags_from_options(), but this will change when we introduce
the "read-only" option.

This patch calls update_flags_from_options() at the beginning of the
function, immediately after creating the QemuOpts.

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

diff --git a/block.c b/block.c
index 324dbe7..a98b75e 100644
--- a/block.c
+++ b/block.c
@@ -961,6 +961,8 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
         goto fail_opts;
     }
 
+    update_flags_from_options(&bs->open_flags, opts);
+
     driver_name = qemu_opt_get(opts, "driver");
     drv = bdrv_find_format(driver_name);
     assert(drv != NULL);
@@ -1022,9 +1024,6 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
     bs->drv = drv;
     bs->opaque = g_malloc0(drv->instance_size);
 
-    /* Apply cache mode options */
-    update_flags_from_options(&bs->open_flags, opts);
-
     /* Open the image, either directly or using a protocol */
     open_flags = bdrv_open_flags(bs, bs->open_flags);
     if (drv->bdrv_file_open) {
-- 
1.8.3.1

[Qemu-devel] [PULL 07/33] block: Add "read-only" to the options QDict

[Kevin Wolf]

From: Alberto Garcia <berto@igalia.com>

This adds the "read-only" option to the QDict. One important effect of
this change is that when a child inherits options from its parent, the
existing "read-only" mode can be preserved if it was explicitly set
previously.

This addresses scenarios like this:

   [E] <- [D] <- [C] <- [B] <- [A]

In this case, if we reopen [D] with read-only=off, and later reopen
[B], then [D] will not inherit read-only=on from its parent during the
bdrv_reopen_queue_child() stage.

The BDRV_O_RDWR flag is not removed yet, but its keep in sync with the
value of the "read-only" option.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c               | 38 +++++++++++++++++++++++++++++++++++---
 block/vvfat.c         |  3 ++-
 blockdev.c            | 16 +++++++---------
 include/block/block.h |  1 +
 4 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/block.c b/block.c
index a98b75e..b724ada 100644
--- a/block.c
+++ b/block.c
@@ -733,6 +733,9 @@ static void bdrv_temp_snapshot_options(int *child_flags, QDict *child_options,
     qdict_set_default_str(child_options, BDRV_OPT_CACHE_DIRECT, "off");
     qdict_set_default_str(child_options, BDRV_OPT_CACHE_NO_FLUSH, "on");
 
+    /* Copy the read-only option from the parent */
+    qdict_copy_default(child_options, parent_options, BDRV_OPT_READ_ONLY);
+
     /* aio=native doesn't work for cache.direct=off, so disable it for the
      * temporary snapshot */
     *child_flags &= ~BDRV_O_NATIVE_AIO;
@@ -755,6 +758,9 @@ static void bdrv_inherited_options(int *child_flags, QDict *child_options,
     qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_DIRECT);
     qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_NO_FLUSH);
 
+    /* Inherit the read-only option from the parent if it's not set */
+    qdict_copy_default(child_options, parent_options, BDRV_OPT_READ_ONLY);
+
     /* Our block drivers take care to send flushes and respect unmap policy,
      * so we can default to enable both on lower layers regardless of the
      * corresponding parent options. */
@@ -808,7 +814,8 @@ static void bdrv_backing_options(int *child_flags, QDict *child_options,
     qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_NO_FLUSH);
 
     /* backing files always opened read-only */
-    flags &= ~(BDRV_O_RDWR | BDRV_O_COPY_ON_READ);
+    qdict_set_default_str(child_options, BDRV_OPT_READ_ONLY, "on");
+    flags &= ~BDRV_O_COPY_ON_READ;
 
     /* snapshot=on is handled on the top layer */
     flags &= ~(BDRV_O_SNAPSHOT | BDRV_O_TEMPORARY);
@@ -855,6 +862,14 @@ static void update_flags_from_options(int *flags, QemuOpts *opts)
     if (qemu_opt_get_bool(opts, BDRV_OPT_CACHE_DIRECT, false)) {
         *flags |= BDRV_O_NOCACHE;
     }
+
+    *flags &= ~BDRV_O_RDWR;
+
+    assert(qemu_opt_find(opts, BDRV_OPT_READ_ONLY));
+    if (!qemu_opt_get_bool(opts, BDRV_OPT_READ_ONLY, false)) {
+        *flags |= BDRV_O_RDWR;
+    }
+
 }
 
 static void update_options_from_flags(QDict *options, int flags)
@@ -867,6 +882,10 @@ static void update_options_from_flags(QDict *options, int flags)
         qdict_put(options, BDRV_OPT_CACHE_NO_FLUSH,
                   qbool_from_bool(flags & BDRV_O_NO_FLUSH));
     }
+    if (!qdict_haskey(options, BDRV_OPT_READ_ONLY)) {
+        qdict_put(options, BDRV_OPT_READ_ONLY,
+                  qbool_from_bool(!(flags & BDRV_O_RDWR)));
+    }
 }
 
 static void bdrv_assign_node_name(BlockDriverState *bs,
@@ -930,6 +949,11 @@ static QemuOptsList bdrv_runtime_opts = {
             .type = QEMU_OPT_BOOL,
             .help = "Ignore flush requests",
         },
+        {
+            .name = BDRV_OPT_READ_ONLY,
+            .type = QEMU_OPT_BOOL,
+            .help = "Node is opened in read-only mode",
+        },
         { /* end of list */ }
     },
 };
@@ -1674,14 +1698,22 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
         goto fail;
     }
 
-    if (flags & BDRV_O_RDWR) {
-        flags |= BDRV_O_ALLOW_RDWR;
+    /* Set the BDRV_O_RDWR and BDRV_O_ALLOW_RDWR flags.
+     * FIXME: we're parsing the QDict to avoid having to create a
+     * QemuOpts just for this, but neither option is optimal. */
+    if (g_strcmp0(qdict_get_try_str(options, BDRV_OPT_READ_ONLY), "on") &&
+        !qdict_get_try_bool(options, BDRV_OPT_READ_ONLY, false)) {
+        flags |= (BDRV_O_RDWR | BDRV_O_ALLOW_RDWR);
+    } else {
+        flags &= ~BDRV_O_RDWR;
     }
 
     if (flags & BDRV_O_SNAPSHOT) {
         snapshot_options = qdict_new();
         bdrv_temp_snapshot_options(&snapshot_flags, snapshot_options,
                                    flags, options);
+        /* Let bdrv_backing_options() override "read-only" */
+        qdict_del(options, BDRV_OPT_READ_ONLY);
         bdrv_backing_options(&flags, options, flags, options);
     }
 
diff --git a/block/vvfat.c b/block/vvfat.c
index ba2620f..ded2109 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2971,7 +2971,8 @@ static BlockDriver vvfat_write_target = {
 static void vvfat_qcow_options(int *child_flags, QDict *child_options,
                                int parent_flags, QDict *parent_options)
 {
-    *child_flags = BDRV_O_RDWR | BDRV_O_NO_FLUSH;
+    qdict_set_default_str(child_options, BDRV_OPT_READ_ONLY, "off");
+    *child_flags = BDRV_O_NO_FLUSH;
 }
 
 static const BdrvChildRole child_vvfat_qcow = {
diff --git a/blockdev.c b/blockdev.c
index 3010393..c3f8da4 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -360,9 +360,6 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
     const char *aio;
 
     if (bdrv_flags) {
-        if (!qemu_opt_get_bool(opts, "read-only", false)) {
-            *bdrv_flags |= BDRV_O_RDWR;
-        }
         if (qemu_opt_get_bool(opts, "copy-on-read", false)) {
             *bdrv_flags |= BDRV_O_COPY_ON_READ;
         }
@@ -471,7 +468,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
     int bdrv_flags = 0;
     int on_read_error, on_write_error;
     bool account_invalid, account_failed;
-    bool writethrough;
+    bool writethrough, read_only;
     BlockBackend *blk;
     BlockDriverState *bs;
     ThrottleConfig cfg;
@@ -567,6 +564,8 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
         bdrv_flags |= BDRV_O_SNAPSHOT;
     }
 
+    read_only = qemu_opt_get_bool(opts, BDRV_OPT_READ_ONLY, false);
+
     /* init */
     if ((!file || !*file) && !qdict_size(bs_opts)) {
         BlockBackendRootState *blk_rs;
@@ -574,7 +573,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
         blk = blk_new();
         blk_rs = blk_get_root_state(blk);
         blk_rs->open_flags    = bdrv_flags;
-        blk_rs->read_only     = !(bdrv_flags & BDRV_O_RDWR);
+        blk_rs->read_only     = read_only;
         blk_rs->detect_zeroes = detect_zeroes;
 
         QDECREF(bs_opts);
@@ -588,6 +587,8 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
          * Apply the defaults here instead. */
         qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_DIRECT, "off");
         qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_NO_FLUSH, "off");
+        qdict_set_default_str(bs_opts, BDRV_OPT_READ_ONLY,
+                              read_only ? "on" : "off");
         assert((bdrv_flags & BDRV_O_CACHE_MASK) == 0);
 
         if (runstate_check(RUN_STATE_INMIGRATE)) {
@@ -682,6 +683,7 @@ static BlockDriverState *bds_tree_init(QDict *bs_opts, Error **errp)
      * Apply the defaults here instead. */
     qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_DIRECT, "off");
     qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_NO_FLUSH, "off");
+    qdict_set_default_str(bs_opts, BDRV_OPT_READ_ONLY, "off");
 
     if (runstate_check(RUN_STATE_INMIGRATE)) {
         bdrv_flags |= BDRV_O_INACTIVE;
@@ -4159,10 +4161,6 @@ static QemuOptsList qemu_root_bds_opts = {
             .type = QEMU_OPT_STRING,
             .help = "host AIO implementation (threads, native)",
         },{
-            .name = "read-only",
-            .type = QEMU_OPT_BOOL,
-            .help = "open drive file as read-only",
-        },{
             .name = "copy-on-read",
             .type = QEMU_OPT_BOOL,
             .help = "copy read data from backing file into image file",
diff --git a/include/block/block.h b/include/block/block.h
index f374e1a..e18233a 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -107,6 +107,7 @@ typedef struct HDGeometry {
 #define BDRV_OPT_CACHE_WB       "cache.writeback"
 #define BDRV_OPT_CACHE_DIRECT   "cache.direct"
 #define BDRV_OPT_CACHE_NO_FLUSH "cache.no-flush"
+#define BDRV_OPT_READ_ONLY      "read-only"
 
 
 #define BDRV_SECTOR_BITS   9
-- 
1.8.3.1

[Qemu-devel] [PULL 08/33] block: Don't queue the same BDS twice in bdrv_reopen_queue_child()

[Kevin Wolf]

From: Alberto Garcia <berto@igalia.com>

bdrv_reopen_queue_child() assumes that a BlockDriverState is never
added twice to BlockReopenQueue.

That's however not the case: commit_start() adds 'base' (and its
children) to a new reopen queue, and then 'overlay_bs' (and its
children, which include 'base') to the same queue. The effect of this
is that the first set of options is ignored and overriden by the
second.

We fixed this by swapping the order in which both BDSs were added to
the queue in 3db2bd5508c86a1605258bc77c9672d93b5c350e. This patch
checks if a BDS is already in the reopen queue and keeps its options.

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

diff --git a/block.c b/block.c
index b724ada..493ecf3 100644
--- a/block.c
+++ b/block.c
@@ -1925,6 +1925,13 @@ static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue,
         options = qdict_new();
     }
 
+    /* Check if this BlockDriverState is already in the queue */
+    QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) {
+        if (bs == bs_entry->state.bs) {
+            break;
+        }
+    }
+
     /*
      * Precedence of options:
      * 1. Explicitly passed in options (highest)
@@ -1945,7 +1952,11 @@ static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue,
     }
 
     /* Old explicitly set values (don't overwrite by inherited value) */
-    old_options = qdict_clone_shallow(bs->explicit_options);
+    if (bs_entry) {
+        old_options = qdict_clone_shallow(bs_entry->state.explicit_options);
+    } else {
+        old_options = qdict_clone_shallow(bs->explicit_options);
+    }
     bdrv_join_options(bs, options, old_options);
     QDECREF(old_options);
 
@@ -1984,8 +1995,13 @@ static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue,
                                 child->role, options, flags);
     }
 
-    bs_entry = g_new0(BlockReopenQueueEntry, 1);
-    QSIMPLEQ_INSERT_TAIL(bs_queue, bs_entry, entry);
+    if (!bs_entry) {
+        bs_entry = g_new0(BlockReopenQueueEntry, 1);
+        QSIMPLEQ_INSERT_TAIL(bs_queue, bs_entry, entry);
+    } else {
+        QDECREF(bs_entry->state.options);
+        QDECREF(bs_entry->state.explicit_options);
+    }
 
     bs_entry->state.bs = bs;
     bs_entry->state.options = options;
-- 
1.8.3.1

[Qemu-devel] [PULL 09/33] commit: Add 'base' to the reopen queue before 'overlay_bs'

[Kevin Wolf]

From: Alberto Garcia <berto@igalia.com>

Now that we're checking for duplicates in the reopen queue, there's no
need to force a specific order in which the queue is constructed so we
can revert 3db2bd5508c86a1605258bc77c9672d93b5c350e.

Since both ways of constructing the queue are now valid, this patch
doesn't have any effect on the behavior of QEMU and is not strictly
necessary. However it can help us check that the fix for the reopen
queue is robust: if it stops working properly at some point, iotest
040 will break.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Kevin Wolf <kwolf@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 a02539b..9f67a8b 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -242,14 +242,14 @@ void commit_start(const char *job_id, BlockDriverState *bs,
     orig_overlay_flags = bdrv_get_flags(overlay_bs);
 
     /* convert base & overlay_bs to r/w, if necessary */
-    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 (!(orig_overlay_flags & BDRV_O_RDWR)) {
+        reopen_queue = bdrv_reopen_queue(reopen_queue, overlay_bs, NULL,
+                                         orig_overlay_flags | BDRV_O_RDWR);
+    }
     if (reopen_queue) {
         bdrv_reopen_multiple(reopen_queue, &local_err);
         if (local_err != NULL) {
-- 
1.8.3.1

[Qemu-devel] [PULL 10/33] block: rename "read-only" to BDRV_OPT_READ_ONLY

[Kevin Wolf]

From: Alberto Garcia <berto@igalia.com>

There were a few instances left. After this patch we're using the
macro in all places.

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

diff --git a/blockdev.c b/blockdev.c
index c3f8da4..fb207cd 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -807,7 +807,7 @@ QemuOptsList qemu_legacy_drive_opts = {
 
         /* Options that are passed on, but have special semantics with -drive */
         {
-            .name = "read-only",
+            .name = BDRV_OPT_READ_ONLY,
             .type = QEMU_OPT_BOOL,
             .help = "open drive file as read-only",
         },{
@@ -873,7 +873,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
 
         { "group",          "throttling.group" },
 
-        { "readonly",       "read-only" },
+        { "readonly",       BDRV_OPT_READ_ONLY },
     };
 
     for (i = 0; i < ARRAY_SIZE(opt_renames); i++) {
@@ -945,7 +945,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
     }
 
     /* copy-on-read is disabled with a warning for read-only devices */
-    read_only |= qemu_opt_get_bool(legacy_opts, "read-only", false);
+    read_only |= qemu_opt_get_bool(legacy_opts, BDRV_OPT_READ_ONLY, false);
     copy_on_read = qemu_opt_get_bool(legacy_opts, "copy-on-read", false);
 
     if (read_only && copy_on_read) {
@@ -953,7 +953,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
         copy_on_read = false;
     }
 
-    qdict_put(bs_opts, "read-only",
+    qdict_put(bs_opts, BDRV_OPT_READ_ONLY,
               qstring_from_str(read_only ? "on" : "off"));
     qdict_put(bs_opts, "copy-on-read",
               qstring_from_str(copy_on_read ? "on" :"off"));
@@ -4042,7 +4042,7 @@ QemuOptsList qemu_common_drive_opts = {
             .type = QEMU_OPT_STRING,
             .help = "write error action",
         },{
-            .name = "read-only",
+            .name = BDRV_OPT_READ_ONLY,
             .type = QEMU_OPT_BOOL,
             .help = "open drive file as read-only",
         },{
-- 
1.8.3.1

[Qemu-devel] [PULL 11/33] block: Fix 'since' for compressed Drive/BlockdevBackup

[Kevin Wolf]

These patches missed 2.7, update the QAPI documentation.

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

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 24223fd..eb0a7d5a 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -900,7 +900,7 @@
 #          otherwise. (Since 2.4)
 #
 # @compress: #optional true to compress data, if the target format supports it.
-#            (default: false) (since 2.7)
+#            (default: false) (since 2.8)
 #
 # @on-source-error: #optional the action to take on an error on the source,
 #                   default 'report'.  'stop' and 'enospc' can only be used
@@ -941,7 +941,7 @@
 #         for unlimited.
 #
 # @compress: #optional true to compress data, if the target format supports it.
-#            (default: false) (since 2.7)
+#            (default: false) (since 2.8)
 #
 # @on-source-error: #optional the action to take on an error on the source,
 #                   default 'report'.  'stop' and 'enospc' can only be used
-- 
1.8.3.1

[Qemu-devel] [PULL 12/33] block: Add blk_by_dev()

[Kevin Wolf]

This finds a BlockBackend given the device model that is attached to it.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block/block-backend.c          | 19 +++++++++++++++++++
 include/sysemu/block-backend.h |  1 +
 2 files changed, 20 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index d1349d9..0bd19ab 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -560,6 +560,25 @@ void *blk_get_attached_dev(BlockBackend *blk)
 }
 
 /*
+ * Return the BlockBackend which has the device model @dev attached if it
+ * exists, else null.
+ *
+ * @dev must not be null.
+ */
+BlockBackend *blk_by_dev(void *dev)
+{
+    BlockBackend *blk = NULL;
+
+    assert(dev != NULL);
+    while ((blk = blk_all_next(blk)) != NULL) {
+        if (blk->dev == dev) {
+            return blk;
+        }
+    }
+    return NULL;
+}
+
+/*
  * Set @blk's device model callbacks to @ops.
  * @opaque is the opaque argument to pass to the callbacks.
  * This is for use by device models.
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 4808a96..410eb68 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -111,6 +111,7 @@ int blk_attach_dev(BlockBackend *blk, void *dev);
 void blk_attach_dev_nofail(BlockBackend *blk, void *dev);
 void blk_detach_dev(BlockBackend *blk, void *dev);
 void *blk_get_attached_dev(BlockBackend *blk);
+BlockBackend *blk_by_dev(void *dev);
 void blk_set_dev_ops(BlockBackend *blk, const BlockDevOps *ops, void *opaque);
 int blk_pread_unthrottled(BlockBackend *blk, int64_t offset, uint8_t *buf,
                           int count);
-- 
1.8.3.1

[Qemu-devel] [PULL 13/33] qdev-monitor: Factor out find_device_state()

[Kevin Wolf]

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 qdev-monitor.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/qdev-monitor.c b/qdev-monitor.c
index e19617f..bc0213f 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -801,7 +801,7 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp)
     object_unref(OBJECT(dev));
 }
 
-void qmp_device_del(const char *id, Error **errp)
+static DeviceState *find_device_state(const char *id, Error **errp)
 {
     Object *obj;
 
@@ -819,15 +819,23 @@ void qmp_device_del(const char *id, Error **errp)
     if (!obj) {
         error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
                   "Device '%s' not found", id);
-        return;
+        return NULL;
     }
 
     if (!object_dynamic_cast(obj, TYPE_DEVICE)) {
         error_setg(errp, "%s is not a hotpluggable device", id);
-        return;
+        return NULL;
     }
 
-    qdev_unplug(DEVICE(obj), errp);
+    return DEVICE(obj);
+}
+
+void qmp_device_del(const char *id, Error **errp)
+{
+    DeviceState *dev = find_device_state(id, errp);
+    if (dev != NULL) {
+        qdev_unplug(dev, errp);
+    }
 }
 
 void qdev_machine_init(void)
-- 
1.8.3.1

[Qemu-devel] [PULL 14/33] qdev-monitor: Add blk_by_qdev_id()

[Kevin Wolf]

This finds the BlockBackend attached to the device model identified by
its qdev ID.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 include/sysemu/block-backend.h |  1 +
 qdev-monitor.c                 | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 410eb68..3b29317 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -112,6 +112,7 @@ void blk_attach_dev_nofail(BlockBackend *blk, void *dev);
 void blk_detach_dev(BlockBackend *blk, void *dev);
 void *blk_get_attached_dev(BlockBackend *blk);
 BlockBackend *blk_by_dev(void *dev);
+BlockBackend *blk_by_qdev_id(const char *id, Error **errp);
 void blk_set_dev_ops(BlockBackend *blk, const BlockDevOps *ops, void *opaque);
 int blk_pread_unthrottled(BlockBackend *blk, int64_t offset, uint8_t *buf,
                           int count);
diff --git a/qdev-monitor.c b/qdev-monitor.c
index bc0213f..4f78ecb 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -28,6 +28,7 @@
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
 #include "qemu/help_option.h"
+#include "sysemu/block-backend.h"
 
 /*
  * Aliases were a bad idea from the start.  Let's keep them
@@ -838,6 +839,23 @@ void qmp_device_del(const char *id, Error **errp)
     }
 }
 
+BlockBackend *blk_by_qdev_id(const char *id, Error **errp)
+{
+    DeviceState *dev;
+    BlockBackend *blk;
+
+    dev = find_device_state(id, errp);
+    if (dev == NULL) {
+        return NULL;
+    }
+
+    blk = blk_by_dev(dev);
+    if (!blk) {
+        error_setg(errp, "Device does not have a block device backend");
+    }
+    return blk;
+}
+
 void qdev_machine_init(void)
 {
     qdev_get_peripheral_anon();
-- 
1.8.3.1

[Qemu-devel] [PULL 15/33] block: Accept device model name for blockdev-open/close-tray

[Kevin Wolf]

In order to remove the need for BlockBackend names in the external API,
we want to allow qdev device names in all device related commands.

This converts blockdev-open/close-tray to accept a qdev device name.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c            | 61 ++++++++++++++++++++++++++++++++++++++-------------
 docs/qmp-commands.txt | 12 ++++++----
 qapi/block-core.json  | 14 ++++++++----
 3 files changed, 64 insertions(+), 23 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index fb207cd..046f9c6 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -56,7 +56,8 @@
 static QTAILQ_HEAD(, BlockDriverState) monitor_bdrv_states =
     QTAILQ_HEAD_INITIALIZER(monitor_bdrv_states);
 
-static int do_open_tray(const char *device, bool force, Error **errp);
+static int do_open_tray(const char *blk_name, const char *qdev_id,
+                        bool force, Error **errp);
 
 static const char *const if_name[IF_COUNT] = {
     [IF_NONE] = "none",
@@ -1198,6 +1199,29 @@ static BlockDriverState *qmp_get_root_bs(const char *name, Error **errp)
     return bs;
 }
 
+static BlockBackend *qmp_get_blk(const char *blk_name, const char *qdev_id,
+                                 Error **errp)
+{
+    BlockBackend *blk;
+
+    if (!blk_name == !qdev_id) {
+        error_setg(errp, "Need exactly one of 'device' and 'id'");
+        return NULL;
+    }
+
+    if (qdev_id) {
+        blk = blk_by_qdev_id(qdev_id, errp);
+    } else {
+        blk = blk_by_name(blk_name);
+        if (blk == NULL) {
+            error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+                      "Device '%s' not found", blk_name);
+        }
+    }
+
+    return blk;
+}
+
 void hmp_commit(Monitor *mon, const QDict *qdict)
 {
     const char *device = qdict_get_str(qdict, "device");
@@ -2250,7 +2274,7 @@ void qmp_eject(const char *device, bool has_force, bool force, Error **errp)
         force = false;
     }
 
-    rc = do_open_tray(device, force, &local_err);
+    rc = do_open_tray(device, NULL, force, &local_err);
     if (rc && rc != -ENOSYS) {
         error_propagate(errp, local_err);
         return;
@@ -2295,15 +2319,15 @@ void qmp_block_passwd(bool has_device, const char *device,
  * If the guest was asked to open the tray, return -EINPROGRESS.
  * Else, return 0.
  */
-static int do_open_tray(const char *device, bool force, Error **errp)
+static int do_open_tray(const char *blk_name, const char *qdev_id,
+                        bool force, Error **errp)
 {
     BlockBackend *blk;
+    const char *device = qdev_id ?: blk_name;
     bool locked;
 
-    blk = blk_by_name(device);
+    blk = qmp_get_blk(blk_name, qdev_id, errp);
     if (!blk) {
-        error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
-                  "Device '%s' not found", device);
         return -ENODEV;
     }
 
@@ -2339,7 +2363,9 @@ static int do_open_tray(const char *device, bool force, Error **errp)
     return 0;
 }
 
-void qmp_blockdev_open_tray(const char *device, bool has_force, bool force,
+void qmp_blockdev_open_tray(bool has_device, const char *device,
+                            bool has_id, const char *id,
+                            bool has_force, bool force,
                             Error **errp)
 {
     Error *local_err = NULL;
@@ -2348,7 +2374,9 @@ void qmp_blockdev_open_tray(const char *device, bool has_force, bool force,
     if (!has_force) {
         force = false;
     }
-    rc = do_open_tray(device, force, &local_err);
+    rc = do_open_tray(has_device ? device : NULL,
+                      has_id ? id : NULL,
+                      force, &local_err);
     if (rc && rc != -ENOSYS && rc != -EINPROGRESS) {
         error_propagate(errp, local_err);
         return;
@@ -2356,19 +2384,22 @@ void qmp_blockdev_open_tray(const char *device, bool has_force, bool force,
     error_free(local_err);
 }
 
-void qmp_blockdev_close_tray(const char *device, Error **errp)
+void qmp_blockdev_close_tray(bool has_device, const char *device,
+                             bool has_id, const char *id,
+                             Error **errp)
 {
     BlockBackend *blk;
 
-    blk = blk_by_name(device);
+    device = has_device ? device : NULL;
+    id = has_id ? id : NULL;
+
+    blk = qmp_get_blk(device, id, errp);
     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);
+        error_setg(errp, "Device '%s' is not removable", device ?: id);
         return;
     }
 
@@ -2564,7 +2595,7 @@ void qmp_blockdev_change_medium(const char *device, const char *filename,
         goto fail;
     }
 
-    rc = do_open_tray(device, false, &err);
+    rc = do_open_tray(device, NULL, false, &err);
     if (rc && rc != -ENOSYS) {
         error_propagate(errp, err);
         goto fail;
@@ -2586,7 +2617,7 @@ void qmp_blockdev_change_medium(const char *device, const char *filename,
 
     blk_apply_root_state(blk, medium_bs);
 
-    qmp_blockdev_close_tray(device, errp);
+    qmp_blockdev_close_tray(true, device, false, NULL, errp);
 
 fail:
     /* If the medium has been inserted, the device has its own reference, so
diff --git a/docs/qmp-commands.txt b/docs/qmp-commands.txt
index acebeb3..9e230f5 100644
--- a/docs/qmp-commands.txt
+++ b/docs/qmp-commands.txt
@@ -3228,7 +3228,9 @@ which no such event will be generated, these include:
 
 Arguments:
 
-- "device": block device name (json-string)
+- "device": block device name (deprecated, use @id instead)
+            (json-string, optional)
+- "id": the name or QOM path of the guest device (json-string, optional)
 - "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
@@ -3237,7 +3239,7 @@ Arguments:
 Example:
 
 -> { "execute": "blockdev-open-tray",
-     "arguments": { "device": "ide1-cd0" } }
+     "arguments": { "id": "ide0-1-0" } }
 
 <- { "timestamp": { "seconds": 1418751016,
                     "microseconds": 716996 },
@@ -3258,12 +3260,14 @@ If the tray was already closed before, this will be a no-op.
 
 Arguments:
 
-- "device": block device name (json-string)
+- "device": block device name (deprecated, use @id instead)
+            (json-string, optional)
+- "id": the name or QOM path of the guest device (json-string, optional)
 
 Example:
 
 -> { "execute": "blockdev-close-tray",
-     "arguments": { "device": "ide1-cd0" } }
+     "arguments": { "id": "ide0-1-0" } }
 
 <- { "timestamp": { "seconds": 1418751345,
                     "microseconds": 272147 },
diff --git a/qapi/block-core.json b/qapi/block-core.json
index eb0a7d5a..cd7b38a 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2363,7 +2363,9 @@
 #   to it
 # - if the guest device does not have an actual tray
 #
-# @device: block device name
+# @device: #optional Block device name (deprecated, use @id instead)
+#
+# @id:     #optional The name or QOM path of the guest device (since: 2.8)
 #
 # @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
@@ -2373,7 +2375,8 @@
 # Since: 2.5
 ##
 { 'command': 'blockdev-open-tray',
-  'data': { 'device': 'str',
+  'data': { '*device': 'str',
+            '*id': 'str',
             '*force': 'bool' } }
 
 ##
@@ -2385,12 +2388,15 @@
 #
 # If the tray was already closed before, this will be a no-op.
 #
-# @device: block device name
+# @device:  #optional Block device name (deprecated, use @id instead)
+#
+# @id:      #optional The name or QOM path of the guest device (since: 2.8)
 #
 # Since: 2.5
 ##
 { 'command': 'blockdev-close-tray',
-  'data': { 'device': 'str' } }
+  'data': { '*device': 'str',
+            '*id': 'str' } }
 
 ##
 # @x-blockdev-remove-medium:
-- 
1.8.3.1

[Qemu-devel] [PULL 16/33] block: Accept device model name for x-blockdev-insert-medium

[Kevin Wolf]

In order to remove the need for BlockBackend names in the external API,
we want to allow qdev device names in all device related commands.

This converts x-blockdev-insert-medium to accept a qdev device name.

As the command is experimental, we can still remove the 'device' option
that uses the BlockBackend name. This requires some test case changes
and is left for another series.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c            | 33 +++++++++++++++++----------------
 docs/qmp-commands.txt |  6 ++++--
 qapi/block-core.json  |  7 +++++--
 3 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 046f9c6..0eb173d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2468,34 +2468,26 @@ out:
     aio_context_release(aio_context);
 }
 
-static void qmp_blockdev_insert_anon_medium(const char *device,
+static void qmp_blockdev_insert_anon_medium(BlockBackend *blk,
                                             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);
+        error_setg(errp, "Device is not removable");
         return;
     }
 
     if (has_device && blk_dev_has_tray(blk) && !blk_dev_is_tray_open(blk)) {
-        error_setg(errp, "Tray of device '%s' is not open", device);
+        error_setg(errp, "Tray of the device is not open");
         return;
     }
 
     if (blk_bs(blk)) {
-        error_setg(errp, "There already is a medium in device '%s'", device);
+        error_setg(errp, "There already is a medium in the device");
         return;
     }
 
@@ -2511,11 +2503,20 @@ static void qmp_blockdev_insert_anon_medium(const char *device,
     }
 }
 
-void qmp_x_blockdev_insert_medium(const char *device, const char *node_name,
-                                  Error **errp)
+void qmp_x_blockdev_insert_medium(bool has_device, const char *device,
+                                  bool has_id, const char *id,
+                                  const char *node_name, Error **errp)
 {
+    BlockBackend *blk;
     BlockDriverState *bs;
 
+    blk = qmp_get_blk(has_device ? device : NULL,
+                      has_id ? id : NULL,
+                      errp);
+    if (!blk) {
+        return;
+    }
+
     bs = bdrv_find_node(node_name);
     if (!bs) {
         error_setg(errp, "Node '%s' not found", node_name);
@@ -2528,7 +2529,7 @@ void qmp_x_blockdev_insert_medium(const char *device, const char *node_name,
         return;
     }
 
-    qmp_blockdev_insert_anon_medium(device, bs, errp);
+    qmp_blockdev_insert_anon_medium(blk, bs, errp);
 }
 
 void qmp_blockdev_change_medium(const char *device, const char *filename,
@@ -2609,7 +2610,7 @@ void qmp_blockdev_change_medium(const char *device, const char *filename,
         goto fail;
     }
 
-    qmp_blockdev_insert_anon_medium(device, medium_bs, &err);
+    qmp_blockdev_insert_anon_medium(blk, medium_bs, &err);
     if (err) {
         error_propagate(errp, err);
         goto fail;
diff --git a/docs/qmp-commands.txt b/docs/qmp-commands.txt
index 9e230f5..ebb65e0 100644
--- a/docs/qmp-commands.txt
+++ b/docs/qmp-commands.txt
@@ -3328,7 +3328,9 @@ Stay away from it unless you want to help with its development.
 
 Arguments:
 
-- "device": block device name (json-string)
+- "device": block device name (deprecated, use @id instead)
+            (json-string, optional)
+- "id": the name or QOM path of the guest device (json-string, optional)
 - "node-name": root node of the BDS tree to insert into the block device
 
 Example:
@@ -3342,7 +3344,7 @@ Example:
 <- { "return": {} }
 
 -> { "execute": "x-blockdev-insert-medium",
-     "arguments": { "device": "ide1-cd0",
+     "arguments": { "id": "ide0-1-0",
                     "node-name": "node0" } }
 
 <- { "return": {} }
diff --git a/qapi/block-core.json b/qapi/block-core.json
index cd7b38a..6ac6809 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2427,14 +2427,17 @@
 # This command is still a work in progress and is considered experimental.
 # Stay away from it unless you want to help with its development.
 #
-# @device:    block device name
+# @device:    #optional Block device name (deprecated, use @id instead)
+#
+# @id:        #optional The name or QOM path of the guest device (since: 2.8)
 #
 # @node-name: name of a node in the block driver state graph
 #
 # Since: 2.5
 ##
 { 'command': 'x-blockdev-insert-medium',
-  'data': { 'device': 'str',
+  'data': { '*device': 'str',
+            '*id': 'str',
             'node-name': 'str'} }
 
 
-- 
1.8.3.1

[Qemu-devel] [PULL 17/33] block: Accept device model name for x-blockdev-remove-medium

[Kevin Wolf]

In order to remove the need for BlockBackend names in the external API,
we want to allow qdev device names in all device related commands.

This converts x-blockdev-remove-medium to accept a qdev device name.

As the command is experimental, we can still remove the 'device' option
that uses the BlockBackend name. This requires some test case changes
and is left for another series.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c            | 28 ++++++++++++++++------------
 docs/qmp-commands.txt | 12 +++++++-----
 qapi/block-core.json  |  7 +++++--
 3 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 0eb173d..a007b22 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2281,7 +2281,7 @@ void qmp_eject(const char *device, bool has_force, bool force, Error **errp)
     }
     error_free(local_err);
 
-    qmp_x_blockdev_remove_medium(device, errp);
+    qmp_x_blockdev_remove_medium(true, device, false, NULL, errp);
 }
 
 void qmp_block_passwd(bool has_device, const char *device,
@@ -2415,30 +2415,34 @@ void qmp_blockdev_close_tray(bool has_device, const char *device,
     blk_dev_change_media_cb(blk, true);
 }
 
-void qmp_x_blockdev_remove_medium(const char *device, Error **errp)
+void qmp_x_blockdev_remove_medium(bool has_device, const char *device,
+                                  bool has_id, const char *id, Error **errp)
 {
     BlockBackend *blk;
     BlockDriverState *bs;
     AioContext *aio_context;
-    bool has_device;
+    bool has_attached_device;
 
-    blk = blk_by_name(device);
+    device = has_device ? device : NULL;
+    id = has_id ? id : NULL;
+
+    blk = qmp_get_blk(device, id, errp);
     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);
+    has_attached_device = blk_get_attached_dev(blk);
 
-    if (has_device && !blk_dev_has_removable_media(blk)) {
-        error_setg(errp, "Device '%s' is not removable", device);
+    if (has_attached_device && !blk_dev_has_removable_media(blk)) {
+        error_setg(errp, "Device '%s' is not removable", device ?: id);
         return;
     }
 
-    if (has_device && blk_dev_has_tray(blk) && !blk_dev_is_tray_open(blk)) {
-        error_setg(errp, "Tray of device '%s' is not open", device);
+    if (has_attached_device && blk_dev_has_tray(blk) &&
+        !blk_dev_is_tray_open(blk))
+    {
+        error_setg(errp, "Tray of device '%s' is not open", device ?: id);
         return;
     }
 
@@ -2604,7 +2608,7 @@ void qmp_blockdev_change_medium(const char *device, const char *filename,
     error_free(err);
     err = NULL;
 
-    qmp_x_blockdev_remove_medium(device, &err);
+    qmp_x_blockdev_remove_medium(true, device, false, NULL, errp);
     if (err) {
         error_propagate(errp, err);
         goto fail;
diff --git a/docs/qmp-commands.txt b/docs/qmp-commands.txt
index ebb65e0..e77bf2f 100644
--- a/docs/qmp-commands.txt
+++ b/docs/qmp-commands.txt
@@ -3290,18 +3290,20 @@ Stay away from it unless you want to help with its development.
 
 Arguments:
 
-- "device": block device name (json-string)
+- "device": block device name (deprecated, use @id instead)
+            (json-string, optional)
+- "id": the name or QOM path of the guest device (json-string, optional)
 
 Example:
 
 -> { "execute": "x-blockdev-remove-medium",
-     "arguments": { "device": "ide1-cd0" } }
+     "arguments": { "id": "ide0-1-0" } }
 
 <- { "error": { "class": "GenericError",
-                "desc": "Tray of device 'ide1-cd0' is not open" } }
+                "desc": "Tray of device 'ide0-1-0' is not open" } }
 
 -> { "execute": "blockdev-open-tray",
-     "arguments": { "device": "ide1-cd0" } }
+     "arguments": { "id": "ide0-1-0" } }
 
 <- { "timestamp": { "seconds": 1418751627,
                     "microseconds": 549958 },
@@ -3312,7 +3314,7 @@ Example:
 <- { "return": {} }
 
 -> { "execute": "x-blockdev-remove-medium",
-     "arguments": { "device": "ide1-cd0" } }
+     "arguments": { "device": "ide0-1-0" } }
 
 <- { "return": {} }
 
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 6ac6809..e370366 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2410,12 +2410,15 @@
 # This command is still a work in progress and is considered experimental.
 # Stay away from it unless you want to help with its development.
 #
-# @device: block device name
+# @device: #optional Block device name (deprecated, use @id instead)
+#
+# @id:     #optional The name or QOM path of the guest device (since: 2.8)
 #
 # Since: 2.5
 ##
 { 'command': 'x-blockdev-remove-medium',
-  'data': { 'device': 'str' } }
+  'data': { '*device': 'str',
+            '*id': 'str' } }
 
 ##
 # @x-blockdev-insert-medium:
-- 
1.8.3.1

[Qemu-devel] [PULL 18/33] block: Accept device model name for eject

[Kevin Wolf]

In order to remove the need for BlockBackend names in the external API,
we want to allow qdev device names in all device related commands.

This converts eject to accept a qdev device name.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c            | 10 +++++++---
 docs/qmp-commands.txt |  8 +++++---
 hmp.c                 |  2 +-
 qapi/block.json       |  9 +++++++--
 4 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index a007b22..3d92724 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2265,7 +2265,9 @@ exit:
     block_job_txn_unref(block_job_txn);
 }
 
-void qmp_eject(const char *device, bool has_force, bool force, Error **errp)
+void qmp_eject(bool has_device, const char *device,
+               bool has_id, const char *id,
+               bool has_force, bool force, Error **errp)
 {
     Error *local_err = NULL;
     int rc;
@@ -2274,14 +2276,16 @@ void qmp_eject(const char *device, bool has_force, bool force, Error **errp)
         force = false;
     }
 
-    rc = do_open_tray(device, NULL, force, &local_err);
+    rc = do_open_tray(has_device ? device : NULL,
+                      has_id ? id : NULL,
+                      force, &local_err);
     if (rc && rc != -ENOSYS) {
         error_propagate(errp, local_err);
         return;
     }
     error_free(local_err);
 
-    qmp_x_blockdev_remove_medium(true, device, false, NULL, errp);
+    qmp_x_blockdev_remove_medium(has_device, device, has_id, id, errp);
 }
 
 void qmp_block_passwd(bool has_device, const char *device,
diff --git a/docs/qmp-commands.txt b/docs/qmp-commands.txt
index e77bf2f..9024ef2 100644
--- a/docs/qmp-commands.txt
+++ b/docs/qmp-commands.txt
@@ -72,12 +72,14 @@ Eject a removable medium.
 
 Arguments:
 
-- force: force ejection (json-bool, optional)
-- device: device name (json-string)
+- "force": force ejection (json-bool, optional)
+- "device": block device name (deprecated, use @id instead)
+            (json-string, optional)
+- "id": the name or QOM path of the guest device (json-string, optional)
 
 Example:
 
--> { "execute": "eject", "arguments": { "device": "ide1-cd0" } }
+-> { "execute": "eject", "arguments": { "id": "ide0-1-0" } }
 <- { "return": {} }
 
 Note: The "force" argument defaults to false.
diff --git a/hmp.c b/hmp.c
index 0a16aef..09827b3 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1376,7 +1376,7 @@ void hmp_eject(Monitor *mon, const QDict *qdict)
     const char *device = qdict_get_str(qdict, "device");
     Error *err = NULL;
 
-    qmp_eject(device, true, force, &err);
+    qmp_eject(true, device, false, NULL, true, force, &err);
     hmp_handle_error(mon, &err);
 }
 
diff --git a/qapi/block.json b/qapi/block.json
index 8b08bd2..c896bd1 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -125,7 +125,9 @@
 #
 # Ejects a device from a removable drive.
 #
-# @device:  The name of the device
+# @device:  #optional Block device name (deprecated, use @id instead)
+#
+# @id:      #optional The name or QOM path of the guest device (since: 2.8)
 #
 # @force:   @optional If true, eject regardless of whether the drive is locked.
 #           If not specified, the default value is false.
@@ -137,7 +139,10 @@
 #
 # Since: 0.14.0
 ##
-{ 'command': 'eject', 'data': {'device': 'str', '*force': 'bool'} }
+{ 'command': 'eject',
+  'data': { '*device': 'str',
+            '*id': 'str',
+            '*force': 'bool' } }
 
 ##
 # @nbd-server-start:
-- 
1.8.3.1

[Qemu-devel] [PULL 19/33] block: Accept device model name for blockdev-change-medium

[Kevin Wolf]

In order to remove the need for BlockBackend names in the external API,
we want to allow qdev device names in all device related commands.

This converts blockdev-change-medium to accept a qdev device name.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c            | 18 +++++++++++-------
 docs/qmp-commands.txt | 10 ++++++----
 hmp.c                 |  5 +++--
 qapi/block-core.json  |  8 ++++++--
 qmp.c                 |  4 ++--
 5 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 3d92724..8c8fcd6 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2540,7 +2540,9 @@ void qmp_x_blockdev_insert_medium(bool has_device, const char *device,
     qmp_blockdev_insert_anon_medium(blk, bs, errp);
 }
 
-void qmp_blockdev_change_medium(const char *device, const char *filename,
+void qmp_blockdev_change_medium(bool has_device, const char *device,
+                                bool has_id, const char *id,
+                                const char *filename,
                                 bool has_format, const char *format,
                                 bool has_read_only,
                                 BlockdevChangeReadOnlyMode read_only,
@@ -2553,10 +2555,10 @@ void qmp_blockdev_change_medium(const char *device, const char *filename,
     QDict *options = NULL;
     Error *err = NULL;
 
-    blk = blk_by_name(device);
+    blk = qmp_get_blk(has_device ? device : NULL,
+                      has_id ? id : NULL,
+                      errp);
     if (!blk) {
-        error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
-                  "Device '%s' not found", device);
         goto fail;
     }
 
@@ -2604,7 +2606,9 @@ void qmp_blockdev_change_medium(const char *device, const char *filename,
         goto fail;
     }
 
-    rc = do_open_tray(device, NULL, false, &err);
+    rc = do_open_tray(has_device ? device : NULL,
+                      has_id ? id : NULL,
+                      false, &err);
     if (rc && rc != -ENOSYS) {
         error_propagate(errp, err);
         goto fail;
@@ -2612,7 +2616,7 @@ void qmp_blockdev_change_medium(const char *device, const char *filename,
     error_free(err);
     err = NULL;
 
-    qmp_x_blockdev_remove_medium(true, device, false, NULL, errp);
+    qmp_x_blockdev_remove_medium(has_device, device, has_id, id, errp);
     if (err) {
         error_propagate(errp, err);
         goto fail;
@@ -2626,7 +2630,7 @@ void qmp_blockdev_change_medium(const char *device, const char *filename,
 
     blk_apply_root_state(blk, medium_bs);
 
-    qmp_blockdev_close_tray(true, device, false, NULL, errp);
+    qmp_blockdev_close_tray(has_device, device, has_id, id, errp);
 
 fail:
     /* If the medium has been inserted, the device has its own reference, so
diff --git a/docs/qmp-commands.txt b/docs/qmp-commands.txt
index 9024ef2..8b2a0ea 100644
--- a/docs/qmp-commands.txt
+++ b/docs/qmp-commands.txt
@@ -3458,7 +3458,9 @@ and loading a new image file which is inserted as the new medium.
 
 Arguments:
 
-- "device": device name (json-string)
+- "device": block device name (deprecated, use @id instead)
+            (json-string, optional)
+- "id": the name or QOM path of the guest device (json-string, optional)
 - "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)
@@ -3469,7 +3471,7 @@ Examples:
 1. Change a removable medium
 
 -> { "execute": "blockdev-change-medium",
-             "arguments": { "device": "ide1-cd0",
+             "arguments": { "id": "ide0-1-0",
                             "filename": "/srv/images/Fedora-12-x86_64-DVD.iso",
                             "format": "raw" } }
 <- { "return": {} }
@@ -3477,7 +3479,7 @@ Examples:
 2. Load a read-only medium into a writable drive
 
 -> { "execute": "blockdev-change-medium",
-             "arguments": { "device": "isa-fd0",
+             "arguments": { "id": "floppyA",
                             "filename": "/srv/images/ro.img",
                             "format": "raw",
                             "read-only-mode": "retain" } }
@@ -3487,7 +3489,7 @@ Examples:
        "desc": "Could not open '/srv/images/ro.img': Permission denied" } }
 
 -> { "execute": "blockdev-change-medium",
-             "arguments": { "device": "isa-fd0",
+             "arguments": { "id": "floppyA",
                             "filename": "/srv/images/ro.img",
                             "format": "raw",
                             "read-only-mode": "read-only" } }
diff --git a/hmp.c b/hmp.c
index 09827b3..336e7bf 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1422,8 +1422,9 @@ void hmp_change(Monitor *mon, const QDict *qdict)
             }
         }
 
-        qmp_blockdev_change_medium(device, target, !!arg, arg,
-                                   !!read_only, read_only_mode, &err);
+        qmp_blockdev_change_medium(true, device, false, NULL, target,
+                                   !!arg, arg, !!read_only, read_only_mode,
+                                   &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 e370366..1d7d4cc 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2470,7 +2470,10 @@
 # combines blockdev-open-tray, x-blockdev-remove-medium,
 # x-blockdev-insert-medium and blockdev-close-tray).
 #
-# @device:          block device name
+# @device:          #optional Block device name (deprecated, use @id instead)
+#
+# @id:              #optional The name or QOM path of the guest device
+#                   (since: 2.8)
 #
 # @filename:        filename of the new image to be loaded
 #
@@ -2483,7 +2486,8 @@
 # Since: 2.5
 ##
 { 'command': 'blockdev-change-medium',
-  'data': { 'device': 'str',
+  'data': { '*device': 'str',
+            '*id': 'str',
             'filename': 'str',
             '*format': 'str',
             '*read-only-mode': 'BlockdevChangeReadOnlyMode' } }
diff --git a/qmp.c b/qmp.c
index 6733463..8b4bc98 100644
--- a/qmp.c
+++ b/qmp.c
@@ -436,8 +436,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, false, 0,
-                                   errp);
+        qmp_blockdev_change_medium(true, device, false, NULL, target,
+                                   has_arg, arg, false, 0, errp);
     }
 }
 
-- 
1.8.3.1

[Qemu-devel] [PULL 20/33] block: Accept device model name for block_set_io_throttle

[Kevin Wolf]

In order to remove the need for BlockBackend names in the external API,
we want to allow qdev device names in all device related commands.

This converts block_set_io_throttle to accept a qdev device name.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c            | 12 +++++++-----
 docs/qmp-commands.txt |  6 ++++--
 qapi/block-core.json  |  8 +++++---
 3 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 8c8fcd6..405145a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2647,10 +2647,10 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp)
     BlockBackend *blk;
     AioContext *aio_context;
 
-    blk = blk_by_name(arg->device);
+    blk = qmp_get_blk(arg->has_device ? arg->device : NULL,
+                      arg->has_id ? arg->id : NULL,
+                      errp);
     if (!blk) {
-        error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
-                  "Device '%s' not found", arg->device);
         return;
     }
 
@@ -2659,7 +2659,7 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp)
 
     bs = blk_bs(blk);
     if (!bs) {
-        error_setg(errp, "Device '%s' has no medium", arg->device);
+        error_setg(errp, "Device has no medium");
         goto out;
     }
 
@@ -2723,7 +2723,9 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp)
          * just update the throttling group. */
         if (!blk_get_public(blk)->throttle_state) {
             blk_io_limits_enable(blk,
-                                 arg->has_group ? arg->group : arg->device);
+                                 arg->has_group ? arg->group :
+                                 arg->has_device ? arg->device :
+                                 arg->id);
         } else if (arg->has_group) {
             blk_io_limits_update_group(blk, arg->group);
         }
diff --git a/docs/qmp-commands.txt b/docs/qmp-commands.txt
index 8b2a0ea..41f5698 100644
--- a/docs/qmp-commands.txt
+++ b/docs/qmp-commands.txt
@@ -1459,7 +1459,9 @@ Change I/O throttle limits for a block drive.
 
 Arguments:
 
-- "device": device name (json-string)
+- "device": block device name (deprecated, use @id instead)
+            (json-string, optional)
+- "id": the name or QOM path of the guest device (json-string, optional)
 - "bps": total throughput limit in bytes per second (json-int)
 - "bps_rd": read throughput limit in bytes per second (json-int)
 - "bps_wr": write throughput limit in bytes per second (json-int)
@@ -1483,7 +1485,7 @@ Arguments:
 
 Example:
 
--> { "execute": "block_set_io_throttle", "arguments": { "device": "virtio0",
+-> { "execute": "block_set_io_throttle", "arguments": { "id": "ide0-1-0",
                                                "bps": 1000000,
                                                "bps_rd": 0,
                                                "bps_wr": 0,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 1d7d4cc..5f04dab 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1377,7 +1377,9 @@
 #
 # A set of parameters describing block throttling.
 #
-# @device: The name of the device
+# @device: #optional Block device name (deprecated, use @id instead)
+#
+# @id: #optional The name or QOM path of the guest device (since: 2.8)
 #
 # @bps: total throughput limit in bytes per second
 #
@@ -1446,8 +1448,8 @@
 # Since: 1.1
 ##
 { 'struct': 'BlockIOThrottle',
-  'data': { 'device': 'str', 'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
-            'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
+  'data': { '*device': 'str', '*id': 'str', 'bps': 'int', 'bps_rd': 'int',
+            'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
             '*bps_max': 'int', '*bps_rd_max': 'int',
             '*bps_wr_max': 'int', '*iops_max': 'int',
             '*iops_rd_max': 'int', '*iops_wr_max': 'int',
-- 
1.8.3.1

[Qemu-devel] [PULL 21/33] qemu-iotests/118: Test media change with qdev name

[Kevin Wolf]

We just added the option to use qdev device names in all device related
block QMP commands. This patch converts some of the test cases in 118 to
use qdev device names instead of BlockBackend names to cover the new
way. It converts cases for each of the media change commands, but only
for CD-ROM and not everywhere, so that the old way is still tested, too.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/qemu-iotests/118        | 85 ++++++++++++++++++++++++++++++++++---------
 tests/qemu-iotests/iotests.py |  5 +++
 2 files changed, 73 insertions(+), 17 deletions(-)

diff --git a/tests/qemu-iotests/118 b/tests/qemu-iotests/118
index 9e5951f..0380069 100755
--- a/tests/qemu-iotests/118
+++ b/tests/qemu-iotests/118
@@ -62,6 +62,9 @@ class ChangeBaseClass(iotests.QMPTestCase):
             self.fail('Timeout while waiting for the tray to close')
 
 class GeneralChangeTestsBaseClass(ChangeBaseClass):
+
+    device_name = None
+
     def test_change(self):
         result = self.vm.qmp('change', device='drive0', target=new_img,
                                        arg=iotests.imgfmt)
@@ -76,9 +79,15 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass):
         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)
+        if self.device_name is not None:
+            result = self.vm.qmp('blockdev-change-medium',
+                                 id=self.device_name, filename=new_img,
+                                 format=iotests.imgfmt)
+        else:
+            result = self.vm.qmp('blockdev-change-medium',
+                                 device='drive0', filename=new_img,
+                                 format=iotests.imgfmt)
+
         self.assert_qmp(result, 'return', {})
 
         self.wait_for_open()
@@ -90,7 +99,10 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass):
         self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
 
     def test_eject(self):
-        result = self.vm.qmp('eject', device='drive0', force=True)
+        if self.device_name is not None:
+            result = self.vm.qmp('eject', id=self.device_name, force=True)
+        else:
+            result = self.vm.qmp('eject', device='drive0', force=True)
         self.assert_qmp(result, 'return', {})
 
         self.wait_for_open()
@@ -101,7 +113,10 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass):
         self.assert_qmp_absent(result, 'return[0]/inserted')
 
     def test_tray_eject_change(self):
-        result = self.vm.qmp('eject', device='drive0', force=True)
+        if self.device_name is not None:
+            result = self.vm.qmp('eject', id=self.device_name, force=True)
+        else:
+            result = self.vm.qmp('eject', device='drive0', force=True)
         self.assert_qmp(result, 'return', {})
 
         self.wait_for_open()
@@ -111,9 +126,12 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass):
             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)
+        if self.device_name is not None:
+            result = self.vm.qmp('blockdev-change-medium', id=self.device_name,
+                                 filename=new_img, format=iotests.imgfmt)
+        else:
+            result = self.vm.qmp('blockdev-change-medium', device='drive0',
+                                 filename=new_img, format=iotests.imgfmt)
         self.assert_qmp(result, 'return', {})
 
         self.wait_for_close()
@@ -124,7 +142,12 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass):
         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)
+        if self.device_name is not None:
+            result = self.vm.qmp('blockdev-open-tray',
+                                 id=self.device_name, force=True)
+        else:
+            result = self.vm.qmp('blockdev-open-tray',
+                                 device='drive0', force=True)
         self.assert_qmp(result, 'return', {})
 
         self.wait_for_open()
@@ -137,7 +160,10 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass):
         else:
             self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img)
 
-        result = self.vm.qmp('blockdev-close-tray', device='drive0')
+        if self.device_name is not None:
+            result = self.vm.qmp('blockdev-close-tray', id=self.device_name)
+        else:
+            result = self.vm.qmp('blockdev-close-tray', device='drive0')
         self.assert_qmp(result, 'return', {})
 
         if self.has_real_tray or not self.was_empty:
@@ -162,7 +188,10 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass):
             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')
+        if self.device_name is not None:
+            result = self.vm.qmp('blockdev-close-tray', id=self.device_name)
+        else:
+            result = self.vm.qmp('blockdev-close-tray', device='drive0')
         self.assert_qmp(result, 'return', {})
 
         self.wait_for_close()
@@ -206,7 +235,12 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass):
                                                'driver': 'file'}})
         self.assert_qmp(result, 'return', {})
 
-        result = self.vm.qmp('blockdev-open-tray', device='drive0', force=True)
+        if self.device_name is not None:
+            result = self.vm.qmp('blockdev-open-tray',
+                                 id=self.device_name, force=True)
+        else:
+            result = self.vm.qmp('blockdev-open-tray',
+                                 device='drive0', force=True)
         self.assert_qmp(result, 'return', {})
 
         self.wait_for_open()
@@ -219,7 +253,11 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass):
         else:
             self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img)
 
-        result = self.vm.qmp('x-blockdev-remove-medium', device='drive0')
+        if self.device_name is not None:
+            result = self.vm.qmp('x-blockdev-remove-medium',
+                                 id=self.device_name)
+        else:
+            result = self.vm.qmp('x-blockdev-remove-medium', device='drive0')
         self.assert_qmp(result, 'return', {})
 
         result = self.vm.qmp('query-block')
@@ -227,8 +265,12 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass):
             self.assert_qmp(result, 'return[0]/tray_open', True)
         self.assert_qmp_absent(result, 'return[0]/inserted')
 
-        result = self.vm.qmp('x-blockdev-insert-medium', device='drive0',
-                                                       node_name='new')
+        if self.device_name is not None:
+            result = self.vm.qmp('x-blockdev-insert-medium',
+                                 id=self.device_name, node_name='new')
+        else:
+            result = self.vm.qmp('x-blockdev-insert-medium',
+                                 device='drive0', node_name='new')
         self.assert_qmp(result, 'return', {})
 
         result = self.vm.qmp('query-block')
@@ -236,7 +278,10 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass):
             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')
+        if self.device_name is not None:
+            result = self.vm.qmp('blockdev-close-tray', id=self.device_name)
+        else:
+            result = self.vm.qmp('blockdev-close-tray', device='drive0')
         self.assert_qmp(result, 'return', {})
 
         self.wait_for_close()
@@ -280,7 +325,13 @@ class TestInitiallyFilled(GeneralChangeTestsBaseClass):
     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 = iotests.VM()
+        if interface == 'ide':
+            self.device_name = 'qdev0'
+            self.vm.add_drive(old_img, 'media=%s' % media, 'none')
+            self.vm.add_device('ide-cd,drive=drive0,id=%s' % self.device_name)
+        else:
+            self.vm.add_drive(old_img, 'media=%s' % media, interface)
         self.vm.launch()
 
     def tearDown(self):
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index f1f36d7..3329bc1 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -139,6 +139,11 @@ class VM(qtest.QEMUQtestMachine):
             self._debug = True
         self._num_drives = 0
 
+    def add_device(self, opts):
+        self._args.append('-device')
+        self._args.append(opts)
+        return self
+
     def add_drive_raw(self, opts):
         self._args.append('-drive')
         self._args.append(opts)
-- 
1.8.3.1

[Qemu-devel] [PULL 22/33] qemu-iotests/041: Avoid blockdev-add with id

[Kevin Wolf]

We want to remove the 'id' option for blockdev-add. This removes one
user of the option and makes it use only node names.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/qemu-iotests/041 | 71 +++++++++++++++++++++++---------------------------
 1 file changed, 32 insertions(+), 39 deletions(-)

diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index 80939c0..d1e1ad8 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -782,7 +782,7 @@ class TestRepairQuorum(iotests.QMPTestCase):
         self.vm.launch()
 
         #assemble the quorum block device from the individual files
-        args = { "options" : { "driver": "quorum", "id": "quorum0",
+        args = { "options" : { "driver": "quorum", "node-name": "quorum0",
                  "vote-threshold": 2, "children": [ "img0", "img1", "img2" ] } }
         if self.has_quorum():
             result = self.vm.qmp("blockdev-add", **args)
@@ -804,13 +804,12 @@ class TestRepairQuorum(iotests.QMPTestCase):
 
         self.assert_no_active_block_jobs()
 
-        result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
-                             node_name="repair0",
-                             replaces="img1",
+        result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
+                             sync='full', node_name="repair0", replaces="img1",
                              target=quorum_repair_img, format=iotests.imgfmt)
         self.assert_qmp(result, 'return', {})
 
-        self.complete_and_wait(drive="quorum0")
+        self.complete_and_wait(drive="job0")
         self.assert_has_block_node("repair0", quorum_repair_img)
         # TODO: a better test requiring some QEMU infrastructure will be added
         #       to check that this file is really driven by quorum
@@ -824,13 +823,12 @@ class TestRepairQuorum(iotests.QMPTestCase):
 
         self.assert_no_active_block_jobs()
 
-        result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
-                             node_name="repair0",
-                             replaces="img1",
+        result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
+                             sync='full', node_name="repair0", replaces="img1",
                              target=quorum_repair_img, format=iotests.imgfmt)
         self.assert_qmp(result, 'return', {})
 
-        self.cancel_and_wait(drive="quorum0", force=True)
+        self.cancel_and_wait(drive="job0", force=True)
         # here we check that the last registered quorum file has not been
         # swapped out and unref
         self.assert_has_block_node(None, quorum_img3)
@@ -842,13 +840,12 @@ class TestRepairQuorum(iotests.QMPTestCase):
 
         self.assert_no_active_block_jobs()
 
-        result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
-                             node_name="repair0",
-                             replaces="img1",
+        result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
+                             sync='full', node_name="repair0", replaces="img1",
                              target=quorum_repair_img, format=iotests.imgfmt)
         self.assert_qmp(result, 'return', {})
 
-        self.wait_ready_and_cancel(drive="quorum0")
+        self.wait_ready_and_cancel(drive="job0")
         # here we check that the last registered quorum file has not been
         # swapped out and unref
         self.assert_has_block_node(None, quorum_img3)
@@ -862,13 +859,12 @@ class TestRepairQuorum(iotests.QMPTestCase):
 
         self.assert_no_active_block_jobs()
 
-        result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
-                             node_name="repair0",
-                             replaces="img1",
+        result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
+                             sync='full', node_name="repair0", replaces="img1",
                              target=quorum_repair_img, format=iotests.imgfmt)
         self.assert_qmp(result, 'return', {})
 
-        result = self.vm.qmp('block-job-pause', device='quorum0')
+        result = self.vm.qmp('block-job-pause', device='job0')
         self.assert_qmp(result, 'return', {})
 
         time.sleep(1)
@@ -879,10 +875,10 @@ class TestRepairQuorum(iotests.QMPTestCase):
         result = self.vm.qmp('query-block-jobs')
         self.assert_qmp(result, 'return[0]/offset', offset)
 
-        result = self.vm.qmp('block-job-resume', device='quorum0')
+        result = self.vm.qmp('block-job-resume', device='job0')
         self.assert_qmp(result, 'return', {})
 
-        self.complete_and_wait(drive="quorum0")
+        self.complete_and_wait(drive="job0")
         self.vm.shutdown()
         self.assertTrue(iotests.compare_images(quorum_img2, quorum_repair_img),
                         'target image does not match source after mirroring')
@@ -894,7 +890,7 @@ class TestRepairQuorum(iotests.QMPTestCase):
         if iotests.qemu_default_machine != 'pc':
             return
 
-        result = self.vm.qmp('drive-mirror', device='drive0', # CD-ROM
+        result = self.vm.qmp('drive-mirror', job_id='job0', device='drive0', # CD-ROM
                              sync='full',
                              node_name='repair0',
                              replaces='img1',
@@ -905,18 +901,18 @@ class TestRepairQuorum(iotests.QMPTestCase):
         if not self.has_quorum():
             return
 
-        result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
-                             node_name='repair0',
-                             replaces='img1',
-                             mode='existing',
-                             target=quorum_repair_img, format=iotests.imgfmt)
+        result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
+                             sync='full', node_name='repair0', replaces='img1',
+                             mode='existing', target=quorum_repair_img,
+                             format=iotests.imgfmt)
         self.assert_qmp(result, 'error/class', 'GenericError')
 
     def test_device_not_found(self):
         if not self.has_quorum():
             return
 
-        result = self.vm.qmp('drive-mirror', device='nonexistent', sync='full',
+        result = self.vm.qmp('drive-mirror', job_id='job0',
+                             device='nonexistent', sync='full',
                              node_name='repair0',
                              replaces='img1',
                              target=quorum_repair_img, format=iotests.imgfmt)
@@ -926,7 +922,7 @@ class TestRepairQuorum(iotests.QMPTestCase):
         if not self.has_quorum():
             return
 
-        result = self.vm.qmp('drive-mirror', device='quorum0',
+        result = self.vm.qmp('drive-mirror', device='quorum0', job_id='job0',
                              node_name='repair0',
                              replaces='img1',
                              target=quorum_repair_img, format=iotests.imgfmt)
@@ -936,8 +932,8 @@ class TestRepairQuorum(iotests.QMPTestCase):
         if not self.has_quorum():
             return
 
-        result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
-                             replaces='img1',
+        result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
+                             sync='full', replaces='img1',
                              target=quorum_repair_img, format=iotests.imgfmt)
         self.assert_qmp(result, 'error/class', 'GenericError')
 
@@ -945,9 +941,8 @@ class TestRepairQuorum(iotests.QMPTestCase):
         if not self.has_quorum():
             return
 
-        result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
-                             node_name='repair0',
-                             replaces='img77',
+        result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
+                             sync='full', node_name='repair0', replaces='img77',
                              target=quorum_repair_img, format=iotests.imgfmt)
         self.assert_qmp(result, 'error/class', 'GenericError')
 
@@ -959,19 +954,17 @@ class TestRepairQuorum(iotests.QMPTestCase):
                              snapshot_file=quorum_snapshot_file,
                              snapshot_node_name="snap1");
 
-        result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
-                             node_name='repair0',
-                             replaces="img1",
+        result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
+                             sync='full', node_name='repair0', replaces="img1",
                              target=quorum_repair_img, format=iotests.imgfmt)
         self.assert_qmp(result, 'error/class', 'GenericError')
 
-        result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
-                             node_name='repair0',
-                             replaces="snap1",
+        result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
+                             sync='full', node_name='repair0', replaces="snap1",
                              target=quorum_repair_img, format=iotests.imgfmt)
         self.assert_qmp(result, 'return', {})
 
-        self.complete_and_wait(drive="quorum0")
+        self.complete_and_wait('job0')
         self.assert_has_block_node("repair0", quorum_repair_img)
         # TODO: a better test requiring some QEMU infrastructure will be added
         #       to check that this file is really driven by quorum
-- 
1.8.3.1

[Qemu-devel] [PULL 23/33] qemu-iotests/067: Avoid blockdev-add with id

[Kevin Wolf]

We want to remove the 'id' option for blockdev-add. This removes one
user of the option and makes it use only node names.

In order to keep the test meaningful, some instances of query-block that
want to check whether the node still exists and would now turn up empty
must be converted to query-named-block-nodes (which also return the
protocol level node, but that shouldn't hurt).

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/qemu-iotests/067     |   6 +-
 tests/qemu-iotests/067.out | 211 +++++++++++++++++++++++++++------------------
 2 files changed, 131 insertions(+), 86 deletions(-)

diff --git a/tests/qemu-iotests/067 b/tests/qemu-iotests/067
index c1df48e..a12125b 100755
--- a/tests/qemu-iotests/067
+++ b/tests/qemu-iotests/067
@@ -121,7 +121,7 @@ run_qemu <<EOF
   "arguments": {
       "options": {
         "driver": "$IMGFMT",
-        "id": "disk",
+        "node-name": "disk",
         "file": {
             "driver": "file",
             "filename": "$TEST_IMG"
@@ -129,13 +129,13 @@ run_qemu <<EOF
       }
     }
   }
-{ "execute": "query-block" }
+{ "execute": "query-named-block-nodes" }
 { "execute": "device_add",
    "arguments": { "driver": "virtio-blk", "drive": "disk",
                   "id": "virtio0" } }
 { "execute": "device_del", "arguments": { "id": "virtio0" } }
 { "execute": "system_reset" }
-{ "execute": "query-block" }
+{ "execute": "query-named-block-nodes" }
 { "execute": "quit" }
 EOF
 
diff --git a/tests/qemu-iotests/067.out b/tests/qemu-iotests/067.out
index 7e25a49..782eae2 100644
--- a/tests/qemu-iotests/067.out
+++ b/tests/qemu-iotests/067.out
@@ -258,49 +258,72 @@ Testing:
 {
     "return": [
         {
-            "device": "disk",
-            "locked": false,
-            "removable": true,
-            "inserted": {
-                "iops_rd": 0,
-                "detect_zeroes": "off",
-                "image": {
-                    "virtual-size": 134217728,
-                    "filename": "TEST_DIR/t.qcow2",
-                    "cluster-size": 65536,
-                    "format": "qcow2",
-                    "actual-size": SIZE,
-                    "format-specific": {
-                        "type": "qcow2",
-                        "data": {
-                            "compat": "1.1",
-                            "lazy-refcounts": false,
-                            "refcount-bits": 16,
-                            "corrupt": false
-                        }
-                    },
-                    "dirty-flag": false
-                },
-                "iops_wr": 0,
-                "ro": false,
-                "node-name": "NODE_NAME",
-                "backing_file_depth": 0,
-                "drv": "qcow2",
-                "iops": 0,
-                "bps_wr": 0,
-                "write_threshold": 0,
-                "encrypted": false,
-                "bps": 0,
-                "bps_rd": 0,
-                "cache": {
-                    "no-flush": false,
-                    "direct": false,
-                    "writeback": true
+            "iops_rd": 0,
+            "detect_zeroes": "off",
+            "image": {
+                "virtual-size": 134217728,
+                "filename": "TEST_DIR/t.qcow2",
+                "cluster-size": 65536,
+                "format": "qcow2",
+                "actual-size": SIZE,
+                "format-specific": {
+                    "type": "qcow2",
+                    "data": {
+                        "compat": "1.1",
+                        "lazy-refcounts": false,
+                        "refcount-bits": 16,
+                        "corrupt": false
+                    }
                 },
-                "file": "TEST_DIR/t.qcow2",
-                "encryption_key_missing": false
+                "dirty-flag": false
             },
-            "type": "unknown"
+            "iops_wr": 0,
+            "ro": false,
+            "node-name": "disk",
+            "backing_file_depth": 0,
+            "drv": "qcow2",
+            "iops": 0,
+            "bps_wr": 0,
+            "write_threshold": 0,
+            "encrypted": false,
+            "bps": 0,
+            "bps_rd": 0,
+            "cache": {
+                "no-flush": false,
+                "direct": false,
+                "writeback": true
+            },
+            "file": "TEST_DIR/t.qcow2",
+            "encryption_key_missing": false
+        },
+        {
+            "iops_rd": 0,
+            "detect_zeroes": "off",
+            "image": {
+                "virtual-size": 197120,
+                "filename": "TEST_DIR/t.qcow2",
+                "format": "file",
+                "actual-size": SIZE,
+                "dirty-flag": false
+            },
+            "iops_wr": 0,
+            "ro": false,
+            "node-name": "NODE_NAME",
+            "backing_file_depth": 0,
+            "drv": "file",
+            "iops": 0,
+            "bps_wr": 0,
+            "write_threshold": 0,
+            "encrypted": false,
+            "bps": 0,
+            "bps_rd": 0,
+            "cache": {
+                "no-flush": false,
+                "direct": false,
+                "writeback": true
+            },
+            "file": "TEST_DIR/t.qcow2",
+            "encryption_key_missing": false
         }
     ]
 }
@@ -319,50 +342,72 @@ Testing:
 {
     "return": [
         {
-            "io-status": "ok",
-            "device": "disk",
-            "locked": false,
-            "removable": true,
-            "inserted": {
-                "iops_rd": 0,
-                "detect_zeroes": "off",
-                "image": {
-                    "virtual-size": 134217728,
-                    "filename": "TEST_DIR/t.qcow2",
-                    "cluster-size": 65536,
-                    "format": "qcow2",
-                    "actual-size": SIZE,
-                    "format-specific": {
-                        "type": "qcow2",
-                        "data": {
-                            "compat": "1.1",
-                            "lazy-refcounts": false,
-                            "refcount-bits": 16,
-                            "corrupt": false
-                        }
-                    },
-                    "dirty-flag": false
+            "iops_rd": 0,
+            "detect_zeroes": "off",
+            "image": {
+                "virtual-size": 134217728,
+                "filename": "TEST_DIR/t.qcow2",
+                "cluster-size": 65536,
+                "format": "qcow2",
+                "actual-size": SIZE,
+                "format-specific": {
+                    "type": "qcow2",
+                    "data": {
+                        "compat": "1.1",
+                        "lazy-refcounts": false,
+                        "refcount-bits": 16,
+                        "corrupt": false
+                    }
                 },
-                "iops_wr": 0,
-                "ro": false,
-                "node-name": "NODE_NAME",
-                "backing_file_depth": 0,
-                "drv": "qcow2",
-                "iops": 0,
-                "bps_wr": 0,
-                "write_threshold": 0,
-                "encrypted": false,
-                "bps": 0,
-                "bps_rd": 0,
-                "cache": {
-                    "no-flush": false,
-                    "direct": false,
-                    "writeback": true
-                },
-                "file": "TEST_DIR/t.qcow2",
-                "encryption_key_missing": false
+                "dirty-flag": false
             },
-            "type": "unknown"
+            "iops_wr": 0,
+            "ro": false,
+            "node-name": "disk",
+            "backing_file_depth": 0,
+            "drv": "qcow2",
+            "iops": 0,
+            "bps_wr": 0,
+            "write_threshold": 0,
+            "encrypted": false,
+            "bps": 0,
+            "bps_rd": 0,
+            "cache": {
+                "no-flush": false,
+                "direct": false,
+                "writeback": true
+            },
+            "file": "TEST_DIR/t.qcow2",
+            "encryption_key_missing": false
+        },
+        {
+            "iops_rd": 0,
+            "detect_zeroes": "off",
+            "image": {
+                "virtual-size": 197120,
+                "filename": "TEST_DIR/t.qcow2",
+                "format": "file",
+                "actual-size": SIZE,
+                "dirty-flag": false
+            },
+            "iops_wr": 0,
+            "ro": false,
+            "node-name": "NODE_NAME",
+            "backing_file_depth": 0,
+            "drv": "file",
+            "iops": 0,
+            "bps_wr": 0,
+            "write_threshold": 0,
+            "encrypted": false,
+            "bps": 0,
+            "bps_rd": 0,
+            "cache": {
+                "no-flush": false,
+                "direct": false,
+                "writeback": true
+            },
+            "file": "TEST_DIR/t.qcow2",
+            "encryption_key_missing": false
         }
     ]
 }
-- 
1.8.3.1

[Qemu-devel] [PULL 24/33] qemu-iotests/071: Avoid blockdev-add with id

[Kevin Wolf]

We want to remove the 'id' option for blockdev-add. This removes one
user of the option and makes it use only node names.

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

diff --git a/tests/qemu-iotests/071 b/tests/qemu-iotests/071
index bdfd91f..6d0864c 100755
--- a/tests/qemu-iotests/071
+++ b/tests/qemu-iotests/071
@@ -118,7 +118,7 @@ run_qemu <<EOF
     "arguments": {
         "options": {
             "driver": "$IMGFMT",
-            "id": "drive0-debug",
+            "node-name": "drive0-debug",
             "file": {
                 "driver": "blkdebug",
                 "image": "drive0",
@@ -159,7 +159,7 @@ run_qemu <<EOF
     "arguments": {
         "options": {
             "driver": "blkverify",
-            "id": "drive0-verify",
+            "node-name": "drive0-verify",
             "test": "drive0",
             "raw": {
                 "driver": "file",
@@ -195,7 +195,7 @@ run_qemu <<EOF
     "arguments": {
         "options": {
             "driver": "blkverify",
-            "id": "drive0-verify",
+            "node-name": "drive0-verify",
             "test": {
                 "driver": "$IMGFMT",
                 "file": {
@@ -234,7 +234,7 @@ run_qemu <<EOF
     "arguments": {
         "options": {
             "driver": "$IMGFMT",
-            "id": "drive0-debug",
+            "node-name": "drive0-debug",
             "file": {
                 "driver": "blkdebug",
                 "image": "drive0",
-- 
1.8.3.1

[Qemu-devel] [PULL 25/33] qemu-iotests/081: Avoid blockdev-add with id

[Kevin Wolf]

We want to remove the 'id' option for blockdev-add. This removes one
user of the option and makes it use only node names.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/qemu-iotests/081 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/081 b/tests/qemu-iotests/081
index d89fabc..0a809f3 100755
--- a/tests/qemu-iotests/081
+++ b/tests/qemu-iotests/081
@@ -119,7 +119,7 @@ run_qemu <<EOF
     "arguments": {
         "options": {
             "driver": "quorum",
-            "id": "drive0-quorum",
+            "node-name": "drive0-quorum",
             "vote-threshold": 2,
             "children": [
                 {
-- 
1.8.3.1

[Qemu-devel] [PULL 26/33] qemu-iotests/087: Avoid blockdev-add with id

[Kevin Wolf]

We want to remove the 'id' option for blockdev-add. This removes one
user of the option and makes it use only node names.

The test cases that test conflicts between the 'id' option to
blockdev-add and existing block devices or the 'node-name' of the same
command can be removed because it won't be possible to specify this at
the end of the series.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/qemu-iotests/087     | 62 ++++------------------------------------------
 tests/qemu-iotests/087.out |  6 +----
 2 files changed, 6 insertions(+), 62 deletions(-)

diff --git a/tests/qemu-iotests/087 b/tests/qemu-iotests/087
index e7bca37..5c04577 100755
--- a/tests/qemu-iotests/087
+++ b/tests/qemu-iotests/087
@@ -77,50 +77,12 @@ echo
 echo === Duplicate ID ===
 echo
 
-run_qemu <<EOF
+run_qemu -drive driver=$IMGFMT,id=disk,node-name=test-node,file="$TEST_IMG" <<EOF
 { "execute": "qmp_capabilities" }
 { "execute": "blockdev-add",
   "arguments": {
       "options": {
         "driver": "$IMGFMT",
-        "id": "disk",
-        "node-name": "test-node",
-        "file": {
-            "driver": "file",
-            "filename": "$TEST_IMG"
-        }
-      }
-    }
-  }
-{ "execute": "blockdev-add",
-  "arguments": {
-      "options": {
-        "driver": "$IMGFMT",
-        "id": "disk",
-        "file": {
-            "driver": "file",
-            "filename": "$TEST_IMG"
-        }
-      }
-    }
-  }
-{ "execute": "blockdev-add",
-  "arguments": {
-      "options": {
-        "driver": "$IMGFMT",
-        "id": "test-node",
-        "file": {
-            "driver": "file",
-            "filename": "$TEST_IMG"
-        }
-      }
-    }
-  }
-{ "execute": "blockdev-add",
-  "arguments": {
-      "options": {
-        "driver": "$IMGFMT",
-        "id": "disk2",
         "node-name": "disk",
         "file": {
             "driver": "file",
@@ -133,7 +95,6 @@ run_qemu <<EOF
   "arguments": {
       "options": {
         "driver": "$IMGFMT",
-        "id": "disk2",
         "node-name": "test-node",
         "file": {
             "driver": "file",
@@ -142,19 +103,6 @@ run_qemu <<EOF
       }
     }
   }
-{ "execute": "blockdev-add",
-  "arguments": {
-      "options": {
-        "driver": "$IMGFMT",
-        "id": "disk3",
-        "node-name": "disk3",
-        "file": {
-            "driver": "file",
-            "filename": "$TEST_IMG"
-        }
-      }
-    }
-  }
 { "execute": "quit" }
 EOF
 
@@ -168,7 +116,7 @@ run_qemu <<EOF
   "arguments": {
       "options": {
         "driver": "$IMGFMT",
-        "id": "disk",
+        "node-name": "disk",
         "aio": "native",
         "file": {
             "driver": "file",
@@ -191,7 +139,7 @@ run_qemu -S <<EOF
   "arguments": {
       "options": {
         "driver": "$IMGFMT",
-        "id": "disk",
+        "node-name": "disk",
         "file": {
             "driver": "file",
             "filename": "$TEST_IMG"
@@ -208,7 +156,7 @@ run_qemu <<EOF
   "arguments": {
       "options": {
         "driver": "$IMGFMT",
-        "id": "disk",
+        "node-name": "disk",
         "file": {
             "driver": "file",
             "filename": "$TEST_IMG"
@@ -229,7 +177,7 @@ run_qemu -S <<EOF
 { "execute": "blockdev-add",
   "arguments": {
       "options": {
-        "id": "disk"
+        "node-name": "disk"
       }
     }
   }
diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out
index a95c4b0..f2d6f96 100644
--- a/tests/qemu-iotests/087.out
+++ b/tests/qemu-iotests/087.out
@@ -13,15 +13,11 @@ QMP_VERSION
 
 === Duplicate ID ===
 
-Testing:
+Testing: -drive driver=IMGFMT,id=disk,node-name=test-node,file=TEST_DIR/t.IMGFMT
 QMP_VERSION
 {"return": {}}
-{"return": {}}
-{"error": {"class": "GenericError", "desc": "Device with id 'disk' already exists"}}
-{"error": {"class": "GenericError", "desc": "Device name 'test-node' conflicts with an existing node name"}}
 {"error": {"class": "GenericError", "desc": "node-name=disk is conflicting with a device id"}}
 {"error": {"class": "GenericError", "desc": "Duplicate node name"}}
-{"error": {"class": "GenericError", "desc": "Device name 'disk3' conflicts with an existing node name"}}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN"}
 
-- 
1.8.3.1

[Qemu-devel] [PULL 27/33] qemu-iotests/117: Avoid blockdev-add with id

[Kevin Wolf]

We want to remove the 'id' option for blockdev-add. This removes one
user of the option and makes it use only node names.

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

diff --git a/tests/qemu-iotests/117 b/tests/qemu-iotests/117
index 9385b3f..5b28039 100755
--- a/tests/qemu-iotests/117
+++ b/tests/qemu-iotests/117
@@ -52,14 +52,14 @@ _send_qemu_cmd $QEMU_HANDLE \
 
 _send_qemu_cmd $QEMU_HANDLE \
     "{ 'execute': 'blockdev-add',
-       'arguments': { 'options': { 'id': 'protocol',
+       'arguments': { 'options': { 'node-name': 'protocol',
                                    'driver': 'file',
                                    'filename': '$TEST_IMG' } } }" \
     'return'
 
 _send_qemu_cmd $QEMU_HANDLE \
     "{ 'execute': 'blockdev-add',
-       'arguments': { 'options': { 'id': 'format',
+       'arguments': { 'options': { 'node-name': 'format',
                                    'driver': '$IMGFMT',
                                    'file': 'protocol' } } }" \
     'return'
-- 
1.8.3.1

[Qemu-devel] [PULL 28/33] qemu-iotests/118: Avoid blockdev-add with id

[Kevin Wolf]

We want to remove the 'id' option for blockdev-add. This removes one
user of the option and makes it use only node names.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/qemu-iotests/118 | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/118 b/tests/qemu-iotests/118
index 0380069..e63a40f 100755
--- a/tests/qemu-iotests/118
+++ b/tests/qemu-iotests/118
@@ -648,13 +648,9 @@ class TestBlockJobsAfterCycle(ChangeBaseClass):
         qemu_img('create', '-f', iotests.imgfmt, old_img, '1M')
 
         self.vm = iotests.VM()
+        self.vm.add_drive_raw("id=drive0,driver=null-co,if=none")
         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]/inserted/image/format', 'null-co')
 
-- 
1.8.3.1

[Qemu-devel] [PULL 29/33] qemu-iotests/124: Avoid blockdev-add with id

[Kevin Wolf]

We want to remove the 'id' option for blockdev-add. This removes one
user of the option and makes it use only node names.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/qemu-iotests/124 | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index de7cdbe..2f0bc24 100644
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -49,8 +49,8 @@ def transaction_bitmap_clear(node, name, **kwargs):
 
 
 def transaction_drive_backup(device, target, **kwargs):
-    return transaction_action('drive-backup', device=device, target=target,
-                              **kwargs)
+    return transaction_action('drive-backup', job_id=device, device=device,
+                              target=target, **kwargs)
 
 
 class Bitmap:
@@ -177,7 +177,8 @@ class TestIncrementalBackupBase(iotests.QMPTestCase):
     def create_anchor_backup(self, drive=None):
         if drive is None:
             drive = self.drives[-1]
-        res = self.do_qmp_backup(device=drive['id'], sync='full',
+        res = self.do_qmp_backup(job_id=drive['id'],
+                                 device=drive['id'], sync='full',
                                  format=drive['fmt'], target=drive['backup'])
         self.assertTrue(res)
         self.files.append(drive['backup'])
@@ -188,7 +189,8 @@ class TestIncrementalBackupBase(iotests.QMPTestCase):
         if bitmap is None:
             bitmap = self.bitmaps[-1]
         _, reference = bitmap.last_target()
-        res = self.do_qmp_backup(device=bitmap.drive['id'], sync='full',
+        res = self.do_qmp_backup(job_id=bitmap.drive['id'],
+                                 device=bitmap.drive['id'], sync='full',
                                  format=bitmap.drive['fmt'], target=reference)
         self.assertTrue(res)
 
@@ -221,7 +223,8 @@ class TestIncrementalBackupBase(iotests.QMPTestCase):
             parent, _ = bitmap.last_target()
 
         target = self.prepare_backup(bitmap, parent)
-        res = self.do_qmp_backup(device=bitmap.drive['id'],
+        res = self.do_qmp_backup(job_id=bitmap.drive['id'],
+                                 device=bitmap.drive['id'],
                                  sync='incremental', bitmap=bitmap.name,
                                  format=bitmap.drive['fmt'], target=target,
                                  mode='existing')
@@ -414,7 +417,7 @@ class TestIncrementalBackup(TestIncrementalBackupBase):
 
         # Create a blkdebug interface to this img as 'drive1'
         result = self.vm.qmp('blockdev-add', options={
-            'id': drive1['id'],
+            'node-name': drive1['id'],
             'driver': drive1['fmt'],
             'file': {
                 'driver': 'blkdebug',
@@ -558,7 +561,7 @@ class TestIncrementalBackupBlkdebug(TestIncrementalBackupBase):
 
         drive0 = self.drives[0]
         result = self.vm.qmp('blockdev-add', options={
-            'id': drive0['id'],
+            'node-name': drive0['id'],
             'driver': drive0['fmt'],
             'file': {
                 'driver': 'blkdebug',
-- 
1.8.3.1

[Qemu-devel] [PULL 30/33] qemu-iotests/139: Avoid blockdev-add with id

[Kevin Wolf]

We want to remove the 'id' option for blockdev-add. This removes one
user of the option and makes it use only node names.

Some test cases that used to work with an unattached BlockBackend are
removed, either because they don't make sense with an attached device or
because the equivalent test case with an attached device already exists.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/qemu-iotests/139     | 178 +++++++++++++++------------------------------
 tests/qemu-iotests/139.out |   4 +-
 2 files changed, 59 insertions(+), 123 deletions(-)

diff --git a/tests/qemu-iotests/139 b/tests/qemu-iotests/139
index a4b9694..47a4c26 100644
--- a/tests/qemu-iotests/139
+++ b/tests/qemu-iotests/139
@@ -31,6 +31,7 @@ class TestBlockdevDel(iotests.QMPTestCase):
     def setUp(self):
         iotests.qemu_img('create', '-f', iotests.imgfmt, base_img, '1M')
         self.vm = iotests.VM()
+        self.vm.add_device("virtio-scsi-pci,id=virtio-scsi")
         self.vm.launch()
 
     def tearDown(self):
@@ -39,18 +40,6 @@ class TestBlockdevDel(iotests.QMPTestCase):
         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')
@@ -58,24 +47,6 @@ class TestBlockdevDel(iotests.QMPTestCase):
         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
@@ -105,23 +76,6 @@ class TestBlockdevDel(iotests.QMPTestCase):
         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)
@@ -133,51 +87,47 @@ class TestBlockdevDel(iotests.QMPTestCase):
         self.checkBlockDriverState(node, expect_error)
 
     # Add a device model
-    def addDeviceModel(self, device, backend):
+    def addDeviceModel(self, device, backend, driver = 'virtio-blk-pci'):
         result = self.vm.qmp('device_add', id = device,
-                             driver = 'virtio-blk-pci', drive = backend)
+                             driver = driver, drive = backend)
         self.assert_qmp(result, 'return', {})
 
     # Delete a device model
-    def delDeviceModel(self, device):
+    def delDeviceModel(self, device, is_virtio_blk = True):
         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)
+        if is_virtio_blk:
+            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,
+    def ejectDrive(self, device, node, expect_error = False,
                    destroys_media = True):
-        self.checkBlockBackend(backend, node)
         self.checkBlockDriverState(node)
-        result = self.vm.qmp('eject', device = backend)
+        result = self.vm.qmp('eject', id = device)
         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)
+    def insertDrive(self, device, node):
         self.checkBlockDriverState(node)
         result = self.vm.qmp('x-blockdev-insert-medium',
-                             device = backend, node_name = node)
+                             id = device, node_name = node)
         self.assert_qmp(result, 'return', {})
-        self.checkBlockBackend(backend, node)
         self.checkBlockDriverState(node)
 
     # Create a snapshot using 'blockdev-snapshot-sync'
@@ -204,26 +154,23 @@ class TestBlockdevDel(iotests.QMPTestCase):
         self.checkBlockDriverState(overlay)
 
     # Create a mirror
-    def createMirror(self, backend, node, new_node):
-        self.checkBlockBackend(backend, node)
+    def createMirror(self, node, new_node):
         self.checkBlockDriverState(new_node, False)
-        opts = {'device': backend,
+        opts = {'device': node,
+                'job-id': node,
                 '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)
+    def completeBlockJob(self, id, node_before, node_after):
+        result = self.vm.qmp('block-job-complete', device=id)
         self.assert_qmp(result, 'return', {})
-        self.wait_until_completed(backend)
-        self.checkBlockBackend(backend, node_after)
+        self.wait_until_completed(id)
 
     # Add a BlkDebug node
     # Note that the purpose of this is to test the x-blockdev-del
@@ -297,89 +244,78 @@ class TestBlockdevDel(iotests.QMPTestCase):
     # 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.addBlockDriverState('node0')
+        self.addDeviceModel('device0', 'node0')
+        self.ejectDrive('device0', 'node0', expect_error = True)
+        self.delBlockDriverState('node0', expect_error = True)
         self.delDeviceModel('device0')
-        self.delBlockBackend('drive0', 'node0')
+        self.delBlockDriverState('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('node0')
+        self.addDeviceModel('device0', 'node0', 'scsi-cd')
+        self.ejectDrive('device0', 'node0', destroys_media = False)
+        self.delBlockDriverState('node0')
+
+        # This creates a new BlockDriverState and inserts it into the device
         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.insertDrive('device0', 'node1')
+        # The node can't be removed: the new device has an extra reference
         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.ejectDrive('device0', 'node1', destroys_media = False)
         self.delBlockDriverState('node1')
-        self.delBlockBackend('drive0', None)
+        self.delDeviceModel('device0', False)
 
     def testSnapshotSync(self):
-        self.addBlockBackend('drive0', 'node0')
+        self.addBlockDriverState('node0')
+        self.addDeviceModel('device0', '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)
+        self.delBlockDriverState('overlay0', expect_error = True)
+        # This succeeds because device0 only has the backend reference
+        self.delDeviceModel('device0')
+        # FIXME Would still be there if blockdev-snapshot-sync took a ref
+        self.checkBlockDriverState('overlay0', False)
+        self.delBlockDriverState('node0')
 
     def testSnapshot(self):
-        self.addBlockBackend('drive0', 'node0')
+        self.addBlockDriverState('node0')
+        self.addDeviceModel('device0', 'node0', 'scsi-cd')
         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.ejectDrive('device0', 'overlay0', destroys_media = False)
         self.delBlockDriverState('node0', expect_error = True)
         self.delBlockDriverState('overlay0')
-        self.checkBlockDriverState('node0', False)
+        self.delBlockDriverState('node0')
 
     def testMirror(self):
-        self.addBlockBackend('drive0', 'node0')
-        self.createMirror('drive0', 'node0', 'mirror0')
+        self.addBlockDriverState('node0')
+        self.addDeviceModel('device0', 'node0', 'scsi-cd')
+        self.createMirror('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.wait_ready('node0')
+        self.completeBlockJob('node0', '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')
+        # This succeeds because the device now points to mirror0
+        self.delBlockDriverState('node0')
+        self.delBlockDriverState('mirror0', expect_error = True)
+        self.delDeviceModel('device0', False)
+        # FIXME mirror0 disappears, drive-mirror doesn't take a reference
+        #self.delBlockDriverState('mirror0')
 
     def testBlkDebug(self):
         self.addBlkDebug('debug0', 'node0')
diff --git a/tests/qemu-iotests/139.out b/tests/qemu-iotests/139.out
index 281b69e..dae404e 100644
--- a/tests/qemu-iotests/139.out
+++ b/tests/qemu-iotests/139.out
@@ -1,5 +1,5 @@
-............
+.........
 ----------------------------------------------------------------------
-Ran 12 tests
+Ran 9 tests
 
 OK
-- 
1.8.3.1

[Qemu-devel] [PULL 31/33] block: Avoid printing NULL string in error messages

[Kevin Wolf]

Even for nodes that have a BlockBackend attached, bdrv_get_parent_name()
can return NULL if the BB is anonymous (e.g. it belongs to a block job
or a device that was created with a drive=<node-name> option).

Remove the information from the error message. The user probably knows
already why the node is still in use.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 blockdev.c                 | 9 +++------
 tests/qemu-iotests/085.out | 6 +++---
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 405145a..17c2671 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1803,8 +1803,7 @@ static void external_snapshot_prepare(BlkActionState *common,
     }
 
     if (bdrv_has_blk(state->new_bs)) {
-        error_setg(errp, "The snapshot is already in use by %s",
-                   bdrv_get_parent_name(state->new_bs));
+        error_setg(errp, "The snapshot is already in use");
         return;
     }
 
@@ -2532,8 +2531,7 @@ void qmp_x_blockdev_insert_medium(bool has_device, const char *device,
     }
 
     if (bdrv_has_blk(bs)) {
-        error_setg(errp, "Node '%s' is already in use by '%s'", node_name,
-                   bdrv_get_parent_name(bs));
+        error_setg(errp, "Node '%s' is already in use", node_name);
         return;
     }
 
@@ -3941,8 +3939,7 @@ void qmp_x_blockdev_del(bool has_id, const char *id,
             return;
         }
         if (bdrv_has_blk(bs)) {
-            error_setg(errp, "Node %s is in use by %s",
-                       node_name, bdrv_get_parent_name(bs));
+            error_setg(errp, "Node %s is in use", node_name);
             return;
         }
         aio_context = bdrv_get_aio_context(bs);
diff --git a/tests/qemu-iotests/085.out b/tests/qemu-iotests/085.out
index 01c78d6..08e4bb7 100644
--- a/tests/qemu-iotests/085.out
+++ b/tests/qemu-iotests/085.out
@@ -68,9 +68,9 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/
 
 === 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"}}
+{"error": {"class": "GenericError", "desc": "The snapshot is already in use"}}
+{"error": {"class": "GenericError", "desc": "The snapshot is already in use"}}
+{"error": {"class": "GenericError", "desc": "The snapshot is already in use"}}
 
 === Invalid command - snapshot node used as backing hd ===
 
-- 
1.8.3.1

[Qemu-devel] [PULL 32/33] qemu-iotests/141: Avoid blockdev-add with id

[Kevin Wolf]

We want to remove the 'id' option for blockdev-add. This removes one
user of the option and makes it use only node names.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/qemu-iotests/141     | 24 ++++++++++++++----------
 tests/qemu-iotests/141.out | 24 ++++++++++++------------
 2 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/tests/qemu-iotests/141 b/tests/qemu-iotests/141
index b2617e5..c092d87 100755
--- a/tests/qemu-iotests/141
+++ b/tests/qemu-iotests/141
@@ -51,7 +51,7 @@ test_blockjob()
         "{'execute': 'blockdev-add',
           'arguments': {
               'options': {
-                  'id': 'drv0',
+                  'node-name': 'drv0',
                   'driver': '$IMGFMT',
                   'file': {
                       'driver': 'file',
@@ -66,18 +66,18 @@ test_blockjob()
 
     # We want this to return an error because the block job is still running
     _send_qemu_cmd $QEMU_HANDLE \
-        "{'execute': 'x-blockdev-remove-medium',
-          'arguments': {'device': 'drv0'}}" \
+        "{'execute': 'x-blockdev-del',
+          'arguments': {'node-name': 'drv0'}}" \
         'error'
 
     _send_qemu_cmd $QEMU_HANDLE \
         "{'execute': 'block-job-cancel',
-          'arguments': {'device': 'drv0'}}" \
+          'arguments': {'device': 'job0'}}" \
         "$3"
 
     _send_qemu_cmd $QEMU_HANDLE \
         "{'execute': 'x-blockdev-del',
-          'arguments': {'id': 'drv0'}}" \
+          'arguments': {'node-name': 'drv0'}}" \
         'return'
 }
 
@@ -101,7 +101,8 @@ echo
 
 test_blockjob \
     "{'execute': 'drive-backup',
-      'arguments': {'device': 'drv0',
+      'arguments': {'job-id': 'job0',
+                    'device': 'drv0',
                     'target': '$TEST_DIR/o.$IMGFMT',
                     'format': '$IMGFMT',
                     'sync': 'none'}}" \
@@ -117,7 +118,8 @@ echo
 
 test_blockjob \
     "{'execute': 'drive-mirror',
-      'arguments': {'device': 'drv0',
+      'arguments': {'job-id': 'job0',
+                    'device': 'drv0',
                     'target': '$TEST_DIR/o.$IMGFMT',
                     'format': '$IMGFMT',
                     'sync': 'none'}}" \
@@ -134,7 +136,7 @@ echo
 
 test_blockjob \
     "{'execute': 'block-commit',
-      'arguments': {'device': 'drv0'}}" \
+      'arguments': {'job-id': 'job0', 'device': 'drv0'}}" \
     'BLOCK_JOB_READY' \
     'BLOCK_JOB_COMPLETED'
 
@@ -150,7 +152,8 @@ $QEMU_IO -c 'write 0 1M' "$TEST_DIR/m.$IMGFMT" | _filter_qemu_io
 
 test_blockjob \
     "{'execute': 'block-commit',
-      'arguments': {'device': 'drv0',
+      'arguments': {'job-id': 'job0',
+                    'device': 'drv0',
                     'top':    '$TEST_DIR/m.$IMGFMT',
                     'speed':  1}}" \
     'return' \
@@ -172,7 +175,8 @@ $QEMU_IO -c 'write 0 1M' "$TEST_DIR/b.$IMGFMT" | _filter_qemu_io
 
 test_blockjob \
     "{'execute': 'block-stream',
-      'arguments': {'device': 'drv0',
+      'arguments': {'job-id': 'job0',
+                    'device': 'drv0',
                     'speed': 1}}" \
     'return' \
     'BLOCK_JOB_CANCELLED'
diff --git a/tests/qemu-iotests/141.out b/tests/qemu-iotests/141.out
index eaf1e60..195ca1a 100644
--- a/tests/qemu-iotests/141.out
+++ b/tests/qemu-iotests/141.out
@@ -9,30 +9,30 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/m.
 {"return": {}}
 Formatting 'TEST_DIR/o.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.IMGFMT backing_fmt=IMGFMT
 {"return": {}}
-{"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device is in use by block job: backup"}}
+{"error": {"class": "GenericError", "desc": "Node drv0 is in use"}}
 {"return": {}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "drv0", "len": 1048576, "offset": 0, "speed": 0, "type": "backup"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "job0", "len": 1048576, "offset": 0, "speed": 0, "type": "backup"}}
 {"return": {}}
 
 === Testing drive-mirror ===
 
 {"return": {}}
 Formatting 'TEST_DIR/o.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.IMGFMT backing_fmt=IMGFMT
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "mirror"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "job0", "len": 0, "offset": 0, "speed": 0, "type": "mirror"}}
 {"return": {}}
-{"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device is in use by block job: mirror"}}
+{"error": {"class": "GenericError", "desc": "Node drv0 is in use"}}
 {"return": {}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "mirror"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "job0", "len": 0, "offset": 0, "speed": 0, "type": "mirror"}}
 {"return": {}}
 
 === Testing active block-commit ===
 
 {"return": {}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "job0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}}
 {"return": {}}
-{"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device is in use by block job: commit"}}
+{"error": {"class": "GenericError", "desc": "Node drv0 is in use"}}
 {"return": {}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "job0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}}
 {"return": {}}
 
 === Testing non-active block-commit ===
@@ -41,9 +41,9 @@ wrote 1048576/1048576 bytes at offset 0
 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 {"return": {}}
 {"return": {}}
-{"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device is in use by block job: commit"}}
+{"error": {"class": "GenericError", "desc": "Node drv0 is in use"}}
 {"return": {}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "drv0", "len": 1048576, "offset": 524288, "speed": 1, "type": "commit"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "job0", "len": 1048576, "offset": 524288, "speed": 1, "type": "commit"}}
 {"return": {}}
 
 === Testing block-stream ===
@@ -52,8 +52,8 @@ wrote 1048576/1048576 bytes at offset 0
 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 {"return": {}}
 {"return": {}}
-{"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device is in use by block job: stream"}}
+{"error": {"class": "GenericError", "desc": "Node drv0 is in use"}}
 {"return": {}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "drv0", "len": 1048576, "offset": 524288, "speed": 1, "type": "stream"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "job0", "len": 1048576, "offset": 524288, "speed": 1, "type": "stream"}}
 {"return": {}}
 *** done
-- 
1.8.3.1

[Qemu-devel] [PULL 33/33] block: Remove BB interface from blockdev-add/del

[Kevin Wolf]

With this patch, blockdev-add always works on a node level, i.e. it
creates a BDS, but no BB. Consequently, x-blockdev-del doesn't need the
'device' option any more, but 'node-name' becomes mandatory.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 blockdev.c                 | 124 +++++++++++++--------------------------------
 docs/qmp-commands.txt      |  24 +++------
 qapi/block-core.json       |  30 +++--------
 tests/qemu-iotests/087.out |   2 +-
 4 files changed, 48 insertions(+), 132 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 17c2671..29c6561 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2844,7 +2844,7 @@ void hmp_drive_del(Monitor *mon, const QDict *qdict)
 
     bs = bdrv_find_node(id);
     if (bs) {
-        qmp_x_blockdev_del(false, NULL, true, id, &local_err);
+        qmp_x_blockdev_del(id, &local_err);
         if (local_err) {
             error_report_err(local_err);
         }
@@ -3827,7 +3827,6 @@ out:
 void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
 {
     BlockDriverState *bs;
-    BlockBackend *blk = NULL;
     QObject *obj;
     Visitor *v = qmp_output_visitor_new(&obj);
     QDict *qdict;
@@ -3859,37 +3858,21 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
 
     qdict_flatten(qdict);
 
-    if (options->has_id) {
-        blk = blockdev_init(NULL, qdict, &local_err);
-        if (local_err) {
-            error_propagate(errp, local_err);
-            goto fail;
-        }
-
-        bs = blk_bs(blk);
-    } else {
-        if (!qdict_get_try_str(qdict, "node-name")) {
-            error_setg(errp, "'id' and/or 'node-name' need to be specified for "
-                       "the root node");
-            goto fail;
-        }
-
-        bs = bds_tree_init(qdict, errp);
-        if (!bs) {
-            goto fail;
-        }
+    if (!qdict_get_try_str(qdict, "node-name")) {
+        error_setg(errp, "'node-name' must be specified for the root node");
+        goto fail;
+    }
 
-        QTAILQ_INSERT_TAIL(&monitor_bdrv_states, bs, monitor_list);
+    bs = bds_tree_init(qdict, errp);
+    if (!bs) {
+        goto fail;
     }
 
+    QTAILQ_INSERT_TAIL(&monitor_bdrv_states, bs, monitor_list);
+
     if (bs && bdrv_key_required(bs)) {
-        if (blk) {
-            monitor_remove_blk(blk);
-            blk_unref(blk);
-        } else {
-            QTAILQ_REMOVE(&monitor_bdrv_states, bs, monitor_list);
-            bdrv_unref(bs);
-        }
+        QTAILQ_REMOVE(&monitor_bdrv_states, bs, monitor_list);
+        bdrv_unref(bs);
         error_setg(errp, "blockdev-add doesn't support encrypted devices");
         goto fail;
     }
@@ -3898,81 +3881,42 @@ fail:
     visit_free(v);
 }
 
-void qmp_x_blockdev_del(bool has_id, const char *id,
-                        bool has_node_name, const char *node_name, Error **errp)
+void qmp_x_blockdev_del(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");
+    bs = bdrv_find_node(node_name);
+    if (!bs) {
+        error_setg(errp, "Cannot find node %s", node_name);
         return;
     }
-
-    if (has_id) {
-        /* blk_by_name() never returns a BB that is not owned by the monitor */
-        blk = blk_by_name(id);
-        if (!blk) {
-            error_setg(errp, "Cannot find block backend %s", id);
-            return;
-        }
-        if (blk_legacy_dinfo(blk)) {
-            error_setg(errp, "Deleting block backend added with drive-add"
-                       " is not supported");
-            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 {
-        blk = NULL;
-        bs = bdrv_find_node(node_name);
-        if (!bs) {
-            error_setg(errp, "Cannot find node %s", node_name);
-            return;
-        }
-        if (bdrv_has_blk(bs)) {
-            error_setg(errp, "Node %s is in use", node_name);
-            return;
-        }
-        aio_context = bdrv_get_aio_context(bs);
+    if (bdrv_has_blk(bs)) {
+        error_setg(errp, "Node %s is in use", node_name);
+        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 (!blk && !QTAILQ_IN_USE(bs, monitor_list)) {
-            error_setg(errp, "Node %s is not owned by the monitor",
-                       bs->node_name);
-            goto out;
-        }
+    if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, errp)) {
+        goto out;
+    }
 
-        if (bs->refcnt > 1) {
-            error_setg(errp, "Block device %s is in use",
-                       bdrv_get_device_or_node_name(bs));
-            goto out;
-        }
+    if (!bs->monitor_list.tqe_prev) {
+        error_setg(errp, "Node %s is not owned by the monitor",
+                   bs->node_name);
+        goto out;
     }
 
-    if (blk) {
-        monitor_remove_blk(blk);
-        blk_unref(blk);
-    } else {
-        QTAILQ_REMOVE(&monitor_bdrv_states, bs, monitor_list);
-        bdrv_unref(bs);
+    if (bs->refcnt > 1) {
+        error_setg(errp, "Block device %s is in use",
+                   bdrv_get_device_or_node_name(bs));
+        goto out;
     }
 
+    QTAILQ_REMOVE(&monitor_bdrv_states, bs, monitor_list);
+    bdrv_unref(bs);
+
 out:
     aio_context_release(aio_context);
 }
diff --git a/docs/qmp-commands.txt b/docs/qmp-commands.txt
index 41f5698..e0adceb 100644
--- a/docs/qmp-commands.txt
+++ b/docs/qmp-commands.txt
@@ -3141,7 +3141,7 @@ Example (2):
      "arguments": {
          "options": {
            "driver": "qcow2",
-           "id": "my_disk",
+           "node-name": "my_disk",
            "discard": "unmap",
            "cache": {
                "direct": true,
@@ -3168,18 +3168,9 @@ 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.
+Deletes a block device that has been added using blockdev-add.
+The command will fail if the node is attached to a device or is
+otherwise being used.
 
 This command is still a work in progress and is considered
 experimental. Stay away from it unless you want to help with its
@@ -3187,8 +3178,7 @@ 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)
+- "node-name": Name of the graph node to delete (json-string)
 
 Example:
 
@@ -3196,7 +3186,7 @@ Example:
      "arguments": {
          "options": {
              "driver": "qcow2",
-             "id": "drive0",
+             "node-name": "node0",
              "file": {
                  "driver": "file",
                  "filename": "test.qcow2"
@@ -3208,7 +3198,7 @@ Example:
 <- { "return": {} }
 
 -> { "execute": "x-blockdev-del",
-     "arguments": { "id": "drive0" }
+     "arguments": { "node-name": "node0" }
    }
 <- { "return": {} }
 
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 5f04dab..92193ab 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2217,13 +2217,8 @@
 # block devices, independent of the block driver:
 #
 # @driver:        block driver name
-# @id:            #optional id by which the new block device can be referred to.
-#                 This option is only allowed on the top level of blockdev-add.
-#                 A BlockBackend will be created by blockdev-add if and only if
-#                 this option is given.
-# @node-name:     #optional the name of a block driver state node (Since 2.0).
-#                 This option is required on the top level of blockdev-add if
-#                 the @id option is not given there.
+# @node-name:     #optional the node name of the new node (Since 2.0).
+#                 This option is required on the top level of blockdev-add.
 # @discard:       #optional discard-related options (default: ignore)
 # @cache:         #optional cache-related options
 # @aio:           #optional AIO backend (default: threads)
@@ -2238,8 +2233,6 @@
 ##
 { 'union': 'BlockdevOptions',
   'base': { 'driver': 'BlockdevDriver',
-# TODO 'id' is a BB-level option, remove it
-            '*id': 'str',
             '*node-name': 'str',
             '*discard': 'BlockdevDiscardOptions',
             '*cache': 'BlockdevCacheOptions',
@@ -2323,29 +2316,18 @@
 # @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.
+# The command will fail if the node is attached to a device or is
+# otherwise being used.
 #
 # 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.
+# @node-name: Name of the graph node to delete.
 #
 # Since: 2.5
 ##
-{ 'command': 'x-blockdev-del', 'data': { '*id': 'str', '*node-name': 'str' } }
+{ 'command': 'x-blockdev-del', 'data': { 'node-name': 'str' } }
 
 ##
 # @blockdev-open-tray:
diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out
index f2d6f96..bef6862 100644
--- a/tests/qemu-iotests/087.out
+++ b/tests/qemu-iotests/087.out
@@ -6,7 +6,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 Testing:
 QMP_VERSION
 {"return": {}}
-{"error": {"class": "GenericError", "desc": "'id' and/or 'node-name' need to be specified for the root node"}}
+{"error": {"class": "GenericError", "desc": "'node-name' must be specified for the root node"}}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN"}
 
-- 
1.8.3.1

Re: [Qemu-devel] [PULL 00/33] Block layer patches

[Peter Maydell]

On 22 September 2016 at 17:29, Kevin Wolf <kwolf@redhat.com> wrote:
> The following changes since commit 430da7a81d356e368ccd88dcca60f38da9aa5b9a:
>
>   Merge remote-tracking branch 'remotes/riku/tags/pull-linux-user-20160915' into staging (2016-09-22 15:39:54 +0100)
>
> are available in the git repository at:
>
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to 5dccdc1b1e0c57746e5f90c69f697029a3686b5f:
>
>   block: Remove BB interface from blockdev-add/del (2016-09-22 18:02:15 +0200)
>
> ----------------------------------------------------------------
> Block layer patches
>
> ----------------------------------------------------------------

Hi; I'm afraid this fails to build on OSX: it looks like you
changed the arguments to some qmp functions but forgot to change
their callsites in ui/cocoa.m.

/Users/pm215/src/qemu-for-merges/ui/cocoa.m:1088:86: error: too few
arguments to function call, expected 7, have 4
    qmp_eject([drive cStringUsingEncoding: NSASCIIStringEncoding],
false, false, &err);
    ~~~~~~~~~
              ^
./qmp-commands.h:109:1: note: 'qmp_eject' declared here
void qmp_eject(bool has_device, const char *device, bool has_id, const
char *id, bool has_force, bool force, Error **errp);
^
/Users/pm215/src/qemu-for-merges/ui/cocoa.m:1127:40: error: too few
arguments to function call, expected 10, have 7
                                   &err);
                                       ^
./qmp-commands.h:61:1: note: 'qmp_blockdev_change_medium' declared here
void qmp_blockdev_change_medium(bool has_device, const char *device,
bool has_id, const char *id, const char *filename, bool has_format,
const char *fo...
^

thanks
-- PMM

Re: [Qemu-devel] [PULL 00/33] Block layer patches

[Kevin Wolf]

Am 23.09.2016 um 13:12 hat Peter Maydell geschrieben:
> On 22 September 2016 at 17:29, Kevin Wolf <kwolf@redhat.com> wrote:
> > The following changes since commit 430da7a81d356e368ccd88dcca60f38da9aa5b9a:
> >
> >   Merge remote-tracking branch 'remotes/riku/tags/pull-linux-user-20160915' into staging (2016-09-22 15:39:54 +0100)
> >
> > are available in the git repository at:
> >
> >
> >   git://repo.or.cz/qemu/kevin.git tags/for-upstream
> >
> > for you to fetch changes up to 5dccdc1b1e0c57746e5f90c69f697029a3686b5f:
> >
> >   block: Remove BB interface from blockdev-add/del (2016-09-22 18:02:15 +0200)
> >
> > ----------------------------------------------------------------
> > Block layer patches
> >
> > ----------------------------------------------------------------
> 
> Hi; I'm afraid this fails to build on OSX: it looks like you
> changed the arguments to some qmp functions but forgot to change
> their callsites in ui/cocoa.m.

I can't test compile any of this, but I'll give it a try...

Kevin

Re: [Qemu-devel] [PULL 17/33] block: Accept device model name for x-blockdev-remove-medium

[Paolo Bonzini]

On 22/09/2016 18:29, Kevin Wolf wrote:
> -    qmp_x_blockdev_remove_medium(device, &err);
> +    qmp_x_blockdev_remove_medium(true, device, false, NULL, errp);
>      if (err) {
>          error_propagate(errp, err);
>          goto fail;

Bug: &err changed to errp, so err is always NULL.

Paolo

Re: [Qemu-devel] [PULL 17/33] block: Accept device model name for x-blockdev-remove-medium

[Kevin Wolf]

Am 26.09.2016 um 12:59 hat Paolo Bonzini geschrieben:
> On 22/09/2016 18:29, Kevin Wolf wrote:
> > -    qmp_x_blockdev_remove_medium(device, &err);
> > +    qmp_x_blockdev_remove_medium(true, device, false, NULL, errp);
> >      if (err) {
> >          error_propagate(errp, err);
> >          goto fail;
> 
> Bug: &err changed to errp, so err is always NULL.

&err vs. errp is kind of hard to spot in a diff. Maybe a good reason to
stick with "local_err" rather than "err" where we introduce new local
error variables.

Anyway, patch sent.

Kevin