* [Qemu-devel] [PATCH 0/2] block: Let blockdev-create return 0 on success
@ 2019-08-23 18:47 Max Reitz
2019-08-23 18:47 ` [Qemu-devel] [PATCH 1/2] " Max Reitz
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Max Reitz @ 2019-08-23 18:47 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, John Snow, qemu-devel, Max Reitz
Jobs are expected to return 0 on success. .bdrv_co_create() on the
other hand is a block layer function, and as such returns a non-negative
value on success.
blockdev_create_run() should translate between the two (patch 1).
Without patch 1, blockdev-create is likely to fail for VPC images.
Hence patch 2.
Max Reitz (2):
block: Let blockdev-create return 0 on success
iotests: Test blockdev-create for vpc
block/create.c | 3 +-
tests/qemu-iotests/266 | 182 +++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/266.out | 107 ++++++++++++++++++++++
tests/qemu-iotests/group | 1 +
4 files changed, 292 insertions(+), 1 deletion(-)
create mode 100755 tests/qemu-iotests/266
create mode 100644 tests/qemu-iotests/266.out
--
2.21.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 1/2] block: Let blockdev-create return 0 on success
2019-08-23 18:47 [Qemu-devel] [PATCH 0/2] block: Let blockdev-create return 0 on success Max Reitz
@ 2019-08-23 18:47 ` Max Reitz
2019-08-23 18:47 ` [Qemu-devel] [PATCH 2/2] iotests: Test blockdev-create for vpc Max Reitz
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Max Reitz @ 2019-08-23 18:47 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, John Snow, qemu-devel, Max Reitz
Block drivers should let their .bdrv_co_create() implementation return a
non-negative value to indicate success. However, jobs should return
exactly 0. Thus, we need to translate positive return values to 0 in
blockdev_create_run().
Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/create.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/create.c b/block/create.c
index 1bd00ed5f8..4b23672e1b 100644
--- a/block/create.c
+++ b/block/create.c
@@ -48,7 +48,8 @@ static int coroutine_fn blockdev_create_run(Job *job, Error **errp)
qapi_free_BlockdevCreateOptions(s->opts);
- return ret;
+ /* Jobs must return 0 to indicate success */
+ return ret < 0 ? ret : 0;
}
static const JobDriver blockdev_create_job_driver = {
--
2.21.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 2/2] iotests: Test blockdev-create for vpc
2019-08-23 18:47 [Qemu-devel] [PATCH 0/2] block: Let blockdev-create return 0 on success Max Reitz
2019-08-23 18:47 ` [Qemu-devel] [PATCH 1/2] " Max Reitz
@ 2019-08-23 18:47 ` Max Reitz
2019-08-27 21:15 ` John Snow
2019-08-27 21:15 ` [Qemu-devel] [PATCH 0/2] block: Let blockdev-create return 0 on success John Snow
2019-09-02 13:32 ` Kevin Wolf
3 siblings, 1 reply; 6+ messages in thread
From: Max Reitz @ 2019-08-23 18:47 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, John Snow, qemu-devel, Max Reitz
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/266 | 182 +++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/266.out | 107 ++++++++++++++++++++++
tests/qemu-iotests/group | 1 +
3 files changed, 290 insertions(+)
create mode 100755 tests/qemu-iotests/266
create mode 100644 tests/qemu-iotests/266.out
diff --git a/tests/qemu-iotests/266 b/tests/qemu-iotests/266
new file mode 100755
index 0000000000..19b7b29535
--- /dev/null
+++ b/tests/qemu-iotests/266
@@ -0,0 +1,182 @@
+#!/usr/bin/env python
+#
+# Test VPC and file image creation
+#
+# Copyright (C) 2019 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+import iotests
+from iotests import imgfmt
+
+iotests.verify_image_format(supported_fmts=['vpc'])
+iotests.verify_protocol(supported=['file'])
+
+def blockdev_create(vm, options):
+ result = vm.qmp_log('blockdev-create', job_id='job0', options=options,
+ filters=[iotests.filter_qmp_testfiles])
+
+ if 'return' in result:
+ assert result['return'] == {}
+ vm.run_job('job0')
+ iotests.log("")
+
+with iotests.FilePath('t.vpc') as disk_path, \
+ iotests.VM() as vm:
+
+ #
+ # Successful image creation (defaults)
+ #
+ iotests.log("=== Successful image creation (defaults) ===")
+ iotests.log("")
+
+ # 8 heads, 964 cyls/head, 17 secs/cyl
+ # (Close to 64 MB)
+ size = 8 * 964 * 17 * 512
+
+ vm.launch()
+ blockdev_create(vm, { 'driver': 'file',
+ 'filename': disk_path,
+ 'size': 0 })
+
+ vm.qmp_log('blockdev-add', driver='file', filename=disk_path,
+ node_name='imgfile', filters=[iotests.filter_qmp_testfiles])
+
+ blockdev_create(vm, { 'driver': imgfmt,
+ 'file': 'imgfile',
+ 'size': size })
+ vm.shutdown()
+
+ iotests.img_info_log(disk_path)
+
+ #
+ # Successful image creation (explicit defaults)
+ #
+ iotests.log("=== Successful image creation (explicit defaults) ===")
+ iotests.log("")
+
+ # 16 heads, 964 cyls/head, 17 secs/cyl
+ # (Close to 128 MB)
+ size = 16 * 964 * 17 * 512
+
+ vm.launch()
+ blockdev_create(vm, { 'driver': 'file',
+ 'filename': disk_path,
+ 'size': 0 })
+
+ vm.qmp_log('blockdev-add', driver='file', filename=disk_path,
+ node_name='imgfile', filters=[iotests.filter_qmp_testfiles])
+
+ blockdev_create(vm, { 'driver': imgfmt,
+ 'file': 'imgfile',
+ 'size': size,
+ 'subformat': 'dynamic',
+ 'force-size': False })
+ vm.shutdown()
+
+ iotests.img_info_log(disk_path)
+
+ #
+ # Successful image creation (non-default options)
+ #
+ iotests.log("=== Successful image creation (non-default options) ===")
+ iotests.log("")
+
+ # Not representable in CHS (fine with force-size=True)
+ size = 1048576
+
+ vm.launch()
+ blockdev_create(vm, { 'driver': 'file',
+ 'filename': disk_path,
+ 'size': 0 })
+
+ vm.qmp_log('blockdev-add', driver='file', filename=disk_path,
+ node_name='imgfile', filters=[iotests.filter_qmp_testfiles])
+
+ blockdev_create(vm, { 'driver': imgfmt,
+ 'file': 'imgfile',
+ 'size': size,
+ 'subformat': 'fixed',
+ 'force-size': True })
+ vm.shutdown()
+
+ iotests.img_info_log(disk_path)
+
+ #
+ # Size not representable in CHS with force-size=False
+ #
+ iotests.log("=== Size not representable in CHS ===")
+ iotests.log("")
+
+ # Not representable in CHS (will not work with force-size=False)
+ size = 1048576
+
+ vm.launch()
+ blockdev_create(vm, { 'driver': 'file',
+ 'filename': disk_path,
+ 'size': 0 })
+
+ vm.qmp_log('blockdev-add', driver='file', filename=disk_path,
+ node_name='imgfile', filters=[iotests.filter_qmp_testfiles])
+
+ blockdev_create(vm, { 'driver': imgfmt,
+ 'file': 'imgfile',
+ 'size': size,
+ 'force-size': False })
+ vm.shutdown()
+
+ #
+ # Zero size
+ #
+ iotests.log("=== Zero size===")
+ iotests.log("")
+
+ vm.add_blockdev('driver=file,filename=%s,node-name=node0' % (disk_path))
+ vm.launch()
+ blockdev_create(vm, { 'driver': imgfmt,
+ 'file': 'node0',
+ 'size': 0 })
+ vm.shutdown()
+
+ iotests.img_info_log(disk_path)
+
+ #
+ # Maximum CHS size
+ #
+ iotests.log("=== Maximum CHS size===")
+ iotests.log("")
+
+ vm.launch()
+ blockdev_create(vm, { 'driver': imgfmt,
+ 'file': 'node0',
+ 'size': 16 * 65535 * 255 * 512 })
+ vm.shutdown()
+
+ iotests.img_info_log(disk_path)
+
+ #
+ # Actual maximum size
+ #
+ iotests.log("=== Actual maximum size===")
+ iotests.log("")
+
+ vm.launch()
+ blockdev_create(vm, { 'driver': imgfmt,
+ 'file': 'node0',
+ 'size': 0xff000000 * 512,
+ 'force-size': True })
+ vm.shutdown()
+
+ iotests.img_info_log(disk_path)
diff --git a/tests/qemu-iotests/266.out b/tests/qemu-iotests/266.out
new file mode 100644
index 0000000000..43244a4574
--- /dev/null
+++ b/tests/qemu-iotests/266.out
@@ -0,0 +1,107 @@
+=== Successful image creation (defaults) ===
+
+{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "file", "filename": "TEST_DIR/PID-t.vpc", "size": 0}}}
+{"return": {}}
+{"execute": "job-dismiss", "arguments": {"id": "job0"}}
+{"return": {}}
+
+{"execute": "blockdev-add", "arguments": {"driver": "file", "filename": "TEST_DIR/PID-t.vpc", "node-name": "imgfile"}}
+{"return": {}}
+{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "vpc", "file": "imgfile", "size": 67125248}}}
+{"return": {}}
+{"execute": "job-dismiss", "arguments": {"id": "job0"}}
+{"return": {}}
+
+image: TEST_IMG
+file format: IMGFMT
+virtual size: 64 MiB (67125248 bytes)
+cluster_size: 2097152
+
+=== Successful image creation (explicit defaults) ===
+
+{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "file", "filename": "TEST_DIR/PID-t.vpc", "size": 0}}}
+{"return": {}}
+{"execute": "job-dismiss", "arguments": {"id": "job0"}}
+{"return": {}}
+
+{"execute": "blockdev-add", "arguments": {"driver": "file", "filename": "TEST_DIR/PID-t.vpc", "node-name": "imgfile"}}
+{"return": {}}
+{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "vpc", "file": "imgfile", "force-size": false, "size": 134250496, "subformat": "dynamic"}}}
+{"return": {}}
+{"execute": "job-dismiss", "arguments": {"id": "job0"}}
+{"return": {}}
+
+image: TEST_IMG
+file format: IMGFMT
+virtual size: 128 MiB (134250496 bytes)
+cluster_size: 2097152
+
+=== Successful image creation (non-default options) ===
+
+{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "file", "filename": "TEST_DIR/PID-t.vpc", "size": 0}}}
+{"return": {}}
+{"execute": "job-dismiss", "arguments": {"id": "job0"}}
+{"return": {}}
+
+{"execute": "blockdev-add", "arguments": {"driver": "file", "filename": "TEST_DIR/PID-t.vpc", "node-name": "imgfile"}}
+{"return": {}}
+{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "vpc", "file": "imgfile", "force-size": true, "size": 1048576, "subformat": "fixed"}}}
+{"return": {}}
+{"execute": "job-dismiss", "arguments": {"id": "job0"}}
+{"return": {}}
+
+image: TEST_IMG
+file format: IMGFMT
+virtual size: 1 MiB (1048576 bytes)
+
+=== Size not representable in CHS ===
+
+{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "file", "filename": "TEST_DIR/PID-t.vpc", "size": 0}}}
+{"return": {}}
+{"execute": "job-dismiss", "arguments": {"id": "job0"}}
+{"return": {}}
+
+{"execute": "blockdev-add", "arguments": {"driver": "file", "filename": "TEST_DIR/PID-t.vpc", "node-name": "imgfile"}}
+{"return": {}}
+{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "vpc", "file": "imgfile", "force-size": false, "size": 1048576}}}
+{"return": {}}
+Job failed: The requested image size cannot be represented in CHS geometry
+{"execute": "job-dismiss", "arguments": {"id": "job0"}}
+{"return": {}}
+
+=== Zero size===
+
+{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "vpc", "file": "node0", "size": 0}}}
+{"return": {}}
+{"execute": "job-dismiss", "arguments": {"id": "job0"}}
+{"return": {}}
+
+image: TEST_IMG
+file format: IMGFMT
+virtual size: 0 B (0 bytes)
+cluster_size: 2097152
+
+=== Maximum CHS size===
+
+{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "vpc", "file": "node0", "size": 136899993600}}}
+{"return": {}}
+{"execute": "job-dismiss", "arguments": {"id": "job0"}}
+{"return": {}}
+
+image: TEST_IMG
+file format: IMGFMT
+virtual size: 127 GiB (136899993600 bytes)
+cluster_size: 2097152
+
+=== Actual maximum size===
+
+{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "vpc", "file": "node0", "force-size": true, "size": 2190433320960}}}
+{"return": {}}
+{"execute": "job-dismiss", "arguments": {"id": "job0"}}
+{"return": {}}
+
+image: TEST_IMG
+file format: IMGFMT
+virtual size: 1.99 TiB (2190433320960 bytes)
+cluster_size: 2097152
+
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index d95d556414..8b96456278 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -274,3 +274,4 @@
257 rw
258 rw quick
262 rw quick migration
+266 rw quick
--
2.21.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] iotests: Test blockdev-create for vpc
2019-08-23 18:47 ` [Qemu-devel] [PATCH 2/2] iotests: Test blockdev-create for vpc Max Reitz
@ 2019-08-27 21:15 ` John Snow
0 siblings, 0 replies; 6+ messages in thread
From: John Snow @ 2019-08-27 21:15 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: Kevin Wolf, qemu-devel
On 8/23/19 2:47 PM, Max Reitz wrote:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> tests/qemu-iotests/266 | 182 +++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/266.out | 107 ++++++++++++++++++++++
> tests/qemu-iotests/group | 1 +
> 3 files changed, 290 insertions(+)
> create mode 100755 tests/qemu-iotests/266
> create mode 100644 tests/qemu-iotests/266.out
>
> diff --git a/tests/qemu-iotests/266 b/tests/qemu-iotests/266
> new file mode 100755
> index 0000000000..19b7b29535
> --- /dev/null
> +++ b/tests/qemu-iotests/266
> @@ -0,0 +1,182 @@
> +#!/usr/bin/env python
> +#
> +# Test VPC and file image creation
> +#
> +# Copyright (C) 2019 Red Hat, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +import iotests
> +from iotests import imgfmt
> +
> +iotests.verify_image_format(supported_fmts=['vpc'])
> +iotests.verify_protocol(supported=['file'])
> +
Oh, I guess I haven't successfully lobbied for the inclusion of the
other thing yet. oh-kay.
> +def blockdev_create(vm, options):
> + result = vm.qmp_log('blockdev-create', job_id='job0', options=options,
> + filters=[iotests.filter_qmp_testfiles])
> +
> + if 'return' in result:
> + assert result['return'] == {}
> + vm.run_job('job0')
> + iotests.log("")
> +
Probably approaching the time when we want a standard for this in
iotests, but I'm not insisting today.
> +with iotests.FilePath('t.vpc') as disk_path, \
> + iotests.VM() as vm:
> +
> + #
> + # Successful image creation (defaults)
> + #
> + iotests.log("=== Successful image creation (defaults) ===")
> + iotests.log("")
> +
> + # 8 heads, 964 cyls/head, 17 secs/cyl
> + # (Close to 64 MB)
> + size = 8 * 964 * 17 * 512
> +
> + vm.launch()
> + blockdev_create(vm, { 'driver': 'file',
> + 'filename': disk_path,
> + 'size': 0 })
> +
> + vm.qmp_log('blockdev-add', driver='file', filename=disk_path,
> + node_name='imgfile', filters=[iotests.filter_qmp_testfiles])
> +
> + blockdev_create(vm, { 'driver': imgfmt,
> + 'file': 'imgfile',
> + 'size': size })
> + vm.shutdown()
> +
> + iotests.img_info_log(disk_path)
> +
> + #
> + # Successful image creation (explicit defaults)
> + #
> + iotests.log("=== Successful image creation (explicit defaults) ===")
> + iotests.log("")
> +
> + # 16 heads, 964 cyls/head, 17 secs/cyl
> + # (Close to 128 MB)
> + size = 16 * 964 * 17 * 512
> +
> + vm.launch()
> + blockdev_create(vm, { 'driver': 'file',
> + 'filename': disk_path,
> + 'size': 0 })
> +
> + vm.qmp_log('blockdev-add', driver='file', filename=disk_path,
> + node_name='imgfile', filters=[iotests.filter_qmp_testfiles])
> +
> + blockdev_create(vm, { 'driver': imgfmt,
> + 'file': 'imgfile',
> + 'size': size,
> + 'subformat': 'dynamic',
> + 'force-size': False })
> + vm.shutdown()
> +
> + iotests.img_info_log(disk_path)
> +
> + #
> + # Successful image creation (non-default options)
> + #
> + iotests.log("=== Successful image creation (non-default options) ===")
> + iotests.log("")
> +
> + # Not representable in CHS (fine with force-size=True)
> + size = 1048576
> +
> + vm.launch()
> + blockdev_create(vm, { 'driver': 'file',
> + 'filename': disk_path,
> + 'size': 0 })
> +
> + vm.qmp_log('blockdev-add', driver='file', filename=disk_path,
> + node_name='imgfile', filters=[iotests.filter_qmp_testfiles])
> +
> + blockdev_create(vm, { 'driver': imgfmt,
> + 'file': 'imgfile',
> + 'size': size,
> + 'subformat': 'fixed',
> + 'force-size': True })
> + vm.shutdown()
> +
> + iotests.img_info_log(disk_path)
> +
> + #
> + # Size not representable in CHS with force-size=False
> + #
> + iotests.log("=== Size not representable in CHS ===")
> + iotests.log("")
> +
> + # Not representable in CHS (will not work with force-size=False)
> + size = 1048576
> +
> + vm.launch()
> + blockdev_create(vm, { 'driver': 'file',
> + 'filename': disk_path,
> + 'size': 0 })
> +
> + vm.qmp_log('blockdev-add', driver='file', filename=disk_path,
> + node_name='imgfile', filters=[iotests.filter_qmp_testfiles])
> +
> + blockdev_create(vm, { 'driver': imgfmt,
> + 'file': 'imgfile',
> + 'size': size,
> + 'force-size': False })
> + vm.shutdown()
> +
> + #
> + # Zero size
> + #
> + iotests.log("=== Zero size===")
> + iotests.log("")
> +
> + vm.add_blockdev('driver=file,filename=%s,node-name=node0' % (disk_path))
> + vm.launch()
> + blockdev_create(vm, { 'driver': imgfmt,
> + 'file': 'node0',
> + 'size': 0 })
> + vm.shutdown()
> +
> + iotests.img_info_log(disk_path)
> +
> + #
> + # Maximum CHS size
> + #
> + iotests.log("=== Maximum CHS size===")
> + iotests.log("")
> +
> + vm.launch()
> + blockdev_create(vm, { 'driver': imgfmt,
> + 'file': 'node0',
> + 'size': 16 * 65535 * 255 * 512 })
> + vm.shutdown()
> +
> + iotests.img_info_log(disk_path)
> +
> + #
> + # Actual maximum size
> + #
> + iotests.log("=== Actual maximum size===")
> + iotests.log("")
> +
> + vm.launch()
> + blockdev_create(vm, { 'driver': imgfmt,
> + 'file': 'node0',
> + 'size': 0xff000000 * 512,
> + 'force-size': True })
> + vm.shutdown()
> +
> + iotests.img_info_log(disk_path)
> diff --git a/tests/qemu-iotests/266.out b/tests/qemu-iotests/266.out
> new file mode 100644
> index 0000000000..43244a4574
> --- /dev/null
> +++ b/tests/qemu-iotests/266.out
> @@ -0,0 +1,107 @@
> +=== Successful image creation (defaults) ===
> +
> +{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "file", "filename": "TEST_DIR/PID-t.vpc", "size": 0}}}
> +{"return": {}}
> +{"execute": "job-dismiss", "arguments": {"id": "job0"}}
> +{"return": {}}
> +
> +{"execute": "blockdev-add", "arguments": {"driver": "file", "filename": "TEST_DIR/PID-t.vpc", "node-name": "imgfile"}}
> +{"return": {}}
> +{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "vpc", "file": "imgfile", "size": 67125248}}}
> +{"return": {}}
> +{"execute": "job-dismiss", "arguments": {"id": "job0"}}
> +{"return": {}}
> +
> +image: TEST_IMG
> +file format: IMGFMT
> +virtual size: 64 MiB (67125248 bytes)
> +cluster_size: 2097152
> +
> +=== Successful image creation (explicit defaults) ===
> +
> +{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "file", "filename": "TEST_DIR/PID-t.vpc", "size": 0}}}
> +{"return": {}}
> +{"execute": "job-dismiss", "arguments": {"id": "job0"}}
> +{"return": {}}
> +
> +{"execute": "blockdev-add", "arguments": {"driver": "file", "filename": "TEST_DIR/PID-t.vpc", "node-name": "imgfile"}}
> +{"return": {}}
> +{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "vpc", "file": "imgfile", "force-size": false, "size": 134250496, "subformat": "dynamic"}}}
> +{"return": {}}
> +{"execute": "job-dismiss", "arguments": {"id": "job0"}}
> +{"return": {}}
> +
> +image: TEST_IMG
> +file format: IMGFMT
> +virtual size: 128 MiB (134250496 bytes)
> +cluster_size: 2097152
> +
> +=== Successful image creation (non-default options) ===
> +
> +{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "file", "filename": "TEST_DIR/PID-t.vpc", "size": 0}}}
> +{"return": {}}
> +{"execute": "job-dismiss", "arguments": {"id": "job0"}}
> +{"return": {}}
> +
> +{"execute": "blockdev-add", "arguments": {"driver": "file", "filename": "TEST_DIR/PID-t.vpc", "node-name": "imgfile"}}
> +{"return": {}}
> +{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "vpc", "file": "imgfile", "force-size": true, "size": 1048576, "subformat": "fixed"}}}
> +{"return": {}}
> +{"execute": "job-dismiss", "arguments": {"id": "job0"}}
> +{"return": {}}
> +
> +image: TEST_IMG
> +file format: IMGFMT
> +virtual size: 1 MiB (1048576 bytes)
> +
> +=== Size not representable in CHS ===
> +
> +{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "file", "filename": "TEST_DIR/PID-t.vpc", "size": 0}}}
> +{"return": {}}
> +{"execute": "job-dismiss", "arguments": {"id": "job0"}}
> +{"return": {}}
> +
> +{"execute": "blockdev-add", "arguments": {"driver": "file", "filename": "TEST_DIR/PID-t.vpc", "node-name": "imgfile"}}
> +{"return": {}}
> +{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "vpc", "file": "imgfile", "force-size": false, "size": 1048576}}}
> +{"return": {}}
> +Job failed: The requested image size cannot be represented in CHS geometry
> +{"execute": "job-dismiss", "arguments": {"id": "job0"}}
> +{"return": {}}
> +
> +=== Zero size===
> +
> +{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "vpc", "file": "node0", "size": 0}}}
> +{"return": {}}
> +{"execute": "job-dismiss", "arguments": {"id": "job0"}}
> +{"return": {}}
> +
> +image: TEST_IMG
> +file format: IMGFMT
> +virtual size: 0 B (0 bytes)
> +cluster_size: 2097152
> +
> +=== Maximum CHS size===
> +
> +{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "vpc", "file": "node0", "size": 136899993600}}}
> +{"return": {}}
> +{"execute": "job-dismiss", "arguments": {"id": "job0"}}
> +{"return": {}}
> +
> +image: TEST_IMG
> +file format: IMGFMT
> +virtual size: 127 GiB (136899993600 bytes)
> +cluster_size: 2097152
> +
> +=== Actual maximum size===
> +
> +{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "vpc", "file": "node0", "force-size": true, "size": 2190433320960}}}
> +{"return": {}}
> +{"execute": "job-dismiss", "arguments": {"id": "job0"}}
> +{"return": {}}
> +
> +image: TEST_IMG
> +file format: IMGFMT
> +virtual size: 1.99 TiB (2190433320960 bytes)
> +cluster_size: 2097152
> +
> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
> index d95d556414..8b96456278 100644
> --- a/tests/qemu-iotests/group
> +++ b/tests/qemu-iotests/group
> @@ -274,3 +274,4 @@
> 257 rw
> 258 rw quick
> 262 rw quick migration
> +266 rw quick
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] block: Let blockdev-create return 0 on success
2019-08-23 18:47 [Qemu-devel] [PATCH 0/2] block: Let blockdev-create return 0 on success Max Reitz
2019-08-23 18:47 ` [Qemu-devel] [PATCH 1/2] " Max Reitz
2019-08-23 18:47 ` [Qemu-devel] [PATCH 2/2] iotests: Test blockdev-create for vpc Max Reitz
@ 2019-08-27 21:15 ` John Snow
2019-09-02 13:32 ` Kevin Wolf
3 siblings, 0 replies; 6+ messages in thread
From: John Snow @ 2019-08-27 21:15 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: Kevin Wolf, qemu-devel
On 8/23/19 2:47 PM, Max Reitz wrote:
> Jobs are expected to return 0 on success. .bdrv_co_create() on the
> other hand is a block layer function, and as such returns a non-negative
> value on success.
>
> blockdev_create_run() should translate between the two (patch 1).
>
> Without patch 1, blockdev-create is likely to fail for VPC images.
> Hence patch 2.
>
>
> Max Reitz (2):
> block: Let blockdev-create return 0 on success
> iotests: Test blockdev-create for vpc
>
> block/create.c | 3 +-
> tests/qemu-iotests/266 | 182 +++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/266.out | 107 ++++++++++++++++++++++
> tests/qemu-iotests/group | 1 +
> 4 files changed, 292 insertions(+), 1 deletion(-)
> create mode 100755 tests/qemu-iotests/266
> create mode 100644 tests/qemu-iotests/266.out
>
Reviewed-by: John Snow <jsnow@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] block: Let blockdev-create return 0 on success
2019-08-23 18:47 [Qemu-devel] [PATCH 0/2] block: Let blockdev-create return 0 on success Max Reitz
` (2 preceding siblings ...)
2019-08-27 21:15 ` [Qemu-devel] [PATCH 0/2] block: Let blockdev-create return 0 on success John Snow
@ 2019-09-02 13:32 ` Kevin Wolf
3 siblings, 0 replies; 6+ messages in thread
From: Kevin Wolf @ 2019-09-02 13:32 UTC (permalink / raw)
To: Max Reitz; +Cc: John Snow, qemu-devel, qemu-block
Am 23.08.2019 um 20:47 hat Max Reitz geschrieben:
> Jobs are expected to return 0 on success. .bdrv_co_create() on the
> other hand is a block layer function, and as such returns a
> non-negative value on success.
I don't agree that >= 0 is the block layer way. The block layer uses
0/-errno for the largest part of its interfaces; and I think the
BlockDriver callbacks might even be consistent in this. Of course, we
never documented this anywhere, maybe we should...
The only historical exceptions I'm aware of are blk/bdrv_pread/pwrite(),
which return the byte count instead of 0. They should be fixed
eventually, but it just never seemed important enough, even though it
did cause bugs every now and then.
> blockdev_create_run() should translate between the two (patch 1).
>
> Without patch 1, blockdev-create is likely to fail for VPC images.
> Hence patch 2.
I'd argue this is a VPC bug. In the success path, it shouldn't return
ret as it happens to be at the end (it comes from bdrv_pwrite()), but
set it to 0 right before the 'fail:' label.
This is really a regression Jeff introduced in commit fef6070eff2,
though the bug was only latent then (five years ago).
Kevin
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-09-02 13:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-23 18:47 [Qemu-devel] [PATCH 0/2] block: Let blockdev-create return 0 on success Max Reitz
2019-08-23 18:47 ` [Qemu-devel] [PATCH 1/2] " Max Reitz
2019-08-23 18:47 ` [Qemu-devel] [PATCH 2/2] iotests: Test blockdev-create for vpc Max Reitz
2019-08-27 21:15 ` John Snow
2019-08-27 21:15 ` [Qemu-devel] [PATCH 0/2] block: Let blockdev-create return 0 on success John Snow
2019-09-02 13:32 ` Kevin Wolf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).