Re: [PATCH v2 3/4] iotests: Add more qemu_img helpers

[Nir Soffer]

On Tue, Jul 28, 2020 at 4:50 PM Vladimir Sementsov-Ogievskiy
<vsementsov@virtuozzo.com> wrote:
>
> 28.07.2020 00:58, Nir Soffer wrote:
> > Add 2 helpers for measuring and checking images:
> > - qemu_img_measure()
> > - qemu_img_check()
> >
> > Both use --output-json and parse the returned json to make easy to use
> > in other tests. I'm going to use them in a new test, and I hope they
> > will be useful in may other tests.
> >
> > Signed-off-by: Nir Soffer <nsoffer@redhat.com>
> > ---
> >   tests/qemu-iotests/iotests.py | 6 ++++++
> >   1 file changed, 6 insertions(+)
> >
> > diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> > index 8f79668435..717b5b652c 100644
> > --- a/tests/qemu-iotests/iotests.py
> > +++ b/tests/qemu-iotests/iotests.py
> > @@ -141,6 +141,12 @@ def qemu_img_create(*args):
> >
> >       return qemu_img(*args)
> >
> > +def qemu_img_measure(*args):
> > +    return json.loads(qemu_img_pipe("measure", "--output", "json", *args))
> > +
> > +def qemu_img_check(*args):
> > +    return json.loads(qemu_img_pipe("check", "--output", "json", *args))
> > +
>
> qemu_img_pipe has type hints, so I assume we should add them here too.

True, but type hints are not use consistently in this module (e.g.
qemu_img_verbose).

>
> Also, qemu-img don't report errors in json format, so in case of error, this 
> will raise a problem about something that json can't parse. Probably we need 
> better error handling.

Yes, this fails in an ugly and unhelpful way now.

Ideally failing command will raise a detailed error with the command,
exit code, output,
and error. Code that want to check for specific return code would do:

    try:
        iotests.qemu_img_check(disk)
    except iotest.Error as e:
        if e.rc == 2:
           ...

But most callers do not need this so they will fail loudly with all the details.

What do you think?

> Still, for 5.1 it's OK as is I think, so if we are in a hurry:
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>
> >   def qemu_img_verbose(*args):
> >       '''Run qemu-img without suppressing its output and return the exit 
> > code'''
> >       exitcode = subprocess.call(qemu_img_args + list(args))
> >
>
>
> --
> Best regards,
> Vladimir
>