All of lore.kernel.org
 help / color / mirror / Atom feed
From: Max Reitz <mreitz@redhat.com>
To: Eric Blake <eblake@redhat.com>, qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 7/7] iotests: Rework 113
Date: Wed, 2 May 2018 20:35:46 +0200	[thread overview]
Message-ID: <c5e4a284-2d3e-4a89-6010-be0875fe8ce1@redhat.com> (raw)
In-Reply-To: <563e34b7-2b97-da1a-9e38-784608197b89@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 3186 bytes --]

On 2018-05-02 20:24, Eric Blake wrote:
> On 04/21/2018 11:54 AM, Max Reitz wrote:
>> This test case has been broken since 398e6ad014df261d (roughly half a
>> year).  qemu-img amend requires its output image to be R/W, so it opens
>> it as such; the node is then turned into an read-only node automatically
>> which is now accompanied by a warning, however.  This warning has not
>> been part of the reference output.
>>
> 
>> The reason nobody has noticed the breakage until now of course is the
>> fact that nobody runs the iotests for nbd+bochs.  There actually was
>> never any reason to set the protocol to "nbd" but because that was
>> technically correct; functionally it made no difference.  So that is the
>> first thing we are going to change: Make the protocol "file" instead so
>> that people might actually notice breakage here.
> 
> There's probably several tests that fail with an explicit pairing of nbd
> as protocol + explicit format (I'm guilty of usually testing either the
> format './check -qcow2' or the protocol './check -nbd', and not
> combining the two).
> 
>> So with this test now being a raw test, we can rework the amend test
>> case to use raw instead.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   tests/qemu-iotests/113     | 19 +++++++++----------
>>   tests/qemu-iotests/113.out |  7 ++++---
>>   2 files changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/113 b/tests/qemu-iotests/113
>> index 19b68b2727..4e09810905 100755
>> --- a/tests/qemu-iotests/113
>> +++ b/tests/qemu-iotests/113
>> @@ -38,16 +38,17 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
>>   . ./common.rc
>>   . ./common.filter
>>   -# We can only test one format here because we need its sample file
>> -_supported_fmt bochs
>> -_supported_proto nbd
>> +# Some of these test cases use bochs, but others do use raw, so this
>> +# is only half a lie.
>> +_supported_fmt raw
>> +_supported_proto file
> 
> Naive question - can we pass a list to _supported_fmt/_supported_proto,
> in which case the test runs under both elements of the list?  That is,
> I'm wondering if:
> 
> _supported_fmt raw bochs
> _supported_proto file nbd
> 
> gives us any additional coverage (theoretically 4 times the testing
> possibilities - but if the rest of the test hard-codes a particular
> format or protocol then it might not actually execute a different
> combination).

Well, anyone who's going to run any of these combinations is probably
going to run raw+file as well.  You could argue that if someone changes
something about NBD, they might just run raw+nbd; but then again, this
test doesn't really test nbd at all.  In fact, it just tests qemu-img
(we simply need a format that does not support image creation, but it's
not like we actually do anything with that format).  Same for bochs.

(And I think anyone who wants to test qemu-img is indeed going to run
the raw+file tests.)

But see my reply to John, I think the best way would be if tests like
these could be just run in "default" mode without forcing some
format/protocol combination on them.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

      reply	other threads:[~2018-05-02 18:35 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-21 16:54 [Qemu-devel] [PATCH 0/7] qemu-img: Improve option help for amend Max Reitz
2018-04-21 16:54 ` [Qemu-devel] [PATCH 1/7] qemu-img: Amendment support implies create_opts Max Reitz
2018-05-02 17:35   ` Eric Blake
2018-04-21 16:54 ` [Qemu-devel] [PATCH 2/7] block: Add Error parameter to bdrv_amend_options Max Reitz
2018-05-01 22:49   ` [Qemu-devel] [Qemu-block] " John Snow
2018-05-02 17:52   ` [Qemu-devel] " Eric Blake
2018-05-02 18:08     ` Max Reitz
2018-04-21 16:54 ` [Qemu-devel] [PATCH 3/7] qemu-option: Pull out "Supported options" print Max Reitz
2018-05-02 18:09   ` Eric Blake
2018-04-21 16:54 ` [Qemu-devel] [PATCH 4/7] qemu-img: Add print_amend_option_help() Max Reitz
2018-05-02 18:13   ` Eric Blake
2018-04-21 16:54 ` [Qemu-devel] [PATCH 5/7] qemu-img: Recognize no creation support in -o help Max Reitz
2018-05-02 17:40   ` [Qemu-devel] [Qemu-block] " John Snow
2018-05-02 18:15   ` [Qemu-devel] " Eric Blake
2018-04-21 16:54 ` [Qemu-devel] [PATCH 6/7] iotests: Test help option for unsupporting formats Max Reitz
2018-05-02 18:00   ` [Qemu-devel] [Qemu-block] " John Snow
2018-05-02 18:17   ` [Qemu-devel] " Eric Blake
2018-04-21 16:54 ` [Qemu-devel] [PATCH 7/7] iotests: Rework 113 Max Reitz
2018-05-02 18:03   ` [Qemu-devel] [Qemu-block] " John Snow
2018-05-02 18:13     ` Max Reitz
2018-05-02 18:18       ` John Snow
2018-05-02 18:48         ` Max Reitz
2018-05-02 18:24   ` [Qemu-devel] " Eric Blake
2018-05-02 18:35     ` Max Reitz [this message]

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=c5e4a284-2d3e-4a89-6010-be0875fe8ce1@redhat.com \
    --to=mreitz@redhat.com \
    --cc=eblake@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.