Re: [Qemu-devel] [PATCH v2 5/6] iotests: implement QemuIoInteractive class

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2023 bytes --]

On 12/07/2017 09:51 AM, Vladimir Sementsov-Ogievskiy wrote:

The subject says what, but there is no commit body that says why.

Presumably this makes writing the test in the next patch easier, but
some details in the commit message would make this obvious.

> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  tests/qemu-iotests/iotests.py | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 

My python is not strong; it looks good overall, although I have a few
questions that may warrant a v3 before I give R-b.

> +class QemuIoInteractive:
> +    def __init__(self, *args):
> +        self.args = qemu_io_args + list(args)
> +        self._p = subprocess.Popen(self.args, stdin=subprocess.PIPE,
> +                                   stdout=subprocess.PIPE,
> +                                   stderr=subprocess.STDOUT)

Why STDOUT instead of STDERR?  Is the redirection intentional?


> +    def _read_output(self):
> +        pattern = 'qemu-io> '
> +        n = len(pattern)
> +        pos = 0
> +        s = []
> +        while pos != n:
> +            c = self._p.stdout.read(1)
> +            #check unexpected EOF

pep8 doesn't like this comment style (it wants space after #).  The file
is already unclean under pep8, but you shouldn't make it worse.

> +            assert c != ''

Is assert really the nicest control flow when early EOF is present? Or
is it because we are only using this in tests, where we don't expect
early EOF, so asserting is just fine for our usage?

> +            s.append(c)
> +            if c == pattern[pos]:
> +                pos += 1
> +            else:
> +                pos = 0
> +
> +        return ''.join(s[:-n])

Is it necessary to use join() on the empty string here, compared to just
returning the array slice directly?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


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