qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Fam Zheng <famz@redhat.com>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: kwolf@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com,
	WenchaoXia <xiawenc@linux.vnet.ibm.com>
Subject: Re: [Qemu-devel] [PATCH v3 3/6] qemu-iotests: Add _supported_cache_modes
Date: Thu, 21 Nov 2013 20:46:31 +0800	[thread overview]
Message-ID: <528E00A7.3010900@redhat.com> (raw)
In-Reply-To: <20131121123957.GA7466@stefanha-thinkpad.redhat.com>

On 2013年11月21日 20:39, Stefan Hajnoczi wrote:
> On Wed, Nov 20, 2013 at 03:44:14PM +0800, Fam Zheng wrote:
>> -_unsupported_qemu_io_options()
>> +_supported_cache_modes()
>>   {
>> -    for bad_opt
>> -    do
>> -        for opt in $QEMU_IO_OPTIONS
>> -        do
>> -            if [ "$bad_opt" = "$opt" ]
>> -            then
>> -                _notrun "not suitable for qemu-io option: $bad_opt"
>> -            fi
>> -        done
>> +    if $CACHEMODE_IS_DEFAULT; then
>> +        QEMU_IO="$QEMU_IO -t $1"
>> +        return
>> +    fi
>> +    for mode; do
>> +        if [ "$mode" = "$CACHEMODE" ]; then
>> +            return
>> +        fi
>>       done
>> +    _notrun "not suitable for cache mode: $CACHEMODE"
>>   }
>
> This seems weird to me:
> By default tests run with CACHEMODE=writethrough but test cases can use
> _supported_cache_modes() to switch to a different "default" behind the
> scenes?
>
> Why not keep it simple:
> If a test doesn't support CACHEMODE, it gets skipped.
>

I thought Kevin wanted to override the cache mode (if not given by 
user), so the cases doesn't get skipped by default. I also feel that a 
test case used to actually run by default shouldn't be skipped unnoticed.

I'm basically adding a meta cache mode "default" here, which means we 
will not have a "implicit yet forced" cache mode any more.

On 2013年11月19日 18:21, Kevin Wolf wrote:
 >
 > Okay, I gave it some testing, too, and it looks like we need some
 > additional changes. There are some test cases that use:
 >
 > _unsupported_qemu_io_options --nocache
 >
 > Which obviously doesn't work any more. We need to replace it by a check
 > against $CACHEMODE (or, perhaps preferably, even override it
 > automatically, so that test cases aren't left out just because of the
 > cache mode)
 >
 > Test case 026 uses the option of having a 026.out.nocache, which differs
 > from the normal output. I suspect the correct differentiation is here
 > between writethrough and writeback modes. And of course, grepping for
 > '--nocache' to detect the condition doesn't work any more.
 >
 > Kevin
 >

Fam

  reply	other threads:[~2013-11-21 12:46 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-20  7:44 [Qemu-devel] [PATCH v3 0/6] Add cache mode option to qemu-iotests, and change default mode to "writeback" Fam Zheng
2013-11-20  7:44 ` [Qemu-devel] [PATCH v3 1/6] qemu-iotests: Add "-c <cache-mode>" option Fam Zheng
2013-11-20  7:44 ` [Qemu-devel] [PATCH v3 2/6] qemu-iotests: Honour cache mode in iotests.py Fam Zheng
2013-11-20  7:44 ` [Qemu-devel] [PATCH v3 3/6] qemu-iotests: Add _supported_cache_modes Fam Zheng
2013-11-21 12:39   ` Stefan Hajnoczi
2013-11-21 12:46     ` Fam Zheng [this message]
2013-11-20  7:44 ` [Qemu-devel] [PATCH v3 4/6] qemu-iotests: Change default cache mode to "writeback" Fam Zheng
2013-11-20  7:44 ` [Qemu-devel] [PATCH v3 5/6] qemu-iotests: Force qcow2 in error path test in 048 Fam Zheng
2013-11-21 12:41   ` Stefan Hajnoczi
2013-11-21 12:50     ` Fam Zheng
2013-11-20  7:44 ` [Qemu-devel] [PATCH v3 6/6] qemu-iotests: Clean up spaces in usage output Fam Zheng
2013-11-21 12:43 ` [Qemu-devel] [PATCH v3 0/6] Add cache mode option to qemu-iotests, and change default mode to "writeback" Stefan Hajnoczi
2013-11-21 14:16   ` Kevin Wolf
2013-11-22  8:57     ` Stefan Hajnoczi
2013-11-22  9:30       ` Kevin Wolf
2013-11-22 15:51         ` Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=528E00A7.3010900@redhat.com \
    --to=famz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@gmail.com \
    --cc=stefanha@redhat.com \
    --cc=xiawenc@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).