| Message ID | 20200707033326.19178-1-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] hw/core/qdev: Increase qdev_realize() kindness | expand |
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); >
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
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 > >
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.
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);
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(-)