core/mount: should not call removeLoop when set autoclear

fuweid · fuweid · commit 41a69eb0d19c · 2025-11-27T12:32:37.000-05:00
In CI we run make root-test via gotestsum, which executes multiple package tests concurrently. TestAutoclearTrueLoop attempts to invoke LOOP_CLR_FD using a device name, which introduces a race condition. Example race: Process P1 represents mount.test which runs TestAutoclearTrueLoop Process P2 represents manager.test which runs TestLoopbackMount T1: P1 closes fd of loop-device (loop3) (kernel unsets backing-file on close) T2: P2 gets loop3 from /dev/loop-control T3: P2 configures loop3 with backing file successfully T4: P1 invokes removeLoop to clear backing file for loop3 You might see that failure like this ``` === FAIL: core/mount/manager TestLoopbackMount (0.05s) log_hook.go:47: time="2025-10-23T21:49:22.532811960Z" level=debug msg="activating mount" func="manager.(*mountManager).Activate" file="/home/runner/work/containerd/containerd/core/mount/manager/manager.go:134" mounts="[{loop /tmp/TestLoopbackMount989607109/001/fs-1621892597 []} {format/ext4 {{ mount 0 }} []}]" name=id1 testcase=TestLoopbackMount helpers.go:100: unmount /tmp/TestLoopbackMount989607109/001/test-mount-3030342351 manager_linux_test.go:80: Error Trace: /home/runner/work/containerd/containerd/core/mount/manager/manager_linux_test.go:80 /home/runner/work/containerd/containerd/core/mount/manager/manager_linux_test.go:105 Error: Received unexpected error: failed to get loop device info: no such device or address Test: TestLoopbackMount ``` To fix this, the test now compares backing-file's inode directly and does not call removeLoop when autoclear is set. Signed-off-by: Wei Fu <fuweid89@gmail.com> (cherry picked from commit a5c8402) Signed-off-by: Wei Fu <fuweid89@gmail.com>
diff --git a/core/mount/loopback_handler_linux.go b/core/mount/loopback_handler_linux.go
@@ -18,6 +18,7 @@ package mount
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"time"
 
@@ -79,7 +80,7 @@ func (loopbackHandler) Unmount(ctx context.Context, path string) error {
 	defer loop.Close()
 
 	if err := setLoopAutoclear(loop, true); err != nil {
-		return err
+		return fmt.Errorf("failed to set auto clear on loop device %q: %w", loopdev, err)
 	}
 
 	if err := os.Remove(path); err != nil {
diff --git a/core/mount/losetup_linux_test.go b/core/mount/losetup_linux_test.go
@@ -17,13 +17,17 @@
 package mount
 
 import (
+	"errors"
+	"fmt"
 	"os"
 	"path/filepath"
+	"syscall"
 	"testing"
 	"time"
 
 	"github.com/containerd/continuity/testutil"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/sys/unix"
 )
 
 var randomData = []byte("randomdata")
@@ -100,23 +104,50 @@ func TestAttachDetachLoopDevice(t *testing.T) {
 func TestAutoclearTrueLoop(t *testing.T) {
 	testutil.RequiresRoot(t)
 
-	dev := func() string {
-		backingFile := createTempFile(t)
+	backingFile := createTempFile(t)
+	bInfo, err := os.Stat(backingFile)
+	require.NoError(t, err)
+	bInode := bInfo.Sys().(*syscall.Stat_t).Ino
+
+	file, err := SetupLoop(backingFile, LoopParams{Autoclear: true})
+	require.NoError(t, err)
+	dev := file.Name()
+	file.Close()
+
+	var checkFn = func(loopDev string, expectedInode uint64) (_shouldRetry bool, _ error) {
+		loop, err := os.Open(loopDev)
+		if err != nil {
+			if os.IsNotExist(err) {
+				return false, nil
+			}
+			return false, fmt.Errorf("failed to open loop device: %w", err)
+		}
+		info, err := unix.IoctlLoopGetStatus64(int(loop.Fd()))
+		loop.Close()
+		if err != nil {
+			if errors.Is(err, unix.ENXIO) {
+				return false, nil
+			}
+			return false, fmt.Errorf("failed to get loop device info: %w", err)
+		}
+
+		if info.Inode != expectedInode {
+			return false, nil
+		}
+
+		t.Logf("loop device %s still present with backing inode %d", loopDev, expectedInode)
+		return true, nil
+	}
 
-		file, err := SetupLoop(backingFile, LoopParams{Autoclear: true})
-		require.NoError(t, err)
-		dev := file.Name()
-		file.Close()
-		return dev
-	}()
 	for range 10 {
-		if err := removeLoop(dev); err != nil {
-			// Expected to fail as Autoclear should have already removed the loop device
+		retry, err := checkFn(dev, bInode)
+		require.NoError(t, err)
+		if !retry {
 			return
 		}
-		time.Sleep(20 * time.Millisecond)
+		time.Sleep(100 * time.Millisecond)
 	}
-	t.Fatalf("removeLoop should fail if Autoclear is true")
+	t.Fatalf("loop device %s still present after autoclear", dev)
 }
 
 func TestAutoclearFalseLoop(t *testing.T) {
