Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run usernet process in background #2595

Merged
merged 1 commit into from
Sep 17, 2024

Conversation

nirs
Copy link
Contributor

@nirs nirs commented Sep 6, 2024

Similar to the how we run the hostagent process[1], we want to run the
usernet process in the background. Now a program using killpg to cleanup
child processes will not terminate the usernet process.

Example run with this change:

% ps -o pid,pgid,ppid,command
  PID  PGID  PPID COMMAND
55768 55768 55767 -zsh
56126 56126 55768 limactl start userv2.yaml --tty=0
56128 56128 56126 /Users/nsoffer/src/lima/_output/bin/limactl usernet ...
56131 56131 56126 /Users/nsoffer/src/lima/_output/bin/limactl hostagent ...

% ps -o pid,pgid,ppid,command
  PID  PGID  PPID COMMAND
55768 55768 55767 -zsh
56128 56128     1 /Users/nsoffer/src/lima/_output/bin/limactl usernet ...
56131 56131     1 /Users/nsoffer/src/lima/_output/bin/limactl hostagent ...

[1] #2574

@nirs
Copy link
Contributor Author

nirs commented Sep 6, 2024

Failed test on fedora seems unrelated:
https://github.com/lima-vm/lima/actions/runs/10746186646/job/29806633152?pr=2595

@AkihiroSuda
Copy link
Member

Similar to the how we run the hostagent process, we want to run the usernet process in the background. Now a program using killpg to cleanup child processes will not terminate the usernet process.

Could you explain the motivation?

Similar to the how we run the hostagent process[1], we want to run the
usernet process in the background. Now a program using killpg to cleanup
child processes will not terminate the usernet process.

Example run with this change:

    % ps -o pid,pgid,ppid,command
      PID  PGID  PPID COMMAND
    55768 55768 55767 -zsh
    56126 56126 55768 limactl start userv2.yaml --tty=0
    56128 56128 56126 /Users/nsoffer/src/lima/_output/bin/limactl usernet ...
    56131 56131 56126 /Users/nsoffer/src/lima/_output/bin/limactl hostagent ...

    % ps -o pid,pgid,ppid,command
      PID  PGID  PPID COMMAND
    55768 55768 55767 -zsh
    56128 56128     1 /Users/nsoffer/src/lima/_output/bin/limactl usernet ...
    56131 56131     1 /Users/nsoffer/src/lima/_output/bin/limactl hostagent ...

[1] lima-vm#2574

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
@nirs
Copy link
Contributor Author

nirs commented Sep 8, 2024

Similar to the how we run the hostagent process, we want to run the usernet process in the background. Now a program using killpg to cleanup child processes will not terminate the usernet process.

Could you explain the motivation?

Sorry, I forgot to link the relevant PR, added in the current version.

The issue with the usernet process (that was fixed in the hostagent process) is that it runs in the same process group of the limactl. Since this is always a background process that must continue to run after limactl exit (otherwise user network breaks), it should at least run in a new process group. Ideally it would run as a proper daemon (like qemu --demonize), but this is a bigger change and running in the background is good enough.

Here is the relevant code in a program running limactl:
https://github.com/RamenDR/ramen/blob/b635a80ff528cd6af64b1f2eeea6df6af22113ed/test/drenv/__main__.py#L42

I can amend the commit message if needed.

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@AkihiroSuda AkihiroSuda added this to the v1.0 milestone Sep 8, 2024
@nirs
Copy link
Contributor Author

nirs commented Sep 17, 2024

@AkihiroSuda any reason to wait this change?

@AkihiroSuda AkihiroSuda merged commit 6163a26 into lima-vm:master Sep 17, 2024
27 checks passed
@nirs nirs deleted the usernet-background branch September 17, 2024 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants