block: flush backing_hd in the right place

The backing device is only modified from bdrv_commit. So instead of flushing it every time bdrv_flush is called for the front-end device only flush it after we're written data to it in bdrv_commit.

Signed-off-by Christoph Hellwig <[email protected]>

Index: qemu/block.c
===================================================================

block.c +0 -0

@@ -623,6 +623,12 @@ int bdrv_commit(BlockDriverState *bs)
     if (drv->bdrv_make_empty)

Quoted 4 lines

block.c +0 -0

@@ -623,6 +623,12 @@ int bdrv_commit(BlockDriverState *bs)
     if (drv->bdrv_make_empty)
 	return drv->bdrv_make_empty(bs);
 

block.c int bdrv_commit(BlockDriverState *bs) · line 633

Quoted 8 lines

block.c +6 -0

+    /*
+     * Make sure all data we wrote to the backing device is actually
+     * stable on disk.
+     */
+    if (bs->backing_hd)
+        bdrv_flush(bs->backing_hd);
     return 0;
 }

Format drivers with a bdrv_make_empty return before the flush, so it won't work for qcow1. Looks good otherwise.

If it has done a bdrv_make_empty we might also want to flush bs?

The backing device is only modified from bdrv_commit. So instead of flushing it every time bdrv_flush is called for the front-end device only flush it after we're written data to it in bdrv_commit.

Also flush the frontend image if we have a make_empty method that possibly writes to it.

Signed-off-by Christoph Hellwig <[email protected]>

Index: qemu/block.c
===================================================================

block.c +1 -0

@@ -589,6 +589,7 @@ int bdrv_commit(BlockDriverState *bs)
     BlockDriver *drv = bs->drv;
     int64_t i, total_sectors;
     int n, j;
+    int ret = 0;
     unsigned char sector[512];
 
     if (!drv)

The backing device is only modified from bdrv_commit. So instead of flushing it every time bdrv_flush is called for the front-end device only flush it after we're written data to it in bdrv_commit.

Signed-off-by Christoph Hellwig<[email protected]>

Index: qemu/block.c
===================================================================

block.c +7 -10

@@ -623,6 +623,12 @@ int bdrv_commit(BlockDriverState *bs)
      if (drv->bdrv_make_empty)
  	return drv->bdrv_make_empty(bs);
 
+    /*
+     * Make sure all data we wrote to the backing device is actually
+     * stable on disk.
+     */
+    if (bs->backing_hd)
+        bdrv_flush(bs->backing_hd);
      return 0;
  }
 

@@ -1124,12 +1130,8 @@ const char *bdrv_get_device_name(BlockDr
 
  void bdrv_flush(BlockDriverState *bs)
  {
-    if (!bs->drv)
-        return;
-    if (bs->drv->bdrv_flush)
+    if (bs->drv&&  bs->drv->bdrv_flush)
          bs->drv->bdrv_flush(bs);
-    if (bs->backing_hd)
-        bdrv_flush(bs->backing_hd);
  }
 
  void bdrv_flush_all(void)

@@ -1806,11 +1808,6 @@ BlockDriverAIOCB *bdrv_aio_flush(BlockDr
 
      if (!drv)
          return NULL;
-
-    /*
-     * Note that unlike bdrv_flush the driver is reponsible for flushing a
-     * backing image if it exists.
-     */
      return drv->bdrv_aio_flush(bs, cb, opaque);
  }
 

Quoted 6 lines

The backing device is only modified from bdrv_commit. So instead of flushing it every time bdrv_flush is called for the front-end device only flush it after we're written data to it in bdrv_commit.

Signed-off-by Christoph Hellwig<[email protected]>

Applied. Thanks.

Anthony, you seem to have missed the v2 patch that considered my review comments. Can you please apply the diff between v1 and v2 on top?

Quoted 3 lines · Kevin Wolf

Anthony, you seem to have missed the v2 patch that considered my review comments. Can you please apply the diff between v1 and v2 on top?

Here is the differences in patch form:

---
From: Christoph Hellwig<[email protected]>
Subject: block: fix cache flushing in bdrv_commit

As pointed out by Kevin Wolf the previous patch returned early if we there is a bdrv_make_empty method and it missed a cache flush for the frontend device after the bdev_make_empty call. This patch fixes it up.

Signed-off-by Christoph Hellwig<[email protected]>

Index: qemu/block.c
===================================================================

block.c +6 -3

@@ -589,6 +589,7 @@ int bdrv_commit(BlockDriverState *bs)
      BlockDriver *drv = bs->drv;
      int64_t i, total_sectors;
      int n, j;
+    int ret = 0;
      unsigned char sector[512];
 
      if (!drv)

@@ -620,8 +621,10 @@ int bdrv_commit(BlockDriverState *bs)
          }
      }
 
-    if (drv->bdrv_make_empty)
-	return drv->bdrv_make_empty(bs);
+    if (drv->bdrv_make_empty) {
+        ret = drv->bdrv_make_empty(bs);
+        bdrv_flush(bs);
+    }
 
      /*
       * Make sure all data we wrote to the backing device is actually

@@ -629,7 +632,7 @@ int bdrv_commit(BlockDriverState *bs)
       */
      if (bs->backing_hd)
          bdrv_flush(bs->backing_hd);
-    return 0;
+    return ret;
  }
 
  /*