From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37662) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XtHRF-0008Mx-1s for qemu-devel@nongnu.org; Tue, 25 Nov 2014 09:50:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XtHR7-0006qZ-7n for qemu-devel@nongnu.org; Tue, 25 Nov 2014 09:49:56 -0500 Date: Tue, 25 Nov 2014 15:49:41 +0100 From: Kevin Wolf Message-ID: <20141125144941.GF4641@noname.redhat.com> References: <1416924485-13304-1-git-send-email-mreitz@redhat.com> <1416924485-13304-11-git-send-email-mreitz@redhat.com> <547490B2.8070105@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <547490B2.8070105@redhat.com> Subject: Re: [Qemu-devel] [PATCH 10/12] qcow2: Flushing the caches in qcow2_close may fail List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: qemu-stable@nongnu.org, Peter Lieven , Markus Armbruster , qemu-devel@nongnu.org, Stefan Hajnoczi Am 25.11.2014 um 15:22 hat Max Reitz geschrieben: > On 2014-11-25 at 15:08, Max Reitz wrote: > >qcow2_cache_flush() may fail; if one of the caches failed to be flushed > >successfully to disk in qcow2_close() the image should not be marked > >clean, and we should emit a warning. > > > >Cc: qemu-stable@nongnu.org > >Signed-off-by: Max Reitz > >--- > > block/qcow2.c | 19 ++++++++++++++++--- > > 1 file changed, 16 insertions(+), 3 deletions(-) > > > >diff --git a/block/qcow2.c b/block/qcow2.c > >index d120494..2bd2a53 100644 > >--- a/block/qcow2.c > >+++ b/block/qcow2.c > >@@ -1428,10 +1428,23 @@ static void qcow2_close(BlockDriverState *bs) > > s->l1_table = NULL; > > if (!(bs->open_flags & BDRV_O_INCOMING)) { > >- qcow2_cache_flush(bs, s->l2_table_cache); > >- qcow2_cache_flush(bs, s->refcount_block_cache); > >+ int ret1, ret2; > >- qcow2_mark_clean(bs); > >+ ret1 = qcow2_cache_flush(bs, s->l2_table_cache); > >+ ret2 = qcow2_cache_flush(bs, s->refcount_block_cache); > >+ > >+ if (ret1) { > >+ error_report("Failed to flush the L2 table cache: %s", > >+ strerror(-ret1)); > >+ } > >+ if (ret2) { > >+ error_report("Failed to flush the refcount block cache: %s", > >+ strerror(-ret2)); > >+ } > >+ > >+ if (!ret1 && !ret2) { > >+ qcow2_mark_clean(bs); > >+ } > > } > > qcow2_cache_destroy(bs, s->l2_table_cache); > > I just noticed this breaks 026, 071 and 089, due to lots of > additional "Failed to flush the .* cache" lines. I can either go the > easy way and remove the error_report() calls from this cache; or I > can amend the test outputs. I'm not sure what I prefer, because I'm > not sure whether having the output is actually useful. Any thoughts > from you? I think I have a slight preference for keeping the error messages and updating the reference output. Kevin