From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51050) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XbWTW-0006tY-Nb for qemu-devel@nongnu.org; Tue, 07 Oct 2014 11:15:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XbWTN-0006bc-BX for qemu-devel@nongnu.org; Tue, 07 Oct 2014 11:14:54 -0400 Received: from mail-qg0-x229.google.com ([2607:f8b0:400d:c04::229]:51553) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XbWTN-0006bV-3m for qemu-devel@nongnu.org; Tue, 07 Oct 2014 11:14:45 -0400 Received: by mail-qg0-f41.google.com with SMTP id f51so5596143qge.28 for ; Tue, 07 Oct 2014 08:14:43 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <5434035E.1040807@redhat.com> Date: Tue, 07 Oct 2014 17:14:38 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1412078370-3555-1-git-send-email-armbru@redhat.com> <87iok46kb8.fsf@blackfin.pond.sub.org> <20141002132119.GD30564@stefanha-thinkpad.redhat.com> <542D5284.1060201@suse.de> <87tx3mblhq.fsf@blackfin.pond.sub.org> <542D6127.7040308@suse.de> <87tx3ma56u.fsf@blackfin.pond.sub.org> <542D8E3C.6000207@redhat.com> <87k34c71gi.fsf@blackfin.pond.sub.org> <5433BA8E.9070101@redhat.com> <87ppe413de.fsf@blackfin.pond.sub.org> In-Reply-To: <87ppe413de.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] IDs in QOM List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: kwolf@redhat.com, peter.maydell@linaro.org, peter.crosthwaite@xilinx.com, Anthony Liguori , Stefan Hajnoczi , qemu-devel@nongnu.org, stefanha@redhat.com, =?ISO-8859-15?Q?Andreas_F=E4rber?= Il 07/10/2014 14:16, Markus Armbruster ha scritto: >> > Possibly, except this would propagate all the way through the APIs. For >> > example, right now [*] is added automatically to MemoryRegion >> > properties, but this can change in the future since many MemoryRegions >> > do not need array-like names. Then you would have two sets of >> > MemoryRegion creation APIs, one that array-ifies names and one that doesn't. > Why couldn't you have a separate name generator there as well? > > QOM: > * object_property_add() takes a non-magical name argument > * object_gen_name() takes a base name and generates a stream of > derived names suitable for object_property_add() > > Memory: > * memory_region_init() takes a non-magical name argument > * memory_gen_name() takes a base name... you get the idea > actually a wrapper around object_gen_name() I see what you mean; you could even reuse object_gen_name(). It looks sane, I guess one has to see a patch to judge if it also _is_ sane. :) > > > Why is it a good idea have two separate restrictions on property names? > > > A loser one that applies always (anything but '\0' and '/'), and a > > > stricter one that applies sometimes (letters, digits, '-', '.', '_', > > > starting with a letter). > > > > > > If yes, how is "sometimes" defined? > > > > It applies to objects created by the user (either in > > /machine/peripheral, or in /objects). Why the restriction? For > > -object, because creating the object involves QemuOpts. You then have > > two ways to satisfy the principle of least astonishment: > > > > 1) always use the same restriction when a user creates objects; > > > > 2) do not introduce restrictions when a user is not using QemuOpts. > > > > We've been doing (2) so far; often it is just because QMP wrappers also > > used QemuOpts, but not necessarily. So object_add just does the same. > > We've been doing (2) so far simply because we've never wasted a thought > on it! Since we're wasting thoughts now: which one do we like better? User interfaces other than QOM have been doing (2) too. netdev-add and device-add have been doing (2) because they use QemuOpts under the hood. blockdev-add has been consciously doing (2) for node-name. chardev-add has been doing (1), and I'd argue that this is a bug in chardev-add. QOM has two families of operations. One is -object/object-add/object-del. This is a high-level operation that only works with specific QOM classes (those that implement UserCreatable) and only operate on a specific part of the QOM tree (/objects). The other is qom-get/qom-set. This is a low-level operation that can explore all of the QOM tree. It cannot _create_ new objects and properties, however, so the user cannot escape the naming sandbox that we put in place for him. I think it's fair to limit the high-level operations to the same id space, no matter if they're QemuOpts based or not. > Based on experience, I'd rather not make "user-created" > vs. "system-created" a hard boundary. Once a system-created funny name > has become ABI, we can't ever let the user create it. One reason for me > to prefer (1). Anything that is outside /objects is "funny", not just anything that has weird characters in its name. The QOM API consists of "magic" object canonical paths and magic property names which, as far as I know, can be easily listed: * the aforementioned /machine.rtc-time that lets you detect missed RTC_CHANGE events * the /backend tree that includes info on the graphic consoles. Not sure if this is considered stable, but it's there. * /machine/peripheral/foo lets you peek at run-time properties of -device id=foo - virtio-ballon has a couple of run-time properties, whose status I am not certain of. Probably stable but undocumented. * /objects/bar lets you reconstruct the properties of -object id=bar - there are no such run-time properties with any promised stability. In other words, practically all of the QOM API is outside /objects. But not all hope is lost. Were we to provide user access to the creation of graphic consoles, we could preserve the /backend API via aliases and links. This way, anything that currently happens in /machine or /backend can tomorrow happen in /objects, without breaking backwards compatibility. Similarly, a QOMified block-backend could be either: * an object that QEMU creates for you when you give -device scsi-disk,id=disk,drive=foo. The canonical path could be something like /machine/peripheral/disk/drive-backend, with a link in /machine/peripheral/disk/backend. * an object that you create with -object block-backend,id=bar,blockdev=myimg and reference with -device scsi-disk,backend=bar. The canonical path would be of course /objects/bar, but the same link would exist in /machine/peripheral/disk/backend. In either case, you would be able to find the block-backend using the same QOM path and property. > So the "automatic arrayification" convenience feature added a property > name restriction. What makes us sure this is the last time we add name > restrictions? Nothing. However, does it matter, as long as the now-disallowed names were not possible at all in -object/object_add? Paolo