On 11/15/21 16:57, Markus Armbruster wrote: > Philippe Mathieu-Daudé <f4...@amsat.org> 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 <arm...@redhat.com> >>> ---
>>> @@ -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.