Re: [PATCH v9 10/14] iotests: add hmp helper with logging

[Kevin Wolf]

Am 25.03.2020 um 00:20 hat John Snow geschrieben:
> Just a mild cleanup while I was here.
> 
> Although we now have universal qmp logging on or off, many existing
> callers to hmp functions don't expect that output to be logged, which
> causes quite a few changes in the test output.
> 
> For now, just offer a use_log parameter.
> 
> Signed-off-by: John Snow 
> ---
>  tests/qemu-iotests/iotests.py | 30 +-
>  1 file changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index e12d6e533e..4faee06f14 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -540,25 +540,29 @@ def add_incoming(self, addr):
>  self._args.append(addr)
>  return self
>  
> -def pause_drive(self, drive, event=None):
> -'''Pause drive r/w operations'''
> +def hmp(self, command_line: str, use_log: bool = False):

Missing return type. Should probably be Dict[str, Any]?

> +cmd = 'human-monitor-command'
> +kwargs = {'command-line': command_line}
> +if use_log:
> +return self.qmp_log(cmd, **kwargs)
> +else:
> +return self.qmp(cmd, **kwargs)
> +
> +def pause_drive(self, drive: str, event: Optional[str] = None) -> None:
> +"""Pause drive r/w operations"""
>  if not event:
>  self.pause_drive(drive, "read_aio")
>  self.pause_drive(drive, "write_aio")
>  return
> -self.qmp('human-monitor-command',
> - command_line='qemu-io %s "break %s bp_%s"'
> - % (drive, event, drive))
> +self.hmp(f'qemu-io {drive} "break {event} bp_{drive}"')
>  
> -def resume_drive(self, drive):
> -self.qmp('human-monitor-command',
> - command_line='qemu-io %s "remove_break bp_%s"'
> - % (drive, drive))
> +def resume_drive(self, drive: str) -> None:
> +"""Resume drive r/w operations"""
> +self.hmp(f'qemu-io {drive} "remove_break bp_{drive}"')
>  
> -def hmp_qemu_io(self, drive, cmd):
> -'''Write to a given drive using an HMP command'''
> -return self.qmp('human-monitor-command',
> -command_line='qemu-io %s "%s"' % (drive, cmd))
> +def hmp_qemu_io(self, drive: str, cmd: str, use_log: bool = False) -> 
> None:
> +"""Write to a given drive using an HMP command"""
> +return self.hmp(f'qemu-io {drive} "{cmd}"', use_log=use_log)

Once you have a non-Any return type for hmp(), this would report that
you return something for a function declared to return None.

Kevin

Re: [PATCH v9 10/14] iotests: add hmp helper with logging

[Max Reitz]

On 25.03.20 00:20, John Snow wrote:
> Just a mild cleanup while I was here.
> 
> Although we now have universal qmp logging on or off, many existing
> callers to hmp functions don't expect that output to be logged, which
> causes quite a few changes in the test output.
> 
> For now, just offer a use_log parameter.
> 
> Signed-off-by: John Snow 
> ---
>  tests/qemu-iotests/iotests.py | 30 +-
>  1 file changed, 17 insertions(+), 13 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

[PATCH v9 10/14] iotests: add hmp helper with logging

[John Snow]

Just a mild cleanup while I was here.

Although we now have universal qmp logging on or off, many existing
callers to hmp functions don't expect that output to be logged, which
causes quite a few changes in the test output.

For now, just offer a use_log parameter.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/iotests.py | 30 +-
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index e12d6e533e..4faee06f14 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -540,25 +540,29 @@ def add_incoming(self, addr):
 self._args.append(addr)
 return self
 
-def pause_drive(self, drive, event=None):
-'''Pause drive r/w operations'''
+def hmp(self, command_line: str, use_log: bool = False):
+cmd = 'human-monitor-command'
+kwargs = {'command-line': command_line}
+if use_log:
+return self.qmp_log(cmd, **kwargs)
+else:
+return self.qmp(cmd, **kwargs)
+
+def pause_drive(self, drive: str, event: Optional[str] = None) -> None:
+"""Pause drive r/w operations"""
 if not event:
 self.pause_drive(drive, "read_aio")
 self.pause_drive(drive, "write_aio")
 return
-self.qmp('human-monitor-command',
- command_line='qemu-io %s "break %s bp_%s"'
- % (drive, event, drive))
+self.hmp(f'qemu-io {drive} "break {event} bp_{drive}"')
 
-def resume_drive(self, drive):
-self.qmp('human-monitor-command',
- command_line='qemu-io %s "remove_break bp_%s"'
- % (drive, drive))
+def resume_drive(self, drive: str) -> None:
+"""Resume drive r/w operations"""
+self.hmp(f'qemu-io {drive} "remove_break bp_{drive}"')
 
-def hmp_qemu_io(self, drive, cmd):
-'''Write to a given drive using an HMP command'''
-return self.qmp('human-monitor-command',
-command_line='qemu-io %s "%s"' % (drive, cmd))
+def hmp_qemu_io(self, drive: str, cmd: str, use_log: bool = False) -> None:
+"""Write to a given drive using an HMP command"""
+return self.hmp(f'qemu-io {drive} "{cmd}"', use_log=use_log)
 
 def flatten_qmp_object(self, obj, output=None, basestr=''):
 if output is None:
-- 
2.21.1