diff mbox

[01/11] block: Use writeback in .bdrv_create() implementations

Message ID 1457454872-12183-2-git-send-email-kwolf@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kevin Wolf March 8, 2016, 4:34 p.m. UTC 
There's no reason to use a writethrough cache mode while creating an
image.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/parallels.c | 3 ++-
 block/qcow.c      | 3 ++-
 block/qcow2.c     | 3 ++-
 block/sheepdog.c  | 6 ++++--
 block/vdi.c       | 3 ++-
 block/vhdx.c      | 3 ++-
 block/vmdk.c      | 9 ++++++---
 block/vpc.c       | 3 ++-
 8 files changed, 22 insertions(+), 11 deletions(-)

Comments

Paolo Bonzini March 9, 2016, 12:14 p.m. UTC | #1 
On 08/03/2016 17:34, Kevin Wolf wrote:
> There's no reason to use a writethrough cache mode while creating an
> image.

There's no reason to do flushes in fact, so you could use
BDRV_O_NO_FLUSH too. :)

Paolo

> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/parallels.c | 3 ++-
>  block/qcow.c      | 3 ++-
>  block/qcow2.c     | 3 ++-
>  block/sheepdog.c  | 6 ++++--
>  block/vdi.c       | 3 ++-
>  block/vhdx.c      | 3 ++-
>  block/vmdk.c      | 9 ++++++---
>  block/vpc.c       | 3 ++-
>  8 files changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/block/parallels.c b/block/parallels.c
> index 645521d..3f9fd48 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -479,7 +479,8 @@ static int parallels_create(const char *filename, QemuOpts *opts, Error **errp)
>  
>      file = NULL;
>      ret = bdrv_open(&file, filename, NULL, NULL,
> -                    BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err);
> +                    BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
> +                    &local_err);
>      if (ret < 0) {
>          error_propagate(errp, local_err);
>          return ret;
> diff --git a/block/qcow.c b/block/qcow.c
> index 251910c..c46810c 100644
> --- a/block/qcow.c
> +++ b/block/qcow.c
> @@ -798,7 +798,8 @@ static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
>  
>      qcow_bs = NULL;
>      ret = bdrv_open(&qcow_bs, filename, NULL, NULL,
> -                    BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err);
> +                    BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
> +                    &local_err);
>      if (ret < 0) {
>          error_propagate(errp, local_err);
>          goto cleanup;
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 8babecd..5a79177 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -2173,7 +2173,8 @@ static int qcow2_create2(const char *filename, int64_t total_size,
>      }
>  
>      bs = NULL;
> -    ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
> +    ret = bdrv_open(&bs, filename, NULL, NULL,
> +                    BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
>                      &local_err);
>      if (ret < 0) {
>          error_propagate(errp, local_err);
> diff --git a/block/sheepdog.c b/block/sheepdog.c
> index 8739acc..5f31ab3 100644
> --- a/block/sheepdog.c
> +++ b/block/sheepdog.c
> @@ -1646,7 +1646,8 @@ static int sd_prealloc(const char *filename, Error **errp)
>      void *buf = NULL;
>      int ret;
>  
> -    ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
> +    ret = bdrv_open(&bs, filename, NULL, NULL,
> +                    BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
>                      errp);
>      if (ret < 0) {
>          goto out_with_err_set;
> @@ -1839,7 +1840,8 @@ static int sd_create(const char *filename, QemuOpts *opts,
>          }
>  
>          bs = NULL;
> -        ret = bdrv_open(&bs, backing_file, NULL, NULL, BDRV_O_PROTOCOL, errp);
> +        ret = bdrv_open(&bs, backing_file, NULL, NULL,
> +                        BDRV_O_PROTOCOL | BDRV_O_CACHE_WB, errp);
>          if (ret < 0) {
>              goto out;
>          }
> diff --git a/block/vdi.c b/block/vdi.c
> index b403243..12407c4 100644
> --- a/block/vdi.c
> +++ b/block/vdi.c
> @@ -766,7 +766,8 @@ static int vdi_create(const char *filename, QemuOpts *opts, Error **errp)
>          error_propagate(errp, local_err);
>          goto exit;
>      }
> -    ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
> +    ret = bdrv_open(&bs, filename, NULL, NULL,
> +                    BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
>                      &local_err);
>      if (ret < 0) {
>          error_propagate(errp, local_err);
> diff --git a/block/vhdx.c b/block/vhdx.c
> index 9a51428..ea030ad 100644
> --- a/block/vhdx.c
> +++ b/block/vhdx.c
> @@ -1838,7 +1838,8 @@ static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
>      }
>  
>      bs = NULL;
> -    ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
> +    ret = bdrv_open(&bs, filename, NULL, NULL,
> +                    BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
>                      &local_err);
>      if (ret < 0) {
>          error_propagate(errp, local_err);
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 03be7f0..dd80936 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -1664,7 +1664,8 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
>      }
>  
>      assert(bs == NULL);
> -    ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
> +    ret = bdrv_open(&bs, filename, NULL, NULL,
> +                    BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
>                      &local_err);
>      if (ret < 0) {
>          error_propagate(errp, local_err);
> @@ -1944,7 +1945,8 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
>              ret = -ENOENT;
>              goto exit;
>          }
> -        ret = bdrv_open(&bs, full_backing, NULL, NULL, BDRV_O_NO_BACKING, errp);
> +        ret = bdrv_open(&bs, full_backing, NULL, NULL,
> +                        BDRV_O_NO_BACKING | BDRV_O_CACHE_WB, errp);
>          g_free(full_backing);
>          if (ret != 0) {
>              goto exit;
> @@ -2015,7 +2017,8 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
>      }
>      assert(new_bs == NULL);
>      ret = bdrv_open(&new_bs, filename, NULL, NULL,
> -                    BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err);
> +                    BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
> +                    &local_err);
>      if (ret < 0) {
>          error_propagate(errp, local_err);
>          goto exit;
> diff --git a/block/vpc.c b/block/vpc.c
> index 318e6d6..1db47d6 100644
> --- a/block/vpc.c
> +++ b/block/vpc.c
> @@ -886,7 +886,8 @@ static int vpc_create(const char *filename, QemuOpts *opts, Error **errp)
>          error_propagate(errp, local_err);
>          goto out;
>      }
> -    ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
> +    ret = bdrv_open(&bs, filename, NULL, NULL,
> +                    BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
>                      &local_err);
>      if (ret < 0) {
>          error_propagate(errp, local_err);
>
Kevin Wolf March 9, 2016, 12:27 p.m. UTC | #2 
Am 09.03.2016 um 13:14 hat Paolo Bonzini geschrieben:
> 
> 
> On 08/03/2016 17:34, Kevin Wolf wrote:
> > There's no reason to use a writethrough cache mode while creating an
> > image.
> 
> There's no reason to do flushes in fact, so you could use
> BDRV_O_NO_FLUSH too. :)

That's true. On the other hand, we don't issue flushes anyway, so
there's little reason to ignore non-existing flushes. :-)

The only part where it might make sense is where image formats don't
open the raw image file, but actually open the image with themselves and
allocate things. qcow2 at least already sets BDRV_O_NO_FLUSH for that
part.

In fact, the only bdrv_open() touched in this patch that doesn't have
BDRV_O_PROTOCOL set, is one instance in VMDK where it opens the backing
file to read some ID from the header. So no flush involved there.

Kevin
diff mbox

Patch

diff --git a/block/parallels.c b/block/parallels.c
index 645521d..3f9fd48 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -479,7 +479,8 @@  static int parallels_create(const char *filename, QemuOpts *opts, Error **errp)
 
     file = NULL;
     ret = bdrv_open(&file, filename, NULL, NULL,
-                    BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err);
+                    BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
+                    &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         return ret;
diff --git a/block/qcow.c b/block/qcow.c
index 251910c..c46810c 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -798,7 +798,8 @@  static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
 
     qcow_bs = NULL;
     ret = bdrv_open(&qcow_bs, filename, NULL, NULL,
-                    BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err);
+                    BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
+                    &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         goto cleanup;
diff --git a/block/qcow2.c b/block/qcow2.c
index 8babecd..5a79177 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2173,7 +2173,8 @@  static int qcow2_create2(const char *filename, int64_t total_size,
     }
 
     bs = NULL;
-    ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
+    ret = bdrv_open(&bs, filename, NULL, NULL,
+                    BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
                     &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 8739acc..5f31ab3 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1646,7 +1646,8 @@  static int sd_prealloc(const char *filename, Error **errp)
     void *buf = NULL;
     int ret;
 
-    ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
+    ret = bdrv_open(&bs, filename, NULL, NULL,
+                    BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
                     errp);
     if (ret < 0) {
         goto out_with_err_set;
@@ -1839,7 +1840,8 @@  static int sd_create(const char *filename, QemuOpts *opts,
         }
 
         bs = NULL;
-        ret = bdrv_open(&bs, backing_file, NULL, NULL, BDRV_O_PROTOCOL, errp);
+        ret = bdrv_open(&bs, backing_file, NULL, NULL,
+                        BDRV_O_PROTOCOL | BDRV_O_CACHE_WB, errp);
         if (ret < 0) {
             goto out;
         }
diff --git a/block/vdi.c b/block/vdi.c
index b403243..12407c4 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -766,7 +766,8 @@  static int vdi_create(const char *filename, QemuOpts *opts, Error **errp)
         error_propagate(errp, local_err);
         goto exit;
     }
-    ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
+    ret = bdrv_open(&bs, filename, NULL, NULL,
+                    BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
                     &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
diff --git a/block/vhdx.c b/block/vhdx.c
index 9a51428..ea030ad 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1838,7 +1838,8 @@  static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
     }
 
     bs = NULL;
-    ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
+    ret = bdrv_open(&bs, filename, NULL, NULL,
+                    BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
                     &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
diff --git a/block/vmdk.c b/block/vmdk.c
index 03be7f0..dd80936 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1664,7 +1664,8 @@  static int vmdk_create_extent(const char *filename, int64_t filesize,
     }
 
     assert(bs == NULL);
-    ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
+    ret = bdrv_open(&bs, filename, NULL, NULL,
+                    BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
                     &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
@@ -1944,7 +1945,8 @@  static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
             ret = -ENOENT;
             goto exit;
         }
-        ret = bdrv_open(&bs, full_backing, NULL, NULL, BDRV_O_NO_BACKING, errp);
+        ret = bdrv_open(&bs, full_backing, NULL, NULL,
+                        BDRV_O_NO_BACKING | BDRV_O_CACHE_WB, errp);
         g_free(full_backing);
         if (ret != 0) {
             goto exit;
@@ -2015,7 +2017,8 @@  static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
     }
     assert(new_bs == NULL);
     ret = bdrv_open(&new_bs, filename, NULL, NULL,
-                    BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err);
+                    BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
+                    &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         goto exit;
diff --git a/block/vpc.c b/block/vpc.c
index 318e6d6..1db47d6 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -886,7 +886,8 @@  static int vpc_create(const char *filename, QemuOpts *opts, Error **errp)
         error_propagate(errp, local_err);
         goto out;
     }
-    ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
+    ret = bdrv_open(&bs, filename, NULL, NULL,
+                    BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
                     &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);