From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:46968)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <famz@redhat.com>) id 1Zp5Uf-0002Xk-Uv
	for qemu-devel@nongnu.org; Wed, 21 Oct 2015 22:20:45 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <famz@redhat.com>) id 1Zp5Ue-0003te-JW
	for qemu-devel@nongnu.org; Wed, 21 Oct 2015 22:20:41 -0400
Date: Thu, 22 Oct 2015 10:20:35 +0800
From: Fam Zheng <famz@redhat.com>
Message-ID: <20151022022035.GD11063@ad.usersys.redhat.com>
References: <1445393209-26545-1-git-send-email-famz@redhat.com>
	<1445393209-26545-6-git-send-email-famz@redhat.com>
	<20151021161118.GD6466@localhost.localdomain>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20151021161118.GD6466@localhost.localdomain>
Subject: Re: [Qemu-devel] [Qemu-block] [PATCH v5 05/12] block: Introduce
 "drained begin/end" API
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Jeff Cody <jcody@redhat.com>
Cc: kwolf@redhat.com, pbonzini@redhat.com, qemu-block@nongnu.org, qemu-devel@nongnu.org, stefanha@redhat.com

On Wed, 10/21 12:11, Jeff Cody wrote:
> On Wed, Oct 21, 2015 at 10:06:42AM +0800, Fam Zheng wrote:
> > The semantics is that after bdrv_drained_begin(bs), bs will not get new external
> > requests until the matching bdrv_drained_end(bs).
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >  block/io.c                | 17 +++++++++++++++++
> >  include/block/block.h     | 21 +++++++++++++++++++++
> >  include/block/block_int.h |  2 ++
> >  3 files changed, 40 insertions(+)
> > 
> > diff --git a/block/io.c b/block/io.c
> > index 2fd7a1d..5ac6256 100644
> > --- a/block/io.c
> > +++ b/block/io.c
> > @@ -2624,3 +2624,20 @@ void bdrv_flush_io_queue(BlockDriverState *bs)
> >      }
> >      bdrv_start_throttled_reqs(bs);
> >  }
> > +
> > +void bdrv_drained_begin(BlockDriverState *bs)
> > +{
> > +    if (!bs->quiesce_counter++) {
> > +        aio_disable_external(bdrv_get_aio_context(bs));
> > +    }
> > +    bdrv_drain(bs);
> > +}
> > +
> > +void bdrv_drained_end(BlockDriverState *bs)
> > +{
> > +    assert(bs->quiesce_counter > 0);
> > +    if (--bs->quiesce_counter > 0) {
> > +        return;
> > +    }
> > +    aio_enable_external(bdrv_get_aio_context(bs));
> > +}
> 
> Why do we need a quiesce counter, given that
> aio_{disable, enable}_external() increments / decrements a counter?

No longer necessary in this version because we call bdrv_drain unconditionally,
but this makes it possible to introducd bdrv_is_quiesced() and assertion on it,
so I lean toward keeping it as it doesn't hurt.

> 
> > diff --git a/include/block/block.h b/include/block/block.h
> > index 28d903c..6d38b62 100644
> > --- a/include/block/block.h
> > +++ b/include/block/block.h
> > @@ -610,4 +610,25 @@ void bdrv_io_plug(BlockDriverState *bs);
> >  void bdrv_io_unplug(BlockDriverState *bs);
> >  void bdrv_flush_io_queue(BlockDriverState *bs);
> >  
> > +BlockAcctStats *bdrv_get_stats(BlockDriverState *bs);
> > +
> 
> Is the above line from a bad rebase?

Yes, will remove.