Re: [Qemu-devel] [PATCH v9 07/13] iotests: prepare 124 and 257 bitmap querying for backup-top filter

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2932 bytes --]

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 <vsementsov@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.)

Max

> +        return None
> +
>  
>  index_re = re.compile(r'([^\[]+)\[([^\]]+)\]')
>  
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]