[Qemu-devel] [RFC PATCH 0/2] Support building qemu-user powered docker test images

[Qemu-devel] [RFC PATCH 0/2] Support building qemu-user powered docker test images

[Alex Bennée]

Hi,

I had a an set of scripts for setting up and running executable
binaries in a $arch-linux-user powered rootfs. However it has been a
while since I last used them and there was various breakage which
spurred me on to a better way. As Fam has been working with Docker
containers for building I investigated if we could use the docker
framework to make all the chroot/bind mount stuff go away.

A couple of caveats as befits an RFC:

  - built on top of Fam's docker series [PATCH v5 00/14] tests: Introducing docker tests
  - assumes binfmt_misc setup an set to /usr/bin/qemu-FOO
  - uses ldd to copy dynamically shared libs across
    - this should be OK for distro's that are multiarch safe

For example to build a Debian Testing armhf distro I run:

  DEB_ARCH=armhf DEB_TYPE=testing \
    ./tests/docker/docker.py build --qemu=qemu-arm debian:armhf \
    ./tests/docker/dockerfiles/debian-bootstrap.docker

This uses Fam's docker helper which now copies qemu-arm into the
docker build context, loads the debian-bootstrap.docker evaluating
HOST_CMD (to do the debootstrap) and then runs the build steps to make
the image. I can then run the final image with something like:

$ docker run -t -i --rm debian:armhf /bin/bash
root@e659ddf8232c:/# uname -a
Linux e659ddf8232c 4.4.0-21-generic #37-Ubuntu SMP Mon Apr 18 18:33:37
UTC 2016 armv7l GNU/Linux

Obviously this requires Fam's stuff to go in and needs some clean-up
but is this worth pursuing?

Cheers,


Alex Bennée (2):
  tests/docker/docker.py: support --qemu option
  add debian-bootstrap.docker target

 tests/docker/docker.py                           | 77 +++++++++++++++++++++---
 tests/docker/dockerfiles/debian-bootstrap.docker | 22 +++++++
 2 files changed, 89 insertions(+), 10 deletions(-)
 create mode 100644 tests/docker/dockerfiles/debian-bootstrap.docker

-- 
2.7.4

[Qemu-devel] [RFC PATCH 1/2] tests/docker/docker.py: support --qemu option

[Alex Bennée]

When passed the name of a qemu-$arch binary we copy it and any linked
libraries into the docker build context. These can then be included by a
dockerfile with the line:

  # Copy all of context into container
  ADD . /

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/docker/docker.py | 58 ++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 52 insertions(+), 6 deletions(-)

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index fe73de7..e9242f3 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -20,6 +20,8 @@ import atexit
 import uuid
 import argparse
 import tempfile
+import re
+from shutil import copyfile
 
 def _text_checksum(text):
     """Calculate a digest string unique to the text content"""
@@ -37,6 +39,27 @@ def _guess_docker_command():
     raise Exception("Cannot find working docker command. Tried:\n%s" % \
                     commands_txt)
 
+def _find_user_binary(binary_name):
+    """ Find a binary in the QEMU source tree. Used for finding qemu-$arch."""
+    top = os.path.abspath("%s/../../.." % sys.argv[0])
+    linux_user = [ x for x in os.listdir(top) if x.endswith("-linux-user") ]
+    for x in linux_user:
+        check_path = "%s/%s/%s" % (top, x, binary_name)
+        if os.path.isfile(check_path):
+            print ("found %s" % check_path)
+            return check_path
+    return None
+
+def _copy_with_mkdir(src, root_dir, sub_path):
+    """Copy src into root_dir, creating sub_path as needed."""
+    full_path = "%s/%s" % (root_dir, sub_path)
+    try:
+        os.makedirs(full_path)
+    except OSError:
+        print "skipping %s" % (full_path)
+
+    copyfile(src, "%s/%s" % (full_path, os.path.basename(src)))
+
 class Docker(object):
     """ Running Docker commands """
     def __init__(self):
@@ -86,18 +109,36 @@ class Docker(object):
         labels = json.loads(resp)[0]["Config"].get("Labels", {})
         return labels.get("com.qemu.dockerfile-checksum", "")
 
-    def build_image(self, tag, dockerfile, df_path, quiet=True, argv=None):
+    def build_image(self, tag, dockerfile, quiet=True, qemu=None, argv=None):
         if argv == None:
             argv = []
+
+        # Create a temporary docker context to build in
+        tmp_dir = tempfile.mkdtemp(prefix="docker_build")
+
+        # Copy the dockerfile into our work space
         tmp = dockerfile + "\n" + \
               "LABEL com.qemu.dockerfile-checksum=%s" % \
               _text_checksum(dockerfile)
-        dirname = os.path.dirname(df_path)
-        tmp_df = tempfile.NamedTemporaryFile(dir=dirname)
+        tmp_df = tempfile.NamedTemporaryFile(dir=tmp_dir, suffix=".docker")
         tmp_df.write(tmp)
         tmp_df.flush()
+
+        # Do we want to copy QEMU into here?
+        if qemu:
+            _copy_with_mkdir(qemu, tmp_dir, "/usr/bin")
+            # do ldd bit here
+            ldd_output = subprocess.check_output(["ldd", qemu])
+            for l in ldd_output.split("\n"):
+                s = re.search("(/.*/)(\S*)", l)
+                if s and len(s.groups())==2:
+                    so_path=s.groups()[0]
+                    so_lib=s.groups()[1]
+                    _copy_with_mkdir("%s/%s" % (so_path, so_lib),
+                                     tmp_dir, so_path)
+
         self._do(["build", "-t", tag, "-f", tmp_df.name] + argv + \
-                 [dirname],
+                 [tmp_dir],
                  quiet=quiet)
 
     def image_matches_dockerfile(self, tag, dockerfile):
@@ -148,6 +189,7 @@ class BuildCommand(SubCommand):
     """ Build docker image out of a dockerfile. Arguments: <tag> <dockerfile>"""
     name = "build"
     def args(self, parser):
+        parser.add_argument("--qemu", help="Include qemu-user binaries")
         parser.add_argument("tag",
                             help="Image Tag")
         parser.add_argument("dockerfile",
@@ -157,14 +199,18 @@ class BuildCommand(SubCommand):
         dockerfile = open(args.dockerfile, "rb").read()
         tag = args.tag
 
+        # find qemu binary
+        qbin=None
+        if args.qemu:
+            qbin=_find_user_binary(args.qemu)
+
         dkr = Docker()
         if dkr.image_matches_dockerfile(tag, dockerfile):
             if not args.quiet:
                 print "Image is up to date."
             return 0
 
-        dkr.build_image(tag, dockerfile, args.dockerfile,
-                        quiet=args.quiet, argv=argv)
+        dkr.build_image(tag, dockerfile, quiet=args.quiet, qemu=qbin, argv=argv)
         return 0
 
 class CleanCommand(SubCommand):
-- 
2.7.4

[Qemu-devel] [RFC PATCH 2/2] add debian-bootstrap.docker target

[Alex Bennée]

Together with some changes to the docker script you can now build an
arbitrary architecture of Debian using debootstrap. To achieve this I
introduce the concept of a HOST_CMD in the docker config file. While
copying the file into workspace the HOST_CMD is run in the docker build
context. This allows debootstrap to set up its first stage before the
container is built.

To build a container you need a command line like:

  DEB_ARCH=armhf DEB_TYPE=testing \
    ./tests/docker/docker.py build --qemu=qemu-arm debian:armhf \
    ./tests/docker/dockerfiles/debian-bootstrap.docker

The DEB_ARCH/DEB_TYPE are expanded in the docker file by the HOST_CMD:

  HOST_CMD fakeroot debootstrap --variant=minbase --foreign \
    --arch=$DEB_ARCH $DEB_TYPE . \
    http://httpredir.debian.org/debian

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/docker/docker.py                           | 27 +++++++++++++++++-------
 tests/docker/dockerfiles/debian-bootstrap.docker | 22 +++++++++++++++++++
 2 files changed, 41 insertions(+), 8 deletions(-)
 create mode 100644 tests/docker/dockerfiles/debian-bootstrap.docker

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index e9242f3..3ba3d4e 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -21,7 +21,7 @@ import uuid
 import argparse
 import tempfile
 import re
-from shutil import copyfile
+from shutil import copy
 
 def _text_checksum(text):
     """Calculate a digest string unique to the text content"""
@@ -46,7 +46,6 @@ def _find_user_binary(binary_name):
     for x in linux_user:
         check_path = "%s/%s/%s" % (top, x, binary_name)
         if os.path.isfile(check_path):
-            print ("found %s" % check_path)
             return check_path
     return None
 
@@ -58,7 +57,7 @@ def _copy_with_mkdir(src, root_dir, sub_path):
     except OSError:
         print "skipping %s" % (full_path)
 
-    copyfile(src, "%s/%s" % (full_path, os.path.basename(src)))
+    copy(src, "%s/%s" % (full_path, os.path.basename(src)))
 
 class Docker(object):
     """ Running Docker commands """
@@ -117,11 +116,23 @@ class Docker(object):
         tmp_dir = tempfile.mkdtemp(prefix="docker_build")
 
         # Copy the dockerfile into our work space
-        tmp = dockerfile + "\n" + \
-              "LABEL com.qemu.dockerfile-checksum=%s" % \
-              _text_checksum(dockerfile)
+        # line by line, stripping and executing HOST_CMDs
+        #
         tmp_df = tempfile.NamedTemporaryFile(dir=tmp_dir, suffix=".docker")
-        tmp_df.write(tmp)
+
+        for l in open(dockerfile).readlines():
+            m = re.match("HOST_CMD ", l)
+            if m:
+                print "l=%s" % (l)
+                cmd = l[m.end():]
+                r = subprocess.check_call(cmd, cwd=tmp_dir, shell=True)
+                tmp_df.write("# HOST_CMD %s# HOST_RES = %d\n" % (cmd, r))
+            else:
+                tmp_df.write(l)
+
+        tmp_df.write("\n")
+        tmp_df.write("LABEL com.qemu.dockerfile-checksum=%s" %
+                     _text_checksum(dockerfile))
         tmp_df.flush()
 
         # Do we want to copy QEMU into here?
@@ -210,7 +221,7 @@ class BuildCommand(SubCommand):
                 print "Image is up to date."
             return 0
 
-        dkr.build_image(tag, dockerfile, quiet=args.quiet, qemu=qbin, argv=argv)
+        dkr.build_image(tag, args.dockerfile, quiet=args.quiet, qemu=qbin, argv=argv)
         return 0
 
 class CleanCommand(SubCommand):
diff --git a/tests/docker/dockerfiles/debian-bootstrap.docker b/tests/docker/dockerfiles/debian-bootstrap.docker
new file mode 100644
index 0000000..44d107d
--- /dev/null
+++ b/tests/docker/dockerfiles/debian-bootstrap.docker
@@ -0,0 +1,22 @@
+# Create Debian Bootstrap Image
+#
+# This is intended to be pre-poluated by:
+#  - a first stage debootstrap
+#  - a native qemu-$arch that binfmt_misc will run
+FROM scratch
+
+# HOST_CMD is executed by docker.py while building the context
+HOST_CMD fakeroot debootstrap --variant=minbase --foreign --arch=$DEB_ARCH $DEB_TYPE . http://httpredir.debian.org/debian
+
+# Add everything from the context into the container
+ADD . /
+
+# Patch all mounts as docker already has stuff set up
+RUN sed -i 's/in_target mount/echo not for docker in_target mount/g' /debootstrap/functions
+
+# Run stage 2
+RUN /debootstrap/debootstrap --second-stage
+
+# At this point we can install additional packages if we want
+#RUN apt-get update
+#RUN apt-get dist-upgrade
-- 
2.7.4

Re: [Qemu-devel] [RFC PATCH 1/2] tests/docker/docker.py: support --qemu option

[Fam Zheng]

On Thu, 05/26 15:27, Alex Bennée wrote:
> When passed the name of a qemu-$arch binary we copy it and any linked
> libraries into the docker build context. These can then be included by a
> dockerfile with the line:
> 
>   # Copy all of context into container
>   ADD . /
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Looks good in general except a few nitpicks below, most important one being the
binary path lookup.

> ---
>  tests/docker/docker.py | 58 ++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 52 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
> index fe73de7..e9242f3 100755
> --- a/tests/docker/docker.py
> +++ b/tests/docker/docker.py
> @@ -20,6 +20,8 @@ import atexit
>  import uuid
>  import argparse
>  import tempfile
> +import re
> +from shutil import copyfile
>  
>  def _text_checksum(text):
>      """Calculate a digest string unique to the text content"""
> @@ -37,6 +39,27 @@ def _guess_docker_command():
>      raise Exception("Cannot find working docker command. Tried:\n%s" % \
>                      commands_txt)
>  
> +def _find_user_binary(binary_name):
> +    """ Find a binary in the QEMU source tree. Used for finding qemu-$arch."""
> +    top = os.path.abspath("%s/../../.." % sys.argv[0])

What if this is an out of tree build?

> +    linux_user = [ x for x in os.listdir(top) if x.endswith("-linux-user") ]
> +    for x in linux_user:
> +        check_path = "%s/%s/%s" % (top, x, binary_name)

os.path.join()?

> +        if os.path.isfile(check_path):
> +            print ("found %s" % check_path)
> +            return check_path
> +    return None
> +
> +def _copy_with_mkdir(src, root_dir, sub_path):
> +    """Copy src into root_dir, creating sub_path as needed."""
> +    full_path = "%s/%s" % (root_dir, sub_path)
> +    try:
> +        os.makedirs(full_path)
> +    except OSError:
> +        print "skipping %s" % (full_path)

Print the error message too? Also do you want to "return"?

> +
> +    copyfile(src, "%s/%s" % (full_path, os.path.basename(src)))
> +
>  class Docker(object):
>      """ Running Docker commands """
>      def __init__(self):
> @@ -86,18 +109,36 @@ class Docker(object):
>          labels = json.loads(resp)[0]["Config"].get("Labels", {})
>          return labels.get("com.qemu.dockerfile-checksum", "")
>  
> -    def build_image(self, tag, dockerfile, df_path, quiet=True, argv=None):
> +    def build_image(self, tag, dockerfile, quiet=True, qemu=None, argv=None):
>          if argv == None:
>              argv = []
> +
> +        # Create a temporary docker context to build in
> +        tmp_dir = tempfile.mkdtemp(prefix="docker_build")
> +
> +        # Copy the dockerfile into our work space
>          tmp = dockerfile + "\n" + \
>                "LABEL com.qemu.dockerfile-checksum=%s" % \
>                _text_checksum(dockerfile)
> -        dirname = os.path.dirname(df_path)
> -        tmp_df = tempfile.NamedTemporaryFile(dir=dirname)
> +        tmp_df = tempfile.NamedTemporaryFile(dir=tmp_dir, suffix=".docker")
>          tmp_df.write(tmp)
>          tmp_df.flush()
> +
> +        # Do we want to copy QEMU into here?
> +        if qemu:
> +            _copy_with_mkdir(qemu, tmp_dir, "/usr/bin")

Looks like "/usr/bin" is combined with tmp_dir as "FOO//usr/bin", superfluous?

> +            # do ldd bit here
> +            ldd_output = subprocess.check_output(["ldd", qemu])
> +            for l in ldd_output.split("\n"):
> +                s = re.search("(/.*/)(\S*)", l)
> +                if s and len(s.groups())==2:
> +                    so_path=s.groups()[0]
> +                    so_lib=s.groups()[1]
> +                    _copy_with_mkdir("%s/%s" % (so_path, so_lib),
> +                                     tmp_dir, so_path)
> +
>          self._do(["build", "-t", tag, "-f", tmp_df.name] + argv + \
> -                 [dirname],
> +                 [tmp_dir],
>                   quiet=quiet)
>  
>      def image_matches_dockerfile(self, tag, dockerfile):
> @@ -148,6 +189,7 @@ class BuildCommand(SubCommand):
>      """ Build docker image out of a dockerfile. Arguments: <tag> <dockerfile>"""
>      name = "build"
>      def args(self, parser):
> +        parser.add_argument("--qemu", help="Include qemu-user binaries")

Can I ask for a rename of this and the variable names in this patch, to a more
generic name (to reflect that it is inherently orthorgonal to the type of the
binary we are copying)? How about:

           parser.add_argument("--executable-inject", "-e",
                               help="""Specify a binary that will be copied to the
                               container together with all its dependent
                               libraries""")

And I think it is reasonable to expect the user (or the calling Makefile) to
designate a working absolute or relative path, instead of looking up it
ourselves.

>          parser.add_argument("tag",
>                              help="Image Tag")
>          parser.add_argument("dockerfile",
> @@ -157,14 +199,18 @@ class BuildCommand(SubCommand):
>          dockerfile = open(args.dockerfile, "rb").read()
>          tag = args.tag
>  
> +        # find qemu binary
> +        qbin=None

Add whitespaces around "="?

> +        if args.qemu:
> +            qbin=_find_user_binary(args.qemu)

Ditto, and some more occasions above.

> +
>          dkr = Docker()
>          if dkr.image_matches_dockerfile(tag, dockerfile):
>              if not args.quiet:
>                  print "Image is up to date."
>              return 0
>  
> -        dkr.build_image(tag, dockerfile, args.dockerfile,
> -                        quiet=args.quiet, argv=argv)
> +        dkr.build_image(tag, dockerfile, quiet=args.quiet, qemu=qbin, argv=argv)
>          return 0
>  
>  class CleanCommand(SubCommand):
> -- 
> 2.7.4
> 

Thanks,

Fam

Re: [Qemu-devel] [RFC PATCH 2/2] add debian-bootstrap.docker target

[Fam Zheng]

On Thu, 05/26 15:27, Alex Bennée wrote:
> Together with some changes to the docker script you can now build an
> arbitrary architecture of Debian using debootstrap. To achieve this I
> introduce the concept of a HOST_CMD in the docker config file. While
> copying the file into workspace the HOST_CMD is run in the docker build
> context. This allows debootstrap to set up its first stage before the
> container is built.

Could you instead introduce the concept of $IMAGE.pre file (in this case
debian-bootstrap.pre, aside debian-bootstrap.docker), and exec it in
docker.py?  It would be much more flexible, and we we wouldn't need to inject a
custom directive to filter it out.

Fam

Re: [Qemu-devel] [RFC PATCH 0/2] Support building qemu-user powered docker test images

[Fam Zheng]

On Thu, 05/26 15:27, Alex Bennée wrote:
> $ docker run -t -i --rm debian:armhf /bin/bash
> root@e659ddf8232c:/# uname -a
> Linux e659ddf8232c 4.4.0-21-generic #37-Ubuntu SMP Mon Apr 18 18:33:37
> UTC 2016 armv7l GNU/Linux
> 
> Obviously this requires Fam's stuff to go in and needs some clean-up
> but is this worth pursuing?

I'm very interested in the feature this brings. Please see my replies to
individual patches regarding some implementation details.

Fam

Re: [Qemu-devel] [RFC PATCH 1/2] tests/docker/docker.py: support --qemu option

[Alex Bennée]

Fam Zheng <famz@redhat.com> writes:

> On Thu, 05/26 15:27, Alex Bennée wrote:
>> When passed the name of a qemu-$arch binary we copy it and any linked
>> libraries into the docker build context. These can then be included by a
>> dockerfile with the line:
>>
>>   # Copy all of context into container
>>   ADD . /
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> Looks good in general except a few nitpicks below, most important one being the
> binary path lookup.
>
>> ---
>>  tests/docker/docker.py | 58 ++++++++++++++++++++++++++++++++++++++++++++------
>>  1 file changed, 52 insertions(+), 6 deletions(-)
>>
>> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
>> index fe73de7..e9242f3 100755
>> --- a/tests/docker/docker.py
>> +++ b/tests/docker/docker.py
>> @@ -20,6 +20,8 @@ import atexit
>>  import uuid
>>  import argparse
>>  import tempfile
>> +import re
>> +from shutil import copyfile
>>
>>  def _text_checksum(text):
>>      """Calculate a digest string unique to the text content"""
>> @@ -37,6 +39,27 @@ def _guess_docker_command():
>>      raise Exception("Cannot find working docker command. Tried:\n%s" % \
>>                      commands_txt)
>>
>> +def _find_user_binary(binary_name):
>> +    """ Find a binary in the QEMU source tree. Used for finding qemu-$arch."""
>> +    top = os.path.abspath("%s/../../.." % sys.argv[0])
>
> What if this is an out of tree build?

Yes I kinda avoided the complexity here. Do we have a programatic way of
finding this out or should we just assume we get based a resolvable path?

>
>> +    linux_user = [ x for x in os.listdir(top) if x.endswith("-linux-user") ]
>> +    for x in linux_user:
>> +        check_path = "%s/%s/%s" % (top, x, binary_name)
>
> os.path.join()?

Ack.

>
>> +        if os.path.isfile(check_path):
>> +            print ("found %s" % check_path)
>> +            return check_path
>> +    return None
>> +
>> +def _copy_with_mkdir(src, root_dir, sub_path):
>> +    """Copy src into root_dir, creating sub_path as needed."""
>> +    full_path = "%s/%s" % (root_dir, sub_path)
>> +    try:
>> +        os.makedirs(full_path)
>> +    except OSError:
>> +        print "skipping %s" % (full_path)
>
> Print the error message too? Also do you want to "return"?

It's really a NOP to behave in a mkdir -p type way. Python 3 provides an
exist_ok parameter but I assume we aren't ready to drop python 2 just
yet ;-)

>
>> +
>> +    copyfile(src, "%s/%s" % (full_path, os.path.basename(src)))
>> +
>>  class Docker(object):
>>      """ Running Docker commands """
>>      def __init__(self):
>> @@ -86,18 +109,36 @@ class Docker(object):
>>          labels = json.loads(resp)[0]["Config"].get("Labels", {})
>>          return labels.get("com.qemu.dockerfile-checksum", "")
>>
>> -    def build_image(self, tag, dockerfile, df_path, quiet=True, argv=None):
>> +    def build_image(self, tag, dockerfile, quiet=True, qemu=None, argv=None):
>>          if argv == None:
>>              argv = []
>> +
>> +        # Create a temporary docker context to build in
>> +        tmp_dir = tempfile.mkdtemp(prefix="docker_build")
>> +
>> +        # Copy the dockerfile into our work space
>>          tmp = dockerfile + "\n" + \
>>                "LABEL com.qemu.dockerfile-checksum=%s" % \
>>                _text_checksum(dockerfile)
>> -        dirname = os.path.dirname(df_path)
>> -        tmp_df = tempfile.NamedTemporaryFile(dir=dirname)
>> +        tmp_df = tempfile.NamedTemporaryFile(dir=tmp_dir, suffix=".docker")
>>          tmp_df.write(tmp)
>>          tmp_df.flush()
>> +
>> +        # Do we want to copy QEMU into here?
>> +        if qemu:
>> +            _copy_with_mkdir(qemu, tmp_dir, "/usr/bin")
>
> Looks like "/usr/bin" is combined with tmp_dir as "FOO//usr/bin",
> superfluous?

Yeah, also a bit hacky. In my original shell script it actually searched
the output of Debian's binfmt_update script to see where it was set to
search for the qemu binary from.

>
>> +            # do ldd bit here
>> +            ldd_output = subprocess.check_output(["ldd", qemu])
>> +            for l in ldd_output.split("\n"):
>> +                s = re.search("(/.*/)(\S*)", l)
>> +                if s and len(s.groups())==2:
>> +                    so_path=s.groups()[0]
>> +                    so_lib=s.groups()[1]
>> +                    _copy_with_mkdir("%s/%s" % (so_path, so_lib),
>> +                                     tmp_dir, so_path)
>> +
>>          self._do(["build", "-t", tag, "-f", tmp_df.name] + argv + \
>> -                 [dirname],
>> +                 [tmp_dir],
>>                   quiet=quiet)
>>
>>      def image_matches_dockerfile(self, tag, dockerfile):
>> @@ -148,6 +189,7 @@ class BuildCommand(SubCommand):
>>      """ Build docker image out of a dockerfile. Arguments: <tag> <dockerfile>"""
>>      name = "build"
>>      def args(self, parser):
>> +        parser.add_argument("--qemu", help="Include qemu-user binaries")
>
> Can I ask for a rename of this and the variable names in this patch, to a more
> generic name (to reflect that it is inherently orthorgonal to the type of the
> binary we are copying)? How about:
>
>            parser.add_argument("--executable-inject", "-e",
>                                help="""Specify a binary that will be copied to the
>                                container together with all its dependent
>                                libraries""")
>
> And I think it is reasonable to expect the user (or the calling Makefile) to
> designate a working absolute or relative path, instead of looking up it
> ourselves.

Makes sense.

>
>>          parser.add_argument("tag",
>>                              help="Image Tag")
>>          parser.add_argument("dockerfile",
>> @@ -157,14 +199,18 @@ class BuildCommand(SubCommand):
>>          dockerfile = open(args.dockerfile, "rb").read()
>>          tag = args.tag
>>
>> +        # find qemu binary
>> +        qbin=None
>
> Add whitespaces around "="?

Ack.

>
>> +        if args.qemu:
>> +            qbin=_find_user_binary(args.qemu)
>
> Ditto, and some more occasions above.

Ack.

>
>> +
>>          dkr = Docker()
>>          if dkr.image_matches_dockerfile(tag, dockerfile):
>>              if not args.quiet:
>>                  print "Image is up to date."
>>              return 0
>>
>> -        dkr.build_image(tag, dockerfile, args.dockerfile,
>> -                        quiet=args.quiet, argv=argv)
>> +        dkr.build_image(tag, dockerfile, quiet=args.quiet, qemu=qbin, argv=argv)
>>          return 0
>>
>>  class CleanCommand(SubCommand):
>> --
>> 2.7.4
>>
>
> Thanks,
>
> Fam


--
Alex Bennée

Re: [Qemu-devel] [RFC PATCH 2/2] add debian-bootstrap.docker target

[Alex Bennée]

Fam Zheng <famz@redhat.com> writes:

> On Thu, 05/26 15:27, Alex Bennée wrote:
>> Together with some changes to the docker script you can now build an
>> arbitrary architecture of Debian using debootstrap. To achieve this I
>> introduce the concept of a HOST_CMD in the docker config file. While
>> copying the file into workspace the HOST_CMD is run in the docker build
>> context. This allows debootstrap to set up its first stage before the
>> container is built.
>
> Could you instead introduce the concept of $IMAGE.pre file (in this case
> debian-bootstrap.pre, aside debian-bootstrap.docker), and exec it in
> docker.py?  It would be much more flexible, and we we wouldn't need to inject a
> custom directive to filter it out.

I'm ambivalent about that. To be honest this is a bit of a gap in
docker's image creation (or if there is a better more docker-y way of
doing things I couldn't find it). It does have the benefit of keeping
everything in one place.

We are copying the dockerfile to the build environment anyway so it
seemed natural to do the operation while copying across. How would
envison the .pre setup? A #! script we just exec inside the temp
directory we create?

>
> Fam


--
Alex Bennée

Re: [Qemu-devel] [RFC PATCH 2/2] add debian-bootstrap.docker target

[Fam Zheng]

On Tue, 05/31 16:27, Alex Bennée wrote:
> 
> Fam Zheng <famz@redhat.com> writes:
> 
> > On Thu, 05/26 15:27, Alex Bennée wrote:
> >> Together with some changes to the docker script you can now build an
> >> arbitrary architecture of Debian using debootstrap. To achieve this I
> >> introduce the concept of a HOST_CMD in the docker config file. While
> >> copying the file into workspace the HOST_CMD is run in the docker build
> >> context. This allows debootstrap to set up its first stage before the
> >> container is built.
> >
> > Could you instead introduce the concept of $IMAGE.pre file (in this case
> > debian-bootstrap.pre, aside debian-bootstrap.docker), and exec it in
> > docker.py?  It would be much more flexible, and we we wouldn't need to inject a
> > custom directive to filter it out.
> 
> I'm ambivalent about that. To be honest this is a bit of a gap in
> docker's image creation (or if there is a better more docker-y way of
> doing things I couldn't find it). It does have the benefit of keeping
> everything in one place.

I don't like that it pollutes the dockerfile, rendering it invalid for raw
"docker build", which is not very good.

> 
> We are copying the dockerfile to the build environment anyway so it
> seemed natural to do the operation while copying across. How would
> envison the .pre setup? A #! script we just exec inside the temp
> directory we create?

Yes, since I'm going to send a v7, I'll merge the tmp dir part in to my series,
and add a new patch for the "pre" idea to see if we like it.

Fam

Re: [Qemu-devel] [RFC PATCH 1/2] tests/docker/docker.py: support --qemu option

[Fam Zheng]

On Tue, 05/31 16:23, Alex Bennée wrote:
> >> +def _find_user_binary(binary_name):
> >> +    """ Find a binary in the QEMU source tree. Used for finding qemu-$arch."""
> >> +    top = os.path.abspath("%s/../../.." % sys.argv[0])
> >
> > What if this is an out of tree build?
> 
> Yes I kinda avoided the complexity here. Do we have a programatic way of
> finding this out or should we just assume we get based a resolvable path?

As said below, let's assume the user provides an absolute path or a relative
path against the working directory, so we don't need to worry about path
guessing. The script caller should have more information.

Fam

[Qemu-devel] [RFC PATCH] docker: Support ".pre" script when building image

[Fam Zheng]

When building "foo.docker", if a "foo.pre" script exists, it will be
executed in the building context before "docker build" is invoked.

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

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index 0151362..7b8f022 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -99,6 +99,12 @@ class Docker(object):
         tmp_df.write("LABEL com.qemu.dockerfile-checksum=%s" %
                      _text_checksum(dockerfile))
         tmp_df.flush()
+        pre_path = os.path.abspath(df_path[:-len(".docker")] + ".pre")
+        if os.path.isfile(pre_path):
+            if quiet:
+                subprocess.check_output([pre_path], cwd=tmp_dir)
+            else:
+                subprocess.check_call([pre_path], cwd=tmp_dir)
         self._do(["build", "-t", tag, "-f", tmp_df.name] + argv + \
                  [tmp_dir],
                  quiet=quiet)
-- 
2.8.2