| Message ID | 1416924485-13304-11-git-send-email-mreitz@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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
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);
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(-)