[PATCH v3] hw/core/qdev: Increase qdev_realize() kindness

Philippe Mathieu-Daudé posted 1 patch 3 years, 8 months ago
Test FreeBSD passed
Test docker-quick@centos7 passed
Test checkpatch passed
Test docker-mingw@fedora passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200707033326.19178-1-f4bug@amsat.org
There is a newer version of this series
hw/core/qdev.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
[PATCH v3] hw/core/qdev: Increase qdev_realize() kindness
Posted by Philippe Mathieu-Daudé 3 years, 8 months ago
Since commit 510ef98dca5, qdev_realize() aborts if bus-less device
is realized on a bus. While commits 514db7710b..007d1dbf72 took
care of converting all mainstream uses, QEMU forks weren't. These
forks are usually maintained by hobbyist with interest in following
mainstream development, but with limited time, so usually rebase
from time to time. To avoid them to spend time on debugging and
reading git-log history, display a kind hint about what is wrong.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Since v2:
- scratch __func__ (armbru)
- reword to justify this is not an impossible case (armbru)
---
 hw/core/qdev.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 2131c7f951..a16f1270f1 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -392,8 +392,11 @@ bool qdev_realize(DeviceState *dev, BusState *bus, Error **errp)
 
     if (bus) {
         qdev_set_parent_bus(dev, bus);
-    } else {
-        assert(!DEVICE_GET_CLASS(dev)->bus_type);
+    } else if (DEVICE_GET_CLASS(dev)->bus_type) {
+        error_setg(errp, "Unexpected bus '%s' for bus-less device '%s'",
+                   DEVICE_GET_CLASS(dev)->bus_type,
+                   object_get_typename(OBJECT(dev)));
+        return false;
     }
 
     object_property_set_bool(OBJECT(dev), true, "realized", &err);
-- 
2.21.3


Re: [PATCH v3] hw/core/qdev: Increase qdev_realize() kindness
Posted by Philippe Mathieu-Daudé 3 years, 8 months ago
On 7/7/20 5:33 AM, Philippe Mathieu-Daudé wrote:
> Since commit 510ef98dca5, qdev_realize() aborts if bus-less device
> is realized on a bus. While commits 514db7710b..007d1dbf72 took
> care of converting all mainstream uses, QEMU forks weren't. These

I guess I missed "weren't [converted]".

> forks are usually maintained by hobbyist with interest in following
> mainstream development, but with limited time, so usually rebase
> from time to time. To avoid them to spend time on debugging and
> reading git-log history, display a kind hint about what is wrong.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Since v2:
> - scratch __func__ (armbru)
> - reword to justify this is not an impossible case (armbru)
> ---
>  hw/core/qdev.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 2131c7f951..a16f1270f1 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -392,8 +392,11 @@ bool qdev_realize(DeviceState *dev, BusState *bus, Error **errp)
>  
>      if (bus) {
>          qdev_set_parent_bus(dev, bus);
> -    } else {
> -        assert(!DEVICE_GET_CLASS(dev)->bus_type);
> +    } else if (DEVICE_GET_CLASS(dev)->bus_type) {
> +        error_setg(errp, "Unexpected bus '%s' for bus-less device '%s'",
> +                   DEVICE_GET_CLASS(dev)->bus_type,
> +                   object_get_typename(OBJECT(dev)));
> +        return false;
>      }
>  
>      object_property_set_bool(OBJECT(dev), true, "realized", &err);
> 

Re: [PATCH v3] hw/core/qdev: Increase qdev_realize() kindness
Posted by Paolo Bonzini 3 years, 8 months ago
On 07/07/20 05:33, Philippe Mathieu-Daudé wrote:
> Since commit 510ef98dca5, qdev_realize() aborts if bus-less device
> is realized on a bus. While commits 514db7710b..007d1dbf72 took
> care of converting all mainstream uses, QEMU forks weren't. These
> forks are usually maintained by hobbyist with interest in following
> mainstream development, but with limited time, so usually rebase
> from time to time. To avoid them to spend time on debugging and
> reading git-log history, display a kind hint about what is wrong.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Since v2:
> - scratch __func__ (armbru)
> - reword to justify this is not an impossible case (armbru)
> ---
>  hw/core/qdev.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 2131c7f951..a16f1270f1 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -392,8 +392,11 @@ bool qdev_realize(DeviceState *dev, BusState *bus, Error **errp)
>  
>      if (bus) {
>          qdev_set_parent_bus(dev, bus);
> -    } else {
> -        assert(!DEVICE_GET_CLASS(dev)->bus_type);
> +    } else if (DEVICE_GET_CLASS(dev)->bus_type) {
> +        error_setg(errp, "Unexpected bus '%s' for bus-less device '%s'",
> +                   DEVICE_GET_CLASS(dev)->bus_type,
> +                   object_get_typename(OBJECT(dev)));
> +        return false;
>      }
>  
>      object_property_set_bool(OBJECT(dev), true, "realized", &err);
> 

Queued, thanks.

Paolo


Re: [PATCH v3] hw/core/qdev: Increase qdev_realize() kindness
Posted by Philippe Mathieu-Daudé 3 years, 8 months ago
On 7/7/20 11:47 AM, Paolo Bonzini wrote:
> On 07/07/20 05:33, Philippe Mathieu-Daudé wrote:
>> Since commit 510ef98dca5, qdev_realize() aborts if bus-less device
>> is realized on a bus. While commits 514db7710b..007d1dbf72 took
>> care of converting all mainstream uses, QEMU forks weren't. These
>> forks are usually maintained by hobbyist with interest in following
>> mainstream development, but with limited time, so usually rebase
>> from time to time. To avoid them to spend time on debugging and
>> reading git-log history, display a kind hint about what is wrong.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> Since v2:
>> - scratch __func__ (armbru)
>> - reword to justify this is not an impossible case (armbru)
>> ---
>>  hw/core/qdev.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>> index 2131c7f951..a16f1270f1 100644
>> --- a/hw/core/qdev.c
>> +++ b/hw/core/qdev.c
>> @@ -392,8 +392,11 @@ bool qdev_realize(DeviceState *dev, BusState *bus, Error **errp)
>>  
>>      if (bus) {
>>          qdev_set_parent_bus(dev, bus);
>> -    } else {
>> -        assert(!DEVICE_GET_CLASS(dev)->bus_type);
>> +    } else if (DEVICE_GET_CLASS(dev)->bus_type) {
>> +        error_setg(errp, "Unexpected bus '%s' for bus-less device '%s'",
>> +                   DEVICE_GET_CLASS(dev)->bus_type,
>> +                   object_get_typename(OBJECT(dev)));
>> +        return false;
>>      }
>>  
>>      object_property_set_bool(OBJECT(dev), true, "realized", &err);
>>
> 
> Queued, thanks.

Thanks! I haven't see that and sent a v4 with example included &
typo fixed, if possible can you take it instead?
https://patchew.org/QEMU/20200727175112.6820-1-f4bug@amsat.org/

> 
> Paolo
> 
> 

Re: [PATCH v3] hw/core/qdev: Increase qdev_realize() kindness
Posted by Markus Armbruster 3 years, 8 months ago
Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> On 7/7/20 11:47 AM, Paolo Bonzini wrote:
>> On 07/07/20 05:33, Philippe Mathieu-Daudé wrote:
>>> Since commit 510ef98dca5, qdev_realize() aborts if bus-less device
>>> is realized on a bus. While commits 514db7710b..007d1dbf72 took
>>> care of converting all mainstream uses, QEMU forks weren't. These
>>> forks are usually maintained by hobbyist with interest in following
>>> mainstream development, but with limited time, so usually rebase
>>> from time to time. To avoid them to spend time on debugging and
>>> reading git-log history, display a kind hint about what is wrong.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>> Since v2:
>>> - scratch __func__ (armbru)
>>> - reword to justify this is not an impossible case (armbru)
>>> ---
>>>  hw/core/qdev.c | 7 +++++--
>>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>>> index 2131c7f951..a16f1270f1 100644
>>> --- a/hw/core/qdev.c
>>> +++ b/hw/core/qdev.c
>>> @@ -392,8 +392,11 @@ bool qdev_realize(DeviceState *dev, BusState *bus, Error **errp)
>>>  
>>>      if (bus) {
>>>          qdev_set_parent_bus(dev, bus);
>>> -    } else {
>>> -        assert(!DEVICE_GET_CLASS(dev)->bus_type);
>>> +    } else if (DEVICE_GET_CLASS(dev)->bus_type) {
>>> +        error_setg(errp, "Unexpected bus '%s' for bus-less device '%s'",
>>> +                   DEVICE_GET_CLASS(dev)->bus_type,
>>> +                   object_get_typename(OBJECT(dev)));
>>> +        return false;
>>>      }
>>>  
>>>      object_property_set_bool(OBJECT(dev), true, "realized", &err);
>>>
>> 
>> Queued, thanks.
>
> Thanks! I haven't see that and sent a v4 with example included &
> typo fixed, if possible can you take it instead?
> https://patchew.org/QEMU/20200727175112.6820-1-f4bug@amsat.org/

Please also consider my objection in reply to v4.