-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Initial addition of 9p code to Podman #20358
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| //go:build linux && (amd64 || arm64) | ||
| // +build linux | ||
| // +build amd64 arm64 | ||
|
|
||
| package machine | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "os" | ||
| "os/exec" | ||
| "path/filepath" | ||
| "strconv" | ||
|
|
||
| "github.com/containers/common/pkg/completion" | ||
| "github.com/containers/podman/v4/cmd/podman/registry" | ||
| "github.com/mdlayher/vsock" | ||
| "github.com/sirupsen/logrus" | ||
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| var ( | ||
| client9pCommand = &cobra.Command{ | ||
| Args: cobra.ExactArgs(2), | ||
| Use: "client9p PORT DIR", | ||
| Hidden: true, | ||
| Short: "Mount a remote directory using 9p over hvsock", | ||
| Long: "Connect to the given hvsock port using 9p and mount the served filesystem at the given directory", | ||
| RunE: remoteDirClient, | ||
| ValidArgsFunction: completion.AutocompleteNone, | ||
| Example: `podman system client9p 55000 /mnt`, | ||
| } | ||
| ) | ||
|
|
||
| func init() { | ||
| registry.Commands = append(registry.Commands, registry.CliCommand{ | ||
| Command: client9pCommand, | ||
| Parent: machineCmd, | ||
| }) | ||
| } | ||
|
|
||
| func remoteDirClient(cmd *cobra.Command, args []string) error { | ||
| port, err := strconv.Atoi(args[0]) | ||
| if err != nil { | ||
| return fmt.Errorf("error parsing port number: %w", err) | ||
| } | ||
|
|
||
| if err := client9p(uint32(port), args[1]); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| // This is Linux-only as we only intend for this function to be used inside the | ||
| // `podman machine` VM, which is guaranteed to be Linux. | ||
| func client9p(portNum uint32, mountPath string) error { | ||
| cleanPath, err := filepath.Abs(mountPath) | ||
| if err != nil { | ||
| return fmt.Errorf("absolute path for %s: %w", mountPath, err) | ||
| } | ||
| mountPath = cleanPath | ||
|
|
||
| // Mountpath needs to exist and be a directory | ||
| stat, err := os.Stat(mountPath) | ||
| if err != nil { | ||
| return fmt.Errorf("stat %s: %w", mountPath, err) | ||
| } | ||
| if !stat.IsDir() { | ||
| return fmt.Errorf("path %s is not a directory", mountPath) | ||
| } | ||
|
|
||
| logrus.Infof("Going to mount 9p on vsock port %d to directory %s", portNum, mountPath) | ||
|
|
||
| // Host connects to non-hypervisor processes on the host running the VM. | ||
| conn, err := vsock.Dial(vsock.Host, portNum, nil) | ||
| if err != nil { | ||
| return fmt.Errorf("dialing vsock port %d: %w", portNum, err) | ||
| } | ||
| defer func() { | ||
| if err := conn.Close(); err != nil { | ||
| logrus.Errorf("Error closing vsock: %v", err) | ||
| } | ||
| }() | ||
|
|
||
| // vsock doesn't give us direct access to the underlying FD. That's kind | ||
| // of inconvenient, because we have to pass it off to mount. | ||
| // However, it does give us the ability to get a syscall.RawConn, which | ||
| // has a method that allows us to run a function that takes the FD | ||
| // number as an argument. | ||
| // Which ought to be good enough? Probably? | ||
| // Overall, this is gross and I hate it, but I don't see a better way. | ||
| rawConn, err := conn.SyscallConn() | ||
| if err != nil { | ||
| return fmt.Errorf("getting vsock raw conn: %w", err) | ||
| } | ||
| errChan := make(chan error, 1) | ||
| runMount := func(fd uintptr) { | ||
| vsock := os.NewFile(fd, "vsock") | ||
| if vsock == nil { | ||
| errChan <- fmt.Errorf("could not convert vsock fd to os.File") | ||
| return | ||
| } | ||
|
|
||
| // This is ugly, but it lets us use real kernel mount code, | ||
| // instead of maintaining our own FUSE 9p implementation. | ||
| cmd := exec.Command("mount", "-t", "9p", "-o", "trans=fd,rfdno=3,wfdno=3,version=9p2000.L", "9p", mountPath) | ||
| cmd.ExtraFiles = []*os.File{vsock} | ||
|
|
||
| output, err := cmd.CombinedOutput() | ||
| if err != nil { | ||
| err = fmt.Errorf("running mount: %w\nOutput: %s", err, string(output)) | ||
| } else { | ||
| logrus.Debugf("Mount output: %s", string(output)) | ||
| logrus.Infof("Mounted directory %s using 9p", mountPath) | ||
| } | ||
|
|
||
| errChan <- err | ||
| close(errChan) | ||
| } | ||
| if err := rawConn.Control(runMount); err != nil { | ||
| return fmt.Errorf("running mount function for dir %s: %w", mountPath, err) | ||
| } | ||
|
|
||
| if err := <-errChan; err != nil { | ||
| return fmt.Errorf("mounting filesystem %s: %w", mountPath, err) | ||
| } | ||
|
|
||
| logrus.Infof("Mount of filesystem %s successful", mountPath) | ||
|
|
||
| return nil | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| //go:build windows && (amd64 || arm64) | ||
| // +build windows | ||
| // +build amd64 arm64 | ||
|
|
||
| package machine | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "strconv" | ||
| "strings" | ||
| "time" | ||
|
|
||
| "github.com/containers/common/pkg/completion" | ||
| "github.com/containers/podman/v4/cmd/podman/registry" | ||
| "github.com/containers/podman/v4/pkg/fileserver" | ||
| psutil "github.com/shirou/gopsutil/v3/process" | ||
| "github.com/sirupsen/logrus" | ||
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| var ( | ||
| server9pCommand = &cobra.Command{ | ||
| Args: cobra.ExactArgs(1), | ||
| Use: "server9p [options] PID", | ||
| Hidden: true, | ||
| Short: "Serve a directory using 9p over hvsock", | ||
| Long: "Start a number of 9p servers on given hvsock UUIDs, and run until the given PID exits", | ||
| RunE: remoteDirServer, | ||
| ValidArgsFunction: completion.AutocompleteNone, | ||
| Example: `podman system server9p --serve C:\Users\myuser:00000050-FACB-11E6-BD58-64006A7986D3 /mnt`, | ||
| } | ||
| ) | ||
|
|
||
| func init() { | ||
| registry.Commands = append(registry.Commands, registry.CliCommand{ | ||
| Command: server9pCommand, | ||
| Parent: machineCmd, | ||
| }) | ||
|
|
||
| flags := server9pCommand.Flags() | ||
|
|
||
| serveFlagName := "serve" | ||
| flags.StringArrayVar(&serveDirs, serveFlagName, []string{}, "directories to serve and UUID of vsock to serve on, colon-separated") | ||
| _ = server9pCommand.RegisterFlagCompletionFunc(serveFlagName, completion.AutocompleteNone) | ||
| } | ||
|
|
||
| var ( | ||
| serveDirs []string | ||
| ) | ||
|
|
||
| func remoteDirServer(cmd *cobra.Command, args []string) error { | ||
| pid, err := strconv.Atoi(args[0]) | ||
| if err != nil { | ||
| return fmt.Errorf("parsing PID: %w", err) | ||
| } | ||
| if pid < 0 { | ||
| return fmt.Errorf("PIDs cannot be negative") | ||
| } | ||
|
|
||
| if len(serveDirs) == 0 { | ||
| return fmt.Errorf("must provide at least one directory to serve") | ||
| } | ||
|
|
||
| // TODO: need to support options here | ||
| shares := make(map[string]string, len(serveDirs)) | ||
| for _, share := range serveDirs { | ||
| splitShare := strings.Split(share, ":") | ||
| if len(splitShare) < 2 { | ||
| return fmt.Errorf("paths passed to --share must include an hvsock GUID") | ||
| } | ||
|
|
||
| // Every element but the last one is the real filepath to share | ||
| path := strings.Join(splitShare[:len(splitShare)-1], ":") | ||
|
|
||
| shares[path] = splitShare[len(splitShare)-1] | ||
| } | ||
|
|
||
| if err := fileserver.StartShares(shares); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| p, err := psutil.NewProcess(int32(pid)) | ||
| if err != nil { | ||
| return fmt.Errorf("opening gvproxy PID: %w", err) | ||
| } | ||
| for { | ||
| running, err := p.IsRunning() | ||
| if err != nil { | ||
| return fmt.Errorf("checking is gvproxy is dead: %w", err) | ||
| } | ||
| if !running { | ||
| break | ||
| } | ||
|
|
||
| time.Sleep(1 * time.Second) | ||
| } | ||
|
|
||
| logrus.Infof("Exiting cleanly as PID %d has died", pid) | ||
|
|
||
| return nil | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not quite sure why this is needed? There seems to be a WaitForExit() call in windows which unlike linux should also work if the process you are waiting for is not a child. Are you something like https://stackoverflow.com/questions/37664015/os-process-wait-after-os-findprocesspid-works-on-windows-not-on-linux does not work and we really need such a big dep?
Alternative why can't we just use the same logic as for gvproxy stop and let the machine commands kill this process and let it sleep forever here instead of this busy loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The WaitForExit call is not accessible via Golang's standard library without DLL loading shenanigans and then calling untyped functions (well, the function itself is, but the ability to get the handle that it requires to work is not) - so this is a lot cleaner, even if it's technically gross.
I don't have objections to using a PID file and terminating on machine stop, though. @baude You have strong feelings on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Further, I'm strongly in favor of keeping the dep even if we drop this code. The things we were doing with Go's native
os.Processcode to do things like check for process exit just did not work - I solved 2 separate bugs elsewhere in the code by replacing our custom-rolled Windows/Linux process code with this library.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I you think it is needed sure, not like I tested it on windows. I just cannot believe that the go std lib wouldn't suffice for that use case but if it is like that then sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I can see, the standard library is fine on basically any POSIX-compliant system, but it starts to fall apart on Windows - things are sufficiently different that the abstractions can't handle it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree here, the stdlib on Windows does not provide the same functionality as you have on Posix-compliant systems. I think this is acceptable for now, as we might spin this code out at a later time.