qemu-block
[Top][All Lists]
Advanced

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

Re: [PATCH 3/7] hw/ide/core: Replace magic '512' value by BDRV_SECTOR_SI


From: John Snow
Subject: Re: [PATCH 3/7] hw/ide/core: Replace magic '512' value by BDRV_SECTOR_SIZE
Date: Wed, 23 Sep 2020 10:53:59 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0

On 8/17/20 7:17 AM, Kevin Wolf wrote:
Am 14.08.2020 um 10:28 hat Philippe Mathieu-Daudé geschrieben:
Use self-explicit definitions instead of magic '512' value.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

BDRV_SECTOR_SIZE is the arbitrary unit in which some block layer
functions and variables work (such as bs->total_sectors). It happens to
be 512.

IDE disks have a sector size, too. Actually, two of them, a physical and
a logical one. The more important one is the logical one. We happen to
emulate only IDE devices for which the logical block size is 512 bytes
(ide_dev_initfn() errors out otherwise).

But just because two constants both happen to be 512 in practice, they
are not the same thing.

So if we want to replace all magic 512 values, we should probably
introduce a new IDE_SECTOR_SIZE and then decide case by case whether
IDE_SECTOR_SIZE or BDRV_SECTOR_SIZE is meant. I think most (if not all)
of the places you converted in this patch need to be IDE_SECTOR_SIZE.

Kevin


I didn't audit the other patches, but be mindful of the distinction that Kevin is pointing out.

Luckily, I think we're low risk for deciding to change the BDRV_SECTOR_SIZE default any time soon, so it probably won't matter in the near future ...

--js




reply via email to

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