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 <vsement...@virtuozzo.com>
>>> ---
>>> 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
