Re: [Qemu-devel] [PATCH v1 03/42] tests/docker: fix "cc" command to work with podman

[John Snow]

On 9/4/19 4:29 PM, Alex Bennée wrote:
> Podman requires a little bit of additional magic to the uid mapping
> which was already done for the normal RunCommand. We simplify the
> logic by pushing it directly into the Docker::run method to avoid
> instantiating an extra Docker() object and ensure the CC command
> always runs as the current user.
> 
> Signed-off-by: Alex Bennée <address@hidden>
> ---
>  tests/docker/docker.py     | 30 +++++++++++++++---------------
>  tests/tcg/Makefile.include |  2 +-
>  2 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/tests/docker/docker.py b/tests/docker/docker.py
> index e23209f71ee..8f391eb278b 100755
> --- a/tests/docker/docker.py
> +++ b/tests/docker/docker.py
> @@ -318,10 +318,20 @@ class Docker(object):
>              return False
>          return checksum == _text_checksum(_dockerfile_preprocess(dockerfile))
>  
> -    def run(self, cmd, keep, quiet):
> +    def run(self, cmd, keep, quiet, as_user=False):
>          label = uuid.uuid1().hex
>          if not keep:
>              self._instances.append(label)
> +
> +        if as_user:
> +            uid = os.getuid()
> +            cmd = [ "-u", str(uid) ] + cmd
> +            # podman requires a bit more fiddling
> +            if self._command[0] == "podman":
> +                cmd = [ "--uidmap", "%d:0:1" % uid,
> +                        "--uidmap", "0:1:%d" % uid,
> +                        "--uidmap", "%d:%d:64536" % (uid + 1, uid + 1)] + cmd
> +

I was having problems with constructs like these recently. I think we
either need to use --userns=keep-id (vastly preferred) or adjust 64536
there to read as "65536 - uid" because not everyone will have a UID of 1000.

(My UID is over 20,000 and for whatever reason, this causes podman to
crash very badly when using this hackaround.)

>          ret = self._do_check(["run", "--label",
>                               "com.qemu.instance.uuid=" + label] + cmd,
>                               quiet=quiet)
> @@ -364,15 +374,8 @@ class RunCommand(SubCommand):
>                              help="Run container using the current user's 
> uid")
>  
>      def run(self, args, argv):
> -        if args.run_as_current_user:
> -            uid = os.getuid()
> -            argv = [ "-u", str(uid) ] + argv
> -            docker = Docker()
> -            if docker._command[0] == "podman":
> -                argv = [ "--uidmap", "%d:0:1" % uid,
> -                         "--uidmap", "0:1:%d" % uid,
> -                         "--uidmap", "%d:%d:64536" % (uid + 1, uid + 1)] + 
> argv
> -        return Docker().run(argv, args.keep, quiet=args.quiet)
> +        return Docker().run(argv, args.keep, quiet=args.quiet,
> +                            as_user=args.run_as_current_user)
>  
>  
>  class BuildCommand(SubCommand):
> @@ -556,8 +559,6 @@ class CcCommand(SubCommand):
>                              help="The docker image in which to run cc")
>          parser.add_argument("--cc", default="cc",
>                              help="The compiler executable to call")
> -        parser.add_argument("--user",
> -                            help="The user-id to run under")
>          parser.add_argument("--source-path", "-s", nargs="*", dest="paths",
>                              help="""Extra paths to (ro) mount into container 
> for
>                              reading sources""")
> @@ -571,11 +572,10 @@ class CcCommand(SubCommand):
>          if args.paths:
>              for p in args.paths:
>                  cmd += ["-v", "%s:%s:ro,z" % (p, p)]
> -        if args.user:
> -            cmd += ["-u", args.user]
>          cmd += [args.image, args.cc]
>          cmd += argv
> -        return Docker().command("run", cmd, args.quiet)
> +        return Docker().run(cmd, False, quiet=args.quiet,
> +                            as_user=True)
>  
>  
>  class CheckCommand(SubCommand):
> diff --git a/tests/tcg/Makefile.include b/tests/tcg/Makefile.include
> index 73b5626fc5f..210f8428237 100644
> --- a/tests/tcg/Makefile.include
> +++ b/tests/tcg/Makefile.include
> @@ -41,7 +41,7 @@ ifneq ($(DOCKER_IMAGE),)
>  # We also need the Docker make rules to depend on
>  include $(SRC_PATH)/tests/docker/Makefile.include
>  
> -DOCKER_COMPILE_CMD="$(DOCKER_SCRIPT) cc --user $(shell id -u) \
> +DOCKER_COMPILE_CMD="$(DOCKER_SCRIPT) cc \
>               --cc $(DOCKER_CROSS_COMPILER) \
>               -i qemu:$(DOCKER_IMAGE) \
>               -s $(SRC_PATH) -- "
> 

-- 
—js