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.
:param check: Enforce a return code of zero.
:param combine_stdio: set to False to keep stdout/stderr separated.
@@ -227,14 +225,13 @@ def qemu_img(*args: str, check: bool = True, combine_stdio: bool = True
handled, the command-line, return code, and all console output
will be included at the bottom of the stack trace.
- :return: a CompletedProcess. This object has args, returncode, and
- stdout properties. If streams are not combined, it will also
- have a stderr property.
+ :return:
+ A CompletedProcess. This object has args, returncode, and stdout
+ properties. If streams are not combined, it will also have a
+ stderr property.
"""
- full_args = qemu_img_args + qemu_img_create_prepare_args(list(args))
-
subp = subprocess.run(
- full_args,
+ args,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT if combine_stdio else subprocess.PIPE,
universal_newlines=True,
@@ -243,7 +240,7 @@ def qemu_img(*args: str, check: bool = True, combine_stdio: bool = True
if check and subp.returncode or (subp.returncode < 0):
raise VerboseProcessError(
- subp.returncode, full_args,
+ subp.returncode, args,
output=subp.stdout,
stderr=subp.stderr,
)
@@ -251,6 +248,20 @@ def qemu_img(*args: str, check: bool = True, combine_stdio: bool = True
return subp
+def qemu_img(*args: str, check: bool = True, combine_stdio: bool = True
+ ) -> subprocess.CompletedProcess[str]:
+ """
+ Run QEMU_IMG_PROG and return its status code and console output.
+
+ This function always prepends QEMU_IMG_OPTIONS and may further alter
+ the args for 'create' commands.
+
+ See `qemu_tool()` for greater detail.
+ """
+ full_args = qemu_img_args + qemu_img_create_prepare_args(list(args))
+ return qemu_tool(*full_args, check=check, combine_stdio=combine_stdio)
+
+
def ordered_qmp(qmsg, conv_keys=True):
# Dictionaries are not ordered prior to 3.6, therefore:
if isinstance(qmsg, list):
--
2.34.1On 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: Reviewed-by: Hanna Reitz <hreitz@redhat.com>
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> > >
On Fri, Mar 18, 2022 at 04:36:45PM -0400, 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]: Does this line need reindentation? > @@ -227,14 +225,13 @@ def qemu_img(*args: str, check: bool = True, combine_stdio: bool = True > handled, the command-line, return code, and all console output > will be included at the bottom of the stack trace. > > - :return: a CompletedProcess. This object has args, returncode, and > - stdout properties. If streams are not combined, it will also > - have a stderr property. > + :return: > + A CompletedProcess. This object has args, returncode, and stdout > + properties. If streams are not combined, it will also have a > + stderr property. Should this reflow be squashed in some earlier patch? As those are both cosemetic only, Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On Mon, Mar 21, 2022, 11:13 AM Eric Blake <eblake@redhat.com> wrote: > On Fri, Mar 18, 2022 at 04:36:45PM -0400, 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]: > > Does this line need reindentation? > Huh, I'll check. Maybe I fixed this by accident in a later patch and didn't notice. Or maybe git diff is playing tricks on me. > > @@ -227,14 +225,13 @@ def qemu_img(*args: str, check: bool = True, > combine_stdio: bool = True > > handled, the command-line, return code, and all console output > > will be included at the bottom of the stack trace. > > > > - :return: a CompletedProcess. This object has args, returncode, and > > - stdout properties. If streams are not combined, it will also > > - have a stderr property. > > + :return: > > + A CompletedProcess. This object has args, returncode, and stdout > > + properties. If streams are not combined, it will also have a > > + stderr property. > > Should this reflow be squashed in some earlier patch? > Aw, you caught me. 😅 I need to respin the qemu-img stuff anyway due to CI failures, so I can fix it where it appears first. (When I wrote this, I didn't realize that the qemu-img series was failing CI yet.) > As those are both cosemetic only, > > Reviewed-by: Eric Blake <eblake@redhat.com> > > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3266 > Virtualization: qemu.org | libvirt.org > >
© 2016 - 2026 Red Hat, Inc.