From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57504) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UtxMN-0006aT-W2 for qemu-devel@nongnu.org; Tue, 02 Jul 2013 05:58:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UtxMM-0007Nu-6Q for qemu-devel@nongnu.org; Tue, 02 Jul 2013 05:58:55 -0400 Date: Tue, 2 Jul 2013 11:38:33 +0200 From: Kevin Wolf Message-ID: <20130702093832.GC3031@dhcp-200-207.str.redhat.com> References: <1372678184-5008-1-git-send-email-stefanha@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1372678184-5008-1-git-send-email-stefanha@redhat.com> Subject: Re: [Qemu-devel] [PATCH] block: fix bdrv_flush() ordering in bdrv_close() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: qemu-devel@nongnu.org, qemu-stable@nongnu.org Am 01.07.2013 um 13:29 hat Stefan Hajnoczi geschrieben: > Since 80ccf93b we flush the block device during close. The > bdrv_drain_all() call should come before bdrv_flush() to ensure guest > write requests have completed. Otherwise we may miss pending writes > when flushing. > > However, there is still a slight change that cancelling a blockjob or > doing bdrv_flush() might leave an I/O request running, so call > bdrv_drain_all() again for safety as the final step. > > Cc: qemu-stable@nongnu.org > Signed-off-by: Stefan Hajnoczi > --- > block.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/block.c b/block.c > index 6c493ad..fca55b1 100644 > --- a/block.c > +++ b/block.c > @@ -1358,11 +1358,12 @@ void bdrv_reopen_abort(BDRVReopenState *reopen_state) > > void bdrv_close(BlockDriverState *bs) > { > - bdrv_flush(bs); > + bdrv_drain_all(); /* complete guest I/O */ > if (bs->job) { > block_job_cancel_sync(bs->job); > } > - bdrv_drain_all(); > + bdrv_flush(bs); > + bdrv_drain_all(); /* in case blockjob or flush started I/O */ > notifier_list_notify(&bs->close_notifiers, bs); I think we need the bdrv_drain_all() immediately before calling bdrv_flush(), so that block job writes are flushed as well. You can probably move the first one there, there doesn't seem to be a reason to drain guest requests and block job requests separately. Kevin