From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:43317) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qy7Qu-0005uH-1u for qemu-devel@nongnu.org; Mon, 29 Aug 2011 15:23:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Qy7Qs-0006lN-O1 for qemu-devel@nongnu.org; Mon, 29 Aug 2011 15:23:44 -0400 Received: from e39.co.us.ibm.com ([32.97.110.160]:41820) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qy7Qs-0006lE-G5 for qemu-devel@nongnu.org; Mon, 29 Aug 2011 15:23:42 -0400 Received: from d03relay02.boulder.ibm.com (d03relay02.boulder.ibm.com [9.17.195.227]) by e39.co.us.ibm.com (8.14.4/8.13.1) with ESMTP id p7TJ8FiQ029358 for ; Mon, 29 Aug 2011 13:08:15 -0600 Received: from d03av03.boulder.ibm.com (d03av03.boulder.ibm.com [9.17.195.169]) by d03relay02.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p7TJNYBT152300 for ; Mon, 29 Aug 2011 13:23:35 -0600 Received: from d03av03.boulder.ibm.com (loopback [127.0.0.1]) by d03av03.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p7TDNXMD031021 for ; Mon, 29 Aug 2011 07:23:33 -0600 Message-ID: <4E5BE735.3090706@us.ibm.com> Date: Mon, 29 Aug 2011 14:23:33 -0500 From: Anthony Liguori MIME-Version: 1.0 References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 6/6] qdev: Generate IDs for anonymous devices List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: qemu-devel , Markus Armbruster On 08/26/2011 09:48 AM, Jan Kiszka wrote: > In order to address devices for that the user forgot or is even unable > (no_user) to provide an ID, assign an automatically generated one. Such > IDs have the format #, thus are outside the name space availing > to users. Don't use them for bus naming to avoid any other user-visible > change. I don't think this is a very nice approach. Why not eliminate anonymous devices entirely and use a parent derived name for devices that are not created by the user? Regards, Anthony Liguori > > CC: Markus Armbruster > Signed-off-by: Jan Kiszka > --- > hw/qdev-properties.c | 2 +- > hw/qdev.c | 25 +++++++++++++------------ > hw/qdev.h | 1 + > 3 files changed, 15 insertions(+), 13 deletions(-) > > diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c > index 0c0c292..4e39cef 100644 > --- a/hw/qdev-properties.c > +++ b/hw/qdev-properties.c > @@ -682,7 +682,7 @@ int qdev_prop_set_drive(DeviceState *dev, const char *name, BlockDriverState *va > if (res< 0) { > error_report("Can't attach drive %s to %s.%s: %s", > bdrv_get_device_name(value), > - dev->id ? dev->id : dev->info->name, > + dev->id != dev->auto_id ? dev->id : dev->info->name, > name, strerror(-res)); > return -1; > } > diff --git a/hw/qdev.c b/hw/qdev.c > index 3b0e1af..807902b 100644 > --- a/hw/qdev.c > +++ b/hw/qdev.c > @@ -85,6 +85,7 @@ static DeviceInfo *qdev_find_info(BusInfo *bus_info, const char *name) > > static DeviceState *qdev_create_from_info(BusState *bus, DeviceInfo *info) > { > + static unsigned int auto_id; > DeviceState *dev; > > assert(bus->info == info->bus_info); > @@ -94,6 +95,8 @@ static DeviceState *qdev_create_from_info(BusState *bus, DeviceInfo *info) > qdev_prop_set_defaults(dev, dev->info->props); > qdev_prop_set_defaults(dev, dev->parent_bus->info->props); > qdev_prop_set_globals(dev); > + snprintf(dev->auto_id, sizeof(dev->auto_id), "#%u", ++auto_id); > + dev->id = dev->auto_id; > QLIST_INSERT_HEAD(&bus->children, dev, sibling); > if (qdev_hotplug) { > assert(bus->allow_hotplug); > @@ -574,8 +577,9 @@ DeviceState *qdev_find_recursive(BusState *bus, const char *id) > BusState *child; > > QLIST_FOREACH(dev,&bus->children, sibling) { > - if (dev->id&& strcmp(dev->id, id) == 0) > + if (strcmp(dev->id, id) == 0) { > return dev; > + } > QLIST_FOREACH(child,&dev->child_bus, sibling) { > ret = qdev_find_recursive(child, id); > if (ret) { > @@ -591,8 +595,8 @@ static void qbus_list_bus(DeviceState *dev) > BusState *child; > const char *sep = " "; > > - error_printf("child busses at \"%s\":", > - dev->id ? dev->id : dev->info->name); > + error_printf("child busses at \"%s\"/\"%s\":", > + dev->info->name, dev->id); > QLIST_FOREACH(child,&dev->child_bus, sibling) { > error_printf("%s\"%s\"", sep, child->name); > sep = ", "; > @@ -607,9 +611,7 @@ static void qbus_list_dev(BusState *bus) > > error_printf("devices at \"%s\":", bus->name); > QLIST_FOREACH(dev,&bus->children, sibling) { > - error_printf("%s\"%s\"", sep, dev->info->name); > - if (dev->id) > - error_printf("/\"%s\"", dev->id); > + error_printf("%s\"%s\"/\"%s\"", sep, dev->info->name, dev->id); > sep = ", "; > } > error_printf("\n"); > @@ -638,7 +640,7 @@ static DeviceState *qbus_find_dev(BusState *bus, char *elem) > * (3) driver alias, if present > */ > QLIST_FOREACH(dev,&bus->children, sibling) { > - if (dev->id&& strcmp(dev->id, elem) == 0) { > + if (strcmp(dev->id, elem) == 0) { > return dev; > } > } > @@ -754,8 +756,8 @@ void qbus_create_inplace(BusState *bus, BusInfo *info, > if (name) { > /* use supplied name */ > bus->name = g_strdup(name); > - } else if (parent&& parent->id) { > - /* parent device has id -> use it for bus name */ > + } else if (parent&& parent->id != parent->auto_id) { > + /* parent device has user-assigned id -> use it for bus name */ > len = strlen(parent->id) + 16; > buf = g_malloc(len); > snprintf(buf, len, "%s.%d", parent->id, parent->num_child_bus); > @@ -850,8 +852,7 @@ static void qdev_print_props(Monitor *mon, DeviceState *dev, Property *props, > static void qdev_print(Monitor *mon, DeviceState *dev, int indent) > { > BusState *child; > - qdev_printf("dev: %s, id \"%s\"\n", dev->info->name, > - dev->id ? dev->id : ""); > + qdev_printf("dev: %s, id \"%s\"\n", dev->info->name, dev->id); > indent += 2; > if (dev->num_gpio_in) { > qdev_printf("gpio-in %d\n", dev->num_gpio_in); > @@ -1196,7 +1197,7 @@ int do_device_show(Monitor *mon, const QDict *qdict, QObject **ret_data) > name = g_malloc(name_len); > snprintf(name, name_len, "%s", dev->info->name); > *ret_data = qobject_from_jsonf("{ 'device': %s, 'id': %s, 'version': %d }", > - name, dev->id ? : "", vmsd->version_id); > + name, dev->id, vmsd->version_id); > g_free(name); > qlist = qlist_new(); > parse_vmstate(vmsd, dev, qlist, qdict_get_try_bool(qdict, "full", false)); > diff --git a/hw/qdev.h b/hw/qdev.h > index 912fc45..d028409 100644 > --- a/hw/qdev.h > +++ b/hw/qdev.h > @@ -31,6 +31,7 @@ enum { > so that it can be embedded in individual device state structures. */ > struct DeviceState { > const char *id; > + char auto_id[16]; > enum DevState state; > QemuOpts *opts; > int hotplugged;