Message ID | 20200403165752.18009-1-berto@igalia.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [for-5.0] qcow2: Check request size in qcow2_co_pwritev_compressed_part() | expand |
On 4/3/20 11:57 AM, Alberto Garcia wrote: > When issuing a compressed write request the number of bytes must be a > multiple of the cluster size. > > With the current code such requests are allowed and we hit an > assertion: > > $ qemu-img create -f qcow2 img.qcow2 1M > $ qemu-io -c 'write -c 0 32k' img.qcow2 > > qemu-io: block/qcow2.c:4257: qcow2_co_pwritev_compressed_task: > Assertion `bytes == s->cluster_size || (bytes < s->cluster_size && > (offset + bytes == bs->total_sectors << BDRV_SECTOR_BITS))' failed. > Aborted > > This patch fixes a regression introduced in 0d483dce38 > > Signed-off-by: Alberto Garcia <berto@igalia.com> > --- > block/qcow2.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Eric Blake <eblake@redhat.com> > > diff --git a/block/qcow2.c b/block/qcow2.c > index 2bb536b014..923ad428f0 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -4345,7 +4345,7 @@ qcow2_co_pwritev_compressed_part(BlockDriverState *bs, > return bdrv_co_truncate(bs->file, len, false, PREALLOC_MODE_OFF, NULL); > } > > - if (offset_into_cluster(s, offset)) { > + if (offset_into_cluster(s, offset | bytes)) { > return -EINVAL; > } > >
Patchew URL: https://patchew.org/QEMU/20200403165752.18009-1-berto@igalia.com/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash make docker-image-centos7 V=1 NETWORK=1 time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1 === TEST SCRIPT END === Not run: 259 Failures: 053 Failed 1 of 116 iotests make: *** [check-tests/check-block.sh] Error 1 make: *** Waiting for unfinished jobs.... TEST check-qtest-aarch64: tests/qtest/qos-test Traceback (most recent call last): --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=bbedd703733d47e09453ee5d9ae135e1', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-u4oill6e/src/docker-src.2020-04-03-22.39.14.28831:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=bbedd703733d47e09453ee5d9ae135e1 make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-u4oill6e/src' make: *** [docker-run-test-quick@centos7] Error 2 real 14m1.546s user 0m8.497s The full log is available at http://patchew.org/logs/20200403165752.18009-1-berto@igalia.com/testing.docker-quick@centos7/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
03.04.2020 20:40, Eric Blake wrote: > On 4/3/20 11:57 AM, Alberto Garcia wrote: >> When issuing a compressed write request the number of bytes must be a >> multiple of the cluster size. >> >> With the current code such requests are allowed and we hit an >> assertion: >> >> $ qemu-img create -f qcow2 img.qcow2 1M >> $ qemu-io -c 'write -c 0 32k' img.qcow2 >> >> qemu-io: block/qcow2.c:4257: qcow2_co_pwritev_compressed_task: >> Assertion `bytes == s->cluster_size || (bytes < s->cluster_size && >> (offset + bytes == bs->total_sectors << BDRV_SECTOR_BITS))' failed. >> Aborted >> >> This patch fixes a regression introduced in 0d483dce38 >> >> Signed-off-by: Alberto Garcia <berto@igalia.com> >> --- >> block/qcow2.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > > Reviewed-by: Eric Blake <eblake@redhat.com> > >> >> diff --git a/block/qcow2.c b/block/qcow2.c >> index 2bb536b014..923ad428f0 100644 >> --- a/block/qcow2.c >> +++ b/block/qcow2.c >> @@ -4345,7 +4345,7 @@ qcow2_co_pwritev_compressed_part(BlockDriverState *bs, >> return bdrv_co_truncate(bs->file, len, false, PREALLOC_MODE_OFF, NULL); >> } >> - if (offset_into_cluster(s, offset)) { >> + if (offset_into_cluster(s, offset | bytes)) { >> return -EINVAL; >> } >> > This will break compressed write to the tail of unaligned to cluster_size image, which is possible as I understand. Check should make an exception for this case, like the assertion does: len = bdrv_getlength(bs); if (offset_into_cluster(s, offset) || (offset_into_cluster(bytes) && offset + bytes != len)) { return -EINVAL; }
diff --git a/block/qcow2.c b/block/qcow2.c index 2bb536b014..923ad428f0 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -4345,7 +4345,7 @@ qcow2_co_pwritev_compressed_part(BlockDriverState *bs, return bdrv_co_truncate(bs->file, len, false, PREALLOC_MODE_OFF, NULL); } - if (offset_into_cluster(s, offset)) { + if (offset_into_cluster(s, offset | bytes)) { return -EINVAL; }
When issuing a compressed write request the number of bytes must be a multiple of the cluster size. With the current code such requests are allowed and we hit an assertion: $ qemu-img create -f qcow2 img.qcow2 1M $ qemu-io -c 'write -c 0 32k' img.qcow2 qemu-io: block/qcow2.c:4257: qcow2_co_pwritev_compressed_task: Assertion `bytes == s->cluster_size || (bytes < s->cluster_size && (offset + bytes == bs->total_sectors << BDRV_SECTOR_BITS))' failed. Aborted This patch fixes a regression introduced in 0d483dce38 Signed-off-by: Alberto Garcia <berto@igalia.com> --- block/qcow2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)