Merge pull request #350 from Aaron-ding-coder/z.d/dev

Fix: prevent snapshotter from deleting in-use devices during snapshot removal

Author: BigVan

.gitignore

@@ -2,4 +2,5 @@ bin/
 .vscode
 vendor/
 tmp_conv/
-tmp/
+tmp/
+.idea

pkg/snapshot/docker.go

@@ -236,7 +236,7 @@ func (o *snapshotter) RemoveDockerLayer(ctx context.Context, key string, initID
 	if parentStype == storageTypeLocalBlock || parentStype == storageTypeRemoteBlock {
 		if _, err := os.Stat(o.overlaybdBackstoreMarkFile(parentID)); err == nil {
 			log.G(ctx).Infof("RemoveDockerLayer: destroying overlaybd device for parent %s", parentID)
-			if err = o.UnmountAndDetachBlockDevice(ctx, parentID); err != nil {
+			if err = o.UnmountAndDetachBlockDevice(ctx, parentID, parentInfo.Name, key); err != nil {
 				log.G(ctx).Warnf("failed to destroy overlaybd device for parent %s: %v", parentID, err)
 				// Don't block remove, device might still be in use by other containers
 			}

pkg/snapshot/overlay.go

@@ -853,7 +853,7 @@ func (o *snapshotter) Commit(ctx context.Context, name, key string, opts ...snap
 		} else if _, err := o.loadBackingStoreConfig(id); err != nil {
 			log.G(ctx).Info("not an overlaybd writable layer")
 		} else {
-			if err := o.UnmountAndDetachBlockDevice(ctx, id); err != nil {
+			if err := o.UnmountAndDetachBlockDevice(ctx, id, key, ""); err != nil {
 				return fmt.Errorf("failed to destroy target device for snapshot %s: %w", key, err)
 			}
 
@@ -1047,7 +1047,7 @@ func (o *snapshotter) Remove(ctx context.Context, key string) (err error) {
 	if stype != storageTypeNormal {
 		_, err = os.Stat(o.overlaybdBackstoreMarkFile(id))
 		if err == nil {
-			err = o.UnmountAndDetachBlockDevice(ctx, id)
+			err = o.UnmountAndDetachBlockDevice(ctx, id, key, "")
 			if err != nil {
 				return mylog.TracedErrorf(ctx, "failed to destroy target device for snapshot %s: %w", key, err)
 			}
@@ -1071,7 +1071,12 @@ func (o *snapshotter) Remove(ctx context.Context, key string) (err error) {
 
 			log.G(ctx).Debugf("try to verify parent snapshots format.")
 			if st, err := o.identifySnapshotStorageType(ctx, s.ParentIDs[0], info); err == nil && st != storageTypeNormal {
-				err = o.UnmountAndDetachBlockDevice(ctx, s.ParentIDs[0])
+				// Get parent snapshot info for key and parent
+				_, parentInfo, _, err := storage.GetInfo(ctx, info.Parent)
+				if err != nil {
+					return mylog.TracedErrorf(ctx, "failed to get parent snapshot info for %s: %w", s.ParentIDs[0], err)
+				}
+				err = o.UnmountAndDetachBlockDevice(ctx, s.ParentIDs[0], parentInfo.Name, key)
 				if err != nil {
 					return mylog.TracedErrorf(ctx, "failed to destroy target device for snapshot %s: %w", key, err)
 				}

pkg/snapshot/storage.go

@@ -231,14 +231,13 @@ func DetachDevice(ctx context.Context, snID string, tenant int) (err error) {
 }
 
 // UnmountAndDetachBlockDevice will do nothing if the device is already destroyed
-func (o *snapshotter) UnmountAndDetachBlockDevice(ctx context.Context, snID string) (err error) {
+func (o *snapshotter) UnmountAndDetachBlockDevice(ctx context.Context, snID string, key string, upperDirKey string) (err error) {
 	devName, err := os.ReadFile(o.overlaybdBackstoreMarkFile(snID))
 	if err != nil {
 		log.G(ctx).Errorf("read device name failed: %s, err: %v", o.overlaybdBackstoreMarkFile(snID), err)
 	}
 
 	mountPoint := o.overlaybdMountpoint(snID)
-	log.G(ctx).Debugf("check overlaybd mountpoint is in use: %s", mountPoint)
 	busy, err := o.checkOverlaybdInUse(ctx, mountPoint)
 	if err != nil {
 		return err
@@ -247,6 +246,18 @@ func (o *snapshotter) UnmountAndDetachBlockDevice(ctx context.Context, snID stri
 		log.G(ctx).Infof("device still in use.")
 		return nil
 	}
+
+	if upperDirKey != "" {
+		inuse, err := checkIsParent(ctx, key, upperDirKey)
+		if err != nil {
+			return fmt.Errorf("failed to check whether snID %v check key %v err: %v", snID, key, err)
+		}
+		if inuse {
+			log.G(ctx).Infof("device is parent to someone else, ignore umount")
+			return nil
+		}
+	}
+
 	log.G(ctx).Infof("umount device, mountpoint: %s", mountPoint)
 	if err := mount.UnmountAll(mountPoint, 0); err != nil {
 		return fmt.Errorf("failed to umount %s: %w", mountPoint, err)
@@ -257,7 +268,24 @@ func (o *snapshotter) UnmountAndDetachBlockDevice(ctx context.Context, snID stri
 	}
 	log.G(ctx).Infof("destroy overlaybd device success(sn: %s): %s", snID, devName)
 	return nil
+}
 
+// checkIsParent checks if the given parent is a parent of any other active snapshot
+func checkIsParent(ctx context.Context, key, exclude string) (bool, error) {
+	found := false
+	if err := storage.WalkInfo(ctx, func(ctx context.Context, info snapshots.Info) error {
+		if found {
+			return nil
+		}
+
+		if info.Name != exclude && info.Parent == key && (info.Kind == snapshots.KindActive || info.Kind == snapshots.KindView) {
+			found = true
+		}
+		return nil
+	}); err != nil {
+		return false, err
+	}
+	return found, nil
 }
 
 // IsErofsFilesystem determines whether the block device represented