[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC v2 3/3] ahci: implement -cdrom and -hd[a-d]
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [RFC v2 3/3] ahci: implement -cdrom and -hd[a-d] |
Date: |
Fri, 19 Sep 2014 11:49:35 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
John Snow <address@hidden> writes:
> Signed-off-by: John Snow <address@hidden>
> ---
> 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 */