qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Anthony Liguori <aliguori@us.ibm.com>
To: Paolo Bonzini <pbonzini@redhat.com>, qemu-devel@nongnu.org
Cc: peter.maydell@linaro.org, Liu Ping Fan <qemulist@gmail.com>
Subject: Re: [Qemu-devel] [PATCH 1.3 0/5] QOM/qdev lifetime fixes
Date: Mon, 26 Nov 2012 15:47:16 -0600	[thread overview]
Message-ID: <87zk24ox3v.fsf@codemonkey.ws> (raw)
In-Reply-To: <1353660436-8897-1-git-send-email-pbonzini@redhat.com>

Paolo Bonzini <pbonzini@redhat.com> writes:

> These patches fix problems in the handling of freeing QOM/qdev
> objects.  Together, they fix hot-unplug of USB mass storage devices,
> which crashed with an assertion failure.
>
> I'm not 100% sure, but I think we were always leaking the scsi-disk in
> pre-QOM days.  Now we're freeing it properly, and the assertion proves it.
>
> However, I don't like particularly the assertion in object_delete.  Once
> we're sure we've fixed all bugs, we should remove it, because it prevents
> a fully correct tracking of references.
>
> In this case, for example, there is still one reference to the scsi-disk
> in the MSDState's scsi_dev member.  We don't have neither an object_ref
> nor an object_unref for it, so it happens to work.  If we had an
> object_ref, the matching object_unref would be in dc->exit.  But then
> we'd trip on the assertion failure again, because the SCSI bus is removed
> (thus calling qdev_free on the scsi-dev) before dc->exit is called.
>
> I have more patches to actually make the reference count of devices
> and buses fully correct, but they are even more scary than these, so
> they should wait for 1.4.
>

Applied. Thanks.

My patches didn't fix Peter's problem but yours do, I figured we'd take
your version in 1.3 and then for 1.4 I can attempt to rework them.

Regards,

Anthony Liguori

> Paolo Bonzini (5):
>   qom: fix refcount of non-heap-allocated objects
>   qdev: move bus removal to object_unparent
>   qom: make object_delete usable for statically-allocated objects
>   qdev: simplify (de)allocation of buses
>   qom: make object_finalize static
>
>  hw/qdev-core.h        |  5 -----
>  hw/qdev.c             | 26 ++++++++++++++------------
>  hw/pci.c              |  2 +-
>  hw/sysbus.c           |  2 +-
>  include/qemu/object.h | 29 ++++++++++++++++++++---------
>  qom/object.c          | 12 +++++++++---
>  6 files changed, 45 insertions(+), 31 deletions(-)
>
> -- 
> 1.8.0

      parent reply	other threads:[~2012-11-26 21:47 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-23  8:47 [Qemu-devel] [PATCH 1.3 0/5] QOM/qdev lifetime fixes Paolo Bonzini
2012-11-23  8:47 ` [Qemu-devel] [PATCH 1.3 1/5] qom: fix refcount of non-heap-allocated objects Paolo Bonzini
2012-11-23 16:49   ` Andreas Färber
2012-11-26 15:49   ` Anthony Liguori
2012-11-26 16:08     ` Paolo Bonzini
2012-11-23  8:47 ` [Qemu-devel] [PATCH 1.3 2/5] qdev: move bus removal to object_unparent Paolo Bonzini
2012-11-23 16:52   ` Andreas Färber
2012-11-27  1:02   ` Andreas Färber
2012-11-23  8:47 ` [Qemu-devel] [PATCH 1.3 3/5] qom: make object_delete usable for statically-allocated objects Paolo Bonzini
2012-11-23 17:02   ` Andreas Färber
2012-11-23 17:10     ` Paolo Bonzini
2012-11-23  8:47 ` [Qemu-devel] [PATCH 1.3 4/5] qdev: simplify (de)allocation of buses Paolo Bonzini
2012-11-23 17:07   ` Andreas Färber
2012-11-23 17:16     ` Paolo Bonzini
2012-11-23  8:47 ` [Qemu-devel] [PATCH 1.3 5/5] qom: make object_finalize static Paolo Bonzini
2012-11-23 17:12   ` Andreas Färber
2012-11-23 17:32     ` Paolo Bonzini
2012-11-26 21:47 ` Anthony Liguori [this message]

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=87zk24ox3v.fsf@codemonkey.ws \
    --to=aliguori@us.ibm.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemulist@gmail.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).