Re: [PATCH v2 4/4] qcow2: Use BDRV_SECTOR_SIZE instead of the hardcoded value

[Alberto Garcia]

On Tue 14 Jan 2020 03:15:48 PM CET, Max Reitz wrote:
>> @@ -219,7 +219,7 @@ static int l2_load(BlockDriverState *bs, uint64_t offset,
>>   * Writes one sector of the L1 table to the disk (can't update single 
>> entries
>>   * and we really don't want bdrv_pread to perform a read-modify-write)
>>   */
>> -#define L1_ENTRIES_PER_SECTOR (512 / 8)
>> +#define L1_ENTRIES_PER_SECTOR (BDRV_SECTOR_SIZE / 8)
>>  int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index)
>
> Here it’s because the comment is wrong: “Can’t update single entries” –
> yes, we can.  We’d just have to do a bdrv_pwrite() to a single entry.

What's the point of qcow2_write_l1_entry() then?

>> @@ -3836,7 +3837,7 @@ qcow2_co_copy_range_from(BlockDriverState *bs,
>>          case QCOW2_CLUSTER_NORMAL:
>>              child = s->data_file;
>>              copy_offset += offset_into_cluster(s, src_offset);
>> -            if ((copy_offset & 511) != 0) {
>> +            if (!QEMU_IS_ALIGNED(copy_offset, BDRV_SECTOR_SIZE)) {
>
> Hm.  I don’t get this one.

Checking the code (e.g. block_copy_do_copy()) it seems that the whole
chunk must be cluster aligned so I don't get this one either.

Berto