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

2019-09-02 Thread Max Reitz 
On 29.08.19 15:22, Vladimir Sementsov-Ogievskiy wrote:
> 28.08.2019 19:40, Max Reitz wrote:
>> On 26.08.19 18:13, Vladimir Sementsov-Ogievskiy wrote:

[...]

>>> +
>>> +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"..

Ah.  OK.  Now it all makes sense...

Max



signature.asc
Description: OpenPGP digital signature

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

2019-08-29 Thread Vladimir Sementsov-Ogievskiy 
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 
>> ---
>>   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

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

2019-08-28 Thread Max Reitz 
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 
> ---
>  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'([^\[]+)\[([^\]]+)\]')
>  
> 




signature.asc
Description: OpenPGP digital signature

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

2019-08-26 Thread Vladimir Sementsov-Ogievskiy 
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 
---
 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')
diff --git a/tests/qemu-iotests/257 b/tests/qemu-iotests/257
index c2a72c577a..8c3e96a7fc 100755
--- a/tests/qemu-iotests/257
+++ b/tests/qemu-iotests/257
@@ -188,25 +188,6 @@ class Drive:
 self.size = size
 self.node = name
 
-def query_bitmaps(vm):
-res = vm.qmp("query-block")
-return {"bitmaps": {device['device'] or device['qdev']:
-device.get('dirty-bitmaps', []) for
-device in res['return']}}
-
-def get_bitmap(bitmaps, drivename, name, recording=None):
-"""
-get a specific bitmap from the object returned by query_bitmaps.
-:param recording: If specified, filter results by the specified value.
-"""
-for bitmap in bitmaps['bitmaps'][drivename]:
-if bitmap.get('name', '') == name:
-if recording is None:
-return bitmap
-elif bitmap.get('recording') == recording:
-return bitmap
-return None
-
 def blockdev_backup(vm, device, target, sync, **kwargs):
 # Strip any arguments explicitly nulled by the caller:
 kwargs = {key: val for key, val in kwargs.items() if val is not None}
@@ -249,7 +230,7 @@ def perform_writes(drive, n):
 pattern.size)
 log(cmd)
 log(drive.vm.hmp_qemu_io(drive.name, cmd))
-bitmaps = query_bitmaps(drive.vm)
+bitmaps = drive.vm.query_bitmaps()
 log(bitmaps, indent=2)
 log('')
 return bitmaps
@@ -370,7 +351,7 @@ def test_bitmap_sync(bsync_mode, msync_mode='bitmap', 
failure=None):
 # 1 - Writes and Reference Backup
 bitmaps = perform_writes(drive0, 1)
 ebitmap.dirty_group(1)
-bitmap = get_bitmap(bitmaps, drive0.device, 'bitmap0')
+bitmap = vm.get_bitmap(bitmaps, drive0.node, 'bitmap0')
 ebitmap.compare(bitmap)
 reference_backup(drive0, 1, fbackup1)
 
@@ -388,11 +369,11 @@ def test_bitmap_sync(bsync_mode, msync_mode='bitmap', 
failure=None):
 log('')
 bitmaps = perform_writes(drive0, 2)
 # Named bitmap (static, should be unchanged)
-ebitmap.compare(get_bitmap(bitmaps, drive0.device, 'bitmap0'))
+ebitmap.compare(vm.get_bitmap(bitmaps, drive0.node, 'bitmap0'))
 # Anonymous bitmap (dynamic, shows new writes)
 anonymous = EmulatedBitmap()
 anonymous.dirty_group(2)
-anonymous.compare(get_bitmap(bitmaps, drive0.device, '',
+anonymous.compare(vm.get_bitmap(bitmaps, drive0.node, '',
  recording=True))
 
 # Simulate the order in which this will happen:
@@ -405,7 +386,7 @@ def test_bitmap_sync(bsync_mode, msync_mode='bitmap', 
failure=None):
 vm.run_job(job, auto_dismiss=True, auto_finalize=False,
pre_finalize=_callback,
cancel=(failure == 'simulated'))
-bitmaps = query_bitmaps(vm)
+bitmaps = vm.query_bitmaps()
 log(bitmaps, indent=2)
 log('')
 
@@ -423,29 +404,29 @@ def test_bitmap_sync(bsync_mode, msync_mode='bitmap', 
failure=None):
 ebitmap.clear()
 ebitmap.dirty_bits(range(fail_bit, SIZE // GRANULARITY))
 
-ebitmap.compare(get_bitmap(bitmaps, drive0.device, 'bitmap0'))
+ebitmap.compare(vm.get_bitmap(bitmaps, drive0.node, 'bitmap0'))
 
 # 2 - Writes and Reference Backup
 bitmaps = perform_writes(drive0, 3)
 ebitmap.dirty_group(3)
-ebitmap.compare(get_bitmap(bitmaps, drive0.device, 'bitmap0'))
+ebitmap.compare(vm.get_bitmap(bitmaps, drive0.node, 'bitmap0'))
 reference_backup(drive0, 2, fbackup2)
 
 # 2 - Bitmap Backup (In failure modes, this is a recovery.)
 job = backup(drive0, 2, bsync2, "bitmap",
  bitmap="bitmap0", bitmap_mode=bsync_mode)
 vm.run_job(job, auto_dismiss=True, auto_finalize=False)