From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40765) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZVFUD-00010I-Lt for qemu-devel@nongnu.org; Fri, 28 Aug 2015 04:58:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZVFUC-00042w-7u for qemu-devel@nongnu.org; Fri, 28 Aug 2015 04:58:13 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42922) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZVFUC-00042r-1U for qemu-devel@nongnu.org; Fri, 28 Aug 2015 04:58:12 -0400 Date: Fri, 28 Aug 2015 09:58:06 +0100 From: "Daniel P. Berrange" Message-ID: <20150828085806.GJ28526@redhat.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH v2] qdev-monitor.c: Add device id generation Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Programmingkid Cc: Kevin Wolf , Peter Maydell , John Snow , Jeff Cody , Markus Armbruster , qemu-devel qemu-devel , Paolo Bonzini , Andreas =?utf-8?Q?F=C3=A4rber?= On Thu, Aug 27, 2015 at 07:14:28PM -0400, Programmingkid wrote: > Add device ID generation to each device if an ID isn't given. > > An auto-generated ID will begin with an underscore character. This will > distinguish it from user-made ID's. > > An user-made ID cannot begin with an underscore, so there is no > problem of collisions with auto-generated ID's. > > A management program like libvirt should have no problems with this > patch since the names for ID's it gives do not start with an underscore. > > Signed-off-by: John Arbuckle > > --- > qdev-monitor.c | 17 ++++++++--------- > 1 files changed, 8 insertions(+), 9 deletions(-) > > diff --git a/qdev-monitor.c b/qdev-monitor.c > index f9e2d62..a9beb6d 100644 > --- a/qdev-monitor.c > +++ b/qdev-monitor.c > @@ -574,18 +574,17 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) > id = qemu_opts_id(opts); > if (id) { > dev->id = id; > + } else { /* create an ID for a device if none is provided */ > + static uint64_t deviceIDCount; > + char *deviceIDString; > + > + /* Add an underscore to show this ID is auto-generated */ > + deviceIDString = g_strdup_printf("_%" PRIu64, deviceIDCount++); > + dev->id = (const char *) deviceIDString; The ID namespace is global to QOM, but this counter is scoped to the qdev code, so we should have a further prefix, in case other bits of code that also use QOM need to have auto-generated IDs. IOW I think we should use deviceIDString = g_strdup_printf("_qdev%" PRIu64, deviceIDCount++); > } > > - if (dev->id) { > - object_property_add_child(qdev_get_peripheral(), dev->id, > - OBJECT(dev), NULL); > - } else { > - static int anon_count; > - gchar *name = g_strdup_printf("device[%d]", anon_count++); > - object_property_add_child(qdev_get_peripheral_anon(), name, > + object_property_add_child(qdev_get_peripheral(), dev->id, > OBJECT(dev), NULL); > - g_free(name); > - } It looks like this is removing the only use of the /machine/peripheral-anon part of the tre, so if we do this, then we should remove the refernces to peripheral-anon from the rest of the codebase. Alternatively we could keep devices with auto-generated IDs under /peripheral-anon, but I don't really see much point. The only reason this was separated was to avoid the auto-generated path name clashing with a user provided name, but we avoid that now by using an underscore. So we might as well just kill off the /peripheral-anon part of the tree and leave just /peripheral Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|