All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Paolo Bonzini <pbonzini@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, 02 Apr 2020 15:40:19 +0200	[thread overview]
Message-ID: <87369m3t18.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <5ae352b1-2653-c69f-f2b7-73e86b804d13@redhat.com> (Paolo Bonzini's message of "Thu, 19 Mar 2020 09:43:29 +0100")

Paolo Bonzini <pbonzini@redhat.com> writes:

> 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.

A year ago, we discussed QOM interfaces:

    Subject: Issues around TYPE_INTERFACE
    Message-ID: <87h8c82woh.fsf@dusky.pond.sub.org>

We touched naming conventions, the fact that interfaces can't have
state, how an interface type should be defined (even though it has no
state), and what it means to convert to such a type.

I'd love to see this covered in qom.rst.



  reply	other threads:[~2020-04-02 13:41 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
2020-04-02 13:40                                       ` Markus Armbruster [this message]
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=87369m3t18.fsf@dusky.pond.sub.org \
    --to=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=pbonzini@redhat.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 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.