qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Fam Zheng <famz@redhat.com>
To: Stefan Hajnoczi <stefanha@redhat.com>
Cc: kwolf@redhat.com, jcody@redhat.com, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v3 0/7] Implement reference count for BlockDriverState
Date: Fri, 2 Aug 2013 10:22:13 +0800	[thread overview]
Message-ID: <20130802022213.GA2864@T430s.nay.redhat.com> (raw)
In-Reply-To: <20130801122821.GC2387@stefanha-thinkpad.redhat.com>

On Thu, 08/01 14:28, Stefan Hajnoczi wrote:
> On Wed, Jul 31, 2013 at 06:13:53PM +0800, Fam Zheng wrote:
> > BlockDriverState lifecycle management is needed by future features such as
> > image fleecing and blockdev-add. This series adds reference count to
> > BlockDriverState.
> > 
> > The first two patches clean up two odd BlockDriverState use cases, so all code
> > uses bdrv_new() to create BlockDriverState instance.
> > 
> > Then implemented bdrv_ref() and bdrv_unref() to operate on refcnt: Initially,
> > refcnt is 1, which means bdrv_unref is effectively a bdrv_delete() here. So
> > patch 04 has a search and replace to convert bdrv_delete to bdrv_unref, before
> > bdrv_ref is used anywhere. 05~08 patches calls bdrv_ref for device attach,
> > block-migration and nbd.
> > 
> > The rule is: Either bdrv_ref() or bdrv_new() must have a matching
> > bdrv_unref() call, and the last matching bdrv_unref deletes the bs.
> > 
> > v3:
> >     03: Removed unnecessary bdrv_close() call.
> > 
> > v2:
> >     05: Removed: "block: use BlockDriverState refcnt for device attach/detach"
> >     07: Fix xen_disk blk_disconnect() as it depended on device attach refcnt.
> > 
> > Fam Zheng (7):
> >   vvfat: use bdrv_new() to allocate BlockDriverState
> >   iscsi: use bdrv_new() instead of stack structure
> >   block: implement reference count for BlockDriverState
> >   block: make bdrv_delete() static
> >   migration: omit drive ref as we have bdrv_ref now
> >   xen_disk: simplify blk_disconnect with refcnt
> >   nbd: use BlockDriverState refcnt
> 
> Follow-up question:
> 
> Did you look at using bdrv_ref() for the BDS <- BlockJob relationship
> too?
> 
> blockdev.c block job code still uses the DriveInfo refcount after your
> series.  The BDS reference would be sufficient since the DriveInfo
> fields are not used by block jobs.
> 
For now they coexist, which works but with redundancy. I will send
follow-up patch(es) to clean up block job to only use BDS refcount.

-- 
Fam

  reply	other threads:[~2013-08-02  4:20 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
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 [this message]
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=20130802022213.GA2864@T430s.nay.redhat.com \
    --to=famz@redhat.com \
    --cc=jcody@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.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).