* [Qemu-devel] [PATCH] block: Remove "options" indirection from blockdev-add
@ 2016-10-07 15:38 Kevin Wolf
2016-10-07 16:16 ` Max Reitz
2016-10-07 17:18 ` Eric Blake
0 siblings, 2 replies; 3+ messages in thread
From: Kevin Wolf @ 2016-10-07 15:38 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, eblake, armbru, qemu-devel
Now that QAPI supports boxed types, we can have unions at the top level
of a command, so let's put our real options directly there for
blockdev-add instead of having a single "options" dict that contains the
real arguments.
blockdev-add is still experimental and we already made substantial
changes to the API recently, so we're free to make changes like this
one, too.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
Yes, that's right. Ignoring the test cases, this is a one-liner in
the schema without any C code changes. :-)
qapi/block-core.json | 2 +-
tests/qemu-iotests/041 | 11 +++--
tests/qemu-iotests/067 | 12 +++--
tests/qemu-iotests/071 | 118 +++++++++++++++++++++----------------------------
tests/qemu-iotests/081 | 52 ++++++++++------------
tests/qemu-iotests/085 | 9 ++--
tests/qemu-iotests/087 | 76 +++++++++++++------------------
tests/qemu-iotests/117 | 12 ++---
tests/qemu-iotests/118 | 42 +++++++++---------
tests/qemu-iotests/124 | 20 ++++-----
tests/qemu-iotests/139 | 10 ++---
tests/qemu-iotests/141 | 13 +++---
tests/qemu-iotests/155 | 10 ++---
13 files changed, 174 insertions(+), 213 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 3d3c0be..2877fcf 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2321,7 +2321,7 @@
#
# Since: 1.7
##
-{ 'command': 'blockdev-add', 'data': { 'options': 'BlockdevOptions' } }
+{ 'command': 'blockdev-add', 'data': 'BlockdevOptions', 'boxed': true }
##
# @x-blockdev-del:
diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index d1e1ad8..30e628f 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -194,10 +194,9 @@ class TestSingleBlockdev(TestSingleDrive):
def setUp(self):
TestSingleDrive.setUp(self)
qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, target_img)
- args = {'options':
- {'driver': iotests.imgfmt,
- 'node-name': self.qmp_target,
- 'file': { 'filename': target_img, 'driver': 'file' } } }
+ args = {'driver': iotests.imgfmt,
+ 'node-name': self.qmp_target,
+ 'file': { 'filename': target_img, 'driver': 'file' } }
result = self.vm.qmp("blockdev-add", **args)
self.assert_qmp(result, 'return', {})
@@ -782,8 +781,8 @@ class TestRepairQuorum(iotests.QMPTestCase):
self.vm.launch()
#assemble the quorum block device from the individual files
- args = { "options" : { "driver": "quorum", "node-name": "quorum0",
- "vote-threshold": 2, "children": [ "img0", "img1", "img2" ] } }
+ args = { "driver": "quorum", "node-name": "quorum0",
+ "vote-threshold": 2, "children": [ "img0", "img1", "img2" ] }
if self.has_quorum():
result = self.vm.qmp("blockdev-add", **args)
self.assert_qmp(result, 'return', {})
diff --git a/tests/qemu-iotests/067 b/tests/qemu-iotests/067
index a12125b..38d23fc 100755
--- a/tests/qemu-iotests/067
+++ b/tests/qemu-iotests/067
@@ -119,13 +119,11 @@ run_qemu <<EOF
{ "execute": "qmp_capabilities" }
{ "execute": "blockdev-add",
"arguments": {
- "options": {
- "driver": "$IMGFMT",
- "node-name": "disk",
- "file": {
- "driver": "file",
- "filename": "$TEST_IMG"
- }
+ "driver": "$IMGFMT",
+ "node-name": "disk",
+ "file": {
+ "driver": "file",
+ "filename": "$TEST_IMG"
}
}
}
diff --git a/tests/qemu-iotests/071 b/tests/qemu-iotests/071
index 6d0864c..48b4955 100755
--- a/tests/qemu-iotests/071
+++ b/tests/qemu-iotests/071
@@ -107,25 +107,21 @@ run_qemu <<EOF
{ "execute": "qmp_capabilities" }
{ "execute": "blockdev-add",
"arguments": {
- "options": {
- "node-name": "drive0",
- "driver": "file",
- "filename": "$TEST_IMG"
- }
+ "node-name": "drive0",
+ "driver": "file",
+ "filename": "$TEST_IMG"
}
}
{ "execute": "blockdev-add",
"arguments": {
- "options": {
- "driver": "$IMGFMT",
- "node-name": "drive0-debug",
- "file": {
- "driver": "blkdebug",
- "image": "drive0",
- "inject-error": [{
- "event": "l2_load"
- }]
- }
+ "driver": "$IMGFMT",
+ "node-name": "drive0-debug",
+ "file": {
+ "driver": "blkdebug",
+ "image": "drive0",
+ "inject-error": [{
+ "event": "l2_load"
+ }]
}
}
}
@@ -145,26 +141,22 @@ run_qemu <<EOF
{ "execute": "qmp_capabilities" }
{ "execute": "blockdev-add",
"arguments": {
- "options": {
- "node-name": "drive0",
- "driver": "$IMGFMT",
- "file": {
- "driver": "file",
- "filename": "$TEST_IMG"
- }
+ "node-name": "drive0",
+ "driver": "$IMGFMT",
+ "file": {
+ "driver": "file",
+ "filename": "$TEST_IMG"
}
}
}
{ "execute": "blockdev-add",
"arguments": {
- "options": {
- "driver": "blkverify",
- "node-name": "drive0-verify",
- "test": "drive0",
- "raw": {
- "driver": "file",
- "filename": "$TEST_IMG.base"
- }
+ "driver": "blkverify",
+ "node-name": "drive0-verify",
+ "test": "drive0",
+ "raw": {
+ "driver": "file",
+ "filename": "$TEST_IMG.base"
}
}
}
@@ -184,27 +176,23 @@ run_qemu <<EOF
{ "execute": "qmp_capabilities" }
{ "execute": "blockdev-add",
"arguments": {
- "options": {
- "node-name": "drive0",
- "driver": "file",
- "filename": "$TEST_IMG.base"
- }
+ "node-name": "drive0",
+ "driver": "file",
+ "filename": "$TEST_IMG.base"
}
}
{ "execute": "blockdev-add",
"arguments": {
- "options": {
- "driver": "blkverify",
- "node-name": "drive0-verify",
- "test": {
- "driver": "$IMGFMT",
- "file": {
- "driver": "file",
- "filename": "$TEST_IMG"
- }
- },
- "raw": "drive0"
- }
+ "driver": "blkverify",
+ "node-name": "drive0-verify",
+ "test": {
+ "driver": "$IMGFMT",
+ "file": {
+ "driver": "file",
+ "filename": "$TEST_IMG"
+ }
+ },
+ "raw": "drive0"
}
}
{ "execute": "human-monitor-command",
@@ -223,30 +211,26 @@ run_qemu <<EOF
{ "execute": "qmp_capabilities" }
{ "execute": "blockdev-add",
"arguments": {
- "options": {
- "node-name": "drive0",
- "driver": "file",
- "filename": "$TEST_IMG"
- }
+ "node-name": "drive0",
+ "driver": "file",
+ "filename": "$TEST_IMG"
}
}
{ "execute": "blockdev-add",
"arguments": {
- "options": {
- "driver": "$IMGFMT",
- "node-name": "drive0-debug",
- "file": {
- "driver": "blkdebug",
- "image": "drive0",
- "inject-error": [{
- "event": "read_aio",
- "state": 42
- }],
- "set-state": [{
- "event": "write_aio",
- "new_state": 42
- }]
- }
+ "driver": "$IMGFMT",
+ "node-name": "drive0-debug",
+ "file": {
+ "driver": "blkdebug",
+ "image": "drive0",
+ "inject-error": [{
+ "event": "read_aio",
+ "state": 42
+ }],
+ "set-state": [{
+ "event": "write_aio",
+ "new_state": 42
+ }]
}
}
}
diff --git a/tests/qemu-iotests/081 b/tests/qemu-iotests/081
index 0a809f3..da3fb09 100755
--- a/tests/qemu-iotests/081
+++ b/tests/qemu-iotests/081
@@ -105,40 +105,36 @@ run_qemu <<EOF
{ "execute": "qmp_capabilities" }
{ "execute": "blockdev-add",
"arguments": {
- "options": {
- "node-name": "drive2",
- "driver": "$IMGFMT",
- "file": {
- "driver": "file",
- "filename": "$TEST_DIR/2.raw"
- }
+ "node-name": "drive2",
+ "driver": "$IMGFMT",
+ "file": {
+ "driver": "file",
+ "filename": "$TEST_DIR/2.raw"
}
}
}
{ "execute": "blockdev-add",
"arguments": {
- "options": {
- "driver": "quorum",
- "node-name": "drive0-quorum",
- "vote-threshold": 2,
- "children": [
- {
- "driver": "$IMGFMT",
- "file": {
- "driver": "file",
- "filename": "$TEST_DIR/1.raw"
- }
- },
- "drive2",
- {
- "driver": "$IMGFMT",
- "file": {
- "driver": "file",
- "filename": "$TEST_DIR/3.raw"
- }
+ "driver": "quorum",
+ "node-name": "drive0-quorum",
+ "vote-threshold": 2,
+ "children": [
+ {
+ "driver": "$IMGFMT",
+ "file": {
+ "driver": "file",
+ "filename": "$TEST_DIR/1.raw"
}
- ]
- }
+ },
+ "drive2",
+ {
+ "driver": "$IMGFMT",
+ "file": {
+ "driver": "file",
+ "filename": "$TEST_DIR/3.raw"
+ }
+ }
+ ]
}
}
{ "execute": "human-monitor-command",
diff --git a/tests/qemu-iotests/085 b/tests/qemu-iotests/085
index aa77eca..c53e97f 100755
--- a/tests/qemu-iotests/085
+++ b/tests/qemu-iotests/085
@@ -100,11 +100,10 @@ function add_snapshot_image()
_make_test_img -b "${base_image}" "$size"
mv "${TEST_IMG}" "${snapshot_file}"
cmd="{ 'execute': 'blockdev-add', 'arguments':
- { 'options':
- { 'driver': 'qcow2', 'node-name': 'snap_${1}', ${extra_params}
- 'file':
- { 'driver': 'file', 'filename': '${snapshot_file}',
- 'node-name': 'file_${1}' } } } }"
+ { 'driver': 'qcow2', 'node-name': 'snap_${1}', ${extra_params}
+ 'file':
+ { 'driver': 'file', 'filename': '${snapshot_file}',
+ 'node-name': 'file_${1}' } } }"
_send_qemu_cmd $h "${cmd}" "return"
}
diff --git a/tests/qemu-iotests/087 b/tests/qemu-iotests/087
index b1ac71f..9de57dd 100755
--- a/tests/qemu-iotests/087
+++ b/tests/qemu-iotests/087
@@ -61,12 +61,10 @@ run_qemu <<EOF
{ "execute": "qmp_capabilities" }
{ "execute": "blockdev-add",
"arguments": {
- "options": {
- "driver": "$IMGFMT",
- "file": {
- "driver": "file",
- "filename": "$TEST_IMG"
- }
+ "driver": "$IMGFMT",
+ "file": {
+ "driver": "file",
+ "filename": "$TEST_IMG"
}
}
}
@@ -81,25 +79,21 @@ run_qemu -drive driver=$IMGFMT,id=disk,node-name=test-node,file="$TEST_IMG" <<EO
{ "execute": "qmp_capabilities" }
{ "execute": "blockdev-add",
"arguments": {
- "options": {
- "driver": "$IMGFMT",
- "node-name": "disk",
- "file": {
- "driver": "file",
- "filename": "$TEST_IMG"
- }
+ "driver": "$IMGFMT",
+ "node-name": "disk",
+ "file": {
+ "driver": "file",
+ "filename": "$TEST_IMG"
}
}
}
{ "execute": "blockdev-add",
"arguments": {
- "options": {
- "driver": "$IMGFMT",
- "node-name": "test-node",
- "file": {
- "driver": "file",
- "filename": "$TEST_IMG"
- }
+ "driver": "$IMGFMT",
+ "node-name": "test-node",
+ "file": {
+ "driver": "file",
+ "filename": "$TEST_IMG"
}
}
}
@@ -114,14 +108,12 @@ run_qemu <<EOF
{ "execute": "qmp_capabilities" }
{ "execute": "blockdev-add",
"arguments": {
- "options": {
- "driver": "$IMGFMT",
- "node-name": "disk",
- "file": {
- "driver": "file",
- "filename": "$TEST_IMG",
- "aio": "native"
- }
+ "driver": "$IMGFMT",
+ "node-name": "disk",
+ "file": {
+ "driver": "file",
+ "filename": "$TEST_IMG",
+ "aio": "native"
}
}
}
@@ -137,13 +129,11 @@ run_qemu -S <<EOF
{ "execute": "qmp_capabilities" }
{ "execute": "blockdev-add",
"arguments": {
- "options": {
- "driver": "$IMGFMT",
- "node-name": "disk",
- "file": {
- "driver": "file",
- "filename": "$TEST_IMG"
- }
+ "driver": "$IMGFMT",
+ "node-name": "disk",
+ "file": {
+ "driver": "file",
+ "filename": "$TEST_IMG"
}
}
}
@@ -154,13 +144,11 @@ run_qemu <<EOF
{ "execute": "qmp_capabilities" }
{ "execute": "blockdev-add",
"arguments": {
- "options": {
- "driver": "$IMGFMT",
- "node-name": "disk",
- "file": {
- "driver": "file",
- "filename": "$TEST_IMG"
- }
+ "driver": "$IMGFMT",
+ "node-name": "disk",
+ "file": {
+ "driver": "file",
+ "filename": "$TEST_IMG"
}
}
}
@@ -176,9 +164,7 @@ run_qemu -S <<EOF
{ "execute": "qmp_capabilities" }
{ "execute": "blockdev-add",
"arguments": {
- "options": {
- "node-name": "disk"
- }
+ "node-name": "disk"
}
}
{ "execute": "quit" }
diff --git a/tests/qemu-iotests/117 b/tests/qemu-iotests/117
index 5b28039..e955d52 100755
--- a/tests/qemu-iotests/117
+++ b/tests/qemu-iotests/117
@@ -52,16 +52,16 @@ _send_qemu_cmd $QEMU_HANDLE \
_send_qemu_cmd $QEMU_HANDLE \
"{ 'execute': 'blockdev-add',
- 'arguments': { 'options': { 'node-name': 'protocol',
- 'driver': 'file',
- 'filename': '$TEST_IMG' } } }" \
+ 'arguments': { 'node-name': 'protocol',
+ 'driver': 'file',
+ 'filename': '$TEST_IMG' } }" \
'return'
_send_qemu_cmd $QEMU_HANDLE \
"{ 'execute': 'blockdev-add',
- 'arguments': { 'options': { 'node-name': 'format',
- 'driver': '$IMGFMT',
- 'file': 'protocol' } } }" \
+ 'arguments': { 'node-name': 'format',
+ 'driver': '$IMGFMT',
+ 'file': 'protocol' } }" \
'return'
_send_qemu_cmd $QEMU_HANDLE \
diff --git a/tests/qemu-iotests/118 b/tests/qemu-iotests/118
index e63a40f..8a9e838 100755
--- a/tests/qemu-iotests/118
+++ b/tests/qemu-iotests/118
@@ -229,10 +229,10 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass):
def test_cycle(self):
result = self.vm.qmp('blockdev-add',
- options={'node-name': 'new',
- 'driver': iotests.imgfmt,
- 'file': {'filename': new_img,
- 'driver': 'file'}})
+ node_name='new',
+ driver=iotests.imgfmt,
+ file={'filename': new_img,
+ 'driver': 'file'})
self.assert_qmp(result, 'return', {})
if self.device_name is not None:
@@ -309,10 +309,10 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass):
return
result = self.vm.qmp('blockdev-add',
- options={'node-name': 'new',
- 'driver': iotests.imgfmt,
- 'file': {'filename': new_img,
- 'driver': 'file'}})
+ node_name='new',
+ driver=iotests.imgfmt,
+ file={'filename': new_img,
+ 'driver': 'file'})
self.assert_qmp(result, 'return', {})
result = self.vm.qmp('x-blockdev-insert-medium', device='drive0',
@@ -341,10 +341,10 @@ class TestInitiallyFilled(GeneralChangeTestsBaseClass):
def test_insert_on_filled(self):
result = self.vm.qmp('blockdev-add',
- options={'node-name': 'new',
- 'driver': iotests.imgfmt,
- 'file': {'filename': new_img,
- 'driver': 'file'}})
+ node_name='new',
+ driver=iotests.imgfmt,
+ file={'filename': new_img,
+ 'driver': 'file'})
self.assert_qmp(result, 'return', {})
result = self.vm.qmp('blockdev-open-tray', device='drive0')
@@ -609,11 +609,11 @@ class TestChangeReadOnly(ChangeBaseClass):
self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img)
result = self.vm.qmp('blockdev-add',
- options={'node-name': 'new',
- 'driver': iotests.imgfmt,
- 'read-only': True,
- 'file': {'filename': new_img,
- 'driver': 'file'}})
+ node_name='new',
+ driver=iotests.imgfmt,
+ read_only=True,
+ file={'filename': new_img,
+ 'driver': 'file'})
self.assert_qmp(result, 'return', {})
result = self.vm.qmp('query-block')
@@ -663,10 +663,10 @@ class TestBlockJobsAfterCycle(ChangeBaseClass):
self.assert_qmp_absent(result, 'return[0]/inserted')
result = self.vm.qmp('blockdev-add',
- options={'node-name': 'node0',
- 'driver': iotests.imgfmt,
- 'file': {'filename': old_img,
- 'driver': 'file'}})
+ node_name='node0',
+ driver=iotests.imgfmt,
+ file={'filename': old_img,
+ 'driver': 'file'})
self.assert_qmp(result, 'return', {})
result = self.vm.qmp('x-blockdev-insert-medium', device='drive0',
diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index 2f0bc24..f06938e 100644
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -416,10 +416,10 @@ class TestIncrementalBackup(TestIncrementalBackupBase):
('0xcd', '32M', '124k')))
# Create a blkdebug interface to this img as 'drive1'
- result = self.vm.qmp('blockdev-add', options={
- 'node-name': drive1['id'],
- 'driver': drive1['fmt'],
- 'file': {
+ result = self.vm.qmp('blockdev-add',
+ node_name=drive1['id'],
+ driver=drive1['fmt'],
+ file={
'driver': 'blkdebug',
'image': {
'driver': 'file',
@@ -438,7 +438,7 @@ class TestIncrementalBackup(TestIncrementalBackupBase):
'once': True
}],
}
- })
+ )
self.assert_qmp(result, 'return', {})
# Create bitmaps and full backups for both drives
@@ -560,10 +560,10 @@ class TestIncrementalBackupBlkdebug(TestIncrementalBackupBase):
'''
drive0 = self.drives[0]
- result = self.vm.qmp('blockdev-add', options={
- 'node-name': drive0['id'],
- 'driver': drive0['fmt'],
- 'file': {
+ result = self.vm.qmp('blockdev-add',
+ node_name=drive0['id'],
+ driver=drive0['fmt'],
+ file={
'driver': 'blkdebug',
'image': {
'driver': 'file',
@@ -582,7 +582,7 @@ class TestIncrementalBackupBlkdebug(TestIncrementalBackupBase):
'once': True
}],
}
- })
+ )
self.assert_qmp(result, 'return', {})
self.create_anchor_backup(drive0)
diff --git a/tests/qemu-iotests/139 b/tests/qemu-iotests/139
index 47a4c26..6a0f6ca 100644
--- a/tests/qemu-iotests/139
+++ b/tests/qemu-iotests/139
@@ -57,7 +57,7 @@ class TestBlockdevDel(iotests.QMPTestCase):
'file': {'driver': 'file',
'node-name': file_node,
'filename': base_img}}
- result = self.vm.qmp('blockdev-add', conv_keys = False, options = opts)
+ result = self.vm.qmp('blockdev-add', conv_keys = False, **opts)
self.assert_qmp(result, 'return', {})
self.checkBlockDriverState(node)
self.checkBlockDriverState(file_node)
@@ -72,7 +72,7 @@ class TestBlockdevDel(iotests.QMPTestCase):
'backing': '',
'file': {'driver': 'file',
'filename': new_img}}
- result = self.vm.qmp('blockdev-add', conv_keys = False, options = opts)
+ result = self.vm.qmp('blockdev-add', conv_keys = False, **opts)
self.assert_qmp(result, 'return', {})
self.checkBlockDriverState(node)
@@ -185,7 +185,7 @@ class TestBlockdevDel(iotests.QMPTestCase):
opts = {'driver': 'blkdebug',
'node-name': debug,
'image': image}
- result = self.vm.qmp('blockdev-add', conv_keys = False, options = opts)
+ result = self.vm.qmp('blockdev-add', conv_keys = False, **opts)
self.assert_qmp(result, 'return', {})
self.checkBlockDriverState(node)
self.checkBlockDriverState(debug)
@@ -210,7 +210,7 @@ class TestBlockdevDel(iotests.QMPTestCase):
'node-name': blkverify,
'test': node_0,
'raw': node_1}
- result = self.vm.qmp('blockdev-add', conv_keys = False, options = opts)
+ result = self.vm.qmp('blockdev-add', conv_keys = False, **opts)
self.assert_qmp(result, 'return', {})
self.checkBlockDriverState(test)
self.checkBlockDriverState(raw)
@@ -234,7 +234,7 @@ class TestBlockdevDel(iotests.QMPTestCase):
'node-name': quorum,
'vote-threshold': 1,
'children': [ child_0, child_1 ]}
- result = self.vm.qmp('blockdev-add', conv_keys = False, options = opts)
+ result = self.vm.qmp('blockdev-add', conv_keys = False, **opts)
self.assert_qmp(result, 'return', {})
self.checkBlockDriverState(child0)
self.checkBlockDriverState(child1)
diff --git a/tests/qemu-iotests/141 b/tests/qemu-iotests/141
index c092d87..3ba79f0 100755
--- a/tests/qemu-iotests/141
+++ b/tests/qemu-iotests/141
@@ -50,13 +50,12 @@ test_blockjob()
_send_qemu_cmd $QEMU_HANDLE \
"{'execute': 'blockdev-add',
'arguments': {
- 'options': {
- 'node-name': 'drv0',
- 'driver': '$IMGFMT',
- 'file': {
- 'driver': 'file',
- 'filename': '$TEST_IMG'
- }}}}" \
+ 'node-name': 'drv0',
+ 'driver': '$IMGFMT',
+ 'file': {
+ 'driver': 'file',
+ 'filename': '$TEST_IMG'
+ }}}" \
'return'
_send_qemu_cmd $QEMU_HANDLE \
diff --git a/tests/qemu-iotests/155 b/tests/qemu-iotests/155
index 4057b5e..0b86ea4 100755
--- a/tests/qemu-iotests/155
+++ b/tests/qemu-iotests/155
@@ -63,10 +63,10 @@ class BaseClass(iotests.QMPTestCase):
# Add the BDS via blockdev-add so it stays around after the mirror block
# job has been completed
result = self.vm.qmp('blockdev-add',
- options={'node-name': 'source',
- 'driver': iotests.imgfmt,
- 'file': {'driver': 'file',
- 'filename': source_img}})
+ node_name='source',
+ driver=iotests.imgfmt,
+ file={'driver': 'file',
+ 'filename': source_img})
self.assert_qmp(result, 'return', {})
result = self.vm.qmp('x-blockdev-insert-medium',
@@ -90,7 +90,7 @@ class BaseClass(iotests.QMPTestCase):
if self.target_blockdev_backing:
options['backing'] = self.target_blockdev_backing
- result = self.vm.qmp('blockdev-add', options=options)
+ result = self.vm.qmp('blockdev-add', **options)
self.assert_qmp(result, 'return', {})
def tearDown(self):
--
1.8.3.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] block: Remove "options" indirection from blockdev-add
2016-10-07 15:38 [Qemu-devel] [PATCH] block: Remove "options" indirection from blockdev-add Kevin Wolf
@ 2016-10-07 16:16 ` Max Reitz
2016-10-07 17:18 ` Eric Blake
1 sibling, 0 replies; 3+ messages in thread
From: Max Reitz @ 2016-10-07 16:16 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: eblake, armbru, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 833 bytes --]
On 07.10.2016 17:38, Kevin Wolf wrote:
> Now that QAPI supports boxed types, we can have unions at the top level
> of a command, so let's put our real options directly there for
> blockdev-add instead of having a single "options" dict that contains the
> real arguments.
>
> blockdev-add is still experimental and we already made substantial
> changes to the API recently, so we're free to make changes like this
> one, too.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>
> Yes, that's right. Ignoring the test cases, this is a one-liner in
> the schema without any C code changes. :-)
While that is cool and correct, you also ignored the documentation
changes, which is probably not correct. :-)
(There are some blockdev-add examples in docs/qmp-commands.txt which
need to be adjusted.)
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 480 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] block: Remove "options" indirection from blockdev-add
2016-10-07 15:38 [Qemu-devel] [PATCH] block: Remove "options" indirection from blockdev-add Kevin Wolf
2016-10-07 16:16 ` Max Reitz
@ 2016-10-07 17:18 ` Eric Blake
1 sibling, 0 replies; 3+ messages in thread
From: Eric Blake @ 2016-10-07 17:18 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: mreitz, armbru, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2376 bytes --]
On 10/07/2016 10:38 AM, Kevin Wolf wrote:
> Now that QAPI supports boxed types, we can have unions at the top level
> of a command, so let's put our real options directly there for
> blockdev-add instead of having a single "options" dict that contains the
> real arguments.
>
> blockdev-add is still experimental and we already made substantial
> changes to the API recently, so we're free to make changes like this
> one, too.
I like it, and it needs to make 2.8. Max was correct that there are
some doc changes missing, so looking forward to v2.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>
> Yes, that's right. Ignoring the test cases, this is a one-liner in
> the schema without any C code changes. :-)
Yay! My qapi refactoring work is paying dividends.
The lack of C code changes is because we convert everything to QDict and
then manually parse it using QemuOpts, rather than sticking to the nicer
qapi structs. Someday, it would be nice to use the actual qapi structs
all the way, but that doesn't have to be today.
> +++ b/qapi/block-core.json
> @@ -2321,7 +2321,7 @@
> #
> # Since: 1.7
> ##
> -{ 'command': 'blockdev-add', 'data': { 'options': 'BlockdevOptions' } }
> +{ 'command': 'blockdev-add', 'data': 'BlockdevOptions', 'boxed': true }
Eventually, when we rename x-blockdev-del, we'll want some sort of doc
note here that even though this command has existed since 1.7, it is
fundamentally different in 2.8 and should not be used in anger unless
blockdev-del is also defined.
> +++ b/tests/qemu-iotests/041
> @@ -194,10 +194,9 @@ class TestSingleBlockdev(TestSingleDrive):
> def setUp(self):
> TestSingleDrive.setUp(self)
> qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, target_img)
> - args = {'options':
> - {'driver': iotests.imgfmt,
> - 'node-name': self.qmp_target,
> - 'file': { 'filename': target_img, 'driver': 'file' } } }
> + args = {'driver': iotests.imgfmt,
> + 'node-name': self.qmp_target,
> + 'file': { 'filename': target_img, 'driver': 'file' } }
Less nesting is indeed a bit easier to read :)
--
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] 3+ messages in thread
end of thread, other threads:[~2016-10-07 17:18 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-07 15:38 [Qemu-devel] [PATCH] block: Remove "options" indirection from blockdev-add Kevin Wolf
2016-10-07 16:16 ` Max Reitz
2016-10-07 17:18 ` Eric Blake
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.