Patchwork [01/34] qdev: Eliminate qdev_free() in favor of QOM

login
register
mail settings
Submitter Andreas Färber
Date Nov. 26, 2012, 12:12 a.m.
Message ID <1353888766-6951-2-git-send-email-afaerber@suse.de>
Download mbox | patch
Permalink /patch/201609/
State New
Headers show

Comments

Andreas Färber - Nov. 26, 2012, 12:12 a.m.
qdev_free() was trivially wrapping object_delete(). Inline it and drop
the wrapper as a further step to QOM'ify qdev.

Signed-off-by: Andreas Färber <afaerber@suse.de>
Cc: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/acpi_piix4.c      |    2 +-
 hw/pci-hotplug.c     |    2 +-
 hw/pci_bridge.c      |    2 +-
 hw/pcie.c            |    2 +-
 hw/qdev-core.h       |    1 -
 hw/qdev-monitor.c    |    2 +-
 hw/qdev.c            |   12 +++---------
 hw/scsi-bus.c        |    4 ++--
 hw/shpc.c            |    2 +-
 hw/usb/bus.c         |    7 ++++---
 hw/usb/dev-storage.c |    2 +-
 hw/usb/host-linux.c  |    2 +-
 hw/xen_platform.c    |    2 +-
 13 Dateien geändert, 18 Zeilen hinzugefügt(+), 24 Zeilen entfernt(-)
Paolo Bonzini - Nov. 26, 2012, 7:20 a.m.
Il 26/11/2012 01:12, Andreas Färber ha scritto:
> qdev_free() was trivially wrapping object_delete(). Inline it and drop
> the wrapper as a further step to QOM'ify qdev.

The reference counts of devices and buses are completely disconnected
from reality, and that's why object_delete() works.

I'd prefer this patch not to be applied until this is straightened out...

Paolo

> Signed-off-by: Andreas Färber <afaerber@suse.de>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/acpi_piix4.c      |    2 +-
>  hw/pci-hotplug.c     |    2 +-
>  hw/pci_bridge.c      |    2 +-
>  hw/pcie.c            |    2 +-
>  hw/qdev-core.h       |    1 -
>  hw/qdev-monitor.c    |    2 +-
>  hw/qdev.c            |   12 +++---------
>  hw/scsi-bus.c        |    4 ++--
>  hw/shpc.c            |    2 +-
>  hw/usb/bus.c         |    7 ++++---
>  hw/usb/dev-storage.c |    2 +-
>  hw/usb/host-linux.c  |    2 +-
>  hw/xen_platform.c    |    2 +-
>  13 Dateien geändert, 18 Zeilen hinzugefügt(+), 24 Zeilen entfernt(-)
> 
> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> index 0fa4c98..e6d09be 100644
> --- a/hw/acpi_piix4.c
> +++ b/hw/acpi_piix4.c
> @@ -361,7 +361,7 @@ static void acpi_piix_eject_slot(PIIX4PMState *s, unsigned slots)
>              if (pc->no_hotplug) {
>                  slot_free = false;
>              } else {
> -                qdev_free(qdev);
> +                object_delete(OBJECT(qdev));
>              }
>          }
>      }
> diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
> index e7fb780..d766eab 100644
> --- a/hw/pci-hotplug.c
> +++ b/hw/pci-hotplug.c
> @@ -204,7 +204,7 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
>          }
>          dev = pci_create(bus, devfn, "virtio-blk-pci");
>          if (qdev_prop_set_drive(&dev->qdev, "drive", dinfo->bdrv) < 0) {
> -            qdev_free(&dev->qdev);
> +            object_delete(OBJECT(dev));
>              dev = NULL;
>              break;
>          }
> diff --git a/hw/pci_bridge.c b/hw/pci_bridge.c
> index 4680501..3689502 100644
> --- a/hw/pci_bridge.c
> +++ b/hw/pci_bridge.c
> @@ -347,7 +347,7 @@ void pci_bridge_exitfn(PCIDevice *pci_dev)
>      pci_bridge_region_cleanup(s, s->windows);
>      memory_region_destroy(&s->address_space_mem);
>      memory_region_destroy(&s->address_space_io);
> -    /* qbus_free() is called automatically by qdev_free() */
> +    /* qbus_free() is called automatically by object_delete() */
>  }
>  
>  /*
> diff --git a/hw/pcie.c b/hw/pcie.c
> index 7c92f19..24bfc95 100644
> --- a/hw/pcie.c
> +++ b/hw/pcie.c
> @@ -235,7 +235,7 @@ static int pcie_cap_slot_hotplug(DeviceState *qdev,
>                                     PCI_EXP_SLTSTA_PDS);
>          pcie_cap_slot_event(d, PCI_EXP_HP_EV_PDC);
>      } else {
> -        qdev_free(&pci_dev->qdev);
> +        object_delete(OBJECT(pci_dev));
>          pci_word_test_and_clear_mask(exp_cap + PCI_EXP_SLTSTA,
>                                       PCI_EXP_SLTSTA_PDS);
>          pcie_cap_slot_event(d, PCI_EXP_HP_EV_PDC);
> diff --git a/hw/qdev-core.h b/hw/qdev-core.h
> index fce9e22..36191e9 100644
> --- a/hw/qdev-core.h
> +++ b/hw/qdev-core.h
> @@ -158,7 +158,6 @@ void qdev_init_nofail(DeviceState *dev);
>  void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
>                                   int required_for_version);
>  void qdev_unplug(DeviceState *dev, Error **errp);
> -void qdev_free(DeviceState *dev);
>  int qdev_simple_unplug_cb(DeviceState *dev);
>  void qdev_machine_creation_done(void);
>  bool qdev_machine_modified(void);
> diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
> index 479eecd..d534b9d 100644
> --- a/hw/qdev-monitor.c
> +++ b/hw/qdev-monitor.c
> @@ -466,7 +466,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>          qdev->id = id;
>      }
>      if (qemu_opt_foreach(opts, set_property, qdev, 1) != 0) {
> -        qdev_free(qdev);
> +        object_delete(OBJECT(qdev));
>          return NULL;
>      }
>      if (qdev->id) {
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 7ddcd24..c2d9b48 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -154,7 +154,7 @@ int qdev_init(DeviceState *dev)
>  
>      rc = dc->init(dev);
>      if (rc < 0) {
> -        qdev_free(dev);
> +        object_delete(OBJECT(dev));
>          return rc;
>      }
>  
> @@ -237,7 +237,7 @@ void qbus_reset_all_fn(void *opaque)
>  int qdev_simple_unplug_cb(DeviceState *dev)
>  {
>      /* just zap it */
> -    qdev_free(dev);
> +    object_delete(OBJECT(dev));
>      return 0;
>  }
>  
> @@ -259,12 +259,6 @@ void qdev_init_nofail(DeviceState *dev)
>      }
>  }
>  
> -/* Unlink device from bus and free the structure.  */
> -void qdev_free(DeviceState *dev)
> -{
> -    object_delete(OBJECT(dev));
> -}
> -
>  void qdev_machine_creation_done(void)
>  {
>      /*
> @@ -765,7 +759,7 @@ static void qbus_finalize(Object *obj)
>  
>      while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) {
>          DeviceState *dev = kid->child;
> -        qdev_free(dev);
> +        object_delete(OBJECT(dev));
>      }
>      if (bus->parent) {
>          QLIST_REMOVE(bus, sibling);
> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
> index dfb2631..68a27eb 100644
> --- a/hw/scsi-bus.c
> +++ b/hw/scsi-bus.c
> @@ -175,7 +175,7 @@ static int scsi_qdev_init(DeviceState *qdev)
>          d = scsi_device_find(bus, dev->channel, dev->id, dev->lun);
>          assert(d);
>          if (d->lun == dev->lun && dev != d) {
> -            qdev_free(&d->qdev);
> +            object_delete(OBJECT(d));
>          }
>      }
>  
> @@ -222,7 +222,7 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv,
>          qdev_prop_set_bit(dev, "removable", removable);
>      }
>      if (qdev_prop_set_drive(dev, "drive", bdrv) < 0) {
> -        qdev_free(dev);
> +        object_delete(OBJECT(dev));
>          return NULL;
>      }
>      if (qdev_init(dev) < 0)
> diff --git a/hw/shpc.c b/hw/shpc.c
> index 4597bbd..24bdfb8 100644
> --- a/hw/shpc.c
> +++ b/hw/shpc.c
> @@ -253,7 +253,7 @@ static void shpc_free_devices_in_slot(SHPCDevice *shpc, int slot)
>           ++devfn) {
>          PCIDevice *affected_dev = shpc->sec_bus->devices[devfn];
>          if (affected_dev) {
> -            qdev_free(&affected_dev->qdev);
> +            object_delete(OBJECT(affected_dev));
>          }
>      }
>  }
> diff --git a/hw/usb/bus.c b/hw/usb/bus.c
> index 99aac7a..1675798 100644
> --- a/hw/usb/bus.c
> +++ b/hw/usb/bus.c
> @@ -337,8 +337,9 @@ void usb_port_location(USBPort *downstream, USBPort *upstream, int portnr)
>  
>  void usb_unregister_port(USBBus *bus, USBPort *port)
>  {
> -    if (port->dev)
> -        qdev_free(&port->dev->qdev);
> +    if (port->dev) {
> +        object_delete(OBJECT(port->dev));
> +    }
>      QTAILQ_REMOVE(&bus->free, port, next);
>      bus->nfree--;
>  }
> @@ -458,7 +459,7 @@ int usb_device_delete_addr(int busnr, int addr)
>          return -1;
>      dev = port->dev;
>  
> -    qdev_free(&dev->qdev);
> +    object_delete(OBJECT(dev));
>      return 0;
>  }
>  
> diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
> index 50af971..2efb4d5 100644
> --- a/hw/usb/dev-storage.c
> +++ b/hw/usb/dev-storage.c
> @@ -664,7 +664,7 @@ static USBDevice *usb_msd_init(USBBus *bus, const char *filename)
>          return NULL;
>      }
>      if (qdev_prop_set_drive(&dev->qdev, "drive", dinfo->bdrv) < 0) {
> -        qdev_free(&dev->qdev);
> +        object_delete(OBJECT(dev));
>          return NULL;
>      }
>      if (qdev_init(&dev->qdev) < 0)
> diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c
> index aa77b77..a0417fc 100644
> --- a/hw/usb/host-linux.c
> +++ b/hw/usb/host-linux.c
> @@ -1584,7 +1584,7 @@ USBDevice *usb_host_device_open(USBBus *bus, const char *devname)
>      return dev;
>  
>  fail:
> -    qdev_free(&dev->qdev);
> +    object_delete(OBJECT(dev));
>      return NULL;
>  }
>  
> diff --git a/hw/xen_platform.c b/hw/xen_platform.c
> index a54e7a2..d56f3e5 100644
> --- a/hw/xen_platform.c
> +++ b/hw/xen_platform.c
> @@ -89,7 +89,7 @@ static void unplug_nic(PCIBus *b, PCIDevice *d, void *o)
>      if (pci_get_word(d->config + PCI_CLASS_DEVICE) ==
>              PCI_CLASS_NETWORK_ETHERNET
>              && strcmp(d->name, "xen-pci-passthrough") != 0) {
> -        qdev_free(&d->qdev);
> +        object_delete(OBJECT(d));
>      }
>  }
>  
>
Andreas Färber - Nov. 26, 2012, 11:52 a.m.
Am 26.11.2012 08:20, schrieb Paolo Bonzini:
> Il 26/11/2012 01:12, Andreas Färber ha scritto:
>> qdev_free() was trivially wrapping object_delete(). Inline it and drop
>> the wrapper as a further step to QOM'ify qdev.
> 
> The reference counts of devices and buses are completely disconnected
> from reality, and that's why object_delete() works.
> 
> I'd prefer this patch not to be applied until this is straightened out...

Your series is marked for-1.3 whereas this is for 1.4, so no obvious
problem there - wanted to mention the overlap in the cover letter but
apparently forgot.

My quest was to kill obsolete things with old "qdev" in the name. :)
I don't really mind if this becomes object_delete() or object_unref() or
anything else as long as it's not such a trivial qdev_* wrapper calling
one QOM object_* function.

Andreas
Paolo Bonzini - Nov. 26, 2012, 12:04 p.m.
Il 26/11/2012 12:52, Andreas Färber ha scritto:
> Am 26.11.2012 08:20, schrieb Paolo Bonzini:
>> Il 26/11/2012 01:12, Andreas Färber ha scritto:
>>> qdev_free() was trivially wrapping object_delete(). Inline it and drop
>>> the wrapper as a further step to QOM'ify qdev.
>>
>> The reference counts of devices and buses are completely disconnected
>> from reality, and that's why object_delete() works.
>>
>> I'd prefer this patch not to be applied until this is straightened out...
> 
> Your series is marked for-1.3 whereas this is for 1.4, so no obvious
> problem there - wanted to mention the overlap in the cover letter but
> apparently forgot.

I have a more complete series for 1.4 (needs a bit more testing, but I
guess I can send out an RFC).  The one I posted for 1.3 is a subset.

> My quest was to kill obsolete things with old "qdev" in the name. :)
> I don't really mind if this becomes object_delete() or object_unref() or
> anything else as long as it's not such a trivial qdev_* wrapper calling
> one QOM object_* function.

Ok, that's fine.

Paolo

Patch

diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 0fa4c98..e6d09be 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -361,7 +361,7 @@  static void acpi_piix_eject_slot(PIIX4PMState *s, unsigned slots)
             if (pc->no_hotplug) {
                 slot_free = false;
             } else {
-                qdev_free(qdev);
+                object_delete(OBJECT(qdev));
             }
         }
     }
diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index e7fb780..d766eab 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -204,7 +204,7 @@  static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
         }
         dev = pci_create(bus, devfn, "virtio-blk-pci");
         if (qdev_prop_set_drive(&dev->qdev, "drive", dinfo->bdrv) < 0) {
-            qdev_free(&dev->qdev);
+            object_delete(OBJECT(dev));
             dev = NULL;
             break;
         }
diff --git a/hw/pci_bridge.c b/hw/pci_bridge.c
index 4680501..3689502 100644
--- a/hw/pci_bridge.c
+++ b/hw/pci_bridge.c
@@ -347,7 +347,7 @@  void pci_bridge_exitfn(PCIDevice *pci_dev)
     pci_bridge_region_cleanup(s, s->windows);
     memory_region_destroy(&s->address_space_mem);
     memory_region_destroy(&s->address_space_io);
-    /* qbus_free() is called automatically by qdev_free() */
+    /* qbus_free() is called automatically by object_delete() */
 }
 
 /*
diff --git a/hw/pcie.c b/hw/pcie.c
index 7c92f19..24bfc95 100644
--- a/hw/pcie.c
+++ b/hw/pcie.c
@@ -235,7 +235,7 @@  static int pcie_cap_slot_hotplug(DeviceState *qdev,
                                    PCI_EXP_SLTSTA_PDS);
         pcie_cap_slot_event(d, PCI_EXP_HP_EV_PDC);
     } else {
-        qdev_free(&pci_dev->qdev);
+        object_delete(OBJECT(pci_dev));
         pci_word_test_and_clear_mask(exp_cap + PCI_EXP_SLTSTA,
                                      PCI_EXP_SLTSTA_PDS);
         pcie_cap_slot_event(d, PCI_EXP_HP_EV_PDC);
diff --git a/hw/qdev-core.h b/hw/qdev-core.h
index fce9e22..36191e9 100644
--- a/hw/qdev-core.h
+++ b/hw/qdev-core.h
@@ -158,7 +158,6 @@  void qdev_init_nofail(DeviceState *dev);
 void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
                                  int required_for_version);
 void qdev_unplug(DeviceState *dev, Error **errp);
-void qdev_free(DeviceState *dev);
 int qdev_simple_unplug_cb(DeviceState *dev);
 void qdev_machine_creation_done(void);
 bool qdev_machine_modified(void);
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index 479eecd..d534b9d 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -466,7 +466,7 @@  DeviceState *qdev_device_add(QemuOpts *opts)
         qdev->id = id;
     }
     if (qemu_opt_foreach(opts, set_property, qdev, 1) != 0) {
-        qdev_free(qdev);
+        object_delete(OBJECT(qdev));
         return NULL;
     }
     if (qdev->id) {
diff --git a/hw/qdev.c b/hw/qdev.c
index 7ddcd24..c2d9b48 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -154,7 +154,7 @@  int qdev_init(DeviceState *dev)
 
     rc = dc->init(dev);
     if (rc < 0) {
-        qdev_free(dev);
+        object_delete(OBJECT(dev));
         return rc;
     }
 
@@ -237,7 +237,7 @@  void qbus_reset_all_fn(void *opaque)
 int qdev_simple_unplug_cb(DeviceState *dev)
 {
     /* just zap it */
-    qdev_free(dev);
+    object_delete(OBJECT(dev));
     return 0;
 }
 
@@ -259,12 +259,6 @@  void qdev_init_nofail(DeviceState *dev)
     }
 }
 
-/* Unlink device from bus and free the structure.  */
-void qdev_free(DeviceState *dev)
-{
-    object_delete(OBJECT(dev));
-}
-
 void qdev_machine_creation_done(void)
 {
     /*
@@ -765,7 +759,7 @@  static void qbus_finalize(Object *obj)
 
     while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) {
         DeviceState *dev = kid->child;
-        qdev_free(dev);
+        object_delete(OBJECT(dev));
     }
     if (bus->parent) {
         QLIST_REMOVE(bus, sibling);
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index dfb2631..68a27eb 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -175,7 +175,7 @@  static int scsi_qdev_init(DeviceState *qdev)
         d = scsi_device_find(bus, dev->channel, dev->id, dev->lun);
         assert(d);
         if (d->lun == dev->lun && dev != d) {
-            qdev_free(&d->qdev);
+            object_delete(OBJECT(d));
         }
     }
 
@@ -222,7 +222,7 @@  SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv,
         qdev_prop_set_bit(dev, "removable", removable);
     }
     if (qdev_prop_set_drive(dev, "drive", bdrv) < 0) {
-        qdev_free(dev);
+        object_delete(OBJECT(dev));
         return NULL;
     }
     if (qdev_init(dev) < 0)
diff --git a/hw/shpc.c b/hw/shpc.c
index 4597bbd..24bdfb8 100644
--- a/hw/shpc.c
+++ b/hw/shpc.c
@@ -253,7 +253,7 @@  static void shpc_free_devices_in_slot(SHPCDevice *shpc, int slot)
          ++devfn) {
         PCIDevice *affected_dev = shpc->sec_bus->devices[devfn];
         if (affected_dev) {
-            qdev_free(&affected_dev->qdev);
+            object_delete(OBJECT(affected_dev));
         }
     }
 }
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 99aac7a..1675798 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -337,8 +337,9 @@  void usb_port_location(USBPort *downstream, USBPort *upstream, int portnr)
 
 void usb_unregister_port(USBBus *bus, USBPort *port)
 {
-    if (port->dev)
-        qdev_free(&port->dev->qdev);
+    if (port->dev) {
+        object_delete(OBJECT(port->dev));
+    }
     QTAILQ_REMOVE(&bus->free, port, next);
     bus->nfree--;
 }
@@ -458,7 +459,7 @@  int usb_device_delete_addr(int busnr, int addr)
         return -1;
     dev = port->dev;
 
-    qdev_free(&dev->qdev);
+    object_delete(OBJECT(dev));
     return 0;
 }
 
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 50af971..2efb4d5 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -664,7 +664,7 @@  static USBDevice *usb_msd_init(USBBus *bus, const char *filename)
         return NULL;
     }
     if (qdev_prop_set_drive(&dev->qdev, "drive", dinfo->bdrv) < 0) {
-        qdev_free(&dev->qdev);
+        object_delete(OBJECT(dev));
         return NULL;
     }
     if (qdev_init(&dev->qdev) < 0)
diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c
index aa77b77..a0417fc 100644
--- a/hw/usb/host-linux.c
+++ b/hw/usb/host-linux.c
@@ -1584,7 +1584,7 @@  USBDevice *usb_host_device_open(USBBus *bus, const char *devname)
     return dev;
 
 fail:
-    qdev_free(&dev->qdev);
+    object_delete(OBJECT(dev));
     return NULL;
 }
 
diff --git a/hw/xen_platform.c b/hw/xen_platform.c
index a54e7a2..d56f3e5 100644
--- a/hw/xen_platform.c
+++ b/hw/xen_platform.c
@@ -89,7 +89,7 @@  static void unplug_nic(PCIBus *b, PCIDevice *d, void *o)
     if (pci_get_word(d->config + PCI_CLASS_DEVICE) ==
             PCI_CLASS_NETWORK_ETHERNET
             && strcmp(d->name, "xen-pci-passthrough") != 0) {
-        qdev_free(&d->qdev);
+        object_delete(OBJECT(d));
     }
 }