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

2020-07-28 Thread Nir Soffer
On Tue, Jul 28, 2020 at 4:50 PM Vladimir Sementsov-Ogievskiy
 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 
> > ---
> >   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 
>
> >   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
>




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

2020-07-28 Thread Vladimir Sementsov-Ogievskiy

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 
---
  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.

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.

Still, for 5.1 it's OK as is I think, so if we are in a hurry:
Reviewed-by: Vladimir Sementsov-Ogievskiy 


  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



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

2020-07-27 Thread Nir Soffer
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 
---
 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))
+
 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))
-- 
2.25.4