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 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 /
> 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