| Message ID | 20200217150246.29180-18-vsementsov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix error handling during bitmap postcopy | expand |
On 17/02/2020 18:02, Vladimir Sementsov-Ogievskiy wrote: > Test wants force bitmap postcopy. Still, resulting postcopy period is > very small. Let's increase it by adding more bitmaps to migrate. Also, > test disabled bitmaps migration. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > tests/qemu-iotests/199 | 58 ++++++++++++++++++++++++++++-------------- > 1 file changed, 39 insertions(+), 19 deletions(-) > > diff --git a/tests/qemu-iotests/199 b/tests/qemu-iotests/199 > index 7914fd0b2b..9a6e8dcb9d 100755 > --- a/tests/qemu-iotests/199 > +++ b/tests/qemu-iotests/199 > @@ -103,29 +103,45 @@ class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase): > > def test_postcopy(self): > granularity = 512 > + nb_bitmaps = 15 > > - result = self.vm_a.qmp('block-dirty-bitmap-add', node='drive0', > - name='bitmap', granularity=granularity) > - self.assert_qmp(result, 'return', {}) > + for i in range(nb_bitmaps): > + result = self.vm_a.qmp('block-dirty-bitmap-add', node='drive0', > + name='bitmap{}'.format(i), > + granularity=granularity) > + self.assert_qmp(result, 'return', {}) > > result = self.vm_a.qmp('x-debug-block-dirty-bitmap-sha256', > - node='drive0', name='bitmap') > + node='drive0', name='bitmap0') > empty_sha256 = result['return']['sha256'] > > - apply_discards(self.vm_a, discards1 + discards2) > + apply_discards(self.vm_a, discards1) > > result = self.vm_a.qmp('x-debug-block-dirty-bitmap-sha256', > - node='drive0', name='bitmap') > - sha256 = result['return']['sha256'] > + node='drive0', name='bitmap0') > + discards1_sha256 = result['return']['sha256'] > > # Check, that updating the bitmap by discards works > - assert sha256 != empty_sha256 > + assert discards1_sha256 != empty_sha256 > > - result = self.vm_a.qmp('block-dirty-bitmap-clear', node='drive0', > - name='bitmap') > - self.assert_qmp(result, 'return', {}) > + # We want to calculate resulting sha256. Do it in bitmap0, so, disable > + # other bitmaps > + for i in range(1, nb_bitmaps): > + result = self.vm_a.qmp('block-dirty-bitmap-disable', node='drive0', > + name='bitmap{}'.format(i)) > + self.assert_qmp(result, 'return', {}) > > - apply_discards(self.vm_a, discards1) > + apply_discards(self.vm_a, discards2) > + > + result = self.vm_a.qmp('x-debug-block-dirty-bitmap-sha256', > + node='drive0', name='bitmap0') > + all_discards_sha256 = result['return']['sha256'] > + > + # Now, enable some bitmaps, to be updated during migration > + for i in range(2, nb_bitmaps, 2): > + result = self.vm_a.qmp('block-dirty-bitmap-enable', node='drive0', > + name='bitmap{}'.format(i)) > + self.assert_qmp(result, 'return', {}) > > caps = [{'capability': 'dirty-bitmaps', 'state': True}, > {'capability': 'events', 'state': True}] > @@ -145,6 +161,7 @@ class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase): > e_resume = self.vm_b.event_wait('RESUME') > self.vm_b_events.append(e_resume) > > + # enabled bitmaps should be updated > apply_discards(self.vm_b, discards2) > > match = {'data': {'status': 'completed'}} > @@ -158,7 +175,7 @@ class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase): > downtime = event_dist(e_stop, e_resume) > postcopy_time = event_dist(e_resume, e_complete) > > - # TODO: assert downtime * 10 < postcopy_time > + assert downtime * 10 < postcopy_time > if debug: > print('downtime:', downtime) > print('postcopy_time:', postcopy_time) > @@ -166,12 +183,15 @@ class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase): > # Assert that bitmap migration is finished (check that successor bitmap > # is removed) > result = self.vm_b.qmp('query-block') > - assert len(result['return'][0]['dirty-bitmaps']) == 1 > - > - # Check content of migrated (and updated by new writes) bitmap > - result = self.vm_b.qmp('x-debug-block-dirty-bitmap-sha256', > - node='drive0', name='bitmap') > - self.assert_qmp(result, 'return/sha256', sha256) > + assert len(result['return'][0]['dirty-bitmaps']) == nb_bitmaps > + > + # Check content of migrated bitmaps. Still, don't waste time checking > + # every bitmap > + for i in range(0, nb_bitmaps, 5): > + result = self.vm_b.qmp('x-debug-block-dirty-bitmap-sha256', > + node='drive0', name='bitmap{}'.format(i)) > + sha256 = discards1_sha256 if i % 2 else all_discards_sha256 > + self.assert_qmp(result, 'return/sha256', sha256) > > > if __name__ == '__main__': > The updated test passed. Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
On 2/17/20 9:02 AM, Vladimir Sementsov-Ogievskiy wrote: > Test wants force bitmap postcopy. Still, resulting postcopy period is The test wants to force a bitmap postcopy. Still, the resulting postcopy period is very small. > very small. Let's increase it by adding more bitmaps to migrate. Also, > test disabled bitmaps migration. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > tests/qemu-iotests/199 | 58 ++++++++++++++++++++++++++++-------------- > 1 file changed, 39 insertions(+), 19 deletions(-) Patches 12-17: Tested-by: Eric Blake <eblake@redhat.com> As they all work without any other patches in this series, and DO make a dramatic difference (cutting the test from over 70 seconds to just 7, on my machine), I'm inclined to stage them now, even while waiting for you to rebase the rest of the series. And 18 is already in the tree.
diff --git a/tests/qemu-iotests/199 b/tests/qemu-iotests/199 index 7914fd0b2b..9a6e8dcb9d 100755 --- a/tests/qemu-iotests/199 +++ b/tests/qemu-iotests/199 @@ -103,29 +103,45 @@ class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase): def test_postcopy(self): granularity = 512 + nb_bitmaps = 15 - result = self.vm_a.qmp('block-dirty-bitmap-add', node='drive0', - name='bitmap', granularity=granularity) - self.assert_qmp(result, 'return', {}) + for i in range(nb_bitmaps): + result = self.vm_a.qmp('block-dirty-bitmap-add', node='drive0', + name='bitmap{}'.format(i), + granularity=granularity) + self.assert_qmp(result, 'return', {}) result = self.vm_a.qmp('x-debug-block-dirty-bitmap-sha256', - node='drive0', name='bitmap') + node='drive0', name='bitmap0') empty_sha256 = result['return']['sha256'] - apply_discards(self.vm_a, discards1 + discards2) + apply_discards(self.vm_a, discards1) result = self.vm_a.qmp('x-debug-block-dirty-bitmap-sha256', - node='drive0', name='bitmap') - sha256 = result['return']['sha256'] + node='drive0', name='bitmap0') + discards1_sha256 = result['return']['sha256'] # Check, that updating the bitmap by discards works - assert sha256 != empty_sha256 + assert discards1_sha256 != empty_sha256 - result = self.vm_a.qmp('block-dirty-bitmap-clear', node='drive0', - name='bitmap') - self.assert_qmp(result, 'return', {}) + # We want to calculate resulting sha256. Do it in bitmap0, so, disable + # other bitmaps + for i in range(1, nb_bitmaps): + result = self.vm_a.qmp('block-dirty-bitmap-disable', node='drive0', + name='bitmap{}'.format(i)) + self.assert_qmp(result, 'return', {}) - apply_discards(self.vm_a, discards1) + apply_discards(self.vm_a, discards2) + + result = self.vm_a.qmp('x-debug-block-dirty-bitmap-sha256', + node='drive0', name='bitmap0') + all_discards_sha256 = result['return']['sha256'] + + # Now, enable some bitmaps, to be updated during migration + for i in range(2, nb_bitmaps, 2): + result = self.vm_a.qmp('block-dirty-bitmap-enable', node='drive0', + name='bitmap{}'.format(i)) + self.assert_qmp(result, 'return', {}) caps = [{'capability': 'dirty-bitmaps', 'state': True}, {'capability': 'events', 'state': True}] @@ -145,6 +161,7 @@ class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase): e_resume = self.vm_b.event_wait('RESUME') self.vm_b_events.append(e_resume) + # enabled bitmaps should be updated apply_discards(self.vm_b, discards2) match = {'data': {'status': 'completed'}} @@ -158,7 +175,7 @@ class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase): downtime = event_dist(e_stop, e_resume) postcopy_time = event_dist(e_resume, e_complete) - # TODO: assert downtime * 10 < postcopy_time + assert downtime * 10 < postcopy_time if debug: print('downtime:', downtime) print('postcopy_time:', postcopy_time) @@ -166,12 +183,15 @@ class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase): # Assert that bitmap migration is finished (check that successor bitmap # is removed) result = self.vm_b.qmp('query-block') - assert len(result['return'][0]['dirty-bitmaps']) == 1 - - # Check content of migrated (and updated by new writes) bitmap - result = self.vm_b.qmp('x-debug-block-dirty-bitmap-sha256', - node='drive0', name='bitmap') - self.assert_qmp(result, 'return/sha256', sha256) + assert len(result['return'][0]['dirty-bitmaps']) == nb_bitmaps + + # Check content of migrated bitmaps. Still, don't waste time checking + # every bitmap + for i in range(0, nb_bitmaps, 5): + result = self.vm_b.qmp('x-debug-block-dirty-bitmap-sha256', + node='drive0', name='bitmap{}'.format(i)) + sha256 = discards1_sha256 if i % 2 else all_discards_sha256 + self.assert_qmp(result, 'return/sha256', sha256) if __name__ == '__main__':
Test wants force bitmap postcopy. Still, resulting postcopy period is very small. Let's increase it by adding more bitmaps to migrate. Also, test disabled bitmaps migration. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- tests/qemu-iotests/199 | 58 ++++++++++++++++++++++++++++-------------- 1 file changed, 39 insertions(+), 19 deletions(-)