From 2c7f96e9c5d740d11958a403208623d42feb1800 Mon Sep 17 00:00:00 2001
From: "zhuangbowei.zbw" <zhuangbowei.zbw@alibaba-inc.com>
Date: Fri, 6 Sep 2024 17:47:48 +0800
Subject: [PATCH] check if snapshot holds mountpoint before remove

Signed-off-by: zhuangbowei.zbw <zhuangbowei.zbw@alibaba-inc.com>
---
 internal/log/helper.go  | 30 ++++++++++++++++++++++++++++++
 pkg/snapshot/overlay.go | 27 +++++++++++++++++++++++++--
 pkg/snapshot/storage.go |  2 +-
 3 files changed, 56 insertions(+), 3 deletions(-)
 create mode 100644 internal/log/helper.go

diff --git a/internal/log/helper.go b/internal/log/helper.go
new file mode 100644
index 00000000..c6115bad
--- /dev/null
+++ b/internal/log/helper.go
@@ -0,0 +1,30 @@
+/*
+   Copyright The Accelerated Container Image Authors
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+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
+}
diff --git a/pkg/snapshot/overlay.go b/pkg/snapshot/overlay.go
index a83e2d8e..90e4d03e 100644
--- a/pkg/snapshot/overlay.go
+++ b/pkg/snapshot/overlay.go
@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"strings"
 	"syscall"
@@ -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"
@@ -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)
 			}
 		}
 	}
@@ -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")
@@ -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)
diff --git a/pkg/snapshot/storage.go b/pkg/snapshot/storage.go
index bd5736f8..92bcba0a 100644
--- a/pkg/snapshot/storage.go
+++ b/pkg/snapshot/storage.go
@@ -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 {