Re: [Qemu-devel] [PATCH v4 03/10] iotests: add test-case to 165 to test reopening qcow2 bitmaps to RW

[Vladimir Sementsov-Ogievskiy]

27.09.2019 10:28, Vladimir Sementsov-Ogievskiy wrote:
> 27.09.2019 1:57, John Snow wrote:
>>
>>
>> On 8/7/19 10:12 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> Reopening bitmaps to RW was broken prior to previous commit. Check that
>>> it works now.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>>> ---
>>>   tests/qemu-iotests/165     | 46 ++++++++++++++++++++++++++++++++++++--
>>>   tests/qemu-iotests/165.out |  4 ++--
>>>   2 files changed, 46 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tests/qemu-iotests/165 b/tests/qemu-iotests/165
>>> index 88f62d3c6d..dd93b5a2d0 100755
>>> --- a/tests/qemu-iotests/165
>>> +++ b/tests/qemu-iotests/165
>>> @@ -43,10 +43,10 @@ class TestPersistentDirtyBitmap(iotests.QMPTestCase):
>>>           os.remove(disk)
>>>       def mkVm(self):
>>> -        return iotests.VM().add_drive(disk)
>>> +        return iotests.VM().add_drive(disk, opts='node-name=node0')
>>>       def mkVmRo(self):
>>> -        return iotests.VM().add_drive(disk, opts='readonly=on')
>>> +        return iotests.VM().add_drive(disk, 
>>> opts='readonly=on,node-name=node0')
>>>       def getSha256(self):
>>>           result = self.vm.qmp('x-debug-block-dirty-bitmap-sha256',
>>> @@ -102,5 +102,47 @@ class TestPersistentDirtyBitmap(iotests.QMPTestCase):
>>>           self.vm.shutdown()
>>> +    def test_reopen_rw(self):
>>> +        self.vm = self.mkVm()
>>> +        self.vm.launch()
>>> +        self.qmpAddBitmap()
>>> +
>>> +        # Calculate sha256 corresponding to regions1
>>> +        self.writeRegions(regions1)
>>> +        sha256 = self.getSha256()
>>> +        result = self.vm.qmp('block-dirty-bitmap-clear', node='drive0',
>>> +                             name='bitmap0')
>>> +        self.assert_qmp(result, 'return', {})
>>> +
>>> +        self.vm.shutdown()
>>> +
>>> +        self.vm = self.mkVmRo()
>>> +        self.vm.launch()
>>> +
>>> +        # We've loaded empty bitmap
>>> +        assert sha256 != self.getSha256()
>>> +
>>> +        # Check that we are in RO mode
>>> +        self.writeRegions(regions1)
>>> +        assert sha256 != self.getSha256()
>>> +
>>
>> the HMP monitor lets you attempt writes to a Read Only drive...? Or does
>> it error out and we just don't check the reply?
> 
> It must fail and we check this by comparing dirty bitmap hash.
> 
>>
>> I would prefer we use an actual dirty sector count here instead; we have
>> the new API that should make it easy to do.
>>
>> If the debug SHA changes this might be a little fragile.
> 
> Hmm, I agree that checking that bitmap is empty by comparing with some hash
> is not very reliable. Will do.
> 
>>
>> ACK otherwise.
>>
>>> +        result = self.vm.qmp('x-blockdev-reopen', **{
>>> +            'node-name': 'node0',
>>> +            'driver': iotests.imgfmt,
>>> +            'file': {
>>> +                'driver': 'file',
>>> +                'filename': disk
>>> +            },
>>> +            'read-only': False
>>> +        })
>>> +        self.assert_qmp(result, 'return', {})
>>> +
>>> +        # Check that bitmap is reopened to RW and we can write to it.
>>> +        self.writeRegions(regions1)
>>> +        assert sha256 == self.getSha256()
>>> +
>>> +        self.vm.shutdown()
>>> +
>>> +
>>>   if __name__ == '__main__':
>>>       iotests.main(supported_fmts=['qcow2'])
>>> diff --git a/tests/qemu-iotests/165.out b/tests/qemu-iotests/165.out
>>> index ae1213e6f8..fbc63e62f8 100644
>>> --- a/tests/qemu-iotests/165.out
>>> +++ b/tests/qemu-iotests/165.out
>>> @@ -1,5 +1,5 @@
>>> -.
>>> +..
>>>   ----------------------------------------------------------------------
>>> -Ran 1 tests
>>> +Ran 2 tests
>>>   OK
>>>
>>
>> This is a suggestion for an even more rigorous test:
>>
>> - Create bitmap
>> - Write a region or two
>> - Record the dirty count and the SHA; assert it is equal to known /

Still, I don't think we need to check count, if we check SHA.

>> predetermined values.
>> - reopen RO
>> - verify the bitmap still exists and that the hash and count are the same.
>> - Stop the VM
>> - Start the VM in readonly mode
>> - verify the bitmap still exists and that the hash and count are the same.
>> - Reopen-RW
>> - verify the bitmap still exists and that the hash and count are the same.
>> - Write further region(s)
>> - Get the new dirty count and SHA, and assert it is equal to known /
>> predetermined values.
>>
> 
> OK
> 


-- 
Best regards,
Vladimir