John Snow <js...@redhat.com> writes: > Signed-off-by: John Snow <js...@redhat.com> > --- > hw/i386/pc_q35.c | 3 +++ > hw/ide/ahci.c | 31 +++++++++++++++++++++++++++++++ > hw/ide/ahci.h | 3 +++ > 3 files changed, 37 insertions(+) > > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > index fd26fe1..0f33696 100644 > --- a/hw/i386/pc_q35.c > +++ b/hw/i386/pc_q35.c > @@ -86,6 +86,7 @@ static void pc_q35_init(MachineState *machine) > DeviceState *icc_bridge; > PcGuestInfo *guest_info; > ram_addr_t lowmem; > + DriveInfo *hd[MAX_SATA_PORTS]; > > /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory > * and 256 Mbytes for PCI Express Enhanced Configuration Access Mapping > @@ -253,6 +254,8 @@ static void pc_q35_init(MachineState *machine) > true, "ich9-ahci"); > idebus[0] = qdev_get_child_bus(&ahci->qdev, "ide.0"); > idebus[1] = qdev_get_child_bus(&ahci->qdev, "ide.1"); > + ahci_drive_get(ahci, hd); > + ahci_ide_create_devs(ahci, hd); > > if (usb_enabled(false)) { > /* Should we create 6 UHCI according to ich9 spec? */ > diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c > index ba69de3..ae28de4 100644 > --- a/hw/ide/ahci.c > +++ b/hw/ide/ahci.c > @@ -1402,3 +1402,34 @@ static void sysbus_ahci_register_types(void) > } > > type_init(sysbus_ahci_register_types) > + > +void ahci_drive_get(PCIDevice *dev, DriveInfo **tab) > +{ > + AHCIPCIState *d = ICH_AHCI(dev); > + AHCIState *ahci = &d->ahci; > + unsigned i; > + > + if ((i = drive_get_max_bus(IF_IDE)) >= ahci->ports) {
I might be one of the strongest advocates for brevity on this list, but even I frown on embedding assignments in conditionals without a genuine need, and on reusing loop counters for unrelated purposes. Moreover, you're mixing signed and unsigned: drive_get_max_bus() returns int, ahci->ports is int32_t, but your i is unsigned. Breaks when drive_get_max_bus() returns -1 because no IF_IDE drives are defined: $ qemu -vnc :0 -M q35 -nodefaults AHCI: Too many IDE buses defined for AHCI (-1 > 5) Stick to int. int n, i; n = drive_get_max_bus(IF_IDE); if (n >= ahci->ports) { > + fprintf(stderr, "AHCI: Too many IDE buses defined for AHCI (%d > > %d)\n", > + i, ahci->ports - 1); > + } > + > + for (i = 0; i < ahci->ports; ++i) { Compares unsigned i with signed ahci->ports. Stick to int. > + tab[i] = drive_get_by_index(IF_IDE, i); > + } > +} > + > +void ahci_ide_create_devs(PCIDevice *dev, DriveInfo **tab) > +{ > + AHCIPCIState *d = ICH_AHCI(dev); > + AHCIState *ahci = &d->ahci; > + unsigned i; > + > + for (i = 0; i < ahci->ports; i++) { Likewise. > + if (tab[i] == NULL) { > + continue; > + } > + ide_create_drive(&ahci->dev[i].port, 0, tab[i]); > + } > + > +} > diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h > index 1543df7..06a18de 100644 > --- a/hw/ide/ahci.h > +++ b/hw/ide/ahci.h > @@ -332,4 +332,7 @@ void ahci_uninit(AHCIState *s); > > void ahci_reset(AHCIState *s); > > +void ahci_drive_get(PCIDevice *dev, DriveInfo **tab); > +void ahci_ide_create_devs(PCIDevice *dev, DriveInfo **tab); > + > #endif /* HW_IDE_AHCI_H */