[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Max Reitz
Subject: Re: [PATCH v2 4/4] qcow2: Use BDRV_SECTOR_SIZE instead of the hardcoded value
Date: Fri, 17 Jan 2020 10:12:10 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.3.1

On 17.01.20 00:26, Alberto Garcia wrote:
> 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)
>>>  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?

I think the point was that we couldn’t, for a long time, because the
block layer only provided sector-granularity access.  This function
simply was never changed when the block layer gained the ability to do
byte-granularity I/O.

(We’d still need this function, but only for the endian swap, I think.)

>>> @@ -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.

Hm, how did you get to block_copy_do_copy()?  That’s part of the
block-copy infrastructure that’s only used for the backup job, as far as
I’m aware.  It’s different from copy_range.

I don’t see any limitation for copy_range.  I suppose maybe it doesn’t
work for anything that isn’t aligned to physical sectors?  But the qcow2
driver shouldn’t care about that.

On thing’s for sure, the raw driver doesn’t care about it.


Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]