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 4/7] qdev-monitor: Fix check for full bus
Date: Sat, 13 Jun 2015 13:18:19 +0200	[thread overview]
Message-ID: <1434194302-26589-5-git-send-email-armbru@redhat.com> (raw)
In-Reply-To: <1434194302-26589-1-git-send-email-armbru@redhat.com>

Property bus has always been too screwed up to be really usable for
values other than plain bus IDs.  This just fixes a bug that crept in
in commit 1395af6 "qdev: add a maximum device allowed field for the
bus."

It doesn't always fail when it should:

    $ qemu-system-x86_64 -nodefaults -device virtio-serial-pci -device virtio-rng-device,bus=pci.0/virtio-serial-pci/virtio-bus

Happily plugs the virtio-rng-device into the virtio-bus provided by
virtio-serial-pci, even though its only slot is already occupied by a
virtio-serial-device.

And sometimes fails when it shouldn't:

    $ qemu-system-x86_64 -nodefaults -device virtio-serial-pci -device virtserialport,bus=virtio-bus/virtio-serial-device

Yes, the virtio-bus is full, but the virtio-serial-bus provided by
virtio-serial-device isn't, and that's the one we're trying to use.

Root cause: we check "bus full" when we resolve the first element of
the path.  That's the correct one only when it's also the last one.

Fix by moving the "bus full" check to right before we return a bus.

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

diff --git a/qdev-monitor.c b/qdev-monitor.c
index 1408c86..a54c368 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -435,10 +435,6 @@ 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;
     }
@@ -449,7 +445,7 @@ static BusState *qbus_find(const char *path)
             pos++;
         }
         if (path[pos] == '\0') {
-            return bus;
+            break;
         }
 
         /* find device */
@@ -474,21 +470,21 @@ static BusState *qbus_find(const char *path)
         if (path[pos] == '\0') {
             /* last specified element is a device.  If it has exactly
              * one child bus accept it nevertheless */
-            switch (dev->num_child_bus) {
-            case 0:
-                qerror_report(ERROR_CLASS_GENERIC_ERROR,
-                              "Device '%s' has no child bus", elem);
-                return NULL;
-            case 1:
-                return QLIST_FIRST(&dev->child_bus);
-            default:
+            if (dev->num_child_bus == 1) {
+                bus = QLIST_FIRST(&dev->child_bus);
+                break;
+            }
+            if (dev->num_child_bus) {
                 qerror_report(ERROR_CLASS_GENERIC_ERROR,
                               "Device '%s' has multiple child busses", elem);
                 if (!monitor_cur_is_qmp()) {
                     qbus_list_bus(dev);
                 }
-                return NULL;
+            } else {
+                qerror_report(ERROR_CLASS_GENERIC_ERROR,
+                              "Device '%s' has no child bus", elem);
             }
+            return NULL;
         }
 
         /* find bus */
@@ -506,6 +502,13 @@ static BusState *qbus_find(const char *path)
             return NULL;
         }
     }
+
+    if (qbus_is_full(bus)) {
+        qerror_report(ERROR_CLASS_GENERIC_ERROR, "Bus '%s' is full",
+                      path);
+        return NULL;
+    }
+    return bus;
 }
 
 DeviceState *qdev_device_add(QemuOpts *opts)
-- 
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 ` [Qemu-devel] [PATCH v2 3/7] qdev-monitor: Stop error avalance in qbus_find_recursive() Markus Armbruster
2015-06-19 12:47   ` Andreas Färber
2015-06-19 13:19     ` Markus Armbruster
2015-06-13 11:18 ` Markus Armbruster [this message]
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-5-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.