Skip to content

Commit

Permalink
virtme-ng: fix --user behavior
Browse files Browse the repository at this point in the history
Properly implement the following behavior regarding the default user in
virtme-ng sessions:

 - default user for non-interactive sessions is root, unless --user is
   specified

 - default user for interactive sessions is the current user, unless
   --user is specified

We want to maintain this distinction to avoid breaking the old behavior,
but also for performance reasons: running non-interactive scripts as
root is faster, because we can get rid of the "user switch" step and
immediately run the target script as soon as the boot is done.

On the other hand, switching automatically to the target user during
interactive sessions has the benefit of giving the user the feeling of
running the target kernel inside the same exact environment of the host.

For the above reasons it makes sense to maintain this behavior.

However, we should also honor the `--user USER` when it's used and right
now we are honoring it for interactive sessions only. Fix this and make
sure the --user option is considered also for non-interactive sessions.

Signed-off-by: Andrea Righi <andrea.righi@canonical.com>
  • Loading branch information
Andrea Righi committed Nov 12, 2023
1 parent 416163c commit 84b9a13
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 20 deletions.
7 changes: 6 additions & 1 deletion virtme/guest/virtme-init
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,12 @@ if [[ -n "${user_cmd}" ]]; then
if [[ ! -n "${virtme_graphics}" ]]; then
# Start the script
log 'starting script'
setsid bash /tmp/.virtme-script </dev/virtio-ports/virtme.stdin >/dev/virtio-ports/virtme.stdout 2>/dev/virtio-ports/virtme.stderr
if [[ -n "${virtme_user}" ]]; then
chmod +x /tmp/.virtme-script
setsid su ${virtme_user} -c /tmp/.virtme-script </dev/virtio-ports/virtme.stdin >/dev/virtio-ports/virtme.stdout 2>/dev/virtio-ports/virtme.stderr
else
setsid bash /tmp/.virtme-script </dev/virtio-ports/virtme.stdin >/dev/virtio-ports/virtme.stdout 2>/dev/virtio-ports/virtme.stderr
fi
log "script returned $?"

# Hmm. We should expose the return value somehow.
Expand Down
44 changes: 26 additions & 18 deletions virtme_ng/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,30 @@ def _get_virtme_name(self, args):
else:
self.virtme_param["name"] = "--name " + socket.gethostname()

def _get_virtme_exec(self, args):
if args.envs:
args.exec = " ".join(args.envs)
if args.exec is not None:
self.virtme_param["exec"] = f'--script-sh "{args.exec}"'
else:
self.virtme_param["exec"] = ""

def _get_virtme_user(self, args):
# Default user for scripts is root, default user for interactive
# sessions is current user.
#
# NOTE: graphic sessions are considered interactive.
if args.exec and not args.graphics:
self.virtme_param["user"] = ""
else:
self.virtme_param["user"] = "--user " + get_username()
# Override default user, if specified by the --user argument.
if args.user is not None:
if args.user != "root":
self.virtme_param["user"] = "--user " + args.user
else:
self.virtme_param["user"] = ""

def _get_virtme_arch(self, args):
if args.arch is not None:
if args.arch not in ARCH_MAPPING:
Expand All @@ -699,14 +723,6 @@ def _get_virtme_arch(self, args):
else:
self.virtme_param["arch"] = ""

def _get_virtme_user(self, args):
if args.user is not None:
self.virtme_param["user"] = "--user " + args.user
elif args.root is None:
self.virtme_param["user"] = "--user " + get_username()
else:
self.virtme_param["user"] = ""

def _get_virtme_root(self, args):
if args.root is not None:
create_root(args.root, args.arch or "amd64")
Expand Down Expand Up @@ -777,14 +793,6 @@ def _get_virtme_no_virtme_ng_init(self, args):
else:
self.virtme_param["no_virtme_ng_init"] = ""

def _get_virtme_exec(self, args):
if args.envs:
args.exec = " ".join(args.envs)
if args.exec is not None:
self.virtme_param["exec"] = f'--script-sh "{args.exec}"'
else:
self.virtme_param["exec"] = ""

def _get_virtme_network(self, args):
if args.network is not None:
self.virtme_param["network"] = f"--net {args.network}"
Expand Down Expand Up @@ -898,6 +906,7 @@ def _get_virtme_qemu_opts(self, args):
def run(self, args):
"""Execute a kernel inside virtme-ng."""
self._get_virtme_name(args)
self._get_virtme_exec(args)
self._get_virtme_user(args)
self._get_virtme_arch(args)
self._get_virtme_root(args)
Expand All @@ -910,7 +919,6 @@ def run(self, args):
self._get_virtme_dry_run(args)
self._get_virtme_no_virtme_ng_init(args)
self._get_virtme_mods(args)
self._get_virtme_exec(args)
self._get_virtme_network(args)
self._get_virtme_disk(args)
self._get_virtme_sound(args)
Expand All @@ -932,6 +940,7 @@ def run(self, args):
cmd = (
"virtme-run "
+ f'{self.virtme_param["name"]} '
+ f'{self.virtme_param["exec"]} '
+ f'{self.virtme_param["user"]} '
+ f'{self.virtme_param["arch"]} '
+ f'{self.virtme_param["root"]} '
Expand All @@ -944,7 +953,6 @@ def run(self, args):
+ f'{self.virtme_param["dry_run"]} '
+ f'{self.virtme_param["no_virtme_ng_init"]} '
+ f'{self.virtme_param["mods"]} '
+ f'{self.virtme_param["exec"]} '
+ f'{self.virtme_param["network"]} '
+ f'{self.virtme_param["disk"]} '
+ f'{self.virtme_param["sound"]} '
Expand Down
2 changes: 1 addition & 1 deletion virtme_ng_init
Submodule virtme_ng_init updated 1 files
+19 −5 src/main.rs

0 comments on commit 84b9a13

Please sign in to comment.