diff --git a/runsc/cmd/restore.go b/runsc/cmd/restore.go index 5ee11b480..bc04e2e56 100644 --- a/runsc/cmd/restore.go +++ b/runsc/cmd/restore.go @@ -39,6 +39,13 @@ type Restore struct { // detach indicates that runsc has to start a process and exit without waiting it. detach bool + + // direct indicates whether O_DIRECT should be used for reading the + // checkpoint pages file. It is faster if the checkpoint files are not + // already in the page cache (for example if its coming from an untouched + // network block device). Usually the restore is done only once, so the cost + // of adding the checkpoint files to the page cache can be redundant. + direct bool } // Name implements subcommands.Command.Name. @@ -62,6 +69,7 @@ func (r *Restore) SetFlags(f *flag.FlagSet) { r.Create.SetFlags(f) f.StringVar(&r.imagePath, "image-path", "", "directory path to saved container image") f.BoolVar(&r.detach, "detach", false, "detach from the container's process") + f.BoolVar(&r.direct, "direct", false, "use O_DIRECT for reading checkpoint pages file") // Unimplemented flags necessary for compatibility with docker. @@ -138,7 +146,7 @@ func (r *Restore) Execute(_ context.Context, f *flag.FlagSet, args ...any) subco } log.Debugf("Restore: %v", r.imagePath) - if err := c.Restore(conf, r.imagePath); err != nil { + if err := c.Restore(conf, r.imagePath, r.direct); err != nil { return util.Errorf("starting container: %v", err) } diff --git a/runsc/container/container.go b/runsc/container/container.go index 87d8cb9bb..77cd5e7f7 100644 --- a/runsc/container/container.go +++ b/runsc/container/container.go @@ -525,7 +525,7 @@ func (c *Container) Start(conf *config.Config) error { // Restore takes a container and replaces its kernel and file system // to restore a container from its state file. -func (c *Container) Restore(conf *config.Config, imagePath string) error { +func (c *Container) Restore(conf *config.Config, imagePath string, direct bool) error { log.Debugf("Restore container, cid: %s", c.ID) if err := c.Saver.lock(BlockAcquire); err != nil { return err @@ -542,7 +542,7 @@ func (c *Container) Restore(conf *config.Config, imagePath string) error { log.Warningf("StartContainer hook skipped because running inside container namespace is not supported") } - if err := c.Sandbox.Restore(conf, c.ID, imagePath); err != nil { + if err := c.Sandbox.Restore(conf, c.ID, imagePath, direct); err != nil { return err } c.changeStatus(Running) diff --git a/runsc/container/container_test.go b/runsc/container/container_test.go index d896d9d89..4d31d77c2 100644 --- a/runsc/container/container_test.go +++ b/runsc/container/container_test.go @@ -1101,7 +1101,7 @@ func testCheckpointRestore(t *testing.T, conf *config.Config, compression statef } defer cont2.Destroy() - if err := cont2.Restore(conf, dir); err != nil { + if err := cont2.Restore(conf, dir, false /* direct */); err != nil { t.Fatalf("error restoring container: %v", err) } @@ -1144,7 +1144,7 @@ func testCheckpointRestore(t *testing.T, conf *config.Config, compression statef } defer cont3.Destroy() - if err := cont3.Restore(conf, dir); err != nil { + if err := cont3.Restore(conf, dir, false /* direct */); err != nil { t.Fatalf("error restoring container: %v", err) } @@ -1263,7 +1263,7 @@ func TestCheckpointRestoreExecKilled(t *testing.T) { } defer cont2.Destroy() - if err := cont2.Restore(conf, dir); err != nil { + if err := cont2.Restore(conf, dir, false /* direct */); err != nil { t.Fatalf("error restoring container: %v", err) } @@ -1377,7 +1377,7 @@ func TestUnixDomainSockets(t *testing.T) { } defer contRestore.Destroy() - if err := contRestore.Restore(conf, dir); err != nil { + if err := contRestore.Restore(conf, dir, false /* direct */); err != nil { t.Fatalf("error restoring container: %v", err) } diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index 930e6bd86..70b4fde60 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -443,7 +443,7 @@ func (s *Sandbox) StartSubcontainer(spec *specs.Spec, conf *config.Config, cid s } // Restore sends the restore call for a container in the sandbox. -func (s *Sandbox) Restore(conf *config.Config, cid string, imagePath string) error { +func (s *Sandbox) Restore(conf *config.Config, cid string, imagePath string, direct bool) error { log.Debugf("Restore sandbox %q", s.ID) stateFileName := path.Join(imagePath, boot.CheckpointStateFileName) @@ -459,9 +459,14 @@ func (s *Sandbox) Restore(conf *config.Config, cid string, imagePath string) err }, } - // If the image file exists, we must pass it in. + // If the pages file exists, we must pass it in. pagesFileName := path.Join(imagePath, boot.CheckpointPagesFileName) - if pf, err := os.Open(pagesFileName); err == nil { + pagesReadFlags := os.O_RDONLY + if direct { + // The contents are page-aligned, so it can be opened with O_DIRECT. + pagesReadFlags |= syscall.O_DIRECT + } + if pf, err := os.OpenFile(pagesFileName, pagesReadFlags, 0); err == nil { defer pf.Close() opt.HavePagesFile = true opt.FilePayload.Files = append(opt.FilePayload.Files, pf) @@ -1289,9 +1294,10 @@ func (s *Sandbox) Checkpoint(cid string, imagePath string, options statefile.Opt // When there is no compression, MemoryFile contents are page-aligned. // It is beneficial to store them separately so certain optimizations can be - // applied during restore. + // applied during restore. See Restore(). if options.Compression == statefile.CompressionLevelNone { pagesFilePath := filepath.Join(imagePath, boot.CheckpointPagesFileName) + // TODO(b/327603247): Implement optional async O_DIRECT write. pf, err := os.OpenFile(pagesFilePath, os.O_CREATE|os.O_EXCL|os.O_RDWR, 0644) if err != nil { return fmt.Errorf("creating checkpoint pages file %q: %w", pagesFilePath, err)