Re: [PATCH 05/15] iotests: create generic qemu_tool() function

[John Snow <jsnow@redhat.com>]

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

On Tue, Mar 22, 2022, 10:49 AM Hanna Reitz <hreitz@redhat.com> wrote:

> On 18.03.22 21:36, John Snow wrote:
> > reimplement qemu_img() in terms of qemu_tool() in preparation for doing
> > the same with qemu_io().
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >   tests/qemu-iotests/iotests.py | 37 +++++++++++++++++++++++------------
> >   1 file changed, 24 insertions(+), 13 deletions(-)
> >
> > diff --git a/tests/qemu-iotests/iotests.py
> b/tests/qemu-iotests/iotests.py
> > index 6cd8374c81..974a2b0c8d 100644
> > --- a/tests/qemu-iotests/iotests.py
> > +++ b/tests/qemu-iotests/iotests.py
> > @@ -207,15 +207,13 @@ def qemu_img_create_prepare_args(args: List[str])
> -> List[str]:
> >
> >       return result
> >
> > -def qemu_img(*args: str, check: bool = True, combine_stdio: bool = True
> > +
> > +def qemu_tool(*args: str, check: bool = True, combine_stdio: bool = True
> >                ) -> subprocess.CompletedProcess[str]:
> >       """
> > -    Run qemu_img and return the status code and console output.
> > +    Run a qemu tool and return its status code and console output.
> >
> > -    This function always prepends QEMU_IMG_OPTIONS and may further alter
> > -    the args for 'create' commands.
> > -
> > -    :param args: command-line arguments to qemu-img.
> > +    :param args: command-line arguments to a QEMU cli tool.
>
> This makes me ask how I am to specify which tool to use.  Perhaps it
> should just be “full command line to run” or something.
>
> Might be nice™, but:
>

I see what you mean. I did away with the "tool name" parameter because we
were only using it for an error message print I also removed.

I'll update the docstring a little to make what's going on more clear.
Maybe someday (tm) I could split args into (tool, args) parameters to make
it more explicit, and change the way the environment variables are parsed
to keep the tool/args separate.

Pretty minor kind of thing, though, so I'm not in a hurry.


> Reviewed-by: Hanna Reitz <hreitz@redhat.com>
>
>

[-- Attachment #2: Type: text/html, Size: 3089 bytes --]