qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Anthony Liguori <anthony@codemonkey.ws>
To: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	Markus Armbruster <armbru@redhat.com>,
	Peter Maydell <peter.maydell@linaro.org>
Subject: Re: [Qemu-devel] [PATCH 00/14] qdev: assign unique names to all devices (part 1)
Date: Mon, 19 Sep 2011 09:05:17 -0500	[thread overview]
Message-ID: <4E774C1D.4050000@codemonkey.ws> (raw)
In-Reply-To: <4E76EE9F.3020109@siemens.com>

On 09/19/2011 02:26 AM, Jan Kiszka wrote:
> On 2011-09-16 20:03, Anthony Liguori wrote:
>> So this is a simplification that I plan on running with.  For now, I think this
>> series is the right next step because it gives us a path name for the name
>> (although different syntax) and let's us enforce that all devices has a
>> canonical path.
>
> For something that changes lots of devices and, at the same time, is
> going to be removed again, I'm hesitating to call it the right direction.
>
> A right step would be, IMHO, to introduce a generic introspectable
> device link so that parent devices can reference their children and a
> visitor can derive a child's relative name from that link name. Then
> make sure this link type is consistently used.
>
> I really dislike this focusing on assigning names internally and using
> them in QEMU-internal APIs. They should just fall out of the core when
> external interaction is required.

I thought a lot about this over the weekend and decided that I should go in a 
different direction based on this discussion.

Starting with the requirement that you have to be able to ask the device for an 
unambiguous reference to itself, I see two possible approaches:

1) Store the unambiguous reference within the child, derive unambiguous 
reference from the composition hierarchy.

2) Store a reference to the composition parent in the child.  When asked for the 
unambiguous reference, use parent point to generate the reference.

These two approaches have subtle differences.  For (1), all devices must be 
given globally unique names.  For (2), all devices must be given names that are 
unique to its composition parent.

I really dislike having a 'Device *parent' pointer because I fear it will be 
abused, but there a number of elegant advantages to this model.  For instance:

a) There is nothing special about user created devices.  All user created 
devices sit on the root of the composition hierarchy.  The names must be unique 
within that level of the composition tree.  However, once you get one level 
down, it's a new namespace.  This means you can achieve uniqueness without 
special characters or any of that nonsense.

b) We have proper path components without parsing a string and assigning special 
meaning to characters.

Point (b) is especially important because I spent a lot of time thinking about 
how to address Kevin's concerns about usability.  I think we can significantly 
improve path usability by trying to do an unambiguous match from right-to-left 
on the path components.  For instance, if you have:

/i440fx
/i440fx/piix3
/i440fx/piix3/ide
/i440fx/slot[1.0]
/i440fx/slot[2.0]

You can do:

  -device isa-serial,bus=piix3  (or)
  -device isa-serial,bus=i440fx/piix3 (or)
  -device isa-serial,bus=/i440fx/piix3

The first two examples are relevant, but unambiguous paths, matched from 
right-to-left.  The last example is an absolute path.  Since the vast majority 
of the time, you have an unambiguous path by simple taking the last and possibly 
second-to-last path component, I think that most of the time you can use very 
short relative paths.

I think this ends up being a big usability improvement and the only way to 
implement this (1) is parsing device names and extract paths which is even more 
evil than having a composition parent link in the Device.

So I'm now rebasing my series and switching to this model.  It's a rather 
significant change but I think I have a grip on how to do it.

Regards,

Anthony Liguori


>
>>
>> Independent of that, Jan suggested that we could have what's essentially an
>> alias.  This is just a short name (could be in the form '%s-%d' %
>> (class.lower(), object_count).  This alias is just a hash table.  It has nothing
>> to do with the core device model.
>
> I can't remember suggesting such thing.
>
> Jan
>

  reply	other threads:[~2011-09-19 14:05 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-16 16:00 [Qemu-devel] [PATCH 00/14] qdev: assign unique names to all devices (part 1) Anthony Liguori
2011-09-16 16:00 ` [Qemu-devel] [PATCH 01/14] apic: rename apic.id -> apic.index Anthony Liguori
2011-09-16 16:00 ` [Qemu-devel] [PATCH 02/14] qdev: enforce that no devices overload the id property Anthony Liguori
2011-09-16 16:00 ` [Qemu-devel] [PATCH 03/14] qdev: push id into qdev_create calls Anthony Liguori
2011-09-16 16:00 ` [Qemu-devel] [PATCH 04/14] qdev: take ownership of id pointer Anthony Liguori
2011-09-19  7:34   ` Gerd Hoffmann
2011-09-19 16:27     ` Anthony Liguori
2011-09-20  6:36       ` Gerd Hoffmann
2011-09-20 13:04         ` Anthony Liguori
2011-09-20 13:21           ` Gerd Hoffmann
2011-09-20 13:55             ` Anthony Liguori
2011-09-20 14:11               ` Gerd Hoffmann
2011-09-16 16:00 ` [Qemu-devel] [PATCH 05/14] qdev: remove opts pointer tracking Anthony Liguori
2011-09-19  7:35   ` Gerd Hoffmann
2011-09-16 16:00 ` [Qemu-devel] [PATCH 06/14] qdev: add ability to do QOM-style derived naming Anthony Liguori
2011-09-17 18:39   ` Blue Swirl
2011-09-16 16:00 ` [Qemu-devel] [PATCH 07/14] sysbus: add an id argument to sysbus_create_simple() Anthony Liguori
2011-09-16 16:00 ` [Qemu-devel] [PATCH 08/14] sysbus: make create_varargs take an id Anthony Liguori
2011-09-16 16:00 ` [Qemu-devel] [PATCH 09/14] fw_cfg: add name to global fw_cfg device Anthony Liguori
2011-09-16 16:00 ` [Qemu-devel] [PATCH 10/14] isa: add name parameter to device creation Anthony Liguori
2011-09-16 16:00 ` [Qemu-devel] [PATCH 11/14] pci: obtain devfn before initializing the device Anthony Liguori
2011-09-16 16:00 ` [Qemu-devel] [PATCH 12/14] pci: give pci devices a default name Anthony Liguori
2011-09-16 16:00 ` [Qemu-devel] [PATCH 13/14] ide: give IDE drives a default name in qdev Anthony Liguori
2011-09-16 16:00 ` [Qemu-devel] [PATCH 14/14] pc: assign names to machine created devices Anthony Liguori
2011-09-16 16:22 ` [Qemu-devel] [PATCH 00/14] qdev: assign unique names to all devices (part 1) Anthony Liguori
2011-09-16 16:48 ` Jan Kiszka
2011-09-16 16:54   ` Anthony Liguori
2011-09-16 17:03     ` Jan Kiszka
2011-09-16 18:06       ` Anthony Liguori
2011-09-16 17:11     ` Kevin Wolf
2011-09-16 18:03       ` Anthony Liguori
2011-09-19  7:26         ` Jan Kiszka
2011-09-19 14:05           ` Anthony Liguori [this message]
2011-09-19 14:24             ` Kevin Wolf
2011-09-20  8:32               ` Jan Kiszka
2011-09-19  7:41         ` Kevin Wolf
2011-09-16 18:21   ` Anthony Liguori
2011-09-19  7:34     ` Jan Kiszka
2011-09-17 18:41 ` Blue Swirl

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E774C1D.4050000@codemonkey.ws \
    --to=anthony@codemonkey.ws \
    --cc=armbru@redhat.com \
    --cc=jan.kiszka@siemens.com \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).