From: Paolo Bonzini <pbonzini@redhat.com>
To: "Andreas Färber" <afaerber@suse.de>,
"Markus Armbruster" <armbru@redhat.com>
Cc: qemu-devel@nongnu.org, "Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] qdev: free qemu-opts when the QOM path goes away
Date: Thu, 5 Nov 2015 13:21:21 +0100 [thread overview]
Message-ID: <563B49C1.9080305@redhat.com> (raw)
In-Reply-To: <563B4629.10903@suse.de>
On 05/11/2015 13:06, Andreas Färber wrote:
> > 1. Wouldn't it be cleaner to delete dev-opts *before* sending
> > DEVICE_DELETED? Like this:
> >
> > +++ b/hw/core/qdev.c
> > @@ -1244,6 +1244,9 @@ static void device_unparent(Object *obj)
> > dev->parent_bus = NULL;
> > }
> >
> > + qemu_opts_del(dev->opts);
> > + dev->opts = NULL;
> > +
> > /* Only send event if the device had been completely realized */
> > if (dev->pending_deleted_event) {
> > gchar *path = object_get_canonical_path(OBJECT(dev));
>
> To me this proposal sounds sane, but I did not get to tracing the code
> flow here. Paolo, which approach do you prefer and why?
It doesn't really matter, because the BQL is being held here.
On the other hand, if the opts are deleted in finalize, there is an
arbitrary delay because finalize is typically called after a
synchronize_rcu period.
>> > 2. If the device is a block device, then unplugging it also deletes its
>> > backend (ugly wart we keep for backward compatibility; *not* for
>> > blockdev-add, though). This backend also has a QemuOpts. It gets
>> > deleted in drive_info_del(). Just like device_finalize(), it runs
>> > within object_unref(), i.e. after DEVICE_DELETED is sent. Same race,
>> > different ID, or am I missing something?
>> >
>> > See also https://bugzilla.redhat.com/show_bug.cgi?id=1256044
>
> If we can leave this patch decoupled from block layer and decide soonish
> on the desired approach, I'd be happy to include it in my upcoming
> qom-devices pull.
I agree with you, the block layer bug is separate.
Paolo
next prev parent reply other threads:[~2015-11-05 12:21 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-19 11:11 [Qemu-devel] [PATCH] qdev: free qemu-opts when the QOM path goes away Paolo Bonzini
2015-11-04 15:53 ` Paolo Bonzini
2015-11-04 18:34 ` Markus Armbruster
2015-11-05 12:06 ` Andreas Färber
2015-11-05 12:21 ` Paolo Bonzini [this message]
2015-11-05 12:47 ` Markus Armbruster
2016-01-15 17:03 ` Andreas Färber
2016-01-15 17:16 ` Paolo Bonzini
2016-01-15 17:36 ` Andreas Färber
2016-01-18 9:45 ` Markus Armbruster
2016-01-08 18:17 ` 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=563B49C1.9080305@redhat.com \
--to=pbonzini@redhat.com \
--cc=afaerber@suse.de \
--cc=armbru@redhat.com \
--cc=mst@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 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).