* [PATCH for-5.0] qcow2: Check request size in qcow2_co_pwritev_compressed_part()
@ 2020-04-03 16:57 Alberto Garcia
2020-04-03 17:40 ` Eric Blake
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Alberto Garcia @ 2020-04-03 16:57 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Alberto Garcia,
qemu-block, Pavel Butsykin, Max Reitz, Andrey Shinkevich
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(-)
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;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH for-5.0] qcow2: Check request size in qcow2_co_pwritev_compressed_part()
2020-04-03 16:57 [PATCH for-5.0] qcow2: Check request size in qcow2_co_pwritev_compressed_part() Alberto Garcia
@ 2020-04-03 17:40 ` Eric Blake
2020-04-06 7:23 ` Vladimir Sementsov-Ogievskiy
2020-04-04 2:53 ` no-reply
2020-04-04 3:37 ` Andrey Shinkevich
2 siblings, 1 reply; 5+ messages in thread
From: Eric Blake @ 2020-04-03 17:40 UTC (permalink / raw)
To: Alberto Garcia, qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, qemu-block,
Pavel Butsykin, Max Reitz, Andrey Shinkevich
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;
> }
>
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH for-5.0] qcow2: Check request size in qcow2_co_pwritev_compressed_part()
2020-04-03 16:57 [PATCH for-5.0] qcow2: Check request size in qcow2_co_pwritev_compressed_part() Alberto Garcia
2020-04-03 17:40 ` Eric Blake
@ 2020-04-04 2:53 ` no-reply
2020-04-04 3:37 ` Andrey Shinkevich
2 siblings, 0 replies; 5+ messages in thread
From: no-reply @ 2020-04-04 2:53 UTC (permalink / raw)
To: berto
Cc: kwolf, vsementsov, berto, qemu-block, pbutsykin, qemu-devel,
mreitz, andrey.shinkevich
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH for-5.0] qcow2: Check request size in qcow2_co_pwritev_compressed_part()
2020-04-03 16:57 [PATCH for-5.0] qcow2: Check request size in qcow2_co_pwritev_compressed_part() Alberto Garcia
2020-04-03 17:40 ` Eric Blake
2020-04-04 2:53 ` no-reply
@ 2020-04-04 3:37 ` Andrey Shinkevich
2 siblings, 0 replies; 5+ messages in thread
From: Andrey Shinkevich @ 2020-04-04 3:37 UTC (permalink / raw)
To: Alberto Garcia, qemu-devel
Cc: Kevin Wolf, Pavel Butsykin, Vladimir Sementsov-Ogievskiy,
qemu-block, Max Reitz
________________________________________
From: Alberto Garcia <berto@igalia.com>
Sent: Friday, April 3, 2020 7:57 PM
To: qemu-devel@nongnu.org
Cc: Alberto Garcia; qemu-block@nongnu.org; Andrey Shinkevich; Max Reitz; Kevin Wolf; Vladimir Sementsov-Ogievskiy; Pavel Butsykin
Subject: [PATCH for-5.0] qcow2: Check request size in qcow2_co_pwritev_compressed_part()
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
The condition that QEMU supports writing compressed data of the size equal to one cluster was introduced with earlier patches.
Andrey
Signed-off-by: Alberto Garcia <berto@igalia.com>
---
block/qcow2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH for-5.0] qcow2: Check request size in qcow2_co_pwritev_compressed_part()
2020-04-03 17:40 ` Eric Blake
@ 2020-04-06 7:23 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 5+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2020-04-06 7:23 UTC (permalink / raw)
To: Eric Blake, Alberto Garcia, qemu-devel
Cc: Kevin Wolf, Andrey Shinkevich, Max Reitz, qemu-block, Pavel Butsykin
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;
}
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-04-06 7:25 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-03 16:57 [PATCH for-5.0] qcow2: Check request size in qcow2_co_pwritev_compressed_part() Alberto Garcia
2020-04-03 17:40 ` Eric Blake
2020-04-06 7:23 ` Vladimir Sementsov-Ogievskiy
2020-04-04 2:53 ` no-reply
2020-04-04 3:37 ` Andrey Shinkevich
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.