Re: [Qemu-devel] [PATCH v4 3/4] qemu-iotest: Add pause_drive and resume_drive methods

[Max Reitz <mreitz@redhat.com>]

On 14.11.2013 10:10, Fam Zheng wrote:
> They wrap blkdebug "break" and "remove_break".
>
> Add optional argument "resume" to cancel_and_wait().
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  tests/qemu-iotests/iotests.py | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index fb10ff4..a483865 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -107,6 +107,18 @@ class VM(object):
>          self._num_drives += 1
>          return self
>  
> +    def pause_drive(self, drive, event=""):
> +        '''Pause drive r/w operations'''
> +        if not event:
> +            self.pause_drive(drive, "read_aio")
> +            self.pause_drive(drive, "write_aio")
> +        self.qmp('human-monitor-command',
> +                    command_line='qemu-io %s "break %s bp_%s"' % (drive, event, drive))

In the default event="" case (on, say, "drive0"), you'll execute the
three HMP commands 'qemu-io drive0 "break read_aio bp_drive0"', 'qemu-io
drive0 "break write_aio bp_drive0"', 'qemu-io drive0 "break 
bp_drive0"', that is, this code will issue three HMP commands with the
last one having an empty string as the event (which is probably not what
you want and will probably result in a silently ignored error from the
break command).

Also, I'd propose using something like None instead of "" for the
default event.

> +
> +    def resume_drive(self, drive):
> +        self.qmp('human-monitor-command',
> +                    command_line='qemu-io %s "remove_break bp_%s"' % (drive, drive))
> +
>      def hmp_qemu_io(self, drive, cmd):
>          '''Write to a given drive using an HMP command'''
>          return self.qmp('human-monitor-command',
> @@ -222,11 +234,14 @@ class QMPTestCase(unittest.TestCase):
>          result = self.vm.qmp('query-block-jobs')
>          self.assert_qmp(result, 'return', [])
>  
> -    def cancel_and_wait(self, drive='drive0', force=False):
> +    def cancel_and_wait(self, drive='drive0', force=False, resume=""):
>          '''Cancel a block job and wait for it to finish, returning the event'''
>          result = self.vm.qmp('block-job-cancel', device=drive, force=force)
>          self.assert_qmp(result, 'return', {})
>  
> +        if resume:
> +            self.vm.resume_drive(drive)
> +

I'd propose using a bool as the resume value (i.e., resume=False as
default). In the next patch, it looks like the parameter should be the
drive name, although it actually doesn't matter at all what string it
contains (as long as it isn't empty). Thus, a boolean value is probably
more appropriate.

Max

>          cancelled = False
>          result = None
>          while not cancelled: