| Message ID | 37ada83704ec5cccacbd9411fe4034c8140d860d.1276641524.git.jan.kiszka@web.de |
|---|---|
| State | New |
| Headers | show |
[cc: kraxel] Jan Kiszka <jan.kiszka@web.de> writes: > From: Jan Kiszka <jan.kiszka@siemens.com> > > Path abbreviations suffer from the inconsistency that bus names can only > be omitted at the end of a path. Drop this special rule, and also remove > the now obsolete QERR_DEVICE_MULTIPLE_BUSSES. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> Affects only -device and device_add option bus. I support this. > --- > hw/qdev.c | 22 ++++------------------ > qerror.c | 4 ---- > qerror.h | 3 --- > 3 files changed, 4 insertions(+), 25 deletions(-) > > diff --git a/hw/qdev.c b/hw/qdev.c > index 61f999c..7c4f039 100644 > --- a/hw/qdev.c > +++ b/hw/qdev.c > @@ -608,32 +608,18 @@ static BusState *qbus_find(const char *path) > } > return NULL; > } > + if (dev->num_child_bus == 0) { > + qerror_report(QERR_DEVICE_NO_BUS, elem); > + return NULL; > + } I'd kill this check as well. The QERR_BUS_NOT_FOUND further down suffices. > > assert(path[pos] == '/' || !path[pos]); > while (path[pos] == '/') { > pos++; > } > - 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(QERR_DEVICE_NO_BUS, elem); > - return NULL; > - case 1: > - return QLIST_FIRST(&dev->child_bus); > - default: > - qerror_report(QERR_DEVICE_MULTIPLE_BUSSES, elem); > - if (!monitor_cur_is_qmp()) { > - qbus_list_bus(dev); > - } > - return NULL; > - } > - } > > /* find bus */ > if (sscanf(path+pos, "%127[^/]%n", elem, &len) != 1) { > - assert(0); > elem[0] = len = 0; > } > pos += len; [...]
Hmm, what happens if the path denotes a device instead of a bus? qemu-system-x86_64: -device e1000,bus=/i440FX-pcihost: Bus '' not found Not so nice.
diff --git a/hw/qdev.c b/hw/qdev.c index 61f999c..7c4f039 100644 --- a/hw/qdev.c +++ b/hw/qdev.c @@ -608,32 +608,18 @@ static BusState *qbus_find(const char *path) } return NULL; } + if (dev->num_child_bus == 0) { + qerror_report(QERR_DEVICE_NO_BUS, elem); + return NULL; + } assert(path[pos] == '/' || !path[pos]); while (path[pos] == '/') { pos++; } - 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(QERR_DEVICE_NO_BUS, elem); - return NULL; - case 1: - return QLIST_FIRST(&dev->child_bus); - default: - qerror_report(QERR_DEVICE_MULTIPLE_BUSSES, elem); - if (!monitor_cur_is_qmp()) { - qbus_list_bus(dev); - } - return NULL; - } - } /* find bus */ if (sscanf(path+pos, "%127[^/]%n", elem, &len) != 1) { - assert(0); elem[0] = len = 0; } pos += len; diff --git a/qerror.c b/qerror.c index 44d0bf8..786b5dc 100644 --- a/qerror.c +++ b/qerror.c @@ -77,10 +77,6 @@ static const QErrorStringTable qerror_table[] = { .desc = "Device '%(device)' is locked", }, { - .error_fmt = QERR_DEVICE_MULTIPLE_BUSSES, - .desc = "Device '%(device)' has multiple child busses", - }, - { .error_fmt = QERR_DEVICE_NOT_ACTIVE, .desc = "Device '%(device)' has not been activated by the guest", }, diff --git a/qerror.h b/qerror.h index 77ae574..88474fb 100644 --- a/qerror.h +++ b/qerror.h @@ -73,9 +73,6 @@ QError *qobject_to_qerror(const QObject *obj); #define QERR_DEVICE_LOCKED \ "{ 'class': 'DeviceLocked', 'data': { 'device': %s } }" -#define QERR_DEVICE_MULTIPLE_BUSSES \ - "{ 'class': 'DeviceMultipleBusses', 'data': { 'device': %s } }" - #define QERR_DEVICE_NOT_ACTIVE \ "{ 'class': 'DeviceNotActive', 'data': { 'device': %s } }"