[Qemu-devel] [PATCH v4 0/4] Add 'x-blockdev-del' command

[Qemu-devel] [PATCH v4 0/4] Add 'x-blockdev-del' command

[Alberto Garcia]

This version has only documentation and test fixes.

Most are cosmetic changes, but there were two missing wait_ready() and
wait_until_completed() calls that could break one of the tests.

Regards,

Berto

v4:
- patch 3: s/being used/in use/ [Max]
- patch 3: add 'blockdev-add' command to the 'x-blockdev-del' example
  in qmp-commands.hx [Max]
- patch 4: clarify that the BlkDebug and BlkVerify are not meant to be
  real use cases of those drivers, but only to test the sanity checks
  of 'x-blockdev-del' [Max]
- patch 4: fix addBlockDriverStateOverlay() documentation [Max]
- patch 4: add missing wait_ready() and wait_until_completed() calls

v3: https://lists.gnu.org/archive/html/qemu-block/2015-10/msg00854.html
- Remove the extra references added in v2 to the mirror and backup
  jobs, and use op blockers instead (for the mirror case only).

v2: https://lists.gnu.org/archive/html/qemu-block/2015-10/msg00828.html
- Rename it as 'x-blockdev-del' and label it as experimental.
- Use two parameters instead of just one. If you try to delete a BDS,
  it must not be attached to any backend.
- New test cases.
- Hold extra references during the mirror and backup block jobs.

v1: https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg02999.html
- Initial implementation

Alberto Garcia (4):
  mirror: block all operations on the target image during the job
  block: Add blk_get_refcnt()
  block: Add 'x-blockdev-del' QMP command
  iotests: Add tests for the x-blockdev-del command

 block/block-backend.c          |   5 +
 block/mirror.c                 |   4 +
 blockdev.c                     |  66 +++++++
 include/sysemu/block-backend.h |   1 +
 qapi/block-core.json           |  32 +++-
 qmp-commands.hx                |  61 +++++-
 tests/qemu-iotests/139         | 414 +++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/139.out     |   5 +
 tests/qemu-iotests/group       |   1 +
 9 files changed, 585 insertions(+), 4 deletions(-)
 create mode 100644 tests/qemu-iotests/139
 create mode 100644 tests/qemu-iotests/139.out

-- 
2.6.1

[Qemu-devel] [PATCH v4 1/4] mirror: block all operations on the target image during the job

[Alberto Garcia]

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

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

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

[Qemu-devel] [PATCH v4 2/4] block: Add blk_get_refcnt()

[Alberto Garcia]

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

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

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

[Qemu-devel] [PATCH v4 3/4] block: Add 'x-blockdev-del' QMP command

[Alberto Garcia]

This command is still experimental, hence the name.

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

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

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

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

[Qemu-devel] [PATCH v4 4/4] iotests: Add tests for the x-blockdev-del command

[Alberto Garcia]

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 tests/qemu-iotests/139     | 414 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/139.out |   5 +
 tests/qemu-iotests/group   |   1 +
 3 files changed, 420 insertions(+)
 create mode 100644 tests/qemu-iotests/139
 create mode 100644 tests/qemu-iotests/139.out

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

Re: [Qemu-devel] [PATCH v4 4/4] iotests: Add tests for the x-blockdev-del command

[Max Reitz]

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

On 02.11.2015 15:51, Alberto Garcia wrote:
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
>  tests/qemu-iotests/139     | 414 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/139.out |   5 +
>  tests/qemu-iotests/group   |   1 +
>  3 files changed, 420 insertions(+)
>  create mode 100644 tests/qemu-iotests/139
>  create mode 100644 tests/qemu-iotests/139.out

Reviewed-by: Max Reitz <mreitz@redhat.com>


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

Re: [Qemu-devel] [PATCH v4 0/4] Add 'x-blockdev-del' command

[Max Reitz]

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

On 02.11.2015 15:51, Alberto Garcia wrote:
> This version has only documentation and test fixes.
> 
> Most are cosmetic changes, but there were two missing wait_ready() and
> wait_until_completed() calls that could break one of the tests.
> 
> Regards,
> 
> Berto
> 
> v4:
> - patch 3: s/being used/in use/ [Max]
> - patch 3: add 'blockdev-add' command to the 'x-blockdev-del' example
>   in qmp-commands.hx [Max]
> - patch 4: clarify that the BlkDebug and BlkVerify are not meant to be
>   real use cases of those drivers, but only to test the sanity checks
>   of 'x-blockdev-del' [Max]
> - patch 4: fix addBlockDriverStateOverlay() documentation [Max]
> - patch 4: add missing wait_ready() and wait_until_completed() calls
> 
> v3: https://lists.gnu.org/archive/html/qemu-block/2015-10/msg00854.html
> - Remove the extra references added in v2 to the mirror and backup
>   jobs, and use op blockers instead (for the mirror case only).
> 
> v2: https://lists.gnu.org/archive/html/qemu-block/2015-10/msg00828.html
> - Rename it as 'x-blockdev-del' and label it as experimental.
> - Use two parameters instead of just one. If you try to delete a BDS,
>   it must not be attached to any backend.
> - New test cases.
> - Hold extra references during the mirror and backup block jobs.
> 
> v1: https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg02999.html
> - Initial implementation
> 
> Alberto Garcia (4):
>   mirror: block all operations on the target image during the job
>   block: Add blk_get_refcnt()
>   block: Add 'x-blockdev-del' QMP command
>   iotests: Add tests for the x-blockdev-del command
> 
>  block/block-backend.c          |   5 +
>  block/mirror.c                 |   4 +
>  blockdev.c                     |  66 +++++++
>  include/sysemu/block-backend.h |   1 +
>  qapi/block-core.json           |  32 +++-
>  qmp-commands.hx                |  61 +++++-
>  tests/qemu-iotests/139         | 414 +++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/139.out     |   5 +
>  tests/qemu-iotests/group       |   1 +
>  9 files changed, 585 insertions(+), 4 deletions(-)
>  create mode 100644 tests/qemu-iotests/139
>  create mode 100644 tests/qemu-iotests/139.out

Thanks, applied to my block tree:

https://github.com/XanClic/qemu/commits/block


Max


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