On 11/15/21 16:57, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <[email protected]> writes:
>> On 11/15/21 13:55, Markus Armbruster wrote:
>>> drive_get_next() is basically a bad idea. It returns the "next" block
>>> backend of a certain interface type. "Next" means bus=0,unit=N, where
>>> subsequent calls count N up from zero, per interface type.
>>>
>>> This lets you define unit numbers implicitly by execution order. If the
>>> order changes, or new calls appear "in the middle", unit numbers change.
>>> ABI break. Hard to spot in review.
>>>
>>> Explicit is better than implicit: use drive_get() directly.
>>>
>>> Signed-off-by: Markus Armbruster <[email protected]>
>>> ---
>>> @@ -435,11 +438,13 @@ static void aspeed_machine_init(MachineState *machine)
>>> }
>>>
>>> for (i = 0; i < bmc->soc.sdhci.num_slots; i++) {
>>> - sdhci_attach_drive(&bmc->soc.sdhci.slots[i],
>>> drive_get_next(IF_SD));
>>> + sdhci_attach_drive(&bmc->soc.sdhci.slots[i],
>>> + drive_get(IF_SD, 0, i));
>>
>> If we put SD on bus #0, ...
>>
>>> }
>>>
>>> if (bmc->soc.emmc.num_slots) {
>>> - sdhci_attach_drive(&bmc->soc.emmc.slots[0], drive_get_next(IF_SD));
>>> + sdhci_attach_drive(&bmc->soc.emmc.slots[0],
>>> + drive_get(IF_SD, 0, bmc->soc.sdhci.num_slots));
>>
>> ... we'd want to put eMMC on bus #1
>
> Using separate buses for different kinds of devices would be neater, but
> it also would be an incompatible change. This patch keeps existing
> bus/unit numbers working. drive_get_next() can only use bus 0.
>
>> but I see having eMMC cards on a
>> IF_SD bus as a bug, since these cards are soldered on the board.
>
> IF_SD is not a bus, it's an "block interface type", which is really just
> a user interface thing.
Why are we discriminating by "block interface type" then?
What is the difference between "block interfaces"? I see a block drive
as a generic unit, usable on multiple hardware devices.
I never really understood how this "block interface type" helps
developers and users. I thought BlockInterfaceType and DriveInfo
were legacy / deprecated APIs we want to get rid of; and we would
come up with a replacement API using BlockDeviceInfo or providing
a BlockFrontend state of the art object.
Anyway, I suppose the explanation is buried in the git history
before the last 8 years. I need to keep reading.
