Re: [Qemu-devel] [RFC 1/2] qbus_find_recursive(): don't abort search for named bus on full bus node

[KONRAD Frédéric]

On 31/01/2013 16:52, Peter Maydell wrote:
> On 31 January 2013 15:42, Laszlo Ersek <address@hidden> wrote:
> > The bus we're looking for could be in the sub-tree rooted at the node
> > being checked; don't skip looping over the children.
> >
> > Signed-off-by: Laszlo Ersek <address@hidden>
> > ---
> >   hw/qdev-monitor.c |   10 +--------
> >   1 files changed, 1 insertions(+), 9 deletions(-)
> >
> > diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
> > index 4e2a92b..34f5014 100644
> > --- a/hw/qdev-monitor.c
> > +++ b/hw/qdev-monitor.c
> > @@ -295,15 +295,7 @@ static BusState *qbus_find_recursive(BusState *bus, const 
> > char *name,
> >           match = 0;
> >       }
> >       if ((bus_class->max_dev != 0) && (bus_class->max_dev <= bus->max_index)) {
> > -        if (name != NULL) {
> > -            /* bus was explicitly specified: return an error. */
> > -            qerror_report(ERROR_CLASS_GENERIC_ERROR, "Bus '%s' is full",
> > -                          bus->name);
> > -            return NULL;
> > -        } else {
> > -            /* bus was not specified: try to find another one. */
> > -            match = 0;
> > -        }
> > +        match = 0;
> >       }
> This looks like the wrong fix to this problem -- if the user passed
> us a specific name to search for and we found it and it was full, then
> we definitely want to stop here. On the other hand, if the user passed
> us a specific name and this bus isn't that named bus then we shouldn't
> be checking the max_index at all.
>
> So I think the right fix is that the condition should be
>    if (match && (bus_class->max_dev != 0)
>        && (bus_class->max_dev <= bus->max_index)) {
>
> and the rest of the code in this function stays as is.
>
> -- PMM
The final result looks the same no?

If you look the qdev-monitor.c code just above the patch, you'll find 
the same thing:

    if (name && (strcmp(bus->name, name) != 0)) {
        match = 0;
    }
    if (bus_typename && !object_dynamic_cast(OBJECT(bus), bus_typename)) {
        match = 0;
    }
    if ((bus_class->max_dev != 0) && (bus_class->max_dev <= 
bus->max_index)) {
        ...

Fred