[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 3/3] qcow2: Use BDRV_SECTOR_SIZE instead of the hardcoded val
From: |
Alberto Garcia |
Subject: |
Re: [PATCH 3/3] qcow2: Use BDRV_SECTOR_SIZE instead of the hardcoded value |
Date: |
Thu, 09 Jan 2020 13:30:36 +0100 |
User-agent: |
Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (i586-pc-linux-gnu) |
On Thu 09 Jan 2020 01:19:00 PM CET, Kevin Wolf wrote:
>> diff --git a/block/qcow2.c b/block/qcow2.c
>> index e8ce966f7f..6427c75409 100644
>> --- a/block/qcow2.c
>> +++ b/block/qcow2.c
>> @@ -2175,7 +2175,7 @@ static coroutine_fn int
>> qcow2_co_preadv_task(BlockDriverState *bs,
>> offset, bytes, qiov, qiov_offset);
>>
>> case QCOW2_CLUSTER_NORMAL:
>> - if ((file_cluster_offset & 511) != 0) {
>> + if ((file_cluster_offset % BDRV_SECTOR_SIZE) != 0) {
>> return -EIO;
>> }
>
> Hm, unrelated to your change, but why do we test for 512 byte
> alignment here? file_cluster_offset should certainly be cluster
> aligned for normal clusters. And if the check fails, that's actually
> an image corruption and not just an I/O error. Am I missing something?
I actually suspect that this is just an old, obsolete check that we have
kept during these years. file_cluster_offset should be not just sector
aligned but also cluster aligned if I'm not wrong, and if not then
qcow2_alloc_cluster_offset() and qcow2_get_cluster_offset() should
return an error.
I can simply remove that check, or replace it with an assertion.
Berto