| Message ID | 20200529134523.8477-13-armbru@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qdev: Rework how we plug into the parent bus | expand |
On 5/29/20 3:44 PM, Markus Armbruster wrote: > Same transformation as in the previous commit. Manual, because > convincing Coccinelle to transform these cases is somewhere between > not worthwhile and infeasible (at least for me). > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > hw/pci-host/prep.c | 3 +-- > hw/ppc/pnv.c | 6 ++---- > hw/s390x/sclp.c | 10 ++++------ > 3 files changed, 7 insertions(+), 12 deletions(-) > > diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c > index c821ef889d..42c7e63a60 100644 > --- a/hw/pci-host/prep.c > +++ b/hw/pci-host/prep.c > @@ -268,7 +268,7 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp) > memory_region_add_subregion(address_space_mem, 0xbffffff0, &s->pci_intack); > > /* TODO Remove once realize propagates to child devices. */ > - object_property_set_bool(OBJECT(&s->pci_dev), true, "realized", errp); > + qdev_realize(DEVICE(&s->pci_dev), BUS(&s->pci_bus), errp); > } > > static void raven_pcihost_initfn(Object *obj) > @@ -308,7 +308,6 @@ static void raven_pcihost_initfn(Object *obj) > > object_initialize(&s->pci_dev, sizeof(s->pci_dev), TYPE_RAVEN_PCI_DEVICE); > pci_dev = DEVICE(&s->pci_dev); > - qdev_set_parent_bus(pci_dev, BUS(&s->pci_bus)); > object_property_set_int(OBJECT(&s->pci_dev), PCI_DEVFN(0, 0), "addr", > NULL); > qdev_prop_set_bit(pci_dev, "multifunction", false); > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > index 8562af3fe0..e0588285a2 100644 > --- a/hw/ppc/pnv.c > +++ b/hw/ppc/pnv.c > @@ -1212,12 +1212,11 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp) > object_property_set_int(OBJECT(phb), i, "index", &error_fatal); > object_property_set_int(OBJECT(phb), chip->chip_id, "chip-id", > &error_fatal); > - object_property_set_bool(OBJECT(phb), true, "realized", &local_err); > + qdev_realize(DEVICE(phb), NULL, &local_err); > if (local_err) { > error_propagate(errp, local_err); > return; > } > - qdev_set_parent_bus(DEVICE(phb), sysbus_get_default()); > > /* Populate the XSCOM address space. */ > pnv_xscom_add_subregion(chip, > @@ -1422,12 +1421,11 @@ static void pnv_chip_power9_phb_realize(PnvChip *chip, Error **errp) > object_property_set_int(obj, PNV_PHB4_DEVICE_ID, "device-id", > &error_fatal); > object_property_set_link(obj, OBJECT(stack), "stack", &error_abort); > - object_property_set_bool(obj, true, "realized", &local_err); > + qdev_realize(DEVICE(obj), NULL, &local_err); > if (local_err) { > error_propagate(errp, local_err); > return; > } > - qdev_set_parent_bus(DEVICE(obj), sysbus_get_default()); > > /* Populate the XSCOM address space. */ > pnv_xscom_add_subregion(chip, > diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c > index 20aca30ac4..40e27a8cb4 100644 > --- a/hw/s390x/sclp.c > +++ b/hw/s390x/sclp.c > @@ -333,17 +333,15 @@ static void sclp_realize(DeviceState *dev, Error **errp) > uint64_t hw_limit; > int ret; > > - object_property_set_bool(OBJECT(sclp->event_facility), true, "realized", > - &err); > - if (err) { > - goto out; > - } > /* > * qdev_device_add searches the sysbus for TYPE_SCLP_EVENTS_BUS. As long > * as we can't find a fitting bus via the qom tree, we have to add the > * event facility to the sysbus, so e.g. a sclp console can be created. > */ > - qdev_set_parent_bus(DEVICE(sclp->event_facility), sysbus_get_default()); > + qdev_realize(DEVICE(sclp->event_facility), NULL, &err); I agree with the API change, but I find using sysbus_get_default() way clearer than using NULL. I'v go the other way around, replace bus=NULL by an explicit bus=sysbus_get_default() in our codebase. Can be done later, I know... But we can also keep sysbus_get_default() in this patch ;) Whichever you prefer (mine is keeping it): Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > + if (err) { > + goto out; > + } > > ret = s390_set_memory_limit(machine->maxram_size, &hw_limit); > if (ret == -E2BIG) { >
Philippe Mathieu-Daudé <philmd@redhat.com> writes: > On 5/29/20 3:44 PM, Markus Armbruster wrote: >> Same transformation as in the previous commit. Manual, because >> convincing Coccinelle to transform these cases is somewhere between >> not worthwhile and infeasible (at least for me). >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> hw/pci-host/prep.c | 3 +-- >> hw/ppc/pnv.c | 6 ++---- >> hw/s390x/sclp.c | 10 ++++------ >> 3 files changed, 7 insertions(+), 12 deletions(-) >> >> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c >> index c821ef889d..42c7e63a60 100644 >> --- a/hw/pci-host/prep.c >> +++ b/hw/pci-host/prep.c >> @@ -268,7 +268,7 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp) >> memory_region_add_subregion(address_space_mem, 0xbffffff0, &s->pci_intack); >> >> /* TODO Remove once realize propagates to child devices. */ >> - object_property_set_bool(OBJECT(&s->pci_dev), true, "realized", errp); >> + qdev_realize(DEVICE(&s->pci_dev), BUS(&s->pci_bus), errp); >> } >> >> static void raven_pcihost_initfn(Object *obj) >> @@ -308,7 +308,6 @@ static void raven_pcihost_initfn(Object *obj) >> >> object_initialize(&s->pci_dev, sizeof(s->pci_dev), TYPE_RAVEN_PCI_DEVICE); >> pci_dev = DEVICE(&s->pci_dev); >> - qdev_set_parent_bus(pci_dev, BUS(&s->pci_bus)); >> object_property_set_int(OBJECT(&s->pci_dev), PCI_DEVFN(0, 0), "addr", >> NULL); >> qdev_prop_set_bit(pci_dev, "multifunction", false); >> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c >> index 8562af3fe0..e0588285a2 100644 >> --- a/hw/ppc/pnv.c >> +++ b/hw/ppc/pnv.c >> @@ -1212,12 +1212,11 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp) >> object_property_set_int(OBJECT(phb), i, "index", &error_fatal); >> object_property_set_int(OBJECT(phb), chip->chip_id, "chip-id", >> &error_fatal); >> - object_property_set_bool(OBJECT(phb), true, "realized", &local_err); >> + qdev_realize(DEVICE(phb), NULL, &local_err); >> if (local_err) { >> error_propagate(errp, local_err); >> return; >> } >> - qdev_set_parent_bus(DEVICE(phb), sysbus_get_default()); >> >> /* Populate the XSCOM address space. */ >> pnv_xscom_add_subregion(chip, >> @@ -1422,12 +1421,11 @@ static void pnv_chip_power9_phb_realize(PnvChip *chip, Error **errp) >> object_property_set_int(obj, PNV_PHB4_DEVICE_ID, "device-id", >> &error_fatal); >> object_property_set_link(obj, OBJECT(stack), "stack", &error_abort); >> - object_property_set_bool(obj, true, "realized", &local_err); >> + qdev_realize(DEVICE(obj), NULL, &local_err); >> if (local_err) { >> error_propagate(errp, local_err); >> return; >> } >> - qdev_set_parent_bus(DEVICE(obj), sysbus_get_default()); >> >> /* Populate the XSCOM address space. */ >> pnv_xscom_add_subregion(chip, >> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c >> index 20aca30ac4..40e27a8cb4 100644 >> --- a/hw/s390x/sclp.c >> +++ b/hw/s390x/sclp.c >> @@ -333,17 +333,15 @@ static void sclp_realize(DeviceState *dev, Error **errp) >> uint64_t hw_limit; >> int ret; >> >> - object_property_set_bool(OBJECT(sclp->event_facility), true, "realized", >> - &err); >> - if (err) { >> - goto out; >> - } >> /* >> * qdev_device_add searches the sysbus for TYPE_SCLP_EVENTS_BUS. As long >> * as we can't find a fitting bus via the qom tree, we have to add the >> * event facility to the sysbus, so e.g. a sclp console can be created. >> */ >> - qdev_set_parent_bus(DEVICE(sclp->event_facility), sysbus_get_default()); >> + qdev_realize(DEVICE(sclp->event_facility), NULL, &err); > > I agree with the API change, but I find using sysbus_get_default() way > clearer than using NULL. I'v go the other way around, replace bus=NULL > by an explicit bus=sysbus_get_default() in our codebase. I agree with you on sysbus_get_default() being clearer than NULL. I actually get rid of NULL in this series, but it takes me many more patches to get there. Starts at PATCH 45. Of course, I must not expect reviewers to look three dozen patches ahead. > Can be done later, I know... But we can also keep sysbus_get_default() > in this patch ;) This patch's purpose is getting rid of separate qdev_set_parent_bus(), in part to ease the conversion from qdev_realize() to sysbus_realize(). > Whichever you prefer (mine is keeping it): > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Thanks! > >> + if (err) { >> + goto out; >> + } >> >> ret = s390_set_memory_limit(machine->maxram_size, &hw_limit); >> if (ret == -E2BIG) { >>
diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c index c821ef889d..42c7e63a60 100644 --- a/hw/pci-host/prep.c +++ b/hw/pci-host/prep.c @@ -268,7 +268,7 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp) memory_region_add_subregion(address_space_mem, 0xbffffff0, &s->pci_intack); /* TODO Remove once realize propagates to child devices. */ - object_property_set_bool(OBJECT(&s->pci_dev), true, "realized", errp); + qdev_realize(DEVICE(&s->pci_dev), BUS(&s->pci_bus), errp); } static void raven_pcihost_initfn(Object *obj) @@ -308,7 +308,6 @@ static void raven_pcihost_initfn(Object *obj) object_initialize(&s->pci_dev, sizeof(s->pci_dev), TYPE_RAVEN_PCI_DEVICE); pci_dev = DEVICE(&s->pci_dev); - qdev_set_parent_bus(pci_dev, BUS(&s->pci_bus)); object_property_set_int(OBJECT(&s->pci_dev), PCI_DEVFN(0, 0), "addr", NULL); qdev_prop_set_bit(pci_dev, "multifunction", false); diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index 8562af3fe0..e0588285a2 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -1212,12 +1212,11 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp) object_property_set_int(OBJECT(phb), i, "index", &error_fatal); object_property_set_int(OBJECT(phb), chip->chip_id, "chip-id", &error_fatal); - object_property_set_bool(OBJECT(phb), true, "realized", &local_err); + qdev_realize(DEVICE(phb), NULL, &local_err); if (local_err) { error_propagate(errp, local_err); return; } - qdev_set_parent_bus(DEVICE(phb), sysbus_get_default()); /* Populate the XSCOM address space. */ pnv_xscom_add_subregion(chip, @@ -1422,12 +1421,11 @@ static void pnv_chip_power9_phb_realize(PnvChip *chip, Error **errp) object_property_set_int(obj, PNV_PHB4_DEVICE_ID, "device-id", &error_fatal); object_property_set_link(obj, OBJECT(stack), "stack", &error_abort); - object_property_set_bool(obj, true, "realized", &local_err); + qdev_realize(DEVICE(obj), NULL, &local_err); if (local_err) { error_propagate(errp, local_err); return; } - qdev_set_parent_bus(DEVICE(obj), sysbus_get_default()); /* Populate the XSCOM address space. */ pnv_xscom_add_subregion(chip, diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c index 20aca30ac4..40e27a8cb4 100644 --- a/hw/s390x/sclp.c +++ b/hw/s390x/sclp.c @@ -333,17 +333,15 @@ static void sclp_realize(DeviceState *dev, Error **errp) uint64_t hw_limit; int ret; - object_property_set_bool(OBJECT(sclp->event_facility), true, "realized", - &err); - if (err) { - goto out; - } /* * qdev_device_add searches the sysbus for TYPE_SCLP_EVENTS_BUS. As long * as we can't find a fitting bus via the qom tree, we have to add the * event facility to the sysbus, so e.g. a sclp console can be created. */ - qdev_set_parent_bus(DEVICE(sclp->event_facility), sysbus_get_default()); + qdev_realize(DEVICE(sclp->event_facility), NULL, &err); + if (err) { + goto out; + } ret = s390_set_memory_limit(machine->maxram_size, &hw_limit); if (ret == -E2BIG) {
Same transformation as in the previous commit. Manual, because convincing Coccinelle to transform these cases is somewhere between not worthwhile and infeasible (at least for me). Signed-off-by: Markus Armbruster <armbru@redhat.com> --- hw/pci-host/prep.c | 3 +-- hw/ppc/pnv.c | 6 ++---- hw/s390x/sclp.c | 10 ++++------ 3 files changed, 7 insertions(+), 12 deletions(-)