[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v1 03/42] tests/docker: fix "cc" command to work
From: |
Alex Bennée |
Subject: |
Re: [Qemu-devel] [PATCH v1 03/42] tests/docker: fix "cc" command to work with podman |
Date: |
Thu, 05 Sep 2019 10:51:09 +0100 |
User-agent: |
mu4e 1.3.4; emacs 27.0.50 |
John Snow <address@hidden> writes:
> 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.
>From Marc-André's original commit:
With a user 1000, the default mapping is: 1000 (host) -> 0 (container).
So write access to /var/tmp/ccache ends will end with permission
denied error.
With "--uidmap 1000:0:1 --uidmap 0:1:1000", the mapping is:
1000 (host) -> 0 (container, 1st namespace) -> 1000 (container, 2nd
namespace).
(the rest is mumbo jumbo to avoid holes in the range of UIDs)
A future podman version may have an option such as --userns-keep-uid.
Thanks to Debarshi Ray <address@hidden> for the help!
So I assumed this doesn't exist for all versions of podman yet. Given
how new the support is I guess we could just say you need a minimum
version for working podman support.
>
> (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) -- "
>>
--
Alex Bennée
[Qemu-devel] [PATCH v1 06/42] tests/tcg: move configuration to a sub-shell script, Alex Bennée, 2019/09/04
[Qemu-devel] [PATCH v1 10/42] tests/docker: add Buster to DOCKER_PARTIAL_IMAGES, Alex Bennée, 2019/09/04
[Qemu-devel] [PATCH v1 04/42] tests/tcg: use EXTRA_CFLAGS everywhere, Alex Bennée, 2019/09/04
[Qemu-devel] [PATCH v1 30/42] .travis.yml: Enable multiple caching features, Alex Bennée, 2019/09/04
[Qemu-devel] [PATCH v1 05/42] tests/tcg: cleanup Makefile inclusions, Alex Bennée, 2019/09/04
[Qemu-devel] [PATCH v1 14/42] tests/docker: move our HPPA cross compile to Buster, Alex Bennée, 2019/09/04