qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	zhanghailiang <zhang.zhanghailiang@huawei.com>,
	"Pan Nengyuan" <pannengyuan@huawei.com>,
	"Mark Cave-Ayland" <mark.cave-ayland@ilande.co.uk>,
	"Laurent Vivier" <laurent@vivier.eu>,
	"QEMU Developers" <qemu-devel@nongnu.org>,
	"Euler Robot" <euler.robot@huawei.com>,
	"Eduardo Habkost" <ehabkost@redhat.com>
Subject: Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via
Date: Thu, 19 Mar 2020 09:43:29 +0100	[thread overview]
Message-ID: <5ae352b1-2653-c69f-f2b7-73e86b804d13@redhat.com> (raw)
In-Reply-To: <87v9n025hc.fsf@dusky.pond.sub.org>

On 19/03/20 08:01, Markus Armbruster wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
>> On 18/03/20 16:06, Markus Armbruster wrote:
>>>> - object initialization should cause no side effects outside the subtree
>>>> of the object
>>>
>>> object_initialize_child() and its users like sysbus_init_child_obj()
>>> violate this rule: they add a child property to the subtree's parent.
>>> Correct?
>>
>> At least object_initialize_child() adds the property to the object
>> itself, so it's fine.
> 
> It seems to
> 
> 1. Initialize @childobj
> 2. Set a bunch of properties
> 3. Add the child property to @parentobj
> 4. Call .complete() if it's a TYPE_USER_CREATABLE
> 5. Adjust reference count
> 
> Step 3. modifies outside the sub-tree rooted at @childobj.  What am I
> missing?
> 
> Hmm, maybe this: using object_initialize_child() when initializing
> @parentobj is fine; while object_initialize_child() leaves the sub-tree
> rooted at @childobj, it still stays within the sub-tree rooted at
> @parentobj.
> 
> If this is how the function must be used, its contract should spell it
> out!

Yes, this is what I meant.

>>>> - realization can also cause side effects outside the subtree of the
>>>> object, but if unrealization is possible, they must be undone by
>>>> unrealization. if an object is realized and unrealization is not
>>>> possible, finalization will never run (and in that case, side effects of
>>>> realization need not be undone at all).
>>>
>>> One possible answer the question what should be done in realize() is
>>> whatever must not be done earlier.  Is that the best we can do?
>>
>> That's the lower bound of descriptivity.  The upper bound is anything
>> that is visible from the guest.  The truth could be in the middle.
> 
> Can we set aside a bit of time to write docs/devel/qom.rst together?  I
> know object.h is lovingly commented, but unless you already know how QOM
> works, you're drowning in detail there.  You'd have to provide most of
> the contents, I'm afraid, because you know so much more about it.  Could
> save you time in the long run, though: fewer questions to answer, fewer
> mistakes to correct.

Yes, this is sorely needed.  I will do it; if you have more random
questions you'd like an answer for, that can help.  I can then stitch
them together in a coherent text.

Paolo



  reply	other threads:[~2020-03-19  8:44 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-05  6:54 [PATCH v4 0/3] delay timer_new from init to realize to fix memleaks Pan Nengyuan
2020-03-05  6:46 ` no-reply
2020-03-05  6:54 ` [PATCH v4 1/3] s390x: fix memleaks in cpu_finalize Pan Nengyuan
2020-03-05  8:34   ` David Hildenbrand
2020-03-05  9:03     ` Pan Nengyuan
2020-03-05  6:54 ` [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via Pan Nengyuan
2020-03-05  7:10   ` Pan Nengyuan
2020-03-05  7:18   ` Pan Nengyuan
2020-03-08 13:29   ` Peter Maydell
2020-03-09  0:56     ` Pan Nengyuan
2020-03-09  9:21       ` Peter Maydell
2020-03-09 10:02         ` Pan Nengyuan
2020-03-09 10:10           ` Peter Maydell
2020-03-09 12:34             ` Markus Armbruster
2020-03-09 12:51               ` Pan Nengyuan
2020-03-09 14:14                 ` Markus Armbruster
2020-03-09 16:16                   ` Mark Cave-Ayland
2020-03-10  0:34                     ` Pan Nengyuan
2020-03-10  9:07             ` Markus Armbruster
2020-03-10  9:41               ` Peter Maydell
2020-03-10 12:38               ` BALATON Zoltan
2020-03-14 13:19               ` Mark Cave-Ayland
2020-03-14 14:03                 ` Paolo Bonzini
2020-03-15 14:56                   ` Markus Armbruster
2020-03-15 17:58                     ` Paolo Bonzini
2020-03-16  6:03                       ` Markus Armbruster
2020-03-16  8:43                         ` Paolo Bonzini
2020-03-18 13:02                           ` Markus Armbruster
2020-03-18 13:21                             ` Paolo Bonzini
2020-03-18 14:58                               ` Peter Maydell
2020-03-18 15:06                               ` Markus Armbruster
2020-03-18 16:44                                 ` Paolo Bonzini
2020-03-19  7:01                                   ` Markus Armbruster
2020-03-19  8:43                                     ` Paolo Bonzini [this message]
2020-04-02 13:40                                       ` Markus Armbruster
2020-03-15 15:16                 ` Markus Armbruster
2020-03-05  6:54 ` [PATCH v4 3/3] hw/misc/mos6522: move timer_new from init() into realize() to avoid memleaks Pan Nengyuan
2020-03-05 22:56   ` David Gibson
2020-03-06  0:50     ` Pan Nengyuan
2020-03-13  6:50     ` David Gibson
2020-03-08 11:58 ` [PATCH v4 0/3] delay timer_new from init to realize to fix memleaks Mark Cave-Ayland
2020-03-08 13:39   ` Peter Maydell
2020-03-09  0:49     ` Pan Nengyuan
2020-03-09 16:14     ` Mark Cave-Ayland

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=5ae352b1-2653-c69f-f2b7-73e86b804d13@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=euler.robot@huawei.com \
    --cc=laurent@vivier.eu \
    --cc=mark.cave-ayland@ilande.co.uk \
    --cc=pannengyuan@huawei.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=zhang.zhanghailiang@huawei.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).