Re: [Qemu-devel] [PATCH v4 03/38] iotests: Only create BB if necessary

[Max Reitz]

On 07.09.2015 18:20, Kevin Wolf wrote:
> Am 20.07.2015 um 19:45 hat Max Reitz geschrieben:
>> Tests 071 and 081 test giving references in blockdev-add. It is not
>> necessary to create a BlockBackend here, so omit it.
>>
>> Signed-off-by: Max Reitz <address@hidden>
>> Reviewed-by: Eric Blake <address@hidden>
>> Reviewed-by: Alberto Garcia <address@hidden>
>> ---
>>  tests/qemu-iotests/071     | 50 
>> ++++++++++++++++++++++++++++++++++++++++++----
>>  tests/qemu-iotests/071.out | 12 +++++++----
>>  tests/qemu-iotests/081     | 14 ++++++++++++-
>>  tests/qemu-iotests/081.out |  5 +++--
>>  4 files changed, 70 insertions(+), 11 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/071 b/tests/qemu-iotests/071
>> index 9eaa49b..68bedd4 100755
>> --- a/tests/qemu-iotests/071
>> +++ b/tests/qemu-iotests/071
>> @@ -104,11 +104,20 @@ echo
>>  echo "=== Testing blkdebug on existing block device ==="
>>  echo
>>  
>> -run_qemu -drive "file=$TEST_IMG,format=raw,if=none,id=drive0" <<EOF
>> +run_qemu <<EOF
>>  { "execute": "qmp_capabilities" }
>>  { "execute": "blockdev-add",
>>      "arguments": {
>>          "options": {
>> +            "node-name": "drive0",
>> +            "driver": "file",
>> +            "filename": "$TEST_IMG"
>> +        }
>> +    }
>> +}
> 
> Any specific reason why format=raw is translated into driver=file here...

This is for the BDS used under blockdev-add, which is supposed to be a
protocol BDS. "raw" was actually wrong, but apparently I felt bad about
specifying a protocol driver on the command line.

>> +{ "execute": "blockdev-add",
>> +    "arguments": {
>> +        "options": {
>>              "driver": "$IMGFMT",
>>              "id": "drive0-debug",
>>              "file": {
>> @@ -133,11 +142,23 @@ echo
>>  echo "=== Testing blkverify on existing block device ==="
>>  echo
>>  
>> -run_qemu -drive "file=$TEST_IMG,format=$IMGFMT,if=none,id=drive0" <<EOF
>> +run_qemu <<EOF
>>  { "execute": "qmp_capabilities" }
>>  { "execute": "blockdev-add",
>>      "arguments": {
>>          "options": {
>> +            "node-name": "drive0",
>> +            "driver": "$IMGFMT",
>> +            "file": {
>> +                "driver": "file",
>> +                "filename": "$TEST_IMG"
>> +            }
>> +        }
>> +    }
>> +}
>> +{ "execute": "blockdev-add",
>> +    "arguments": {
>> +        "options": {
>>              "driver": "blkverify",
>>              "id": "drive0-verify",
>>              "test": "drive0",
>> @@ -163,11 +184,23 @@ echo
>>  echo "=== Testing blkverify on existing raw block device ==="
>>  echo
>>  
>> -run_qemu -drive "file=$TEST_IMG.base,format=raw,if=none,id=drive0" <<EOF
>> +run_qemu <<EOF
>>  { "execute": "qmp_capabilities" }
>>  { "execute": "blockdev-add",
>>      "arguments": {
>>          "options": {
>> +            "node-name": "drive0",
>> +            "driver": "raw",
>> +            "file": {
>> +                "driver": "file",
>> +                "filename": "$TEST_IMG.base"
>> +            }
>> +        }
>> +    }
>> +}
> 
> ...but into driver=raw,file.driver=file here?

Probably because I assumed based on the test above this one that the
"raw" BDS for blkverify would be a format BDS. Apparently it's not and
both this and the test above this one should be fixed.

>> +{ "execute": "blockdev-add",
>> +    "arguments": {
>> +        "options": {
>>              "driver": "blkverify",
>>              "id": "drive0-verify",
>>              "test": {
> 
>> diff --git a/tests/qemu-iotests/081 b/tests/qemu-iotests/081
>> index d9b042c..5c8a8fa 100755
>> --- a/tests/qemu-iotests/081
>> +++ b/tests/qemu-iotests/081
>> @@ -101,11 +101,23 @@ $QEMU_IO -c "open -o $quorum" -c "read -P 0x32 0 
>> $size" | _filter_qemu_io
>>  echo
>>  echo "== checking mixed reference/option specification =="
>>  
>> -run_qemu -drive "file=$TEST_DIR/2.raw,format=$IMGFMT,if=none,id=drive2" 
>> <<EOF
>> +run_qemu <<EOF
>>  { "execute": "qmp_capabilities" }
>>  { "execute": "blockdev-add",
>>      "arguments": {
>>          "options": {
>> +            "node-name": "drive2",
>> +            "driver": "raw",
>> +            "file": {
>> +                "driver": "file",
>> +                "filename": "$TEST_DIR/2.raw"
>> +            }
>> +        }
>> +    }
>> +}
> 
> And here $IMGFMT becomes raw (which is equivalent, but...)

The other blockdev-add in this example uses hard-coded "raw", too. And
then there's the hard-coded ".raw" extension everywhere in this test...

I'll fix it, but that means fixing the pre-existing blockdev-add, too,
and for such matters the most difficult thing always tends to be
explaining it in the commit message. :-)

Max

>> +{ "execute": "blockdev-add",
>> +    "arguments": {
>> +        "options": {
>>              "driver": "quorum",
>>              "id": "drive0-quorum",
>>              "vote-threshold": 2,
> 
> Kevin
>

signature.asc
Description: OpenPGP digital signature