All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: qemu-devel@nongnu.org
Cc: pbonzini@redhat.com, afaerber@suse.de, kraxel@redhat.com
Subject: [Qemu-devel] [PATCH v2 3/7] qdev-monitor: Stop error avalance in qbus_find_recursive()
Date: Sat, 13 Jun 2015 13:18:18 +0200	[thread overview]
Message-ID: <1434194302-26589-4-git-send-email-armbru@redhat.com> (raw)
In-Reply-To: <1434194302-26589-1-git-send-email-armbru@redhat.com>

Reproducer:

    $ qemu-system-x86_64 -nodefaults -device virtio-rng-pci -device virtio-rng-pci -device virtio-rng-device,bus=virtio-bus
    qemu-system-x86_64: -device virtio-rng-device,bus=virtio-bus: Bus 'virtio-bus' is full
    qemu-system-x86_64: -device virtio-rng-device,bus=virtio-bus: Bus 'virtio-bus' is full
    qemu-system-x86_64: -device virtio-rng-device,bus=virtio-bus: Bus 'virtio-bus' not found

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".

Screwed up when commit 1395af6 added the max_dev limit and the "is
full" error condition to enforce it.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 qdev-monitor.c | 62 ++++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 39 insertions(+), 23 deletions(-)

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;
 }
 
+static inline bool qbus_is_full(BusState *bus)
+{
+    BusClass *bus_class = BUS_GET_CLASS(bus);
+    return bus_class->max_dev && bus->max_index >= bus_class->max_dev;
+}
+
+/**
+ * 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 = BUS_GET_CLASS(bus);
     BusChild *kid;
-    BusState *child, *ret;
-    int match = 1;
+    BusState *pick, *child, *ret;
+    bool match;
 
-    if (name && (strcmp(bus->name, name) != 0)) {
-        match = 0;
-    } else if (bus_typename && !object_dynamic_cast(OBJECT(bus), bus_typename)) {
-        match = 0;
-    } else 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;
-        }
+    assert(name || bus_typename);
+    if (name) {
+        match = !strcmp(bus->name, name);
+    } else {
+        match = !!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 */
     }
 
+    pick = match ? bus : NULL;
+
     QTAILQ_FOREACH(kid, &bus->children, sibling) {
         DeviceState *dev = kid->child;
         QLIST_FOREACH(child, &dev->child_bus, sibling) {
             ret = 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 = ret;
             }
         }
     }
-    return NULL;
+
+    /* root or a descendant matches, but is full */
+    return pick;
 }
 
 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 = len;
     }
@@ -529,7 +545,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
         }
     } else if (dc->bus_type != NULL) {
         bus = qbus_find_recursive(sysbus_get_default(), NULL, dc->bus_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);
-- 
1.9.3

  parent reply	other threads:[~2015-06-13 11:18 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-13 11:18 [Qemu-devel] [PATCH v2 0/7] qdev: Mostly wean off QError Markus Armbruster
2015-06-13 11:18 ` [Qemu-devel] [PATCH v2 1/7] qdev: Deprecated qdev_init() is finally unused, drop Markus Armbruster
2015-06-19 12:36   ` Andreas Färber
2015-06-19 13:17     ` Markus Armbruster
2015-06-13 11:18 ` [Qemu-devel] [PATCH v2 2/7] qdev: Un-deprecate qdev_init_nofail() Markus Armbruster
2015-06-19 12:42   ` Andreas Färber
2015-06-19 13:18     ` Markus Armbruster
2015-06-13 11:18 ` Markus Armbruster [this message]
2015-06-19 12:47   ` [Qemu-devel] [PATCH v2 3/7] qdev-monitor: Stop error avalance in qbus_find_recursive() Andreas Färber
2015-06-19 13:19     ` Markus Armbruster
2015-06-13 11:18 ` [Qemu-devel] [PATCH v2 4/7] qdev-monitor: Fix check for full bus Markus Armbruster
2015-06-13 11:18 ` [Qemu-devel] [PATCH v2 5/7] qdev-monitor: Convert qbus_find() to Error Markus Armbruster
2015-06-13 11:18 ` [Qemu-devel] [PATCH v2 6/7] qdev-monitor: Propagate errors through set_property() Markus Armbruster
2015-06-19 12:49   ` Andreas Färber
2015-06-13 11:18 ` [Qemu-devel] [PATCH v2 7/7] qdev-monitor: Propagate errors through qdev_device_add() Markus Armbruster
2015-06-19 13:08   ` Andreas Färber
2015-06-19 13:49     ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1434194302-26589-4-git-send-email-armbru@redhat.com \
    --to=armbru@redhat.com \
    --cc=afaerber@suse.de \
    --cc=kraxel@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.