Re: [Qemu-devel] [PATCH 10/12] qcow2: Flushing the caches in qcow2_close may fail

[Kevin Wolf <kwolf@redhat.com>]

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 <mreitz@redhat.com>
> >---
> >  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