volume/local: Don't unmount, restore mounted status

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 2689484402)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit is contained in:
Paweł Gronowski 2023-08-17 16:44:28 +02:00 committed by Sebastiaan van Stijn
parent ecdca2b06f
commit 8dac634ebd
No known key found for this signature in database
GPG key ID: 76698F39D527CE8C
3 changed files with 41 additions and 8 deletions

View file

@ -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
}
@ -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()
}

View file

@ -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
@ -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 {

View file

@ -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 {