[Qemu-devel] [PATCH 0/2] Two small improvements for docker.py

[Qemu-devel] [PATCH 0/2] Two small improvements for docker.py

[Fam Zheng]

The first one is a small simplification, the second one is an error handling
improvement.

Fam Zheng (2):
  docker.py: Drop infile parameter
  docker.py: Improve subprocess exit code handling

 tests/docker/docker.py | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

-- 
2.9.4

[Qemu-devel] [PATCH 1/2] docker.py: Drop infile parameter

[Fam Zheng]

The **kwargs can do this just well.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/docker/docker.py | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index e707e5b..f5ac86b 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -112,11 +112,9 @@ class Docker(object):
         signal.signal(signal.SIGTERM, self._kill_instances)
         signal.signal(signal.SIGHUP, self._kill_instances)
 
-    def _do(self, cmd, quiet=True, infile=None, **kwargs):
+    def _do(self, cmd, quiet=True, **kwargs):
         if quiet:
             kwargs["stdout"] = DEVNULL
-        if infile:
-            kwargs["stdin"] = infile
         return subprocess.call(self._command + cmd, **kwargs)
 
     def _do_kill_instances(self, only_known, only_active=True):
@@ -184,7 +182,7 @@ class Docker(object):
     def update_image(self, tag, tarball, quiet=True):
         "Update a tagged image using "
 
-        self._do(["build", "-t", tag, "-"], quiet=quiet, infile=tarball)
+        self._do(["build", "-t", tag, "-"], quiet=quiet, stdin=tarball)
 
     def image_matches_dockerfile(self, tag, dockerfile):
         try:
-- 
2.9.4

[Qemu-devel] [PATCH 2/2] docker.py: Improve subprocess exit code handling

[Fam Zheng]

A few error handlings are missing because we ignore the subprocess exit
code, for example "docker build" errors are currently ignored.

Introduce _do_check() aside the existing _do() method and use it in a
few places.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/docker/docker.py | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index f5ac86b..ee40ca0 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -117,6 +117,11 @@ class Docker(object):
             kwargs["stdout"] = DEVNULL
         return subprocess.call(self._command + cmd, **kwargs)
 
+    def _do_check(self, cmd, quiet=True, **kwargs):
+        if quiet:
+            kwargs["stdout"] = DEVNULL
+        return subprocess.check_call(self._command + cmd, **kwargs)
+
     def _do_kill_instances(self, only_known, only_active=True):
         cmd = ["ps", "-q"]
         if not only_active:
@@ -175,14 +180,14 @@ class Docker(object):
                                     extra_files_cksum)))
         tmp_df.flush()
 
-        self._do(["build", "-t", tag, "-f", tmp_df.name] + argv + \
-                 [docker_dir],
-                 quiet=quiet)
+        self._do_check(["build", "-t", tag, "-f", tmp_df.name] + argv + \
+                       [docker_dir],
+                       quiet=quiet)
 
     def update_image(self, tag, tarball, quiet=True):
         "Update a tagged image using "
 
-        self._do(["build", "-t", tag, "-"], quiet=quiet, stdin=tarball)
+        self._do_check(["build", "-t", tag, "-"], quiet=quiet, stdin=tarball)
 
     def image_matches_dockerfile(self, tag, dockerfile):
         try:
@@ -195,9 +200,9 @@ class Docker(object):
         label = uuid.uuid1().hex
         if not keep:
             self._instances.append(label)
-        ret = self._do(["run", "--label",
-                        "com.qemu.instance.uuid=" + label] + cmd,
-                       quiet=quiet)
+        ret = self._do_check(["run", "--label",
+                             "com.qemu.instance.uuid=" + label] + cmd,
+                             quiet=quiet)
         if not keep:
             self._instances.remove(label)
         return ret
-- 
2.9.4

Re: [Qemu-devel] [PATCH 1/2] docker.py: Drop infile parameter

[Philippe Mathieu-Daudé]

On 07/12/2017 04:55 AM, Fam Zheng wrote:
> The **kwargs can do this just well.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>   tests/docker/docker.py | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
> index e707e5b..f5ac86b 100755
> --- a/tests/docker/docker.py
> +++ b/tests/docker/docker.py
> @@ -112,11 +112,9 @@ class Docker(object):
>           signal.signal(signal.SIGTERM, self._kill_instances)
>           signal.signal(signal.SIGHUP, self._kill_instances)
>   
> -    def _do(self, cmd, quiet=True, infile=None, **kwargs):
> +    def _do(self, cmd, quiet=True, **kwargs):
>           if quiet:
>               kwargs["stdout"] = DEVNULL
> -        if infile:
> -            kwargs["stdin"] = infile
>           return subprocess.call(self._command + cmd, **kwargs)
>   
>       def _do_kill_instances(self, only_known, only_active=True):
> @@ -184,7 +182,7 @@ class Docker(object):
>       def update_image(self, tag, tarball, quiet=True):
>           "Update a tagged image using "
>   
> -        self._do(["build", "-t", tag, "-"], quiet=quiet, infile=tarball)
> +        self._do(["build", "-t", tag, "-"], quiet=quiet, stdin=tarball)
>   
>       def image_matches_dockerfile(self, tag, dockerfile):
>           try:
> 

Re: [Qemu-devel] [PATCH 2/2] docker.py: Improve subprocess exit code handling

[Philippe Mathieu-Daudé]

On 07/12/2017 04:55 AM, Fam Zheng wrote:
> A few error handlings are missing because we ignore the subprocess exit
> code, for example "docker build" errors are currently ignored.
> 
> Introduce _do_check() aside the existing _do() method and use it in a
> few places.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>

Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>   tests/docker/docker.py | 19 ++++++++++++-------
>   1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
> index f5ac86b..ee40ca0 100755
> --- a/tests/docker/docker.py
> +++ b/tests/docker/docker.py
> @@ -117,6 +117,11 @@ class Docker(object):
>               kwargs["stdout"] = DEVNULL
>           return subprocess.call(self._command + cmd, **kwargs)
>   
> +    def _do_check(self, cmd, quiet=True, **kwargs):
> +        if quiet:
> +            kwargs["stdout"] = DEVNULL
> +        return subprocess.check_call(self._command + cmd, **kwargs)
> +
>       def _do_kill_instances(self, only_known, only_active=True):
>           cmd = ["ps", "-q"]
>           if not only_active:
> @@ -175,14 +180,14 @@ class Docker(object):
>                                       extra_files_cksum)))
>           tmp_df.flush()
>   
> -        self._do(["build", "-t", tag, "-f", tmp_df.name] + argv + \
> -                 [docker_dir],
> -                 quiet=quiet)
> +        self._do_check(["build", "-t", tag, "-f", tmp_df.name] + argv + \
> +                       [docker_dir],
> +                       quiet=quiet)
>   
>       def update_image(self, tag, tarball, quiet=True):
>           "Update a tagged image using "
>   
> -        self._do(["build", "-t", tag, "-"], quiet=quiet, stdin=tarball)
> +        self._do_check(["build", "-t", tag, "-"], quiet=quiet, stdin=tarball)
>   
>       def image_matches_dockerfile(self, tag, dockerfile):
>           try:
> @@ -195,9 +200,9 @@ class Docker(object):
>           label = uuid.uuid1().hex
>           if not keep:
>               self._instances.append(label)
> -        ret = self._do(["run", "--label",
> -                        "com.qemu.instance.uuid=" + label] + cmd,
> -                       quiet=quiet)
> +        ret = self._do_check(["run", "--label",
> +                             "com.qemu.instance.uuid=" + label] + cmd,
> +                             quiet=quiet)
>           if not keep:
>               self._instances.remove(label)
>           return ret
> 

Re: [Qemu-devel] [PATCH 0/2] Two small improvements for docker.py

[Fam Zheng]

On Wed, 07/12 15:55, Fam Zheng wrote:
> The first one is a small simplification, the second one is an error handling
> improvement.
> 
> Fam Zheng (2):
>   docker.py: Drop infile parameter
>   docker.py: Improve subprocess exit code handling

Thanks, queued:

    https://github.com/famz/qemu/tree/staging