diff --git a/runsc/boot/controller.go b/runsc/boot/controller.go index 86f06bff1..f09c1bd85 100644 --- a/runsc/boot/controller.go +++ b/runsc/boot/controller.go @@ -211,12 +211,6 @@ type StartArgs struct { func (cm *containerManager) Start(args *StartArgs, _ *struct{}) error { log.Debugf("containerManager.Start: %+v", args) - defer func() { - for _, f := range args.FilePayload.Files { - f.Close() - } - }() - // Validate arguments. if args == nil { return errors.New("start missing arguments") @@ -305,21 +299,19 @@ type RestoreOpts struct { func (cm *containerManager) Restore(o *RestoreOpts, _ *struct{}) error { log.Debugf("containerManager.Restore") - var specFile *os.File - deviceFD := -1 + var specFile, deviceFile *os.File switch numFiles := len(o.FilePayload.Files); numFiles { case 2: - var err error // The device file is donated to the platform. // Can't take ownership away from os.File. dup them to get a new FD. - deviceFD, err = syscall.Dup(int(o.FilePayload.Files[1].Fd())) + fd, err := syscall.Dup(int(o.FilePayload.Files[1].Fd())) if err != nil { return fmt.Errorf("failed to dup file: %v", err) } + deviceFile = os.NewFile(uintptr(fd), "platform device") fallthrough case 1: specFile = o.FilePayload.Files[0] - defer specFile.Close() case 0: return fmt.Errorf("at least one file must be passed to Restore") default: @@ -331,7 +323,7 @@ func (cm *containerManager) Restore(o *RestoreOpts, _ *struct{}) error { cm.l.k.Pause() cm.l.k.Destroy() - p, err := createPlatform(cm.l.conf, deviceFD) + p, err := createPlatform(cm.l.conf, deviceFile) if err != nil { return fmt.Errorf("creating platform: %v", err) } @@ -357,7 +349,7 @@ func (cm *containerManager) Restore(o *RestoreOpts, _ *struct{}) error { if eps, ok := networkStack.(*epsocket.Stack); ok { stack.StackFromEnv = eps.Stack // FIXME(b/36201077) } - info, err := o.FilePayload.Files[0].Stat() + info, err := specFile.Stat() if err != nil { return err } @@ -366,9 +358,7 @@ func (cm *containerManager) Restore(o *RestoreOpts, _ *struct{}) error { } // Load the state. - loadOpts := state.LoadOpts{ - Source: o.FilePayload.Files[0], - } + loadOpts := state.LoadOpts{Source: specFile} if err := loadOpts.Load(k, networkStack); err != nil { return err } diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index 05122a6a8..6ac6b94dd 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -152,8 +152,8 @@ type Args struct { Conf *Config // ControllerFD is the FD to the URPC controller. ControllerFD int - // DeviceFD is an optional argument that is passed to the platform. - DeviceFD int + // Device is an optional argument that is passed to the platform. + Device *os.File // GoferFDs is an array of FDs used to connect with the Gofer. GoferFDs []int // StdioFDs is the stdio for the application. @@ -183,7 +183,7 @@ func New(args Args) (*Loader, error) { } // Create kernel and platform. - p, err := createPlatform(args.Conf, args.DeviceFD) + p, err := createPlatform(args.Conf, args.Device) if err != nil { return nil, fmt.Errorf("creating platform: %v", err) } @@ -401,17 +401,17 @@ func (l *Loader) Destroy() { l.watchdog.Stop() } -func createPlatform(conf *Config, deviceFD int) (platform.Platform, error) { +func createPlatform(conf *Config, deviceFile *os.File) (platform.Platform, error) { switch conf.Platform { case PlatformPtrace: log.Infof("Platform: ptrace") return ptrace.New() case PlatformKVM: log.Infof("Platform: kvm") - if deviceFD < 0 { - return nil, fmt.Errorf("kvm device FD must be provided") + if deviceFile == nil { + return nil, fmt.Errorf("kvm device file must be provided") } - return kvm.New(os.NewFile(uintptr(deviceFD), "kvm device")) + return kvm.New(deviceFile) default: return nil, fmt.Errorf("invalid platform %v", conf.Platform) } @@ -590,18 +590,22 @@ func (l *Loader) startContainer(k *kernel.Kernel, spec *specs.Spec, conf *Config return fmt.Errorf("creating new process: %v", err) } + // setupContainerFS() dups stdioFDs, so we don't need to dup them here. + var stdioFDs []int + for _, f := range files[:3] { + stdioFDs = append(stdioFDs, int(f.Fd())) + } + // Can't take ownership away from os.File. dup them to get a new FDs. - var ioFDs []int - for _, f := range files { + var goferFDs []int + for _, f := range files[3:] { fd, err := syscall.Dup(int(f.Fd())) if err != nil { return fmt.Errorf("failed to dup file: %v", err) } - ioFDs = append(ioFDs, fd) + goferFDs = append(goferFDs, fd) } - stdioFDs := ioFDs[:3] - goferFDs := ioFDs[3:] if err := setupContainerFS( &procArgs, spec, @@ -616,13 +620,6 @@ func (l *Loader) startContainer(k *kernel.Kernel, spec *specs.Spec, conf *Config return fmt.Errorf("configuring container FS: %v", err) } - // setFileSystemForProcess dup'd stdioFDs, so we can close them. - for i, fd := range stdioFDs { - if err := syscall.Close(fd); err != nil { - return fmt.Errorf("closing stdio FD #%d: %v", i, fd) - } - } - ctx := procArgs.NewContext(l.k) mns := k.RootMountNamespace() if err := setExecutablePath(ctx, mns, &procArgs); err != nil { diff --git a/runsc/boot/loader_test.go b/runsc/boot/loader_test.go index 9a864ad3f..4603f751d 100644 --- a/runsc/boot/loader_test.go +++ b/runsc/boot/loader_test.go @@ -115,7 +115,6 @@ func createLoader() (*Loader, func(), error) { Spec: spec, Conf: conf, ControllerFD: fd, - DeviceFD: -1, GoferFDs: []int{sandEnd}, StdioFDs: stdio, } diff --git a/runsc/cmd/boot.go b/runsc/cmd/boot.go index ac937f7bc..3a547d4aa 100644 --- a/runsc/cmd/boot.go +++ b/runsc/cmd/boot.go @@ -213,7 +213,7 @@ func (b *Boot) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) Spec: spec, Conf: conf, ControllerFD: b.controllerFD, - DeviceFD: b.deviceFD, + Device: os.NewFile(uintptr(b.deviceFD), "platform device"), GoferFDs: b.ioFDs.GetArray(), StdioFDs: b.stdioFDs.GetArray(), Console: b.console,