[PULL,12/17] qcow2: Implement .bdrv_inactivate

Commit Message

Kevin Wolf Jan. 20, 2016, 4:25 p.m. UTC

The callback has to ensure that closing or flushing the image afterwards
wouldn't cause a write access to the image files. This means that just
the caches have to be written out, which is part of the existing
.bdrv_close implementation.

Signed-off-by: Kevin Wolf <[e-mail]>
Reviewed-by: Eric Blake <[e-mail]>
---
 block/qcow2.c | 45 ++++++++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 17 deletions(-)

Comments

Paolo Bonzini Jan. 22, 2016, 11:24 a.m. UTC | #1

On 20/01/2016 17:25, Kevin Wolf wrote:
> +    ret = qcow2_cache_flush(bs, s->refcount_block_cache);
> +    if (ret) {
> +        result = ret;
> +        error_report("Failed to flush the refcount block cache: %s",
> +                     strerror(-ret));
> +    }
> +
> +    if (result == 0) {
> +        qcow2_mark_clean(bs);

Should you return an error if qcow2_mark_clean does?  (Coverity noticed
that its return value is usually checked).

Paolo

> +    }
> +
> +    return result;

Patch

diff --git a/block/qcow2.c b/block/qcow2.c
index 340ae8f..bfc80ea 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1686,6 +1686,32 @@  fail:
     return ret;
 }
 
+static int qcow2_inactivate(BlockDriverState *bs)
+{
+    BDRVQcow2State *s = bs->opaque;
+    int ret, result = 0;
+
+    ret = qcow2_cache_flush(bs, s->l2_table_cache);
+    if (ret) {
+        result = ret;
+        error_report("Failed to flush the L2 table cache: %s",
+                     strerror(-ret));
+    }
+
+    ret = qcow2_cache_flush(bs, s->refcount_block_cache);
+    if (ret) {
+        result = ret;
+        error_report("Failed to flush the refcount block cache: %s",
+                     strerror(-ret));
+    }
+
+    if (result == 0) {
+        qcow2_mark_clean(bs);
+    }
+
+    return result;
+}
+
 static void qcow2_close(BlockDriverState *bs)
 {
     BDRVQcow2State *s = bs->opaque;
@@ -1694,23 +1720,7 @@  static void qcow2_close(BlockDriverState *bs)
     s->l1_table = NULL;
 
     if (!(bs->open_flags & BDRV_O_INACTIVE)) {
-        int ret1, ret2;
-
-        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_inactivate(bs);
     }
 
     cache_clean_timer_del(bs);
@@ -3340,6 +3350,7 @@  BlockDriver bdrv_qcow2 = {
 
     .bdrv_refresh_limits        = qcow2_refresh_limits,
     .bdrv_invalidate_cache      = qcow2_invalidate_cache,
+    .bdrv_inactivate            = qcow2_inactivate,
 
     .create_opts         = &qcow2_create_opts,
     .bdrv_check          = qcow2_check,