Re: [Qemu-devel] [PATCH 0/2] block: Let blockdev-create return 0 on success
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
Re: [Qemu-devel] [PATCH 0/2] block: Let blockdev-create return 0 on success
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
[Qemu-devel] [PATCH 0/2] block: Let blockdev-create return 0 on success
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
