* [Qemu-devel] [PATCH v2 01/11] qemu-iotests/041: Avoid blockdev-add with id
2016-09-20 13:03 [Qemu-devel] [PATCH v2 00/11] block: Remove BB interface from blockdev-add/del Kevin Wolf
@ 2016-09-20 13:03 ` Kevin Wolf
2016-09-20 20:22 ` Eric Blake
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 02/11] qemu-iotests/067: " Kevin Wolf
` (9 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Kevin Wolf @ 2016-09-20 13:03 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, eblake, mreitz, 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/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] 23+ messages in thread* [Qemu-devel] [PATCH v2 02/11] qemu-iotests/067: Avoid blockdev-add with id
2016-09-20 13:03 [Qemu-devel] [PATCH v2 00/11] block: Remove BB interface from blockdev-add/del Kevin Wolf
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 01/11] qemu-iotests/041: Avoid blockdev-add with id Kevin Wolf
@ 2016-09-20 13:03 ` Kevin Wolf
2016-09-20 20:25 ` Eric Blake
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 03/11] qemu-iotests/071: " Kevin Wolf
` (8 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Kevin Wolf @ 2016-09-20 13:03 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, eblake, mreitz, 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>
---
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] 23+ messages in thread* Re: [Qemu-devel] [PATCH v2 02/11] qemu-iotests/067: Avoid blockdev-add with id
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 02/11] qemu-iotests/067: " Kevin Wolf
@ 2016-09-20 20:25 ` Eric Blake
0 siblings, 0 replies; 23+ messages in thread
From: Eric Blake @ 2016-09-20 20:25 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: mreitz, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 847 bytes --]
On 09/20/2016 08:03 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.
>
> 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>
> ---
> tests/qemu-iotests/067 | 6 +-
> tests/qemu-iotests/067.out | 211 +++++++++++++++++++++++++++------------------
> 2 files changed, 131 insertions(+), 86 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] 23+ messages in thread
* [Qemu-devel] [PATCH v2 03/11] qemu-iotests/071: Avoid blockdev-add with id
2016-09-20 13:03 [Qemu-devel] [PATCH v2 00/11] block: Remove BB interface from blockdev-add/del Kevin Wolf
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 01/11] qemu-iotests/041: Avoid blockdev-add with id Kevin Wolf
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 02/11] qemu-iotests/067: " Kevin Wolf
@ 2016-09-20 13:03 ` Kevin Wolf
2016-09-20 20:26 ` Eric Blake
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 04/11] qemu-iotests/081: " Kevin Wolf
` (7 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Kevin Wolf @ 2016-09-20 13:03 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, eblake, mreitz, 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/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] 23+ messages in thread* [Qemu-devel] [PATCH v2 04/11] qemu-iotests/081: Avoid blockdev-add with id
2016-09-20 13:03 [Qemu-devel] [PATCH v2 00/11] block: Remove BB interface from blockdev-add/del Kevin Wolf
` (2 preceding siblings ...)
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 03/11] qemu-iotests/071: " Kevin Wolf
@ 2016-09-20 13:03 ` Kevin Wolf
2016-09-20 20:28 ` Eric Blake
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 05/11] qemu-iotests/087: " Kevin Wolf
` (6 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Kevin Wolf @ 2016-09-20 13:03 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, eblake, mreitz, 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/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] 23+ messages in thread* Re: [Qemu-devel] [PATCH v2 04/11] qemu-iotests/081: Avoid blockdev-add with id
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 04/11] qemu-iotests/081: " Kevin Wolf
@ 2016-09-20 20:28 ` Eric Blake
0 siblings, 0 replies; 23+ messages in thread
From: Eric Blake @ 2016-09-20 20:28 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: mreitz, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 939 bytes --]
On 09/20/2016 08:03 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/081 | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
> 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": [
> {
>
--
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] 23+ messages in thread
* [Qemu-devel] [PATCH v2 05/11] qemu-iotests/087: Avoid blockdev-add with id
2016-09-20 13:03 [Qemu-devel] [PATCH v2 00/11] block: Remove BB interface from blockdev-add/del Kevin Wolf
` (3 preceding siblings ...)
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 04/11] qemu-iotests/081: " Kevin Wolf
@ 2016-09-20 13:03 ` Kevin Wolf
2016-09-20 20:33 ` Eric Blake
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 06/11] qemu-iotests/117: " Kevin Wolf
` (5 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Kevin Wolf @ 2016-09-20 13:03 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, eblake, mreitz, 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>
---
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] 23+ messages in thread* [Qemu-devel] [PATCH v2 06/11] qemu-iotests/117: Avoid blockdev-add with id
2016-09-20 13:03 [Qemu-devel] [PATCH v2 00/11] block: Remove BB interface from blockdev-add/del Kevin Wolf
` (4 preceding siblings ...)
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 05/11] qemu-iotests/087: " Kevin Wolf
@ 2016-09-20 13:03 ` Kevin Wolf
2016-09-20 21:03 ` Eric Blake
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 07/11] qemu-iotests/118: " Kevin Wolf
` (4 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Kevin Wolf @ 2016-09-20 13:03 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, eblake, mreitz, 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/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] 23+ messages in thread* [Qemu-devel] [PATCH v2 07/11] qemu-iotests/118: Avoid blockdev-add with id
2016-09-20 13:03 [Qemu-devel] [PATCH v2 00/11] block: Remove BB interface from blockdev-add/del Kevin Wolf
` (5 preceding siblings ...)
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 06/11] qemu-iotests/117: " Kevin Wolf
@ 2016-09-20 13:03 ` Kevin Wolf
2016-09-20 21:20 ` Eric Blake
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 08/11] qemu-iotests/124: " Kevin Wolf
` (3 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Kevin Wolf @ 2016-09-20 13:03 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, eblake, mreitz, 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/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] 23+ messages in thread* [Qemu-devel] [PATCH v2 08/11] qemu-iotests/124: Avoid blockdev-add with id
2016-09-20 13:03 [Qemu-devel] [PATCH v2 00/11] block: Remove BB interface from blockdev-add/del Kevin Wolf
` (6 preceding siblings ...)
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 07/11] qemu-iotests/118: " Kevin Wolf
@ 2016-09-20 13:03 ` Kevin Wolf
2016-09-20 21:27 ` Eric Blake
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 09/11] qemu-iotests/139: " Kevin Wolf
` (2 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Kevin Wolf @ 2016-09-20 13:03 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, eblake, mreitz, 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/124 | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index de7cdbe..33525bf 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] 23+ messages in thread* Re: [Qemu-devel] [PATCH v2 08/11] qemu-iotests/124: Avoid blockdev-add with id
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 08/11] qemu-iotests/124: " Kevin Wolf
@ 2016-09-20 21:27 ` Eric Blake
0 siblings, 0 replies; 23+ messages in thread
From: Eric Blake @ 2016-09-20 21:27 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: mreitz, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1160 bytes --]
On 09/20/2016 08:03 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/124 | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
> index de7cdbe..33525bf 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,
Umm, why the whitespace after =?
pep8 already complains about this (and other) pythonic iotests, but no
need to make it noisier :)
With that fixed,
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] 23+ messages in thread
* [Qemu-devel] [PATCH v2 09/11] qemu-iotests/139: Avoid blockdev-add with id
2016-09-20 13:03 [Qemu-devel] [PATCH v2 00/11] block: Remove BB interface from blockdev-add/del Kevin Wolf
` (7 preceding siblings ...)
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 08/11] qemu-iotests/124: " Kevin Wolf
@ 2016-09-20 13:03 ` Kevin Wolf
2016-09-20 21:33 ` Eric Blake
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 10/11] qemu-iotests/141: " Kevin Wolf
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 11/11] block: Remove BB interface from blockdev-add/del Kevin Wolf
10 siblings, 1 reply; 23+ messages in thread
From: Kevin Wolf @ 2016-09-20 13:03 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, eblake, mreitz, 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/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] 23+ messages in thread* Re: [Qemu-devel] [PATCH v2 09/11] qemu-iotests/139: Avoid blockdev-add with id
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 09/11] qemu-iotests/139: " Kevin Wolf
@ 2016-09-20 21:33 ` Eric Blake
0 siblings, 0 replies; 23+ messages in thread
From: Eric Blake @ 2016-09-20 21:33 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: mreitz, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 871 bytes --]
On 09/20/2016 08:03 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/139 | 178 +++++++++++++++------------------------------
> tests/qemu-iotests/139.out | 4 +-
> 2 files changed, 59 insertions(+), 123 deletions(-)
>
> @@ -1,5 +1,5 @@
> -............
> +.........
> ----------------------------------------------------------------------
> -Ran 12 tests
> +Ran 9 tests
Might be worth expanding the commit message to mention that deleted test
cases are intentional (similar to the longer message in 2/11).
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] 23+ messages in thread
* [Qemu-devel] [PATCH v2 10/11] qemu-iotests/141: Avoid blockdev-add with id
2016-09-20 13:03 [Qemu-devel] [PATCH v2 00/11] block: Remove BB interface from blockdev-add/del Kevin Wolf
` (8 preceding siblings ...)
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 09/11] qemu-iotests/139: " Kevin Wolf
@ 2016-09-20 13:03 ` Kevin Wolf
2016-09-20 21:35 ` Eric Blake
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 11/11] block: Remove BB interface from blockdev-add/del Kevin Wolf
10 siblings, 1 reply; 23+ messages in thread
From: Kevin Wolf @ 2016-09-20 13:03 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, eblake, mreitz, 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..da9e841 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 by (null)"}}
{"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 by (null)"}}
{"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 by (null)"}}
{"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 by (null)"}}
{"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 by (null)"}}
{"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] 23+ messages in thread* Re: [Qemu-devel] [PATCH v2 10/11] qemu-iotests/141: Avoid blockdev-add with id
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 10/11] qemu-iotests/141: " Kevin Wolf
@ 2016-09-20 21:35 ` Eric Blake
0 siblings, 0 replies; 23+ messages in thread
From: Eric Blake @ 2016-09-20 21:35 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: mreitz, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1246 bytes --]
On 09/20/2016 08:03 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(-)
>
> +++ 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 by (null)"}}
Are we calling printf("%s",NULL) (not portable) or
printf("%s","(null)")? At any rate, is the poorer error message quality
intentional? I'd feel more comfortable if we knew why the error message
regressed, before giving R-b here.
--
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] 23+ messages in thread
* [Qemu-devel] [PATCH v2 11/11] block: Remove BB interface from blockdev-add/del
2016-09-20 13:03 [Qemu-devel] [PATCH v2 00/11] block: Remove BB interface from blockdev-add/del Kevin Wolf
` (9 preceding siblings ...)
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 10/11] qemu-iotests/141: " Kevin Wolf
@ 2016-09-20 13:03 ` Kevin Wolf
2016-09-20 21:59 ` Eric Blake
10 siblings, 1 reply; 23+ messages in thread
From: Kevin Wolf @ 2016-09-20 13:03 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, eblake, mreitz, 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 | 127 ++++++++++++++------------------------------------
docs/qmp-commands.txt | 24 +++-------
qapi/block-core.json | 30 +++---------
3 files changed, 49 insertions(+), 132 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 405145a..a51434c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2846,7 +2846,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);
}
@@ -3829,7 +3829,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;
@@ -3861,37 +3860,22 @@ 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, "'id' and/or 'node-name' need to 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;
}
@@ -3900,82 +3884,43 @@ 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 by %s",
- node_name, bdrv_get_parent_name(bs));
- return;
- }
- aio_context = bdrv_get_aio_context(bs);
+ if (bdrv_has_blk(bs)) {
+ error_setg(errp, "Node %s is in use by %s",
+ node_name, bdrv_get_parent_name(bs));
+ 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..860251a 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..46661e6 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] 23+ messages in thread* Re: [Qemu-devel] [PATCH v2 11/11] block: Remove BB interface from blockdev-add/del
2016-09-20 13:03 ` [Qemu-devel] [PATCH v2 11/11] block: Remove BB interface from blockdev-add/del Kevin Wolf
@ 2016-09-20 21:59 ` Eric Blake
0 siblings, 0 replies; 23+ messages in thread
From: Eric Blake @ 2016-09-20 21:59 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: mreitz, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 7585 bytes --]
On 09/20/2016 08:03 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.
How close are we to promoting x-blockdev-del out of the x- namespace? Do
we "just" need the remaining blockdev devices exposed in QMP?
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> blockdev.c | 127 ++++++++++++++------------------------------------
> docs/qmp-commands.txt | 24 +++-------
> qapi/block-core.json | 30 +++---------
> 3 files changed, 49 insertions(+), 132 deletions(-)
>
> void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
> {
> BlockDriverState *bs;
> - BlockBackend *blk = NULL;
> QObject *obj;
> Visitor *v = qmp_output_visitor_new(&obj);
> QDict *qdict;
> @@ -3861,37 +3860,22 @@ 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, "'id' and/or 'node-name' need to be specified for "
> + "the root node");
Umm, 'id' can't be specified, after this change :)
> -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 by %s",
> - node_name, bdrv_get_parent_name(bs));
> - return;
> - }
> - aio_context = bdrv_get_aio_context(bs);
> + if (bdrv_has_blk(bs)) {
> + error_setg(errp, "Node %s is in use by %s",
> + node_name, bdrv_get_parent_name(bs));
Is this the source of the "(null)" I was asking about in 10/11?
> +++ 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
Spurious #.
> +++ 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.
Removing an option. Not backward-compatible, but we've been warning
users that blockdev-add is not ready for primetime yet, so I'm okay with
it (and at least introspection shows it).
> -# @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.
Spurious # (copy-and-paste strikes again)
Overall, I like the result, but I think it needs just a bit of polish
before I give R-b.
--
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] 23+ messages in thread