Skip to content

Commit

Permalink
check if snapshot holds mountpoint before remove
Browse files Browse the repository at this point in the history
  • Loading branch information
WaberZhuang committed Sep 6, 2024
1 parent 1fce068 commit 05aeff4
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 3 deletions.
14 changes: 14 additions & 0 deletions internal/log/helper.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package log

import (
"context"
"fmt"

clog "github.com/containerd/log"
)

func TracedErrorf(ctx context.Context, format string, args ...any) error {
err := fmt.Errorf(format, args...)
clog.G(ctx).Error(err)
return err
}
27 changes: 25 additions & 2 deletions pkg/snapshot/overlay.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"fmt"
"os"
"os/exec"
"path/filepath"
"strings"
"syscall"
Expand All @@ -29,6 +30,7 @@ import (
"github.com/containerd/accelerated-container-image/pkg/label"
"github.com/containerd/accelerated-container-image/pkg/snapshot/diskquota"

mylog "github.com/containerd/accelerated-container-image/internal/log"
"github.com/containerd/accelerated-container-image/pkg/metrics"
"github.com/data-accelerator/zdfs"
"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -944,7 +946,7 @@ func (o *snapshotter) Remove(ctx context.Context, key string) (err error) {
if err == nil {
err = o.unmountAndDetachBlockDevice(ctx, id, key)
if err != nil {
return errors.Wrapf(err, "failed to destroy target device for snapshot %s", key)
return mylog.TracedErrorf(ctx, "failed to destroy target device for snapshot %s: %w", key, err)
}
}
}
Expand All @@ -961,12 +963,20 @@ func (o *snapshotter) Remove(ctx context.Context, key string) (err error) {
if st, err := o.identifySnapshotStorageType(ctx, s.ParentIDs[0], info); err == nil && st != storageTypeNormal {
err = o.unmountAndDetachBlockDevice(ctx, s.ParentIDs[0], "")
if err != nil {
return errors.Wrapf(err, "failed to destroy target device for snapshot %s", key)
return mylog.TracedErrorf(ctx, "failed to destroy target device for snapshot %s: %w", key, err)
}
}
}
}

// Just in case, check if snapshot contains mountpoint
mounted, err := o.hasMountpoint(ctx, id)
if err != nil {
return mylog.TracedErrorf(ctx, "failed to check mountpoint: %w", err)
} else if mounted {
return mylog.TracedErrorf(ctx, "try to remove snapshot %s which still have mountpoint", id)
}

_, _, err = storage.Remove(ctx, key)
if err != nil {
return errors.Wrap(err, "failed to remove")
Expand All @@ -980,6 +990,19 @@ func (o *snapshotter) Remove(ctx context.Context, key string) (err error) {
return nil
}

func (o *snapshotter) hasMountpoint(ctx context.Context, id string) (bool, error) {
if out, err := exec.CommandContext(ctx, "/bin/bash", "-c",
fmt.Sprintf("mount -l | grep %s", o.overlaybdMountpoint(id)),
).CombinedOutput(); err == nil {
log.G(ctx).Infof("snapshot %s has mountpoint: %s", id, string(out))
return true, nil
} else if string(out) == "" {
return false, nil
} else {
return false, err
}
}

// Walk the snapshots.
func (o *snapshotter) Walk(ctx context.Context, fn snapshots.WalkFunc, fs ...string) (err error) {
log.G(ctx).Infof("Walk (fs: %s)", fs)
Expand Down
2 changes: 1 addition & 1 deletion pkg/snapshot/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func (o *snapshotter) parseAndCheckMounted(ctx context.Context, r io.Reader, dir
return false, nil
}

// unmountAndDetachBlockDevice
// unmountAndDetachBlockDevice will do nothing if the device is already destroyed
func (o *snapshotter) unmountAndDetachBlockDevice(ctx context.Context, snID string, snKey string) (err error) {
devName, err := os.ReadFile(o.overlaybdBackstoreMarkFile(snID))
if err != nil {
Expand Down

0 comments on commit 05aeff4

Please sign in to comment.