Re: [PATCH v2 07/58] qdev: Convert to qdev_unrealize() with Coccinelle

[Philippe Mathieu-Daudé]

On 5/29/20 3:44 PM, Markus Armbruster wrote:
> For readability, and consistency with qbus_realize().
> 
> Coccinelle script:
> 
>     @ depends on !(file in "hw/core/qdev.c")@
>     typedef DeviceState;
>     DeviceState *dev;
>     symbol false, error_abort;
>     @@
>     -    object_property_set_bool(OBJECT(dev), false, "realized", 
> &error_abort);
>     +    qdev_unrealize(dev);
> 
>     @ depends on !(file in "hw/core/qdev.c")@
>     expression dev;
>     symbol false, error_abort;
>     @@
>     -    object_property_set_bool(OBJECT(dev), false, "realized", 
> &error_abort);
>     +    qdev_unrealize(DEVICE(dev));
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/acpi/pcihp.c            | 2 +-
>  hw/char/serial-pci-multi.c | 2 +-
>  hw/char/serial-pci.c       | 2 +-
>  hw/core/bus.c              | 3 +--
>  hw/i386/pc.c               | 4 ++--
>  hw/pci/pcie.c              | 2 +-
>  hw/pci/shpc.c              | 2 +-
>  hw/ppc/spapr.c             | 8 ++++----
>  hw/ppc/spapr_pci.c         | 3 +--
>  hw/s390x/css-bridge.c      | 2 +-
>  hw/s390x/s390-pci-bus.c    | 4 ++--
>  11 files changed, 16 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> index d42906ea19..33ea2b76ae 100644
> --- a/hw/acpi/pcihp.c
> +++ b/hw/acpi/pcihp.c
> @@ -266,7 +266,7 @@ void acpi_pcihp_device_unplug_cb(HotplugHandler 
> *hotplug_dev, AcpiPciHpState *s,
>  {
>      trace_acpi_pci_unplug(PCI_SLOT(PCI_DEVICE(dev)->devfn),
>                            acpi_pcihp_get_bsel(pci_get_bus(PCI_DEVICE(dev))));
> -    object_property_set_bool(OBJECT(dev), false, "realized", &error_abort);
> +    qdev_unrealize(dev);
>  }
>  
>  void acpi_pcihp_device_unplug_request_cb(HotplugHandler *hotplug_dev,
> diff --git a/hw/char/serial-pci-multi.c b/hw/char/serial-pci-multi.c
> index 5f9ccfcc93..23d0ebe2cd 100644
> --- a/hw/char/serial-pci-multi.c
> +++ b/hw/char/serial-pci-multi.c
> @@ -56,7 +56,7 @@ static void multi_serial_pci_exit(PCIDevice *dev)
>  
>      for (i = 0; i < pci->ports; i++) {
>          s = pci->state + i;
> -        object_property_set_bool(OBJECT(s), false, "realized", &error_abort);
> +        qdev_unrealize(DEVICE(s));
>          memory_region_del_subregion(&pci->iobar, &s->io);
>          g_free(pci->name[i]);
>      }
> diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c
> index 37818db156..65eacfae0e 100644
> --- a/hw/char/serial-pci.c
> +++ b/hw/char/serial-pci.c
> @@ -68,7 +68,7 @@ static void serial_pci_exit(PCIDevice *dev)
>      PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev);
>      SerialState *s = &pci->state;
>  
> -    object_property_set_bool(OBJECT(s), false, "realized", &error_abort);
> +    qdev_unrealize(DEVICE(s));
>      qemu_free_irq(s->irq);

Ah, unrealize() isn't finalize(), so 's' is still a valid memory.
(Comment unrelated to your patch, just reviewing non-obvious code).

>  }
>  
> diff --git a/hw/core/bus.c b/hw/core/bus.c
> index 6f6071f5fa..6cc28b334e 100644
> --- a/hw/core/bus.c
> +++ b/hw/core/bus.c
> @@ -200,8 +200,7 @@ static void bus_set_realized(Object *obj, bool value, 
> Error **errp)
>      } else if (!value && bus->realized) {
>          QTAILQ_FOREACH(kid, &bus->children, sibling) {
>              DeviceState *dev = kid->child;
> -            object_property_set_bool(OBJECT(dev), false, "realized",
> -                                     &error_abort);
> +            qdev_unrealize(dev);
>          }
>          if (bc->unrealize) {
>              bc->unrealize(bus);
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 2128f3d6fe..f9d51479b1 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1385,7 +1385,7 @@ static void pc_memory_unplug(HotplugHandler 
> *hotplug_dev,
>      }
>  
>      pc_dimm_unplug(PC_DIMM(dev), MACHINE(pcms));
> -    object_property_set_bool(OBJECT(dev), false, "realized", &error_abort);
> +    qdev_unrealize(dev);
>   out:
>      error_propagate(errp, local_err);
>  }
> @@ -1493,7 +1493,7 @@ static void pc_cpu_unplug_cb(HotplugHandler 
> *hotplug_dev,
>  
>      found_cpu = pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, NULL);
>      found_cpu->cpu = NULL;
> -    object_property_set_bool(OBJECT(dev), false, "realized", &error_abort);
> +    qdev_unrealize(dev);
>  
>      /* decrement the number of CPUs */
>      x86ms->boot_cpus--;
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index f50e10b8fb..582f81fdff 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -457,7 +457,7 @@ void pcie_cap_slot_plug_cb(HotplugHandler *hotplug_dev, 
> DeviceState *dev,
>  void pcie_cap_slot_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
>                               Error **errp)
>  {
> -    object_property_set_bool(OBJECT(dev), false, "realized", &error_abort);
> +    qdev_unrealize(dev);
>  }
>  
>  static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque)
> diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
> index b76d3d2c9a..99d65d5c4c 100644
> --- a/hw/pci/shpc.c
> +++ b/hw/pci/shpc.c
> @@ -547,7 +547,7 @@ void shpc_device_plug_cb(HotplugHandler *hotplug_dev, 
> DeviceState *dev,
>  void shpc_device_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
>                             Error **errp)
>  {
> -    object_property_set_bool(OBJECT(dev), false, "realized", &error_abort);
> +    qdev_unrealize(dev);
>  }
>  
>  void shpc_device_unplug_request_cb(HotplugHandler *hotplug_dev,
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 3b1a5ed865..6a315c0dc8 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3671,7 +3671,7 @@ static void spapr_memory_unplug(HotplugHandler 
> *hotplug_dev, DeviceState *dev)
>      SpaprDimmState *ds = spapr_pending_dimm_unplugs_find(spapr, 
> PC_DIMM(dev));
>  
>      pc_dimm_unplug(PC_DIMM(dev), MACHINE(hotplug_dev));
> -    object_property_set_bool(OBJECT(dev), false, "realized", &error_abort);
> +    qdev_unrealize(dev);
>      spapr_pending_dimm_unplugs_remove(spapr, ds);
>  }
>  
> @@ -3764,7 +3764,7 @@ static void spapr_core_unplug(HotplugHandler 
> *hotplug_dev, DeviceState *dev)
>  
>      assert(core_slot);
>      core_slot->cpu = NULL;
> -    object_property_set_bool(OBJECT(dev), false, "realized", &error_abort);
> +    qdev_unrealize(dev);
>  }
>  
>  static
> @@ -4037,7 +4037,7 @@ void spapr_phb_release(DeviceState *dev)
>  
>  static void spapr_phb_unplug(HotplugHandler *hotplug_dev, DeviceState *dev)
>  {
> -    object_property_set_bool(OBJECT(dev), false, "realized", &error_abort);
> +    qdev_unrealize(dev);
>  }
>  
>  static void spapr_phb_unplug_request(HotplugHandler *hotplug_dev,
> @@ -4073,7 +4073,7 @@ static void spapr_tpm_proxy_unplug(HotplugHandler 
> *hotplug_dev, DeviceState *dev
>  {
>      SpaprMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev));
>  
> -    object_property_set_bool(OBJECT(dev), false, "realized", &error_abort);
> +    qdev_unrealize(dev);
>      object_unparent(OBJECT(dev));
>      spapr->tpm_proxy = NULL;

Ditto, OK.

>  }
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 83f1453096..329002ac04 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1587,8 +1587,7 @@ static void spapr_pci_unplug(HotplugHandler 
> *plug_handler,
>          return;
>      }
>  
> -    object_property_set_bool(OBJECT(plugged_dev), false, "realized",
> -                             &error_abort);
> +    qdev_unrealize(plugged_dev);
>  }
>  
>  static void spapr_pci_unplug_request(HotplugHandler *plug_handler,
> diff --git a/hw/s390x/css-bridge.c b/hw/s390x/css-bridge.c
> index 3f6aec6b6a..813bfc768a 100644
> --- a/hw/s390x/css-bridge.c
> +++ b/hw/s390x/css-bridge.c
> @@ -54,7 +54,7 @@ static void ccw_device_unplug(HotplugHandler *hotplug_dev,
>  
>      css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid, 1, 0);
>  
> -    object_property_set_bool(OBJECT(dev), false, "realized", &error_abort);
> +    qdev_unrealize(dev);
>  }
>  
>  static void virtual_css_bus_reset(BusState *qbus)
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index c4a4259f0c..7a4bfb7383 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -1003,7 +1003,7 @@ static void s390_pcihost_unplug(HotplugHandler 
> *hotplug_dev, DeviceState *dev,
>                                       pbdev->fh, pbdev->fid);
>          bus = pci_get_bus(pci_dev);
>          devfn = pci_dev->devfn;
> -        object_property_set_bool(OBJECT(dev), false, "realized", 
> &error_abort);
> +        qdev_unrealize(dev);
>  
>          s390_pci_msix_free(pbdev);
>          s390_pci_iommu_free(s, bus, devfn);
> @@ -1014,7 +1014,7 @@ static void s390_pcihost_unplug(HotplugHandler 
> *hotplug_dev, DeviceState *dev,
>          pbdev->fid = 0;
>          QTAILQ_REMOVE(&s->zpci_devs, pbdev, link);
>          g_hash_table_remove(s->zpci_table, &pbdev->idx);
> -        object_property_set_bool(OBJECT(dev), false, "realized", 
> &error_abort);
> +        qdev_unrealize(dev);
>      }
>  }
>  
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>