From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57783) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z3jSI-0000N4-Qt for qemu-devel@nongnu.org; Sat, 13 Jun 2015 07:18:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z3jSE-0005aV-LI for qemu-devel@nongnu.org; Sat, 13 Jun 2015 07:18:30 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41210) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z3jSE-0005a7-Dn for qemu-devel@nongnu.org; Sat, 13 Jun 2015 07:18:26 -0400 From: Markus Armbruster Date: Sat, 13 Jun 2015 13:18:19 +0200 Message-Id: <1434194302-26589-5-git-send-email-armbru@redhat.com> In-Reply-To: <1434194302-26589-1-git-send-email-armbru@redhat.com> References: <1434194302-26589-1-git-send-email-armbru@redhat.com> Subject: [Qemu-devel] [PATCH v2 4/7] qdev-monitor: Fix check for full bus List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: pbonzini@redhat.com, afaerber@suse.de, kraxel@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 Reviewed-by: Eric Blake --- 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