Re: [Qemu-devel] [PATCH 09/30] qdev: move qdev->info to class

["Andreas Färber" <afaerber@suse.de>]

Am 03.01.2012 01:51, schrieb Anthony Liguori:
> Right now, DeviceInfo acts as the class for qdev.  In order to switch to a
> proper ObjectClass derivative, we need to ween all of the callers off of
> interacting directly with the info pointer.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---

> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> index 3473345..c0e3450 100644
> --- a/hw/ide/piix.c
> +++ b/hw/ide/piix.c
> @@ -195,7 +195,6 @@ PCIDevice *pci_piix3_xen_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
>      PCIDevice *dev;
>  
>      dev = pci_create_simple(bus, devfn, "piix3-ide-xen");
> -    dev->qdev.info->unplug = pci_piix3_xen_ide_unplug;
>      pci_ide_create_devs(dev, hd_table);
>      return dev;
>  }
> @@ -253,6 +252,7 @@ static PCIDeviceInfo piix_ide_info[] = {
>          .qdev.name    = "piix3-ide-xen",
>          .qdev.size    = sizeof(PCIIDEState),
>          .qdev.no_user = 1,
> +        .qdev.unplug  = pci_piix3_xen_ide_unplug,
>          .init         = pci_piix_ide_initfn,
>          .vendor_id    = PCI_VENDOR_ID_INTEL,
>          .device_id    = PCI_DEVICE_ID_INTEL_82371SB_1,

> diff --git a/hw/pci.c b/hw/pci.c
> index fd575ac..64cd08f 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c

> @@ -1544,7 +1544,9 @@ static int pci_unplug_device(DeviceState *qdev)
>  void pci_qdev_register(PCIDeviceInfo *info)
>  {
>      info->qdev.init = pci_qdev_init;
> -    info->qdev.unplug = pci_unplug_device;
> +    if (!info->qdev.unplug) {
> +        info->qdev.unplug = pci_unplug_device;
> +    }
>      info->qdev.exit = pci_unregister_device;
>      info->qdev.bus_info = &pci_bus_info;
>      qdev_register(&info->qdev);

Would've been worth its own preparatory patch IMO since going beyond
pure qdev_get_info(&dev->qdev)->unplug conversion for piix3-ide-xen.

> @@ -1757,10 +1759,10 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
>          size = 1 << qemu_fls(size);
>      }
>  
> -    if (pdev->qdev.info->vmsd)
> -        snprintf(name, sizeof(name), "%s.rom", pdev->qdev.info->vmsd->name);
> +    if (qdev_get_info(&pdev->qdev)->vmsd)
> +        snprintf(name, sizeof(name), "%s.rom", qdev_get_info(&pdev->qdev)->vmsd->name);
>      else
> -        snprintf(name, sizeof(name), "%s.rom", pdev->qdev.info->name);
> +        snprintf(name, sizeof(name), "%s.rom", qdev_get_info(&pdev->qdev)->name);

Add braces while you're touching it?

Otherwise mostly mechanical and looks okay:

Reviewed-by: Andreas Färber <afaerber@suse.de>

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg