28.08.2019 19:40, Max Reitz wrote: > On 26.08.19 18:13, Vladimir Sementsov-Ogievskiy wrote: >> After backup-top filter appearing it's not possible to see dirty >> bitmaps in top node, so use node-name instead. >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >> --- >> tests/qemu-iotests/124 | 3 +- >> tests/qemu-iotests/257 | 39 +--- >> tests/qemu-iotests/257.out | 364 +++++++++++++--------------------- >> tests/qemu-iotests/iotests.py | 22 ++ >> 4 files changed, 173 insertions(+), 255 deletions(-) >> >> diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124 >> index 3440f54781..8b6024769c 100755 >> --- a/tests/qemu-iotests/124 >> +++ b/tests/qemu-iotests/124 >> @@ -749,8 +749,7 @@ class >> TestIncrementalBackupBlkdebug(TestIncrementalBackupBase): >> >> # Bitmap Status Check >> query = self.vm.qmp('query-block') >> - ret = [bmap for bmap in query['return'][0]['dirty-bitmaps'] >> - if bmap.get('name') == bitmap.name][0] >> + ret = self.vm.get_bitmap(None, drive0['id'], bitmap.name) >> self.assert_qmp(ret, 'count', 458752) >> self.assert_qmp(ret, 'granularity', 65536) >> self.assert_qmp(ret, 'status', 'frozen') > > I see a couple more instances of querying the bitmaps through > query-block here. Wouldn’t it make sense to replace them all with > get_bitmap()? > > [...] > >> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py >> index 84438e837c..9381964d9f 100644 >> --- a/tests/qemu-iotests/iotests.py >> +++ b/tests/qemu-iotests/iotests.py >> @@ -643,6 +643,28 @@ class VM(qtest.QEMUQtestMachine): >> return x >> return None >> >> + def query_bitmaps(self): >> + res = self.qmp("query-named-block-nodes") >> + return {"bitmaps": {device['node-name']: device['dirty-bitmaps'] >> + for device in res['return'] >> + if 'dirty-bitmaps' in device}} > > I’d leave the wrapping in {"bitmaps": x} to the callers, if they need it. > >> + >> + def get_bitmap(self, bitmaps, node_name, name, recording=None): >> + """ >> + get a specific bitmap from the object returned by query_bitmaps. >> + :param recording: If specified, filter results by the specified >> value. >> + """ >> + if bitmaps is None: >> + bitmaps = self.query_bitmaps() >> + >> + for bitmap in bitmaps['bitmaps'][node_name]: >> + if bitmap.get('name', '') == name: >> + if recording is None: >> + return bitmap >> + elif bitmap.get('recording') == recording: >> + return bitmap > > Maybe add a “break” or “return None” here? > > (Yes, yes, you just moved existing code. Still.) >
No, as we may have several unnamed bitmaps, which should be selected by "recording".. > >> + return None >> + >> >> index_re = re.compile(r'([^\[]+)\[([^\]]+)\]') >> >> > > -- Best regards, Vladimir