Re: [Qemu-devel] [PATCH v2 7/7] iotests: add iotest 236 for testing bitmap merge

[John Snow]

On 12/12/18 9:27 PM, Eric Blake wrote:
> On 12/12/18 7:50 PM, John Snow wrote:
>> New interface, new smoke test.
>>
>> Signed-off-by: John Snow <address@hidden>
>> ---
>>   tests/qemu-iotests/236     | 123 +++++++++++++++++
>>   tests/qemu-iotests/236.out | 265 +++++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/group   |   1 +
>>   3 files changed, 389 insertions(+)
>>   create mode 100755 tests/qemu-iotests/236
>>   create mode 100644 tests/qemu-iotests/236.out
>>
>> diff --git a/tests/qemu-iotests/236 b/tests/qemu-iotests/236
> 
>> +    log('')
>> +    log('--- Disabling B & Adding C ---\n')
>> +    vm.qmp_log("transaction", actions=[
>> +        { "type": "block-dirty-bitmap-disable",
>> +          "data": { "node": "drive0", "name": "bitmapB" }},
>> +        { "type": "block-dirty-bitmap-add",
>> +          "data": { "node": "drive0", "name": "bitmapC" }}
>> +    ])
> 
> Just for grins, what happens if we extend that transaction to
> additionally call:
> 
>        { "type": "block-dirty-bitmap-disable",
>          "data": { "node": "drive0", "name": "bitmapC" }},
>        { "type": "block-dirty-bitmap-enable",
>          "data": { "node": "drive0", "name": "bitmapC" }}
> 
> Yes, we have a redundancy where plain '-add' coupled with '-disable' in
> a transaction does the same as '-add' with 'enabled':false.  And I'd
> rather keep 'enabled':false as part of '-add', as it's handy to do that
> without having to code up a transaction.  But the specific act of
> toggling the enabled bit twice on a brand new bitmap as part of the same
> transaction, while unlikely to be done by libvirt, may still prove to be
> a useful validation of our transaction semantics.
> 
>> +++ b/tests/qemu-iotests/group
>> @@ -233,3 +233,4 @@
>>   233 auto quick
>>   234 auto quick migration
>>   235 auto quick
>> +236 auto quick
>> \ No newline at end of file
> 
> You'll want to fix that.
> 

Now that you've pointed it out, I have to. :)

> Reviewed-by: Eric Blake <address@hidden>
> 

OK. I want more tests, too; mostly around malformed requests (bad
target, single bad source, multiple bad sources, mixed bad sources).
your suggestion to merge self into self,
and maybe anything else I've forgotten by now.

--js