[Qemu-devel] [PATCH] qemu-iotests: Add "-c <cache-mode>" to check

[Qemu-devel] [PATCH] qemu-iotests: Add "-c <cache-mode>" to check

[Fam Zheng]

The default cache mode for drive options is changed to writethrough, and
overridable with "./check -c <mode>".

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/qemu-iotests/common     | 13 ++++++++++++-
 tests/qemu-iotests/iotests.py |  3 ++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
index 2932e14..f870c52 100644
--- a/tests/qemu-iotests/common
+++ b/tests/qemu-iotests/common
@@ -42,12 +42,14 @@ expunge=true
 have_test_arg=false
 randomize=false
 valgrind=false
+cachemode=false
 rm -f $tmp.list $tmp.tmp $tmp.sed
 
 export IMGFMT=raw
 export IMGFMT_GENERIC=true
 export IMGPROTO=file
 export IMGOPTS=""
+export CACHEMODE="writethrough"
 export QEMU_IO_OPTIONS=""
 
 for r
@@ -113,7 +115,11 @@ s/ .*//p
         IMGOPTS="$r"
         imgopts=false
         continue
-
+    elif $cachemode
+    then
+        CACHEMODE="$r"
+        cachemode=false
+        continue
     fi
 
     xpand=true
@@ -147,6 +153,7 @@ check options
     -o options          -o options to pass to qemu-img create/convert
     -T                        output timestamps
     -r                         randomize test order
+    -c                  cache mode
 
 testlist options
     -g group[,group...]        include tests from these groups
@@ -259,6 +266,10 @@ testlist options
             imgopts=true
             xpand=false
             ;;
+        -c)
+            cachemode=true
+            xpand=false
+            ;;
         -r)        # randomize test order
             randomize=true
             xpand=false
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index fb10ff4..c84a1a5 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -37,6 +37,7 @@ qemu_args = os.environ.get('QEMU', 'qemu').strip().split(' ')
 imgfmt = os.environ.get('IMGFMT', 'raw')
 imgproto = os.environ.get('IMGPROTO', 'file')
 test_dir = os.environ.get('TEST_DIR', '/var/tmp')
+cachemode = os.environ.get('CACHEMODE')
 
 socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 'socket_scm_helper')
 
@@ -96,7 +97,7 @@ class VM(object):
         '''Add a virtio-blk drive to the VM'''
         options = ['if=virtio',
                    'format=%s' % imgfmt,
-                   'cache=none',
+                   'cache=%s' % cachemode,
                    'file=%s' % path,
                    'id=drive%d' % self._num_drives]
         if opts:
-- 
1.8.4.2

Re: [Qemu-devel] [PATCH] qemu-iotests: Add "-c <cache-mode>" to check

[Wenchao Xia]

Tried on RH6.4 x86_64 on upstream, the value can be passed to the final
vm boot command.

Tested-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>

Re: [Qemu-devel] [PATCH] qemu-iotests: Add "-c <cache-mode>" to check

[Stefan Hajnoczi]

On Thu, Nov 14, 2013 at 10:24:04AM +0800, Fam Zheng wrote:
> The default cache mode for drive options is changed to writethrough, and
> overridable with "./check -c <mode>".

Please make the default "writeback" so that ./check completes more
quickly.

Also, please also indicate in the commit description why the change was
made (i.e.  you want qemu-iotests to succeed on tmpfs by default).

Stefan

Re: [Qemu-devel] [PATCH] qemu-iotests: Add "-c <cache-mode>" to check

[Kevin Wolf]

Am 18.11.2013 um 16:29 hat Stefan Hajnoczi geschrieben:
> On Thu, Nov 14, 2013 at 10:24:04AM +0800, Fam Zheng wrote:
> > The default cache mode for drive options is changed to writethrough, and
> > overridable with "./check -c <mode>".
> 
> Please make the default "writeback" so that ./check completes more
> quickly.

Changing the cache mode should be a separate patch.

The current default for all shell script based tests is
cache=writethrough (can be overridden with -nocache) and Python test
cases should respect the same setting.

This is also the problem that I see with this patch: It doesn't make
'-nocache' an alias of '-c none', but both control different aspects.
What should happen is that '-c <mode>' sets

    QEMU_IO_OPTIONS="$QEMU_IO_OPTIONS -t <mode>"

like -nocache does today, and the Python scripts should refer to the
same cache settings as the bash scripts do.

Kevin

Re: [Qemu-devel] [PATCH] qemu-iotests: Add "-c <cache-mode>" to check

[Fam Zheng]

On 2013年11月18日 23:38, Kevin Wolf wrote:
> Am 18.11.2013 um 16:29 hat Stefan Hajnoczi geschrieben:
>> On Thu, Nov 14, 2013 at 10:24:04AM +0800, Fam Zheng wrote:
>>> The default cache mode for drive options is changed to writethrough, and
>>> overridable with "./check -c <mode>".
>>
>> Please make the default "writeback" so that ./check completes more
>> quickly.
>
> Changing the cache mode should be a separate patch.
>
> The current default for all shell script based tests is
> cache=writethrough (can be overridden with -nocache) and Python test
> cases should respect the same setting.
>
> This is also the problem that I see with this patch: It doesn't make
> '-nocache' an alias of '-c none', but both control different aspects.
> What should happen is that '-c <mode>' sets
>
>      QEMU_IO_OPTIONS="$QEMU_IO_OPTIONS -t <mode>"
>
> like -nocache does today, and the Python scripts should refer to the
> same cache settings as the bash scripts do.
>

Right. Thanks for explaining.

Fam

Re: [Qemu-devel] [PATCH] qemu-iotests: Add "-c <cache-mode>" to check

[Stefan Hajnoczi]

On Thu, Nov 14, 2013 at 10:24:04AM +0800, Fam Zheng wrote:
> The default cache mode for drive options is changed to writethrough, and
> overridable with "./check -c <mode>".
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  tests/qemu-iotests/common     | 13 ++++++++++++-
>  tests/qemu-iotests/iotests.py |  3 ++-
>  2 files changed, 14 insertions(+), 2 deletions(-)

BTW, I looked back at Kevin's reply to your earlier patch and saw he
suggested making the default "writethrough".

Kevin: Any reason not to use "writeback" by default?

Re: [Qemu-devel] [PATCH] qemu-iotests: Add "-c <cache-mode>" to check

[Kevin Wolf]

Am 18.11.2013 um 16:32 hat Stefan Hajnoczi geschrieben:
> On Thu, Nov 14, 2013 at 10:24:04AM +0800, Fam Zheng wrote:
> > The default cache mode for drive options is changed to writethrough, and
> > overridable with "./check -c <mode>".
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >  tests/qemu-iotests/common     | 13 ++++++++++++-
> >  tests/qemu-iotests/iotests.py |  3 ++-
> >  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> BTW, I looked back at Kevin's reply to your earlier patch and saw he
> suggested making the default "writethrough".
> 
> Kevin: Any reason not to use "writeback" by default?

I'm not opposed to changing the default in a second step. It's just that
writethrough is the default today and tests not respecting this are
buggy. So the incremental fix is to make them obey the global setting,
and then that setting can be tweaked in a second step.

Kevin

Re: [Qemu-devel] [PATCH] qemu-iotests: Add "-c <cache-mode>" to check

[Stefan Hajnoczi]

On Mon, Nov 18, 2013 at 04:40:39PM +0100, Kevin Wolf wrote:
> Am 18.11.2013 um 16:32 hat Stefan Hajnoczi geschrieben:
> > On Thu, Nov 14, 2013 at 10:24:04AM +0800, Fam Zheng wrote:
> > > The default cache mode for drive options is changed to writethrough, and
> > > overridable with "./check -c <mode>".
> > > 
> > > Signed-off-by: Fam Zheng <famz@redhat.com>
> > > ---
> > >  tests/qemu-iotests/common     | 13 ++++++++++++-
> > >  tests/qemu-iotests/iotests.py |  3 ++-
> > >  2 files changed, 14 insertions(+), 2 deletions(-)
> > 
> > BTW, I looked back at Kevin's reply to your earlier patch and saw he
> > suggested making the default "writethrough".
> > 
> > Kevin: Any reason not to use "writeback" by default?
> 
> I'm not opposed to changing the default in a second step. It's just that
> writethrough is the default today and tests not respecting this are
> buggy. So the incremental fix is to make them obey the global setting,
> and then that setting can be tweaked in a second step.

Makes sense.

Stefan