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

Support preserve-fds with non-continuous fd numbers #20844

Closed
scanon opened this issue Nov 29, 2023 · 5 comments · Fixed by #20866
Closed

Support preserve-fds with non-continuous fd numbers #20844

scanon opened this issue Nov 29, 2023 · 5 comments · Fixed by #20866
Labels
kind/feature Categorizes issue or PR as related to a new feature. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Comments

@scanon
Copy link

scanon commented Nov 29, 2023

Feature request description

Currently the preserve-fds allows for a single number that specifies how many additional file descriptors to preserve above fd 2. This means the additional file descriptors have to start at 3 and be contiguous. I have a situation where the file descriptor I need to pass is some high number. I have a work around that duplicates the fd of interest as 3 and then can pass --preserve-fds 1. But it would be much better if we could pass a list of FDs.

Suggest potential solution

I would like to be able to pass a list of fds to preserve. Since it needs to preserve backwards compatibility maybe there could be some way to indicate it is a list even if it is just one number.

So something like

podman run --preserve-fds 4,5 ....

or if there was just one something like

podman run --preserve-fds 5, ....

Have you considered any alternatives?

As mentioned I have a workaround but it is a bit of a hack.

Additional context

Add any other context or screenshots about the feature request here.

@scanon scanon added the kind/feature Categorizes issue or PR as related to a new feature. label Nov 29, 2023
@eriksjolund
Copy link
Contributor

Previous discussion:

@giuseppe
Copy link
Member

it is a consequence of the Go API, that expects the FDs to be continuous:

if preserveFDs > 0 {
for fd := 3; fd < int(3+preserveFDs); fd++ {
f := os.NewFile(uintptr(fd), fmt.Sprintf("fd-%d", fd))
filesToClose = append(filesToClose, f)
cmd.ExtraFiles = append(cmd.ExtraFiles, f)
}
}

so for this to work, we'd need to change the OCI runtime, conmon, and use directly fork+exec instead of the Go os/exec package

@Luap99
Copy link
Member

Luap99 commented Nov 30, 2023

@giuseppe I think it is valid to pass nil in the ExtraFiles array to skip the fd. Try

package main

import (
	"fmt"
	"os"
	"os/exec"
)

func main() {
	cmd := exec.Command("ls", "-l", "/proc/self/fd")
	cmd.Stdout = os.Stdout
	cmd.Stderr = os.Stderr

	f1, _ := os.Open(os.DevNull)

	cmd.ExtraFiles = []*os.File{nil, f1, nil, f1, nil, f1}
	fmt.Println(cmd.Run())
}

@giuseppe
Copy link
Member

@Luap99 neat! Thanks, that is good to know. In this case, we could probably handle it completely in Podman, passing --preserve-fds=N to be the maximum value in the FDs set

@giuseppe
Copy link
Member

giuseppe commented Dec 1, 2023

opened a PR: #20866

giuseppe added a commit to giuseppe/libpod that referenced this issue Dec 5, 2023
add a new option --preserve-fd that allows to specify a list of FDs to
pass down to the container.

It is similar to --preserve-fds but it allows to specify a list of FDs
instead of the maximum FD number to preserve.

--preserve-fd and --preserve-fds are mutually exclusive.

It requires crun since runc would complain if any fd below
--preserve-fds is not preserved.

Closes: containers#20844

Signed-off-by: Giuseppe Scrivano <[email protected]>
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Mar 6, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/feature Categorizes issue or PR as related to a new feature. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants