* [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).