On 9/27/19 6:29 AM, Vladimir Sementsov-Ogievskiy wrote:
> 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 /
>>> 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
>>
>
> Ah, stop. We can't check reopen RO at this point, as it will be fixed in
> later patch
>
Oh :(
Is it a reopen bug, a qcow2 bug, or a qcow2-bitmap bug?
