[v3,for,3.0,17/18] docker: perform basic binfmt_misc validation in docker.py

Setting up binfmt_misc is outside of the scope of the docker.py script
but we can at least validate it with any given executable so we have a
more useful error message than the sed line of deboostrap failing
cryptically.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reported-by: Richard Henderson <richard.henderson@linaro.org>
---
 tests/docker/docker.py | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

On Wed, Jul 18, 2018 at 4:02 AM Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Setting up binfmt_misc is outside of the scope of the docker.py script
> but we can at least validate it with any given executable so we have a
> more useful error message than the sed line of deboostrap failing
> cryptically.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reported-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tests/docker/docker.py | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
> index 523f4b95a2..a3f5b0c1b0 100755
> --- a/tests/docker/docker.py
> +++ b/tests/docker/docker.py
> @@ -112,6 +112,31 @@ def _copy_binary_with_libs(src, dest_dir):
>              so_path = os.path.dirname(l)
>              _copy_with_mkdir(l , dest_dir, so_path)
>
> +
> +def _check_binfmt_misc(executable):
> +    """Check binfmt_misc has entry for executable in the right place.
> +
> +    The details of setting up binfmt_misc are outside the scope of
> +    this script but we should at least fail early with a useful
> +    message if it won't work."""
> +
> +    binary = os.path.basename(executable)
> +    binfmt_entry = "/proc/sys/fs/binfmt_misc/%s" % (binary)
> +
> +    if not os.path.exists(binfmt_entry):
> +        print ("No binfmt_misc entry for %s" % (binary))
> +        return False
> +
> +    with open(binfmt_entry) as x: entry = x.read()
> +
> +    qpath = "/usr/bin/%s" % (binary)

Is it intended to hardcode this to /usr/bin? I thought we were more
flexible than that..

Fam

> +    if not re.search("interpreter %s\n" % (qpath), entry):
> +        print ("binfmt_misc for %s does not point to %s" % (binary, qpath))
> +        return False
> +
> +    return True
> +
> +
>  def _read_qemu_dockerfile(img_name):
>      # special case for Debian linux-user images
>      if img_name.startswith("debian") and img_name.endswith("user"):
> @@ -315,6 +340,11 @@ class BuildCommand(SubCommand):
>              # Create a docker context directory for the build
>              docker_dir = tempfile.mkdtemp(prefix="docker_build")
>
> +            # Validate binfmt_misc will work
> +            if args.include_executable:
> +                if not _check_binfmt_misc(args.include_executable):
> +                    return 1
> +
>              # 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):
> --
> 2.17.1
>

Fam Zheng <famz@redhat.com> writes:

> On Wed, Jul 18, 2018 at 4:02 AM Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Setting up binfmt_misc is outside of the scope of the docker.py script
>> but we can at least validate it with any given executable so we have a
>> more useful error message than the sed line of deboostrap failing
>> cryptically.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Reported-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  tests/docker/docker.py | 30 ++++++++++++++++++++++++++++++
>>  1 file changed, 30 insertions(+)
>>
>> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
>> index 523f4b95a2..a3f5b0c1b0 100755
>> --- a/tests/docker/docker.py
>> +++ b/tests/docker/docker.py
>> @@ -112,6 +112,31 @@ def _copy_binary_with_libs(src, dest_dir):
>>              so_path = os.path.dirname(l)
>>              _copy_with_mkdir(l , dest_dir, so_path)
>>
>> +
>> +def _check_binfmt_misc(executable):
>> +    """Check binfmt_misc has entry for executable in the right place.
>> +
>> +    The details of setting up binfmt_misc are outside the scope of
>> +    this script but we should at least fail early with a useful
>> +    message if it won't work."""
>> +
>> +    binary = os.path.basename(executable)
>> +    binfmt_entry = "/proc/sys/fs/binfmt_misc/%s" % (binary)
>> +
>> +    if not os.path.exists(binfmt_entry):
>> +        print ("No binfmt_misc entry for %s" % (binary))
>> +        return False
>> +
>> +    with open(binfmt_entry) as x: entry = x.read()
>> +
>> +    qpath = "/usr/bin/%s" % (binary)
>
> Is it intended to hardcode this to /usr/bin? I thought we were more
> flexible than that..

That's where we currently copy it. However we are certainly capable of
being more flexible. For example as long at there is an entry visible to
the host file-system with the "F" flag we can actually create and run
docker images without copying QEMU inside the container.

However if we don't we certainly need to copy it to the same place in
the container than it is used outside the container.

>
> Fam
>
>> +    if not re.search("interpreter %s\n" % (qpath), entry):
>> +        print ("binfmt_misc for %s does not point to %s" % (binary, qpath))
>> +        return False
>> +
>> +    return True
>> +
>> +
>>  def _read_qemu_dockerfile(img_name):
>>      # special case for Debian linux-user images
>>      if img_name.startswith("debian") and img_name.endswith("user"):
>> @@ -315,6 +340,11 @@ class BuildCommand(SubCommand):
>>              # Create a docker context directory for the build
>>              docker_dir = tempfile.mkdtemp(prefix="docker_build")
>>
>> +            # Validate binfmt_misc will work
>> +            if args.include_executable:
>> +                if not _check_binfmt_misc(args.include_executable):
>> +                    return 1
>> +
>>              # 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):
>> --
>> 2.17.1
>>


--
Alex Bennée