[Qemu-devel] [PATCH v3 0/8] docker tests fixes

[Qemu-devel] [PATCH v3 0/8] docker tests fixes

[Sascha Silbe]

A couple of fixes for issues encountered while trying out the new
docker test support.

v2→v3:
  - fix non-portable sort -V usage
  - send debootstrap.pre error messages to stderr
  - whitespace changes

v1→v2:
  - found a good place to stick the warning about EXECUTABLE
  - additional fixes for the debian-debootstrap image

Sascha Silbe (8):
  docker.py: don't hang on large docker output
  docker: avoid dependency on 'realpath' package
  docker: debian-bootstrap.pre: print error messages to stderr
  docker: debian-bootstrap.pre: print helpful message if
    DEB_ARCH/DEB_TYPE unset
  docker: print warning if EXECUTABLE is not set when building
    debootstrap image
  docker: make sure debootstrap is at least 1.0.67
  docker: build debootstrap after cloning
  docker: silence debootstrap when --quiet is given

 tests/docker/Makefile.include                 |  5 ++++-
 tests/docker/docker.py                        | 12 ++++++----
 tests/docker/dockerfiles/debian-bootstrap.pre | 32 +++++++++++++++++++++++----
 3 files changed, 40 insertions(+), 9 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH v3 1/8] docker.py: don't hang on large docker output

[Sascha Silbe]

Unlike Popen.communicate(), subprocess.call() doesn't read from the
stdout file descriptor. If the child process produces more output than
fits into the pipe buffer, it will block indefinitely.

If we don't intend to consume the output, just send it straight to
/dev/null to avoid this issue.

Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
---
This fixes a hang for me when building the Ubuntu docker image (empty
docker image cache).

 tests/docker/docker.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index 222a105..efb2bf4 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -25,6 +25,10 @@ from tarfile import TarFile, TarInfo
 from StringIO import StringIO
 from shutil import copy, rmtree
 
+
+DEVNULL = open(os.devnull, 'wb')
+
+
 def _text_checksum(text):
     """Calculate a digest string unique to the text content"""
     return hashlib.sha1(text).hexdigest()
@@ -34,8 +38,7 @@ def _guess_docker_command():
     commands = [["docker"], ["sudo", "-n", "docker"]]
     for cmd in commands:
         if subprocess.call(cmd + ["images"],
-                           stdout=subprocess.PIPE,
-                           stderr=subprocess.PIPE) == 0:
+                           stdout=DEVNULL, stderr=DEVNULL) == 0:
             return cmd
     commands_txt = "\n".join(["  " + " ".join(x) for x in commands])
     raise Exception("Cannot find working docker command. Tried:\n%s" % \
@@ -98,7 +101,7 @@ class Docker(object):
 
     def _do(self, cmd, quiet=True, infile=None, **kwargs):
         if quiet:
-            kwargs["stdout"] = subprocess.PIPE
+            kwargs["stdout"] = DEVNULL
         if infile:
             kwargs["stdin"] = infile
         return subprocess.call(self._command + cmd, **kwargs)
-- 
1.9.1

[Qemu-devel] [PATCH v3 2/8] docker: avoid dependency on 'realpath' package

[Sascha Silbe]

The 'realpath' executable is shipped in a separate package that isn't
installed by default on some distros.

We already use 'readlink -e' (provided by GNU coreutils) in some other
part of the code, so let's settle for that instead.

Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
---
Too bad there isn't a POSIX equivalent of this.

 tests/docker/Makefile.include | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index 4f4707d..1b20db0 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -116,7 +116,7 @@ docker-run-%: docker-qemu-src
 				-e EXTRA_CONFIGURE_OPTS=$(EXTRA_CONFIGURE_OPTS) \
 				-e V=$V -e J=$J -e DEBUG=$(DEBUG)\
 				-e CCACHE_DIR=/var/tmp/ccache \
-				-v $$(realpath $(DOCKER_SRC_COPY)):/var/tmp/qemu:z$(COMMA)ro \
+				-v $$(readlink -e $(DOCKER_SRC_COPY)):/var/tmp/qemu:z$(COMMA)ro \
 				-v $(DOCKER_CCACHE_DIR):/var/tmp/ccache:z \
 				qemu:$(IMAGE) \
 				/var/tmp/qemu/run \
-- 
1.9.1

[Qemu-devel] [PATCH v3 3/8] docker: debian-bootstrap.pre: print error messages to stderr

[Sascha Silbe]

Send error messages where they belong so they're seen even if stdout
is redirected to /dev/null.

Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
---
v2→v3:
  - new patch

 tests/docker/dockerfiles/debian-bootstrap.pre | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/docker/dockerfiles/debian-bootstrap.pre b/tests/docker/dockerfiles/debian-bootstrap.pre
index 5d9c8d5..9b95a6b 100755
--- a/tests/docker/dockerfiles/debian-bootstrap.pre
+++ b/tests/docker/dockerfiles/debian-bootstrap.pre
@@ -13,7 +13,7 @@ exit_and_skip()
 # fakeroot is needed to run the bootstrap stage
 #
 if [ -z $FAKEROOT ]; then
-    echo "Please install fakeroot to enable bootstraping"
+    echo "Please install fakeroot to enable bootstraping" >&2
     exit_and_skip
 fi
 
@@ -38,7 +38,7 @@ if [ -z $DEBOOTSTRAP_DIR ]; then
 else
     DEBOOTSTRAP=${DEBOOTSTRAP_DIR}/debootstrap
     if [ ! -f $DEBOOTSTRAP ]; then
-        echo "Couldn't find script at ${DEBOOTSTRAP}"
+        echo "Couldn't find script at ${DEBOOTSTRAP}" >&2
         exit_and_skip
     fi
 fi
@@ -48,7 +48,7 @@ fi
 #
 BINFMT_DIR=/proc/sys/fs/binfmt_misc
 if [ ! -e $BINFMT_DIR ]; then
-   echo "binfmt_misc needs enabling for a QEMU bootstrap to work"
+   echo "binfmt_misc needs enabling for a QEMU bootstrap to work" >&2
    exit_and_skip
 else
     # DEB_ARCH and QEMU arch names are not totally aligned
@@ -76,7 +76,7 @@ else
         ;;
     esac
     if [ ! -e "${BINFMT_DIR}/$QEMU" ]; then
-        echo "No binfmt_misc rule to run $QEMU, can't bootstrap"
+        echo "No binfmt_misc rule to run $QEMU, can't bootstrap" >&2
         exit_and_skip
     fi
 fi
-- 
1.9.1

[Qemu-devel] [PATCH v3 4/8] docker: debian-bootstrap.pre: print helpful message if DEB_ARCH/DEB_TYPE unset

[Sascha Silbe]

The debian-bootstrap image doesn't choose a default architecture and
distribution version, instead the user has to set both DEB_ARCH and
DEB_TYPE in the environment. Print a reasonably helpful message if
either of them isn't set instead of complaining about "qemu-" being
missing or erroring out because we cannot cd to the mirror URL.

Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
---
v2→v3:
  - send error messages to stderr

 tests/docker/dockerfiles/debian-bootstrap.pre | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tests/docker/dockerfiles/debian-bootstrap.pre b/tests/docker/dockerfiles/debian-bootstrap.pre
index 9b95a6b..3d9f7f4 100755
--- a/tests/docker/dockerfiles/debian-bootstrap.pre
+++ b/tests/docker/dockerfiles/debian-bootstrap.pre
@@ -15,6 +15,19 @@ exit_and_skip()
 if [ -z $FAKEROOT ]; then
     echo "Please install fakeroot to enable bootstraping" >&2
     exit_and_skip
+
+fi
+
+if [ -z "${DEB_ARCH}" ]; then
+    echo "Please set DEB_ARCH to choose an architecture (e.g. armhf)" >&2
+    exit_and_skip
+
+fi
+
+if [ -z "${DEB_TYPE}" ]; then
+    echo "Please set DEB_TYPE to a Debian archive name (e.g. testing)" >&2
+    exit_and_skip
+
 fi
 
 # We check in order for
-- 
1.9.1

[Qemu-devel] [PATCH v3 5/8] docker: print warning if EXECUTABLE is not set when building debootstrap image

[Sascha Silbe]

Building the debian-debootstrap image will usually fail if EXECUTABLE
isn't set (when using the Makefile). Warn the user in this case so
they know why it's failing.

Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
---
 tests/docker/Makefile.include | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index 1b20db0..19d4cc7 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -44,6 +44,9 @@ docker-image: ${DOCKER_TARGETS}
 
 # General rule for building docker images
 docker-image-%: $(DOCKER_FILES_DIR)/%.docker
+	@if test "$@" = docker-image-debian-bootstrap -a -z "$(EXECUTABLE)"; then \
+		echo WARNING: EXECUTABLE is not set, debootstrap may fail. 2>&1 ; \
+	fi
 	$(call quiet-command,\
 		$(SRC_PATH)/tests/docker/docker.py build qemu:$* $< \
 		$(if $V,,--quiet) $(if $(NOCACHE),--no-cache) \
-- 
1.9.1

[Qemu-devel] [PATCH v3 6/8] docker: make sure debootstrap is at least 1.0.67

[Sascha Silbe]

debootstrap prior to 1.0.67 generated an empty sources.list during
foreign bootstraps (Debian#732255 [1]). Fall back to the git checkout
if the installed debootstrap version is too old.

[1] https://bugs.debian.org/732255

Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
---
v2→v3:
  - fix unbalanced white space around pipes
  - replaced GNU-specific version sort option with POSIX compliant set
    of options

 tests/docker/dockerfiles/debian-bootstrap.pre | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tests/docker/dockerfiles/debian-bootstrap.pre b/tests/docker/dockerfiles/debian-bootstrap.pre
index 3d9f7f4..ee69e89 100755
--- a/tests/docker/dockerfiles/debian-bootstrap.pre
+++ b/tests/docker/dockerfiles/debian-bootstrap.pre
@@ -3,6 +3,8 @@
 # Simple wrapper for debootstrap, run in the docker build context
 #
 FAKEROOT=`which fakeroot 2> /dev/null`
+# debootstrap < 1.0.67 generates empty sources.list, see Debian#732255
+MIN_DEBOOTSTRAP_VERSION=1.0.67
 
 exit_and_skip()
 {
@@ -40,9 +42,17 @@ fi
 #
 
 if [ -z $DEBOOTSTRAP_DIR ]; then
+    NEED_DEBOOTSTRAP=false
     DEBOOTSTRAP=`which debootstrap 2> /dev/null`
     if [ -z $DEBOOTSTRAP ]; then
         echo "No debootstrap installed, attempting to install from SCM"
+        NEED_DEBOOTSTRAP=true
+    elif ! (echo "${MIN_DEBOOTSTRAP_VERSION}" ; "${DEBOOTSTRAP}" --version \
+            | cut -d ' ' -f 2) | sort -t . -n -k 1,1 -k 2,2 -k 3,3 -C; then
+        echo "debootstrap too old, attempting to install from SCM"
+        NEED_DEBOOTSTRAP=true
+    fi
+    if $NEED_DEBOOTSTRAP; then
         DEBOOTSTRAP_SOURCE=https://anonscm.debian.org/git/d-i/debootstrap.git
         git clone ${DEBOOTSTRAP_SOURCE} ./debootstrap.git
         export DEBOOTSTRAP_DIR=./debootstrap.git
-- 
1.9.1

[Qemu-devel] [PATCH v3 7/8] docker: build debootstrap after cloning

[Sascha Silbe]

When using the git version of debootstrap (because no usable version
of debootstrap was installed on the host), we need to run 'make' so
that devices.tar.gz gets built. Otherwise the first debootstrap stage
will fail without printing any error message.

Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
---
 tests/docker/dockerfiles/debian-bootstrap.pre | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/docker/dockerfiles/debian-bootstrap.pre b/tests/docker/dockerfiles/debian-bootstrap.pre
index ee69e89..4322fbb 100755
--- a/tests/docker/dockerfiles/debian-bootstrap.pre
+++ b/tests/docker/dockerfiles/debian-bootstrap.pre
@@ -57,6 +57,7 @@ if [ -z $DEBOOTSTRAP_DIR ]; then
         git clone ${DEBOOTSTRAP_SOURCE} ./debootstrap.git
         export DEBOOTSTRAP_DIR=./debootstrap.git
         DEBOOTSTRAP=./debootstrap.git/debootstrap
+        (cd "${DEBOOTSTRAP_DIR}" && "${FAKEROOT}" make )
     fi
 else
     DEBOOTSTRAP=${DEBOOTSTRAP_DIR}/debootstrap
-- 
1.9.1

[Qemu-devel] [PATCH v3 8/8] docker: silence debootstrap when --quiet is given

[Sascha Silbe]

If we silence docker when --quiet is given, we should also silence the
.pre script (i.e. debootstrap).

Only discards stdout, so some diagnostics (e.g. from git clone) are
still printed. Most of the verbose output is gone however and this way
we still have a chance to see error messages.

Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
---
 tests/docker/docker.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index efb2bf4..b85c165 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -239,8 +239,9 @@ class BuildCommand(SubCommand):
             # Is there a .pre file to run in the build context?
             docker_pre = os.path.splitext(args.dockerfile)[0]+".pre"
             if os.path.exists(docker_pre):
+                stdout = DEVNULL if args.quiet else None
                 rc = subprocess.call(os.path.realpath(docker_pre),
-                                     cwd=docker_dir)
+                                     cwd=docker_dir, stdout=stdout)
                 if rc == 3:
                     print "Skip"
                     return 0
-- 
1.9.1

Re: [Qemu-devel] [PATCH v3 6/8] docker: make sure debootstrap is at least 1.0.67

[Fam Zheng]

On Tue, 09/06 22:05, Sascha Silbe wrote:
> debootstrap prior to 1.0.67 generated an empty sources.list during
> foreign bootstraps (Debian#732255 [1]). Fall back to the git checkout
> if the installed debootstrap version is too old.
> 
> [1] https://bugs.debian.org/732255
> 
> Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
> ---
> v2→v3:
>   - fix unbalanced white space around pipes
>   - replaced GNU-specific version sort option with POSIX compliant set
>     of options
> 
>  tests/docker/dockerfiles/debian-bootstrap.pre | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/tests/docker/dockerfiles/debian-bootstrap.pre b/tests/docker/dockerfiles/debian-bootstrap.pre
> index 3d9f7f4..ee69e89 100755
> --- a/tests/docker/dockerfiles/debian-bootstrap.pre
> +++ b/tests/docker/dockerfiles/debian-bootstrap.pre
> @@ -3,6 +3,8 @@
>  # Simple wrapper for debootstrap, run in the docker build context
>  #
>  FAKEROOT=`which fakeroot 2> /dev/null`
> +# debootstrap < 1.0.67 generates empty sources.list, see Debian#732255
> +MIN_DEBOOTSTRAP_VERSION=1.0.67
>  
>  exit_and_skip()
>  {
> @@ -40,9 +42,17 @@ fi
>  #
>  
>  if [ -z $DEBOOTSTRAP_DIR ]; then
> +    NEED_DEBOOTSTRAP=false
>      DEBOOTSTRAP=`which debootstrap 2> /dev/null`
>      if [ -z $DEBOOTSTRAP ]; then
>          echo "No debootstrap installed, attempting to install from SCM"
> +        NEED_DEBOOTSTRAP=true
> +    elif ! (echo "${MIN_DEBOOTSTRAP_VERSION}" ; "${DEBOOTSTRAP}" --version \
> +            | cut -d ' ' -f 2) | sort -t . -n -k 1,1 -k 2,2 -k 3,3 -C; then

Like -V, -C is not available on OSX either. Maybe use "-c"? If so, I can fix it
when applying.

https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man1/sort.1.html

Fam

> +        echo "debootstrap too old, attempting to install from SCM"
> +        NEED_DEBOOTSTRAP=true
> +    fi
> +    if $NEED_DEBOOTSTRAP; then
>          DEBOOTSTRAP_SOURCE=https://anonscm.debian.org/git/d-i/debootstrap.git
>          git clone ${DEBOOTSTRAP_SOURCE} ./debootstrap.git
>          export DEBOOTSTRAP_DIR=./debootstrap.git
> -- 
> 1.9.1
> 
> 

Re: [Qemu-devel] [PATCH v3 6/8] docker: make sure debootstrap is at least 1.0.67

[Sascha Silbe]

Dear Fam,

Fam Zheng <famz@redhat.com> writes:

[...]
>> +        NEED_DEBOOTSTRAP=true
>> +    elif ! (echo "${MIN_DEBOOTSTRAP_VERSION}" ; "${DEBOOTSTRAP}" --version \
>> +            | cut -d ' ' -f 2) | sort -t . -n -k 1,1 -k 2,2 -k 3,3 -C; then
>
> Like -V, -C is not available on OSX either. Maybe use "-c"? If so, I can fix it
> when applying.
>
> https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man1/sort.1.html

Thanks for the pointer. So Apple took a 2005 version of GNU sort,
removed version sort support (introduced in 1997) and neglected to
update it for POSIX.1-2008 compliance [1]?

Guess using "-c" and redirecting stderr to /dev/null is our only option
then. Feel free to fix that up when applying.

Thanks for pointing out these portability issues!

Sascha

[1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/sort.html
-- 
Softwareentwicklung Sascha Silbe, Niederhofenstraße 5/1, 71229 Leonberg
https://se-silbe.de/
USt-IdNr. DE281696641

Re: [Qemu-devel] [PATCH v3 0/8] docker tests fixes

[Fam Zheng]

On Tue, 09/06 22:05, Sascha Silbe wrote:
> A couple of fixes for issues encountered while trying out the new
> docker test support.
> 
> v2→v3:
>   - fix non-portable sort -V usage
>   - send debootstrap.pre error messages to stderr
>   - whitespace changes
> 
> v1→v2:
>   - found a good place to stick the warning about EXECUTABLE
>   - additional fixes for the debian-debootstrap image
> 
> Sascha Silbe (8):
>   docker.py: don't hang on large docker output
>   docker: avoid dependency on 'realpath' package
>   docker: debian-bootstrap.pre: print error messages to stderr
>   docker: debian-bootstrap.pre: print helpful message if
>     DEB_ARCH/DEB_TYPE unset
>   docker: print warning if EXECUTABLE is not set when building
>     debootstrap image
>   docker: make sure debootstrap is at least 1.0.67
>   docker: build debootstrap after cloning
>   docker: silence debootstrap when --quiet is given
> 
>  tests/docker/Makefile.include                 |  5 ++++-
>  tests/docker/docker.py                        | 12 ++++++----
>  tests/docker/dockerfiles/debian-bootstrap.pre | 32 +++++++++++++++++++++++----
>  3 files changed, 40 insertions(+), 9 deletions(-)
> 
> -- 
> 1.9.1
> 

Thanks. Fixed the 'sort -C' portability issue and applied to my docker branch:

http://github.com/famz/qemu/tree/docker

Fam