Message ID | 20200529134523.8477-19-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: > I'm converting from qdev_create()/qdev_init_nofail() to > qdev_new()/qdev_realize_and_unref(); recent commit "qdev: New > qdev_new(), qdev_realize(), etc." explains why. > > ISA devices use qdev_create() through isa_create() and > isa_try_create(). > > Provide isa_new(), isa_try_new(), and isa_realize_and_unref() for > converting ISA devices. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > include/hw/isa/isa.h | 3 +++ > hw/isa/isa-bus.c | 15 +++++++++++++++ > 2 files changed, 18 insertions(+) > > diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h > index 02c2350274..3b6215fafe 100644 > --- a/include/hw/isa/isa.h > +++ b/include/hw/isa/isa.h > @@ -105,6 +105,9 @@ MemoryRegion *isa_address_space(ISADevice *dev); > MemoryRegion *isa_address_space_io(ISADevice *dev); > ISADevice *isa_create(ISABus *bus, const char *name); > ISADevice *isa_try_create(ISABus *bus, const char *name); > +ISADevice *isa_new(const char *name); > +ISADevice *isa_try_new(const char *name); > +bool isa_realize_and_unref(ISADevice *dev, ISABus *bus, Error **errp); > ISADevice *isa_create_simple(ISABus *bus, const char *name); > > ISADevice *isa_vga_init(ISABus *bus); > diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c > index 1c9d7e19ab..e6412d39b4 100644 > --- a/hw/isa/isa-bus.c > +++ b/hw/isa/isa-bus.c > @@ -176,6 +176,16 @@ ISADevice *isa_try_create(ISABus *bus, const char *name) > return ISA_DEVICE(dev); > } > > +ISADevice *isa_new(const char *name) > +{ > + return ISA_DEVICE(qdev_new(name)); > +} > + > +ISADevice *isa_try_new(const char *name) > +{ > + return ISA_DEVICE(qdev_try_new(name)); We have: #define ISA_DEVICE(obj) \ OBJECT_CHECK(ISADevice, (obj), TYPE_ISA_DEVICE) With: #define OBJECT_CHECK(type, obj, name) \ ((type *)object_dynamic_cast_assert(OBJECT(obj), (name), \ __FILE__, __LINE__, __func__)) "If an invalid object is passed to this function, a run time assert will be generated." I'd expect isa_try_new() to return NULL if the type_name is not registered... > +} > + > ISADevice *isa_create_simple(ISABus *bus, const char *name) > { > ISADevice *dev; > @@ -185,6 +195,11 @@ ISADevice *isa_create_simple(ISABus *bus, const char *name) > return dev; > } > > +bool isa_realize_and_unref(ISADevice *dev, ISABus *bus, Error **errp) > +{ > + return qdev_realize_and_unref(&dev->parent_obj, &bus->parent_obj, errp); > +} > + > ISADevice *isa_vga_init(ISABus *bus) > { > switch (vga_interface_type) { >
On 6/9/20 10:19 AM, Philippe Mathieu-Daudé wrote: > On 5/29/20 3:44 PM, Markus Armbruster wrote: >> I'm converting from qdev_create()/qdev_init_nofail() to >> qdev_new()/qdev_realize_and_unref(); recent commit "qdev: New >> qdev_new(), qdev_realize(), etc." explains why. >> >> ISA devices use qdev_create() through isa_create() and >> isa_try_create(). >> >> Provide isa_new(), isa_try_new(), and isa_realize_and_unref() for >> converting ISA devices. >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> include/hw/isa/isa.h | 3 +++ >> hw/isa/isa-bus.c | 15 +++++++++++++++ >> 2 files changed, 18 insertions(+) >> >> diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h >> index 02c2350274..3b6215fafe 100644 >> --- a/include/hw/isa/isa.h >> +++ b/include/hw/isa/isa.h >> @@ -105,6 +105,9 @@ MemoryRegion *isa_address_space(ISADevice *dev); >> MemoryRegion *isa_address_space_io(ISADevice *dev); >> ISADevice *isa_create(ISABus *bus, const char *name); >> ISADevice *isa_try_create(ISABus *bus, const char *name); >> +ISADevice *isa_new(const char *name); >> +ISADevice *isa_try_new(const char *name); >> +bool isa_realize_and_unref(ISADevice *dev, ISABus *bus, Error **errp); >> ISADevice *isa_create_simple(ISABus *bus, const char *name); >> >> ISADevice *isa_vga_init(ISABus *bus); >> diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c >> index 1c9d7e19ab..e6412d39b4 100644 >> --- a/hw/isa/isa-bus.c >> +++ b/hw/isa/isa-bus.c >> @@ -176,6 +176,16 @@ ISADevice *isa_try_create(ISABus *bus, const char *name) >> return ISA_DEVICE(dev); >> } >> >> +ISADevice *isa_new(const char *name) >> +{ >> + return ISA_DEVICE(qdev_new(name)); >> +} >> + >> +ISADevice *isa_try_new(const char *name) >> +{ >> + return ISA_DEVICE(qdev_try_new(name)); > > We have: > > #define ISA_DEVICE(obj) \ > OBJECT_CHECK(ISADevice, (obj), TYPE_ISA_DEVICE) > > With: > > #define OBJECT_CHECK(type, obj, name) \ > ((type *)object_dynamic_cast_assert(OBJECT(obj), (name), \ > __FILE__, __LINE__, __func__)) > > "If an invalid object is passed to this function, a run time > assert will be generated." Looking at object_dynamic_cast_assert() implementation, NULL is a "valid" object... > > I'd expect isa_try_new() to return NULL if the type_name is not > registered... This is what is returned, as object_dynamic_cast_assert(NULL) = NULL. This is very unclear (and not the first time I dig to understand that). So this patch is logically correct. Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > >> +} >> + >> ISADevice *isa_create_simple(ISABus *bus, const char *name) >> { >> ISADevice *dev; >> @@ -185,6 +195,11 @@ ISADevice *isa_create_simple(ISABus *bus, const char *name) >> return dev; >> } >> >> +bool isa_realize_and_unref(ISADevice *dev, ISABus *bus, Error **errp) >> +{ >> + return qdev_realize_and_unref(&dev->parent_obj, &bus->parent_obj, errp); >> +} >> + >> ISADevice *isa_vga_init(ISABus *bus) >> { >> switch (vga_interface_type) { >> >
diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h index 02c2350274..3b6215fafe 100644 --- a/include/hw/isa/isa.h +++ b/include/hw/isa/isa.h @@ -105,6 +105,9 @@ MemoryRegion *isa_address_space(ISADevice *dev); MemoryRegion *isa_address_space_io(ISADevice *dev); ISADevice *isa_create(ISABus *bus, const char *name); ISADevice *isa_try_create(ISABus *bus, const char *name); +ISADevice *isa_new(const char *name); +ISADevice *isa_try_new(const char *name); +bool isa_realize_and_unref(ISADevice *dev, ISABus *bus, Error **errp); ISADevice *isa_create_simple(ISABus *bus, const char *name); ISADevice *isa_vga_init(ISABus *bus); diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c index 1c9d7e19ab..e6412d39b4 100644 --- a/hw/isa/isa-bus.c +++ b/hw/isa/isa-bus.c @@ -176,6 +176,16 @@ ISADevice *isa_try_create(ISABus *bus, const char *name) return ISA_DEVICE(dev); } +ISADevice *isa_new(const char *name) +{ + return ISA_DEVICE(qdev_new(name)); +} + +ISADevice *isa_try_new(const char *name) +{ + return ISA_DEVICE(qdev_try_new(name)); +} + ISADevice *isa_create_simple(ISABus *bus, const char *name) { ISADevice *dev; @@ -185,6 +195,11 @@ ISADevice *isa_create_simple(ISABus *bus, const char *name) return dev; } +bool isa_realize_and_unref(ISADevice *dev, ISABus *bus, Error **errp) +{ + return qdev_realize_and_unref(&dev->parent_obj, &bus->parent_obj, errp); +} + ISADevice *isa_vga_init(ISABus *bus) { switch (vga_interface_type) {
I'm converting from qdev_create()/qdev_init_nofail() to qdev_new()/qdev_realize_and_unref(); recent commit "qdev: New qdev_new(), qdev_realize(), etc." explains why. ISA devices use qdev_create() through isa_create() and isa_try_create(). Provide isa_new(), isa_try_new(), and isa_realize_and_unref() for converting ISA devices. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- include/hw/isa/isa.h | 3 +++ hw/isa/isa-bus.c | 15 +++++++++++++++ 2 files changed, 18 insertions(+)