qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Andreas Färber" <afaerber@suse.de>
To: Liviu Ionescu <ilg@livius.net>, QEMU Developers <Qemu-devel@nongnu.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Subject: Re: [Qemu-devel] [RFC] QDev explicit constructors & destructors
Date: Tue, 23 Jun 2015 12:39:12 +0200	[thread overview]
Message-ID: <55893750.3020502@suse.de> (raw)
In-Reply-To: <803004FB-2F23-439C-AF59-E71FA7DDC2C4@livius.net>

Hi Liviu,

Am 15.06.2015 um 15:35 schrieb Liviu Ionescu:
>> On 15 Jun 2015, at 13:48, Liviu Ionescu <ilg@livius.net> wrote:
>>
>> ... One natural solution would be to add an explicit constructor, like in C++. 
> 
> if this solution is considered too complicated, it might also be possible to arrange for the constructor data to be passed to the existing .instance_init call.
> 
> the use case would be even simpler:
> 
>   DeviceState *mcu = qdev_create_with_data(NULL, TYPE_STM32F103RB, &constructor_data);
>   {
>       /* Set various properties. */
>       qdev_prop_set_uint32(mcu, "hse-freq-hz", 8000000); /* 8.0 MHz */
>       qdev_prop_set_uint32(mcu, "lse-freq-hz", 32768); /* 32 KHz */
>   }

These {...} blocks have no justification at all, so please avoid them
for the sake of readability. Just use empty lines for grouping, that
spares indentation.

>   qdev_realize(mcu); /* QDev specific step */
[snip]

Please don't add new infrastructure with "qdev" in the name, qdev was
the old device infrastructure that has been replaced with QOM; use
"device" naming for any new APIs.

But really, you should read up on the two discussions, a) when Anthony
introduced QOM (several looong threads with Paolo, Avi and others) and
b) when I ran into the issue of virtual methods (start with the
resulting documentation in object.h).

You seem to be opening a can of worms without understanding where we're
coming from, how it's being used and where this is headed: QOM is not
just a random C++-in-C framework. We've specifically banned pointers
except for a handful by converting to link<> properties, and I see
adding random opaque pointers to the generic constructor model - be it
for devices or all objects - as a clear no-no. Look at how QOM objects
are being instantiated from QMP commands or config files as a hint to
what you're missing here (apart from CC'ing the right maintainers ;)).
(Note that quite similarly Linux has been replacing platform data
structs with Device Tree nodes.)

If you do in some local case want to pass local C data to the object,
you can already do so via .class_data if all instances are the same, as
in your case of a single MCU on a board probably.

If I'm not seeing the problem, you'll need to explain better why
class_init + instance_init + properties + realize doesn't fit your use case.

That said, your efforts of simplifying STM32 instantiation are
appreciated. I've been wanting to add an F429 Discovery machine for my
Linux port.

Cheers,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)

  parent reply	other threads:[~2015-06-23 10:39 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-15 10:48 [Qemu-devel] [RFC] QDev explicit constructors & destructors Liviu Ionescu
2015-06-15 11:13 ` Liviu Ionescu
2015-06-15 13:35 ` Liviu Ionescu
2015-06-22 20:48   ` Liviu Ionescu
2015-06-23  7:47     ` Markus Armbruster
2015-06-23  9:12       ` Liviu Ionescu
2015-06-23 10:39   ` Andreas Färber [this message]
2015-06-23 12:58     ` Liviu Ionescu
2015-06-23 14:25       ` Andreas Färber
2015-06-23 16:15         ` Liviu Ionescu
2015-06-23 18:31         ` Peter Crosthwaite
2015-06-23 19:10           ` Liviu Ionescu
2015-06-23 20:57             ` Peter Crosthwaite
2015-06-23 21:24               ` Liviu Ionescu
2015-06-23 19:27           ` Andreas Färber
2015-06-23 20:10           ` Liviu Ionescu
2015-06-24  7:30             ` Liviu Ionescu
2015-06-24  8:29               ` Peter Crosthwaite
2015-06-24  9:29                 ` Liviu Ionescu
2015-06-24  9:51                   ` Paolo Bonzini
2015-06-24 13:41                     ` Andreas Färber
2015-06-24 13:50                   ` Andreas Färber
2015-06-24 14:11                     ` Liviu Ionescu
2015-06-24 14:18                       ` Andreas Färber
2015-06-24 14:39                         ` Liviu Ionescu
2015-06-24 14:41                           ` Andreas Färber
2015-06-24 20:14                       ` Liviu Ionescu
2015-06-24  8:51             ` Paolo Bonzini

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=55893750.3020502@suse.de \
    --to=afaerber@suse.de \
    --cc=Qemu-devel@nongnu.org \
    --cc=ilg@livius.net \
    --cc=pbonzini@redhat.com \
    --cc=peter.crosthwaite@xilinx.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 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).