Patchwork [v7,4/5] Qemu: Add commandline -drive option 'hostcache'

login
register
mail settings
Submitter Supriya Kannery
Date Oct. 11, 2011, 3:11 a.m.
Message ID <20111011031145.9587.93507.sendpatchset@skannery.in.ibm.com>
Download mbox | patch
Permalink /patch/118857/
State New
Headers show

Comments

Supriya Kannery - Oct. 11, 2011, 3:11 a.m.
qemu command option 'hostcache' added to -drive for block devices.
While starting a VM from qemu commandline, this option can be used 
for setting host cache usage for block data access. Simultaneous
use of 'hostcache' and 'cache' options not allowed.

Signed-off-by: Supriya Kannery <supriyak@linux.vnet.ibm.com>

---
 blockdev.c      |   13 +++++++++++++
 qemu-config.c   |    4 ++++
 qemu-options.hx |    2 +-
 3 files changed, 18 insertions(+), 1 deletion(-)
Kevin Wolf - Oct. 12, 2011, 2:30 p.m.
Am 11.10.2011 05:11, schrieb Supriya Kannery:
> qemu command option 'hostcache' added to -drive for block devices.
> While starting a VM from qemu commandline, this option can be used 
> for setting host cache usage for block data access. Simultaneous
> use of 'hostcache' and 'cache' options not allowed.
> 
> Signed-off-by: Supriya Kannery <supriyak@linux.vnet.ibm.com>

I'm not sure if introducing this alone makes sense. I think I would only
do it when we introduce more options that allow replacing all cache=...
options by other parameters.

> 
> ---
>  blockdev.c      |   13 +++++++++++++
>  qemu-config.c   |    4 ++++
>  qemu-options.hx |    2 +-
>  3 files changed, 18 insertions(+), 1 deletion(-)
> 
> Index: qemu/blockdev.c
> ===================================================================
> --- qemu.orig/blockdev.c
> +++ qemu/blockdev.c
> @@ -237,6 +237,7 @@ DriveInfo *drive_init(QemuOpts *opts, in
>      DriveInfo *dinfo;
>      int snapshot = 0;
>      int ret;
> +    int hostcache = 0;
>  
>      translation = BIOS_ATA_TRANSLATION_AUTO;
>      media = MEDIA_DISK;
> @@ -319,7 +320,19 @@ DriveInfo *drive_init(QemuOpts *opts, in
>  	}
>      }
>  
> +    if ((hostcache = qemu_opt_get_bool(opts, "hostcache", -1)) != -1) {
> +        if (!hostcache) {
> +            bdrv_flags |= BDRV_O_NOCACHE;
> +        } else {
> +            bdrv_flags &= ~BDRV_O_NOCACHE;
> +        }

bdrv_flags is initialised to 0, so the else branch is unnecessary.

Kevin
Supriya Kannery - Oct. 14, 2011, 11:19 a.m.
On 10/12/2011 08:00 PM, Kevin Wolf wrote:
> Am 11.10.2011 05:11, schrieb Supriya Kannery:
>> qemu command option 'hostcache' added to -drive for block devices.
>> While starting a VM from qemu commandline, this option can be used
>> for setting host cache usage for block data access. Simultaneous
>> use of 'hostcache' and 'cache' options not allowed.
>>
>> Signed-off-by: Supriya Kannery<supriyak@linux.vnet.ibm.com>
>
> I'm not sure if introducing this alone makes sense. I think I would only
> do it when we introduce more options that allow replacing all cache=...
> options by other parameters.
>

Can we do transition to alternatives for 'cache=' in a phased manner?
Until all other params are ready, we can allow hostcache (as well
as other params as and when they are ready) in cmdline with the
condition that 'cache=x', if specified, overrides these params.
Once we have all other params ready, 'cache=' can be replaced completely.

thanks, Supriya
Kevin Wolf - Oct. 14, 2011, 11:26 a.m.
Am 14.10.2011 13:19, schrieb Supriya Kannery:
> On 10/12/2011 08:00 PM, Kevin Wolf wrote:
>> Am 11.10.2011 05:11, schrieb Supriya Kannery:
>>> qemu command option 'hostcache' added to -drive for block devices.
>>> While starting a VM from qemu commandline, this option can be used
>>> for setting host cache usage for block data access. Simultaneous
>>> use of 'hostcache' and 'cache' options not allowed.
>>>
>>> Signed-off-by: Supriya Kannery<supriyak@linux.vnet.ibm.com>
>>
>> I'm not sure if introducing this alone makes sense. I think I would only
>> do it when we introduce more options that allow replacing all cache=...
>> options by other parameters.
>>
> 
> Can we do transition to alternatives for 'cache=' in a phased manner?
> Until all other params are ready, we can allow hostcache (as well
> as other params as and when they are ready) in cmdline with the
> condition that 'cache=x', if specified, overrides these params.
> Once we have all other params ready, 'cache=' can be replaced completely.

I guess that would be good enough. There's still not much use in
specifying hostcache=... at the same time as cache=... but at least it
doesn't take away other options then.

Kevin

Patch

Index: qemu/blockdev.c
===================================================================
--- qemu.orig/blockdev.c
+++ qemu/blockdev.c
@@ -237,6 +237,7 @@  DriveInfo *drive_init(QemuOpts *opts, in
     DriveInfo *dinfo;
     int snapshot = 0;
     int ret;
+    int hostcache = 0;
 
     translation = BIOS_ATA_TRANSLATION_AUTO;
     media = MEDIA_DISK;
@@ -319,7 +320,19 @@  DriveInfo *drive_init(QemuOpts *opts, in
 	}
     }
 
+    if ((hostcache = qemu_opt_get_bool(opts, "hostcache", -1)) != -1) {
+        if (!hostcache) {
+            bdrv_flags |= BDRV_O_NOCACHE;
+        } else {
+            bdrv_flags &= ~BDRV_O_NOCACHE;
+        }
+    }
+
     if ((buf = qemu_opt_get(opts, "cache")) != NULL) {
+        if (hostcache != -1) {
+            error_report("'hostcache' and 'cache' cannot co-exist");
+            return NULL;
+        }
         if (bdrv_parse_cache_flags(buf, &bdrv_flags) != 0) {
             error_report("invalid cache option");
             return NULL;
Index: qemu/qemu-options.hx
===================================================================
--- qemu.orig/qemu-options.hx
+++ qemu/qemu-options.hx
@@ -135,7 +135,7 @@  DEF("drive", HAS_ARG, QEMU_OPTION_drive,
     "       [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
     "       [,cache=writethrough|writeback|none|directsync|unsafe][,format=f]\n"
     "       [,serial=s][,addr=A][,id=name][,aio=threads|native]\n"
-    "       [,readonly=on|off]\n"
+    "       [,readonly=on|off][,hostcache=on|off]\n"
     "                use 'file' as a drive image\n", QEMU_ARCH_ALL)
 STEXI
 @item -drive @var{option}[,@var{option}[,@var{option}[,...]]]
Index: qemu/qemu-config.c
===================================================================
--- qemu.orig/qemu-config.c
+++ qemu/qemu-config.c
@@ -85,6 +85,10 @@  static QemuOptsList qemu_drive_opts = {
             .name = "readonly",
             .type = QEMU_OPT_BOOL,
             .help = "open drive file as read-only",
+        },{
+            .name = "hostcache",
+            .type = QEMU_OPT_BOOL,
+            .help = "set or reset hostcache (on/off)"
         },
         { /* end of list */ }
     },