| Message ID | 20190807141226.193501-4-vsementsov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
| Series | qcow2-bitmaps: rewrite reopening logic | expand |
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 <vsementsov@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? 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. 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.
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 <vsementsov@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
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 <vsementsov@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 >
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 <vsementsov@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
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 <vsementsov@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?
27 сент. 2019 г. 21:30 пользователь John Snow <jsnow@redhat.com> написал: 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 <vsementsov@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? It`s a bug fixed later in this series :) -- Best regards, Vladimir
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() + + 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
Reopening bitmaps to RW was broken prior to previous commit. Check that it works now. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- tests/qemu-iotests/165 | 46 ++++++++++++++++++++++++++++++++++++-- tests/qemu-iotests/165.out | 4 ++-- 2 files changed, 46 insertions(+), 4 deletions(-)