All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Andreas Färber" <afaerber@suse.de>
To: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	Stefan Hajnoczi <stefanha@gmail.com>,
	"qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>,
	Markus Armbruster <armbru@redhat.com>,
	Anthony Liguori <anthony@codemonkey.ws>,
	Jan Kiszka <jan.kiszka@siemens.com>,
	"Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Subject: Re: [Qemu-devel] Selecting device variant types based on bdrv size
Date: Mon, 27 May 2013 15:40:52 +0200	[thread overview]
Message-ID: <51A36264.6070002@suse.de> (raw)
In-Reply-To: <CAEgOgz4JEzFi8TMrkBC9CYkK+PZL3HNRFREgumYBz__S8ee8aw@mail.gmail.com>

Hi,

Am 27.05.2013 09:50, schrieb Peter Crosthwaite:
> I have a bit of a chicken and egg problem trying to refactor Jans AT24
> I2C EEPROM model. I'm trying to migrate static class properties up to
> the class level rather than down on the device property level (as we
> did for EHCI in the sysbusification a while back). Problem is the
> device model has part autodetection logic based on the size of the
> backing image - you can instantiate the "abstract" class and selection
> of what part it is depends on the backing file size. And so if we go
> for one class for each separate part, we don't actually know what
> concrete class to instantiate until we have a handle on the bdrv at
> realize time which is way to late. Any Ideas?

I think the practical question to ask is: What's the difference between
those subclasses? Then maybe you can initialize YourAutoType::field from
DerivedTypeClass::template_field or the like. I.e., the class is
supposed to exist only once, so you can't modify it beyond class_init,
but you can modify the instance including field values and per-instance
callback hooks.

For a vaguely related example, you may want to look at the history of
target-ppc/kvm.c for how I previously mutated the host-ppc-cpu type -
depending on the host, not user parameters (today it uses inheritance
from a dynamically chosen base type instead; reason was not a technical
one but that target-ppc/kvm.c does not get compile-tested on x86 if
someone changes/adds ppc-cpu fields - no concern for regular devices).

FWIW bdrv not fitting well into the realize scheme was the main reason
behind going for DeviceState rather than Object for realize. ;)

BTW do we have any guidance of when to use properties vs. subclasses?
Might be a good addition to the QOMConventions page since it recently
came up for CAN as well.

> Can you safely change a devices type at realize time?
> 
> realize() {
>    ...
>    OBJECT(dev)->class = the_now_known_correct_child_class;
>    ...
> }
>
> Obviously this would need an API call in QOM to sanity check it.

Short answer: No, such a mutation is generally unsafe.

Instance sizes can differ between types - could be sanity-checked.
A type can expect to get access to its final class on instance_init.
instance_init may init fields that you can only get by instantiating.
A type mutation would change child<> or link<> properties at runtime.
Realize will be too late to tweak the resulting instance further.
Real OO languages don't support it, causing QOM lock-in.

So I think this is rather hinting into the direction of a three-stage
construction - instance_init, open, realize - as discussed by
Kevin/Markus some time ago.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

  reply	other threads:[~2013-05-27 13:41 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-27  7:50 [Qemu-devel] Selecting device variant types based on bdrv size Peter Crosthwaite
2013-05-27 13:40 ` Andreas Färber [this message]
2013-05-27 22:48   ` Peter Crosthwaite

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=51A36264.6070002@suse.de \
    --to=afaerber@suse.de \
    --cc=anthony@codemonkey.ws \
    --cc=armbru@redhat.com \
    --cc=edgar.iglesias@gmail.com \
    --cc=jan.kiszka@siemens.com \
    --cc=kwolf@redhat.com \
    --cc=peter.crosthwaite@xilinx.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@gmail.com \
    /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.