Skip to content

Commit

Permalink
volume/local: Don't unmount, restore mounted status
Browse files Browse the repository at this point in the history
On startup all local volumes were unmounted as a cleanup mechanism for
the non-clean exit of the last engine process.

This caused live-restored volumes that used special volume opt mount
flags to be broken. While the refcount was restored, the _data directory
was just unmounted, so all new containers mounting this volume would
just have the access to the empty _data directory instead of the real
volume.

With this patch, the mountpoint isn't unmounted. Instead, if the volume
is already mounted, just mark it as mounted, so the next time Mount is
called only the ref count is incremented, but no second attempt to mount
it is performed.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
(cherry picked from commit 2689484)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
  • Loading branch information
vvoland authored and thaJeztah committed Aug 29, 2023
1 parent 5d4cc0b commit c35376c
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 8 deletions.
17 changes: 13 additions & 4 deletions volume/local/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,18 @@ func New(scope string, rootIdentity idtools.Identity) (*Root, error) {
quotaCtl: r.quotaCtl,
}

// unmount anything that may still be mounted (for example, from an
// unclean shutdown). This is a no-op on windows
unmount(v.path)

if err := v.loadOpts(); err != nil {
return nil, err
}

if err := v.restoreIfMounted(); err != nil {
log.G(context.TODO()).WithFields(log.Fields{
"volume": v.name,
"path": v.path,
"error": err,
}).Warn("restoreIfMounted failed")
}

r.volumes[name] = v
}

Expand Down Expand Up @@ -338,6 +343,10 @@ func (v *localVolume) Unmount(id string) error {
return nil
}

if !v.active.mounted {
return nil
}

logger.Debug("Unmounting volume")
return v.unmount()
}
Expand Down
27 changes: 23 additions & 4 deletions volume/local/local_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,6 @@ func (v *localVolume) setOpts(opts map[string]string) error {
return v.saveOpts()
}

func unmount(path string) {
_ = mount.Unmount(path)
}

func (v *localVolume) needsMount() bool {
if v.opts == nil {
return false
Expand Down Expand Up @@ -163,6 +159,29 @@ func (v *localVolume) unmount() error {
return nil
}

// restoreIfMounted restores the mounted status if the _data directory is already mounted.
func (v *localVolume) restoreIfMounted() error {
if v.needsMount() {
// Check if the _data directory is already mounted.
mounted, err := mountinfo.Mounted(v.path)
if err != nil {
return fmt.Errorf("failed to determine if volume _data path is already mounted: %w", err)
}

if mounted {
// Mark volume as mounted, but don't increment active count. If
// any container needs this, the refcount will be incremented
// by the live-restore (if enabled).
// In other case, refcount will be zero but the volume will
// already be considered as mounted when Mount is called, and
// only the refcount will be incremented.
v.active.mounted = true
}
}

return nil
}

func (v *localVolume) CreatedAt() (time.Time, error) {
fileInfo, err := os.Stat(v.rootPath)
if err != nil {
Expand Down
5 changes: 5 additions & 0 deletions volume/local/local_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ func (v *localVolume) postMount() error {
return nil
}

// restoreIfMounted is a no-op on Windows (because mounts are not supported).
func (v *localVolume) restoreIfMounted() error {
return nil
}

func (v *localVolume) CreatedAt() (time.Time, error) {
fileInfo, err := os.Stat(v.rootPath)
if err != nil {
Expand Down

0 comments on commit c35376c

Please sign in to comment.