[Qemu-devel] [5839] qemu-img: open files with cache=writeback

[Qemu-devel] [5839] qemu-img: open files with cache=writeback

[Aurelien Jarno]

Revision: 5839
          http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5839
Author:   aurel32
Date:     2008-11-30 19:12:49 +0000 (Sun, 30 Nov 2008)

Log Message:
-----------
qemu-img: open files with cache=writeback

Data integrity is not important in qemu-img, so open the files with
cache=writeback. This fixes the performance regression seen with qemu-img
since revision 5485, and most particularly with the qcow2 format.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>

Revision Links:
--------------
    http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5485

Modified Paths:
--------------
    trunk/qemu-img.c

Modified: trunk/qemu-img.c
===================================================================
--- trunk/qemu-img.c	2008-11-30 17:54:28 UTC (rev 5838)
+++ trunk/qemu-img.c	2008-11-30 19:12:49 UTC (rev 5839)
@@ -30,6 +30,9 @@
 #include <windows.h>
 #endif
 
+/* Default to cache=writeback as data integrity is not important for qemu-tcg. */
+#define BRDV_O_FLAGS BDRV_O_CACHE_WB
+
 static void __attribute__((noreturn)) error(const char *fmt, ...)
 {
     va_list ap;
@@ -186,7 +189,7 @@
     } else {
         drv = NULL;
     }
-    if (bdrv_open2(bs, filename, 0, drv) < 0) {
+    if (bdrv_open2(bs, filename, BRDV_O_FLAGS, drv) < 0) {
         error("Could not open '%s'", filename);
     }
     if (bdrv_is_encrypted(bs)) {
@@ -317,7 +320,7 @@
     } else {
         drv = NULL;
     }
-    if (bdrv_open2(bs, filename, 0, drv) < 0) {
+    if (bdrv_open2(bs, filename, BRDV_O_FLAGS, drv) < 0) {
         error("Could not open '%s'", filename);
     }
     ret = bdrv_commit(bs);
@@ -691,7 +694,7 @@
     } else {
         drv = NULL;
     }
-    if (bdrv_open2(bs, filename, 0, drv) < 0) {
+    if (bdrv_open2(bs, filename, BRDV_O_FLAGS, drv) < 0) {
         error("Could not open '%s'", filename);
     }
     bdrv_get_format(bs, fmt_name, sizeof(fmt_name));

Re: [Qemu-devel] [5839] qemu-img: open files with cache=writeback

[Anthony Liguori]

Aurelien Jarno wrote:
> Revision: 5839
>           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5839
> Author:   aurel32
> Date:     2008-11-30 19:12:49 +0000 (Sun, 30 Nov 2008)
>
> Log Message:
> -----------
> qemu-img: open files with cache=writeback
>
> Data integrity is not important in qemu-img, so open the files with
> cache=writeback. This fixes the performance regression seen with qemu-img
> since revision 5485, and most particularly with the qcow2 format.
>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>   

My thinking was to just make qcow2 always default to cache=writeback.  I 
posted an RFC but received no feedback.  I guess that means noone 
disagreed :-)

Regards,

Anthony Liguori

> Revision Links:
> --------------
>     http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5485
>
> Modified Paths:
> --------------
>     trunk/qemu-img.c
>
> Modified: trunk/qemu-img.c
> ===================================================================
> --- trunk/qemu-img.c	2008-11-30 17:54:28 UTC (rev 5838)
> +++ trunk/qemu-img.c	2008-11-30 19:12:49 UTC (rev 5839)
> @@ -30,6 +30,9 @@
>  #include <windows.h>
>  #endif
>  
> +/* Default to cache=writeback as data integrity is not important for qemu-tcg. */
> +#define BRDV_O_FLAGS BDRV_O_CACHE_WB
> +
>  static void __attribute__((noreturn)) error(const char *fmt, ...)
>  {
>      va_list ap;
> @@ -186,7 +189,7 @@
>      } else {
>          drv = NULL;
>      }
> -    if (bdrv_open2(bs, filename, 0, drv) < 0) {
> +    if (bdrv_open2(bs, filename, BRDV_O_FLAGS, drv) < 0) {
>          error("Could not open '%s'", filename);
>      }
>      if (bdrv_is_encrypted(bs)) {
> @@ -317,7 +320,7 @@
>      } else {
>          drv = NULL;
>      }
> -    if (bdrv_open2(bs, filename, 0, drv) < 0) {
> +    if (bdrv_open2(bs, filename, BRDV_O_FLAGS, drv) < 0) {
>          error("Could not open '%s'", filename);
>      }
>      ret = bdrv_commit(bs);
> @@ -691,7 +694,7 @@
>      } else {
>          drv = NULL;
>      }
> -    if (bdrv_open2(bs, filename, 0, drv) < 0) {
> +    if (bdrv_open2(bs, filename, BRDV_O_FLAGS, drv) < 0) {
>          error("Could not open '%s'", filename);
>      }
>      bdrv_get_format(bs, fmt_name, sizeof(fmt_name));
>
>
>
>
>   

Re: [Qemu-devel] [5839] qemu-img: open files with cache=writeback

[Aurelien Jarno]

On Sun, Nov 30, 2008 at 04:56:23PM -0600, Anthony Liguori wrote:
> Aurelien Jarno wrote:
> >Revision: 5839
> >          http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5839
> >Author:   aurel32
> >Date:     2008-11-30 19:12:49 +0000 (Sun, 30 Nov 2008)
> >
> >Log Message:
> >-----------
> >qemu-img: open files with cache=writeback
> >
> >Data integrity is not important in qemu-img, so open the files with
> >cache=writeback. This fixes the performance regression seen with qemu-img
> >since revision 5485, and most particularly with the qcow2 format.
> >
> >Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> >  
> 
> My thinking was to just make qcow2 always default to cache=writeback.  I 
> posted an RFC but received no feedback.  I guess that means noone 
> disagreed :-)
> 

Sorry if I haven't answered to your RFC, but as you said no complains
means the patch is ok. I think you can commit it.

I have tried your patch to solve the problem, but it was not fixing all
the problems: other formats are also affected by the slowdown, as the
I/O operations in qemu-img are very different than in qemu.

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net