diff mbox series

[v2,18/58] isa: New isa_new(), isa_realize_and_unref() etc.

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

Commit Message

Markus Armbruster May 29, 2020, 1:44 p.m. UTC
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(+)

Comments

Philippe Mathieu-Daudé June 9, 2020, 8:19 a.m. UTC | #1
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) {
>
Philippe Mathieu-Daudé June 9, 2020, 8:26 a.m. UTC | #2
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 mbox series

Patch

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) {