From: Stefan Hajnoczi <stefanha@redhat.com>
To: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Cc: kwolf@redhat.com, jcody@redhat.com, Fam Zheng <famz@redhat.com>,
qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v3 5/7] migration: omit drive ref as we have bdrv_ref now
Date: Fri, 2 Aug 2013 17:04:32 +0200 [thread overview]
Message-ID: <20130802150432.GH815@stefanha-thinkpad.redhat.com> (raw)
In-Reply-To: <51FB6A43.1050108@linux.vnet.ibm.com>
On Fri, Aug 02, 2013 at 04:13:55PM +0800, Wenchao Xia wrote:
> There should be a section of code in device hot unplug, checking
> DriverInfo's ref, fail or do nothing when ref != 1. But I haven't found
> that code, so not sure whether this patch will change the behavior in
> device hot unplug.
It is not necessary to refuse device hot unplug when bs.ref > 0. But a
couple of fixes are required to make it safe:
Background
----------
DriveInfo always holds a BDS refcount, so BDS can never be deleted while
the DriveInfo exists.
DriveInfo is the metadata that connects an emulated storage controller
with its BDS. Therefore, hot unplugging an emulated storage controller
may release the last DriveInfo reference and delete it.
Anything that still holds the BDS reference when DriveInfo is deleted
will keep the BDS alive. It turns out that a lot of commands only use
BDS with bdrv_find(), not DriveInfo, so you can continue to do useful
things with the BDS after its DriveInfo is deleted.
The problem
-----------
A couple of places have not been converted to use bdrv_ref() yet, so
they still go through drive_get_ref(drive_get_by_blockdev(bs)). These
cases will now fail! I pointed out the blockjob cases but please grep
to make sure there are no others.
You need to fix them before this series is safe, otherwise
drive_put_ref(NULL) will segfault!
Stefan
next prev parent reply other threads:[~2013-08-02 19:57 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-31 10:13 [Qemu-devel] [PATCH v3 0/7] Implement reference count for BlockDriverState Fam Zheng
2013-07-31 10:13 ` [Qemu-devel] [PATCH v3 1/7] vvfat: use bdrv_new() to allocate BlockDriverState Fam Zheng
2013-08-02 7:23 ` Wenchao Xia
2013-07-31 10:13 ` [Qemu-devel] [PATCH v3 2/7] iscsi: use bdrv_new() instead of stack structure Fam Zheng
2013-08-02 7:34 ` Wenchao Xia
2013-07-31 10:13 ` [Qemu-devel] [PATCH v3 3/7] block: implement reference count for BlockDriverState Fam Zheng
2013-08-02 7:36 ` Wenchao Xia
2013-07-31 10:13 ` [Qemu-devel] [PATCH v3 4/7] block: make bdrv_delete() static Fam Zheng
2013-08-02 7:40 ` Wenchao Xia
2013-07-31 10:13 ` [Qemu-devel] [PATCH v3 5/7] migration: omit drive ref as we have bdrv_ref now Fam Zheng
2013-08-02 8:13 ` Wenchao Xia
2013-08-02 15:04 ` Stefan Hajnoczi [this message]
2013-07-31 10:13 ` [Qemu-devel] [PATCH v3 6/7] xen_disk: simplify blk_disconnect with refcnt Fam Zheng
2013-08-02 8:24 ` Wenchao Xia
2013-07-31 10:14 ` [Qemu-devel] [PATCH v3 7/7] nbd: use BlockDriverState refcnt Fam Zheng
2013-08-02 8:28 ` Wenchao Xia
2013-08-01 12:23 ` [Qemu-devel] [PATCH v3 0/7] Implement reference count for BlockDriverState Stefan Hajnoczi
2013-08-01 12:28 ` Stefan Hajnoczi
2013-08-02 2:22 ` Fam Zheng
2013-08-02 15:06 ` Stefan Hajnoczi
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=20130802150432.GH815@stefanha-thinkpad.redhat.com \
--to=stefanha@redhat.com \
--cc=famz@redhat.com \
--cc=jcody@redhat.com \
--cc=kwolf@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=xiawenc@linux.vnet.ibm.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).