From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44666) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z5vhL-0003zM-Vk for qemu-devel@nongnu.org; Fri, 19 Jun 2015 08:47:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z5vhH-0003Ug-Qv for qemu-devel@nongnu.org; Fri, 19 Jun 2015 08:47:07 -0400 Received: from cantor2.suse.de ([195.135.220.15]:43051 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z5vhH-0003SP-FO for qemu-devel@nongnu.org; Fri, 19 Jun 2015 08:47:03 -0400 Message-ID: <55840F46.7000009@suse.de> Date: Fri, 19 Jun 2015 14:47:02 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1434194302-26589-1-git-send-email-armbru@redhat.com> <1434194302-26589-4-git-send-email-armbru@redhat.com> In-Reply-To: <1434194302-26589-4-git-send-email-armbru@redhat.com> Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 3/7] qdev-monitor: Stop error avalance in qbus_find_recursive() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , qemu-devel@nongnu.org Cc: pbonzini@redhat.com, kraxel@redhat.com Did you mean avalanche? Am 13.06.2015 um 13:18 schrieb Markus Armbruster: > Reproducer: >=20 > $ qemu-system-x86_64 -nodefaults -device virtio-rng-pci -device vir= tio-rng-pci -device virtio-rng-device,bus=3Dvirtio-bus > qemu-system-x86_64: -device virtio-rng-device,bus=3Dvirtio-bus: Bus= 'virtio-bus' is full > qemu-system-x86_64: -device virtio-rng-device,bus=3Dvirtio-bus: Bus= 'virtio-bus' is full > qemu-system-x86_64: -device virtio-rng-device,bus=3Dvirtio-bus: Bus= 'virtio-bus' not found >=20 > qbus_find_recursive() reports the "is full" error itself, and leaves > reporting "not found" to its caller. The result is confusion. Write > it a function contract that permits leaving all error reporting to the > caller, and implement it. Update callers to detect and report "is > full". >=20 > Screwed up when commit 1395af6 added the max_dev limit and the "is > full" error condition to enforce it. >=20 > Signed-off-by: Markus Armbruster > Reviewed-by: Eric Blake > --- > qdev-monitor.c | 62 ++++++++++++++++++++++++++++++++++++--------------= -------- > 1 file changed, 39 insertions(+), 23 deletions(-) >=20 > diff --git a/qdev-monitor.c b/qdev-monitor.c > index 7dd62dd..1408c86 100644 > --- a/qdev-monitor.c > +++ b/qdev-monitor.c > @@ -364,43 +364,55 @@ static DeviceState *qbus_find_dev(BusState *bus, = char *elem) > return NULL; > } > =20 > +static inline bool qbus_is_full(BusState *bus) > +{ > + BusClass *bus_class =3D BUS_GET_CLASS(bus); > + return bus_class->max_dev && bus->max_index >=3D bus_class->max_de= v; > +} > + > +/** This should probably just be "/*". I'll trust you on getting the functional logic right, can't do a full review right now. Andreas > + * Search the tree rooted at @bus for a bus. > + * If @name, search for a bus with that name. Note that bus names > + * need not be unique. Yes, that's screwed up. > + * Else search for a bus that is a subtype of @bus_typename. > + * If more than one exists, prefer one that can take another device. > + * Return the bus if found, else %NULL. > + */ > static BusState *qbus_find_recursive(BusState *bus, const char *name, > const char *bus_typename) > { > - BusClass *bus_class =3D BUS_GET_CLASS(bus); > BusChild *kid; > - BusState *child, *ret; > - int match =3D 1; > + BusState *pick, *child, *ret; > + bool match; > =20 > - if (name && (strcmp(bus->name, name) !=3D 0)) { > - match =3D 0; > - } else if (bus_typename && !object_dynamic_cast(OBJECT(bus), bus_t= ypename)) { > - match =3D 0; > - } else if ((bus_class->max_dev !=3D 0) && (bus_class->max_dev <=3D= bus->max_index)) { > - if (name !=3D 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 =3D 0; > - } > + assert(name || bus_typename); > + if (name) { > + match =3D !strcmp(bus->name, name); > + } else { > + match =3D !!object_dynamic_cast(OBJECT(bus), bus_typename); > } > - if (match) { > - return bus; > + > + if (match && !qbus_is_full(bus)) { > + return bus; /* root matches and isn't full */ > } > =20 > + pick =3D match ? bus : NULL; > + > QTAILQ_FOREACH(kid, &bus->children, sibling) { > DeviceState *dev =3D kid->child; > QLIST_FOREACH(child, &dev->child_bus, sibling) { > ret =3D qbus_find_recursive(child, name, bus_typename); > - if (ret) { > - return ret; > + if (ret && !qbus_is_full(ret)) { > + return ret; /* a descendant matches and isn't full= */ > + } > + if (ret && !pick) { > + pick =3D ret; > } > } > } > - return NULL; > + > + /* root or a descendant matches, but is full */ > + return pick; > } > =20 > static BusState *qbus_find(const char *path) > @@ -423,6 +435,10 @@ static BusState *qbus_find(const char *path) > if (!bus) { > qerror_report(QERR_BUS_NOT_FOUND, elem); > return NULL; > + } else if (qbus_is_full(bus)) { > + qerror_report(ERROR_CLASS_GENERIC_ERROR, "Bus '%s' is full= ", > + elem); > + return NULL; > } > pos =3D len; > } > @@ -529,7 +545,7 @@ DeviceState *qdev_device_add(QemuOpts *opts) > } > } else if (dc->bus_type !=3D NULL) { > bus =3D qbus_find_recursive(sysbus_get_default(), NULL, dc->bu= s_type); > - if (!bus) { > + if (!bus || qbus_is_full(bus)) { > qerror_report(ERROR_CLASS_GENERIC_ERROR, > "No '%s' bus found for device '%s'", > dc->bus_type, driver); >=20 --=20 SUSE Linux GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Felix Imend=F6rffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB 21284 (AG N=FCrnberg)