From: Markus Armbruster <armbru@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrange" <berrange@redhat.com>,
Markus Armbruster <armbru@redhat.com>,
Eduardo Habkost <ehabkost@redhat.com>,
qemu-devel@nongnu.org
Subject: Re: Infinite loop in bus_unparent(), qdev bug or qdev misuse?
Date: Tue, 05 May 2020 18:03:25 +0200 [thread overview]
Message-ID: <875zda8j3m.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <6fc8633a-6d91-b83a-e6cd-5f714ccaf9ea@redhat.com> (Paolo Bonzini's message of "Mon, 4 May 2020 16:58:54 +0200")
Paolo Bonzini <pbonzini@redhat.com> writes:
> On 04/05/20 16:38, Markus Armbruster wrote:
>> makes no progreess because OBJECT(dev)->parent is still null, and
>> therefore object_unparent() does nothing.
>>
>> Possible culprit: qdev_try_create() calls qdev_set_parent_bus(), which
>> adds the device to the bus, but leaves ->parent null. If this isn't
>> wrong outright, it's at least a dangerous state.
>>
>> Work-around: call qdev_set_id(dev, NULL) right after qdev_create().
>> This sets ->parent.
>
> That's a good one, and especially a safe one, since it matches
> qdev_device_add. It has the disadvantage of having to touch all
> qdev_create() calls.
Also, it moves onboard devices from /machine/unattached/ to
/machine/peripheral-anon/.
I really regard it as a work-around, not a proper solution.
> Even better however would be to move the bus argument (and thus
> qdev_set_parent_bus) to qdev_init, and likewise in qdev_device_add move
> qdev_set_id after qemu_opt_foreach. I looked at the property setters
> and couldn't find anything suspicious (somewhat to my surprise), but I
> haven't honestly tried.
Thus, we satisfy bus_unparent()'s precondition "bus children have a QOM
parent"[*] by moving "add to parent bus" next to the place where we
ensure "has QOM parent" by putting orphans under /machine/unattached/.
Makes sense.
If we add to the bus first, the precondition ceases to hold until we
realize. Ugly, but harmless unless we manage to actually call the
function then.
I suspect we can't realize first, because the realize method may want to
use the parent bus.
We could lift putting orphans under /machine/unattached from
device_set_realized() into those callers that don't already assign a QOM
parent. Possibly hundreds of places, hmm.
We could factor it out into a helper, and call it right before we add to
the parent bus. Orphans with a parent bus would get adopted ealier,
orphans without one would still get adopted at realize time.
I'll play with it.
[*] Confusing terminology; two separate parent-child relationships
next prev parent reply other threads:[~2020-05-05 16:07 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-04 14:38 Infinite loop in bus_unparent(), qdev bug or qdev misuse? Markus Armbruster
2020-05-04 14:58 ` Paolo Bonzini
2020-05-04 15:25 ` Peter Maydell
2020-05-05 16:03 ` Markus Armbruster [this message]
2020-05-05 16:26 ` Paolo Bonzini
2020-05-06 6:39 ` Markus Armbruster
2020-05-12 15:58 ` Markus Armbruster
2020-05-12 18:43 ` Paolo Bonzini
2020-05-05 8:23 ` no-reply
2020-05-05 8:24 ` no-reply
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=875zda8j3m.fsf@dusky.pond.sub.org \
--to=armbru@redhat.com \
--cc=berrange@redhat.com \
--cc=ehabkost@redhat.com \
--cc=pbonzini@redhat.com \
--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.