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

[Max Reitz <mreitz@redhat.com>]

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?

Max