On 12/20/18 7:12 AM, Vladimir Sementsov-Ogievskiy wrote:
> 20.12.2018 5:29, John Snow wrote:
>> New interface, new smoke test.
>>
>> Signed-off-by: John Snow <js...@redhat.com>
>> ---
>
> [...]
>
>> + # A: 7 clusters
>> + # B: 4 clusters
>> + # C: 6 clusters
>> + log(query_bitmaps(vm), indent=2)
>> +
>> + log('\n--- Submitting Bad Merge ---\n')
>
> aha, spent some time, trying to understand, what is bad with merge, until
> understand that
> that is abort. I didn't sleep enough last night, but anyway, 'Aborting Merge
> Transaction'
> is a bit clearer, I think.
>
Sure, I'll rephrase it: "Submitting & Aborting Merge Transaction"
>> + vm.qmp_log("transaction", indent=2, actions=[
>> + { "type": "block-dirty-bitmap-add",
>> + "data": { "node": "drive0", "name": "bitmapD",
>> + "disabled": True, "granularity": granularity }},
>> + { "type": "block-dirty-bitmap-merge",
>> + "data": { "node": "drive0", "target": "bitmapD",
>> + "bitmaps": ["bitmapB", "bitmapC"] }},
>> + { "type": "abort", "data": {}}
>> + ])
>> + log(query_bitmaps(vm), indent=2)
>> +
>> + log('\n--- Creating D as a merge of B & C ---\n')
>> + # Good hygiene: create a disabled bitmap as a merge target.
>> + vm.qmp_log("transaction", indent=2, actions=[
>> + { "type": "block-dirty-bitmap-add",
>> + "data": { "node": "drive0", "name": "bitmapD",
>> + "disabled": True, "granularity": granularity }},
>> + { "type": "block-dirty-bitmap-merge",
>> + "data": { "node": "drive0", "target": "bitmapD",
>> + "bitmaps": ["bitmapB", "bitmapC"] }}
>> + ])
>> +
>> + # A and D should now both have 7 clusters apiece.
>> + # B and C remain unchanged with 4 and 6 respectively.
>> + log(query_bitmaps(vm), indent=2)
>> +
>> + # A and D should be equivalent.
>> + # Some formats round the size of the disk, so don't print the checksums.
>
> Just interested: round 64M? to what?
>
VPC does weird stuff. If you ask for 64M you get 64M+16K. "round" is
maybe a bad adjective here, but VPC really won't give you what you ask
for. Loosening the restriction to "generic" was a good idea.
>> + check_a = vm.qmp('x-debug-block-dirty-bitmap-sha256',
>> + node="drive0", name="bitmapA")['return']['sha256']
>> + check_b = vm.qmp('x-debug-block-dirty-bitmap-sha256',
>> + node="drive0", name="bitmapD")['return']['sha256']
>> + assert(check_a == check_b)
>
> hmm, a funny suggestion: s/check_b/check_d/
Oh, yes, that would be better.
>
>> +
>> + log('\n--- Removing bitmaps A, B, C, and D ---\n')
>
> what about failed transaction with remove command, for a full kit?
>
Remove isn't transactionable!
>> + vm.qmp_log("block-dirty-bitmap-remove", node="drive0", name="bitmapA")
>> + vm.qmp_log("block-dirty-bitmap-remove", node="drive0", name="bitmapB")
>> + vm.qmp_log("block-dirty-bitmap-remove", node="drive0", name="bitmapC")
>> + vm.qmp_log("block-dirty-bitmap-remove", node="drive0", name="bitmapD")
>> +
>> + log('\n--- Final Query ---\n')
>> + log(query_bitmaps(vm), indent=2)
>> +
>> + log('\n--- Done ---\n')
>> + vm.shutdown()
>
>
> with or without any of my suggestions:
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
>
Thanks!
--js