All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3 00/12] block: Remove BB interface from blockdev-add/del
@ 2016-09-21 12:55 Kevin Wolf
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 01/12] qemu-iotests/041: Avoid blockdev-add with id Kevin Wolf
                   ` (12 more replies)
  0 siblings, 13 replies; 17+ messages in thread
From: Kevin Wolf @ 2016-09-21 12:55 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, mreitz, eblake, qemu-devel

This series makes the next step towards a QAPI interface that doesn't require
clients to know about BlockBackends. By removing the support for 'id' from
blockdev-add, it becomes a command that always only creates a BDS (with a node
name). Existing interfaces have already been changed to accept node names
everywhere and BlockBackends are created whenever they are needed.

The actual change is in the last patch and pretty trivial, but many test cases
use blockdev-add to create a BlockBackend and must be updated first.

v3:
- Patch 8: Fixed whitespace error
- Patch 9: Mention dropped cases in commit message
- Patch 10: New: Fix NULL use for %s in error messages
- Patch 11 (was 10): Update output after new patch 10
- Patch 12 (was 11):
  Remove 'id' from blockdev-add error message
  Remove spurious # characters in QAPI documentation

v2:
- Rebased on top of qmp-commands.hx removal

Kevin Wolf (12):
  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

 blockdev.c                 | 131 ++++++++--------------------
 docs/qmp-commands.txt      |  24 ++----
 qapi/block-core.json       |  30 ++-----
 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 |   6 +-
 tests/qemu-iotests/117     |   4 +-
 tests/qemu-iotests/118     |   6 +-
 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 +++---
 18 files changed, 324 insertions(+), 490 deletions(-)

-- 
1.8.3.1

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

* [Qemu-devel] [PATCH v3 01/12] qemu-iotests/041: Avoid blockdev-add with id
  2016-09-21 12:55 [Qemu-devel] [PATCH v3 00/12] block: Remove BB interface from blockdev-add/del Kevin Wolf
@ 2016-09-21 12:56 ` Kevin Wolf
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 02/12] qemu-iotests/067: " Kevin Wolf
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Kevin Wolf @ 2016-09-21 12:56 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, mreitz, eblake, qemu-devel

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

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

* [Qemu-devel] [PATCH v3 02/12] qemu-iotests/067: Avoid blockdev-add with id
  2016-09-21 12:55 [Qemu-devel] [PATCH v3 00/12] block: Remove BB interface from blockdev-add/del Kevin Wolf
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 01/12] qemu-iotests/041: Avoid blockdev-add with id Kevin Wolf
@ 2016-09-21 12:56 ` Kevin Wolf
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 03/12] qemu-iotests/071: " Kevin Wolf
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Kevin Wolf @ 2016-09-21 12:56 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, mreitz, eblake, qemu-devel

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

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

* [Qemu-devel] [PATCH v3 03/12] qemu-iotests/071: Avoid blockdev-add with id
  2016-09-21 12:55 [Qemu-devel] [PATCH v3 00/12] block: Remove BB interface from blockdev-add/del Kevin Wolf
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 01/12] qemu-iotests/041: Avoid blockdev-add with id Kevin Wolf
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 02/12] qemu-iotests/067: " Kevin Wolf
@ 2016-09-21 12:56 ` Kevin Wolf
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 04/12] qemu-iotests/081: " Kevin Wolf
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Kevin Wolf @ 2016-09-21 12:56 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, mreitz, eblake, qemu-devel

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

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

* [Qemu-devel] [PATCH v3 04/12] qemu-iotests/081: Avoid blockdev-add with id
  2016-09-21 12:55 [Qemu-devel] [PATCH v3 00/12] block: Remove BB interface from blockdev-add/del Kevin Wolf
                   ` (2 preceding siblings ...)
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 03/12] qemu-iotests/071: " Kevin Wolf
@ 2016-09-21 12:56 ` Kevin Wolf
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 05/12] qemu-iotests/087: " Kevin Wolf
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Kevin Wolf @ 2016-09-21 12:56 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, mreitz, eblake, qemu-devel

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

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

* [Qemu-devel] [PATCH v3 05/12] qemu-iotests/087: Avoid blockdev-add with id
  2016-09-21 12:55 [Qemu-devel] [PATCH v3 00/12] block: Remove BB interface from blockdev-add/del Kevin Wolf
                   ` (3 preceding siblings ...)
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 04/12] qemu-iotests/081: " Kevin Wolf
@ 2016-09-21 12:56 ` Kevin Wolf
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 06/12] qemu-iotests/117: " Kevin Wolf
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Kevin Wolf @ 2016-09-21 12:56 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, mreitz, eblake, qemu-devel

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

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

* [Qemu-devel] [PATCH v3 06/12] qemu-iotests/117: Avoid blockdev-add with id
  2016-09-21 12:55 [Qemu-devel] [PATCH v3 00/12] block: Remove BB interface from blockdev-add/del Kevin Wolf
                   ` (4 preceding siblings ...)
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 05/12] qemu-iotests/087: " Kevin Wolf
@ 2016-09-21 12:56 ` Kevin Wolf
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 07/12] qemu-iotests/118: " Kevin Wolf
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Kevin Wolf @ 2016-09-21 12:56 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, mreitz, eblake, qemu-devel

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

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

* [Qemu-devel] [PATCH v3 07/12] qemu-iotests/118: Avoid blockdev-add with id
  2016-09-21 12:55 [Qemu-devel] [PATCH v3 00/12] block: Remove BB interface from blockdev-add/del Kevin Wolf
                   ` (5 preceding siblings ...)
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 06/12] qemu-iotests/117: " Kevin Wolf
@ 2016-09-21 12:56 ` Kevin Wolf
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 08/12] qemu-iotests/124: " Kevin Wolf
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Kevin Wolf @ 2016-09-21 12:56 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, mreitz, eblake, qemu-devel

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

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

* [Qemu-devel] [PATCH v3 08/12] qemu-iotests/124: Avoid blockdev-add with id
  2016-09-21 12:55 [Qemu-devel] [PATCH v3 00/12] block: Remove BB interface from blockdev-add/del Kevin Wolf
                   ` (6 preceding siblings ...)
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 07/12] qemu-iotests/118: " Kevin Wolf
@ 2016-09-21 12:56 ` Kevin Wolf
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 09/12] qemu-iotests/139: " Kevin Wolf
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Kevin Wolf @ 2016-09-21 12:56 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, mreitz, eblake, qemu-devel

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

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

* [Qemu-devel] [PATCH v3 09/12] qemu-iotests/139: Avoid blockdev-add with id
  2016-09-21 12:55 [Qemu-devel] [PATCH v3 00/12] block: Remove BB interface from blockdev-add/del Kevin Wolf
                   ` (7 preceding siblings ...)
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 08/12] qemu-iotests/124: " Kevin Wolf
@ 2016-09-21 12:56 ` Kevin Wolf
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 10/12] block: Avoid printing NULL string in error messages Kevin Wolf
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 17+ messages in thread
From: Kevin Wolf @ 2016-09-21 12:56 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, mreitz, eblake, qemu-devel

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

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

* [Qemu-devel] [PATCH v3 10/12] block: Avoid printing NULL string in error messages
  2016-09-21 12:55 [Qemu-devel] [PATCH v3 00/12] block: Remove BB interface from blockdev-add/del Kevin Wolf
                   ` (8 preceding siblings ...)
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 09/12] qemu-iotests/139: " Kevin Wolf
@ 2016-09-21 12:56 ` Kevin Wolf
  2016-09-21 14:16   ` Eric Blake
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 11/12] qemu-iotests/141: Avoid blockdev-add with id Kevin Wolf
                   ` (2 subsequent siblings)
  12 siblings, 1 reply; 17+ messages in thread
From: Kevin Wolf @ 2016-09-21 12:56 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, mreitz, eblake, qemu-devel

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>
---
 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

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

* [Qemu-devel] [PATCH v3 11/12] qemu-iotests/141: Avoid blockdev-add with id
  2016-09-21 12:55 [Qemu-devel] [PATCH v3 00/12] block: Remove BB interface from blockdev-add/del Kevin Wolf
                   ` (9 preceding siblings ...)
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 10/12] block: Avoid printing NULL string in error messages Kevin Wolf
@ 2016-09-21 12:56 ` Kevin Wolf
  2016-09-21 14:17   ` Eric Blake
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 12/12] block: Remove BB interface from blockdev-add/del Kevin Wolf
  2016-09-21 14:48 ` [Qemu-devel] [PATCH v3 00/12] " Kevin Wolf
  12 siblings, 1 reply; 17+ messages in thread
From: Kevin Wolf @ 2016-09-21 12:56 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, mreitz, eblake, qemu-devel

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>
---
 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

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

* [Qemu-devel] [PATCH v3 12/12] block: Remove BB interface from blockdev-add/del
  2016-09-21 12:55 [Qemu-devel] [PATCH v3 00/12] block: Remove BB interface from blockdev-add/del Kevin Wolf
                   ` (10 preceding siblings ...)
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 11/12] qemu-iotests/141: Avoid blockdev-add with id Kevin Wolf
@ 2016-09-21 12:56 ` Kevin Wolf
  2016-09-21 14:20   ` Eric Blake
  2016-09-21 14:48 ` [Qemu-devel] [PATCH v3 00/12] " Kevin Wolf
  12 siblings, 1 reply; 17+ messages in thread
From: Kevin Wolf @ 2016-09-21 12:56 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, mreitz, eblake, qemu-devel

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>
---
 blockdev.c            | 124 ++++++++++++++------------------------------------
 docs/qmp-commands.txt |  24 +++-------
 qapi/block-core.json  |  30 +++---------
 3 files changed, 47 insertions(+), 131 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:
-- 
1.8.3.1

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

* Re: [Qemu-devel] [PATCH v3 10/12] block: Avoid printing NULL string in error messages
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 10/12] block: Avoid printing NULL string in error messages Kevin Wolf
@ 2016-09-21 14:16   ` Eric Blake
  0 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2016-09-21 14:16 UTC (permalink / raw)
  To: Kevin Wolf, qemu-block; +Cc: mreitz, qemu-devel

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

On 09/21/2016 07:56 AM, Kevin Wolf wrote:
> 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>
> ---
>  blockdev.c                 | 9 +++------
>  tests/qemu-iotests/085.out | 6 +++---
>  2 files changed, 6 insertions(+), 9 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

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


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

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

* Re: [Qemu-devel] [PATCH v3 11/12] qemu-iotests/141: Avoid blockdev-add with id
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 11/12] qemu-iotests/141: Avoid blockdev-add with id Kevin Wolf
@ 2016-09-21 14:17   ` Eric Blake
  0 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2016-09-21 14:17 UTC (permalink / raw)
  To: Kevin Wolf, qemu-block; +Cc: mreitz, qemu-devel

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

On 09/21/2016 07:56 AM, Kevin Wolf wrote:
> 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>
> ---
>  tests/qemu-iotests/141     | 24 ++++++++++++++----------
>  tests/qemu-iotests/141.out | 24 ++++++++++++------------
>  2 files changed, 26 insertions(+), 22 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

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


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

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

* Re: [Qemu-devel] [PATCH v3 12/12] block: Remove BB interface from blockdev-add/del
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 12/12] block: Remove BB interface from blockdev-add/del Kevin Wolf
@ 2016-09-21 14:20   ` Eric Blake
  0 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2016-09-21 14:20 UTC (permalink / raw)
  To: Kevin Wolf, qemu-block; +Cc: mreitz, qemu-devel

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

On 09/21/2016 07:56 AM, Kevin Wolf wrote:
> 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>
> ---
>  blockdev.c            | 124 ++++++++++++++------------------------------------
>  docs/qmp-commands.txt |  24 +++-------
>  qapi/block-core.json  |  30 +++---------
>  3 files changed, 47 insertions(+), 131 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

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


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

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

* Re: [Qemu-devel] [PATCH v3 00/12] block: Remove BB interface from blockdev-add/del
  2016-09-21 12:55 [Qemu-devel] [PATCH v3 00/12] block: Remove BB interface from blockdev-add/del Kevin Wolf
                   ` (11 preceding siblings ...)
  2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 12/12] block: Remove BB interface from blockdev-add/del Kevin Wolf
@ 2016-09-21 14:48 ` Kevin Wolf
  12 siblings, 0 replies; 17+ messages in thread
From: Kevin Wolf @ 2016-09-21 14:48 UTC (permalink / raw)
  To: qemu-block; +Cc: mreitz, eblake, qemu-devel

Am 21.09.2016 um 14:55 hat Kevin Wolf geschrieben:
> This series makes the next step towards a QAPI interface that doesn't require
> clients to know about BlockBackends. By removing the support for 'id' from
> blockdev-add, it becomes a command that always only creates a BDS (with a node
> name). Existing interfaces have already been changed to accept node names
> everywhere and BlockBackends are created whenever they are needed.
> 
> The actual change is in the last patch and pretty trivial, but many test cases
> use blockdev-add to create a BlockBackend and must be updated first.

Applied to the block branch.

Kevin

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

end of thread, other threads:[~2016-09-21 14:50 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-21 12:55 [Qemu-devel] [PATCH v3 00/12] block: Remove BB interface from blockdev-add/del Kevin Wolf
2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 01/12] qemu-iotests/041: Avoid blockdev-add with id Kevin Wolf
2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 02/12] qemu-iotests/067: " Kevin Wolf
2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 03/12] qemu-iotests/071: " Kevin Wolf
2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 04/12] qemu-iotests/081: " Kevin Wolf
2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 05/12] qemu-iotests/087: " Kevin Wolf
2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 06/12] qemu-iotests/117: " Kevin Wolf
2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 07/12] qemu-iotests/118: " Kevin Wolf
2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 08/12] qemu-iotests/124: " Kevin Wolf
2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 09/12] qemu-iotests/139: " Kevin Wolf
2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 10/12] block: Avoid printing NULL string in error messages Kevin Wolf
2016-09-21 14:16   ` Eric Blake
2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 11/12] qemu-iotests/141: Avoid blockdev-add with id Kevin Wolf
2016-09-21 14:17   ` Eric Blake
2016-09-21 12:56 ` [Qemu-devel] [PATCH v3 12/12] block: Remove BB interface from blockdev-add/del Kevin Wolf
2016-09-21 14:20   ` Eric Blake
2016-09-21 14:48 ` [Qemu-devel] [PATCH v3 00/12] " Kevin Wolf

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