All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Anthony Liguori <anthony@codemonkey.ws>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v2 00/18] qom: dynamic properties and composition tree (v2)
Date: Mon, 05 Dec 2011 16:47:19 +0100	[thread overview]
Message-ID: <4EDCE787.8070306@redhat.com> (raw)
In-Reply-To: <4EDCD6EA.7090500@codemonkey.ws>

On 12/05/2011 03:36 PM, Anthony Liguori wrote:
> It's to support method inheritance. In qdev, the various DeviceInfo
> structures correspond roughly to the class of the object. When you
> create an ISADeviceInfo (the ISA subclass), you declare it statically.
>
> Any methods you aren't overriding are going to be initialized to zero.
> You want those methods to inherit their values from the base class. To
> do this in qdev, you have to introduce a base-class specific
> registration function (isa_qdev_register).
>
> There's not a lot of discipline in how these functions are implemented
> and generally makes type registration more complicated as you have to
> understand what methods get overridden.

Yeah, that's true.  I think in general our class hierarchy is shallow 
enough that we could live with that, but I appreciate that dynamic 
initialization has advantages.

> links are nullable and usually start out as NULL.
>
> childs are not nullable. I can't really think of a reason why they
> should be nullable. What are you thinking here?

Ok, I understand now better what children are.

> I've thought a lot about bus properties. I've looked at a lot of code at
> this point and for the most part, I think that the reason they even
> exist is because we can't inherit a default set of properties.
>
> SCSI is a good example. The bus properties really make more sense as
> SCSIDevice properties that are inherited.

Yeah, bus properties *are* most of the time properties that you add to 
the abstract class, so...

> I dislike these properties in the first place, but I'd like to find a
> way to convert everything to the QOM type system before we start
> rearchitecting how hotplug works.

... just change them to properties on the abstract class.

>> Perhaps hidden with some macro that lets me just write
>> SCSI_BUS_INTERFACE(dev), but that's the idea; such a lookup function is
>> pretty much what all object models do. GObject has
>> G_TYPE_INSTANCE_GET_INTERFACE, COM/XPCOM has QueryInterface, etc.
>>
>> If I understood everything so far, then here is my question. Are
>> interfaces properties?
>
> No. A device is-a interface. Hopefully the above example will make it
> more clear.

No, but I'm confident that there will be a sane way to access the list 
of interfaces that you embed in the Object type. :)

>> That's not what I meant. The legacy<> namespace splits the set of QOM
>> properties in two parts, sane ones and legacy ones. That's wrong,
>> because the old broken interface remains there. Worse, it remains
>> there as user-visible API in the JSON etc., and it will remain forever
>> since we
>> cannot get rid of -device overnight.
>>
>> What I suggested is to provide two implementations for each old-style
>> property: an old string-based one (used for -device etc.) and a modern
>> visitor-based one (used for qom_*). In other words, old-style
>> properties would expose both a print/parse legacy interface, and a sane
>> get/set visitor interface. No need for a legacy<> namespace, because
>> new-style consumers would not see the old-style string ABI.
>
> Yeah, I'd like to do something like this but I'm in no rush. I agree
> that when we declare QOM as a supported interface, we should have
> replacements for anything that's in the legacy<> space. That may be from
> some magic Property tricks where we introduce Visitor to parse/print or
> because we introduce new and improved properties.

Yeah, extending Property looks like a feasible plan.  The get/set pair 
is an adaptor between JSON/Visitor-type data and C struct fields, the 
parse/print pair is an adaptor between strings and C struct fields.

> Maybe now is the right time to rename the legacy properties to all be
> prefixed with qdev-? That way we don't need to introduce two different
> types for a single property.

Why do you need such a prefix?

Paolo

  parent reply	other threads:[~2011-12-05 15:47 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-03  0:56 [Qemu-devel] [PATCH v2 00/18] qom: dynamic properties and composition tree (v2) Paolo Bonzini
2011-12-03  2:40 ` Anthony Liguori
2011-12-03 14:24   ` Paolo Bonzini
2011-12-03 21:34     ` Anthony Liguori
2011-12-04 21:01       ` Anthony Liguori
2011-12-05  9:52         ` Paolo Bonzini
2011-12-05 14:36           ` Anthony Liguori
2011-12-05 14:50             ` Peter Maydell
2011-12-05 15:04               ` Anthony Liguori
2011-12-05 15:33                 ` Peter Maydell
2011-12-05 19:28                   ` Anthony Liguori
2011-12-05 15:47             ` Paolo Bonzini [this message]
2011-12-05 16:13               ` Anthony Liguori
2011-12-05 16:29                 ` Paolo Bonzini
2011-12-05 16:38                   ` Anthony Liguori
2011-12-05 17:01                     ` Paolo Bonzini
  -- strict thread matches above, loose matches on Subject: below --
2011-12-02 20:20 Anthony Liguori

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=4EDCE787.8070306@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=anthony@codemonkey.ws \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.