use xfs_repair to check and repair xfs filesystem

Signed-off-by: Lou <luogj@cn.ibm.com>

mount_linux.go

@@ -256,6 +256,54 @@ func (mounter *Mounter) GetMountRefs(pathname string) ([]string, error) {
 	return SearchMountPoints(realpath, procMountInfoPath)
 }
 
+// checkAndRepairFileSystem checks and repairs filesystems using command fsck.
+func (mounter *SafeFormatAndMount) checkAndRepairFilesystem(source string) error {
+	klog.V(4).Infof("Checking for issues with fsck on disk: %s", source)
+	args := []string{"-a", source}
+	out, err := mounter.Exec.Command("fsck", args...).CombinedOutput()
+	if err != nil {
+		ee, isExitError := err.(utilexec.ExitError)
+		switch {
+		case err == utilexec.ErrExecutableNotFound:
+			klog.Warningf("'fsck' not found on system; continuing mount without running 'fsck'.")
+		case isExitError && ee.ExitStatus() == fsckErrorsCorrected:
+			klog.Infof("Device %s has errors which were corrected by fsck.", source)
+		case isExitError && ee.ExitStatus() == fsckErrorsUncorrected:
+			return NewMountError(HasFilesystemErrors, "'fsck' found errors on device %s but could not correct them: %s", source, string(out))
+		case isExitError && ee.ExitStatus() > fsckErrorsUncorrected:
+			klog.Infof("`fsck` error %s", string(out))
+		}
+	}
+	return nil
+}
+
+// checkAndRepairXfsFilesystem checks and repairs xfs filesystem using command xfs_repair.
+func (mounter *SafeFormatAndMount) checkAndRepairXfsFilesystem(source string) error {
+	klog.V(4).Infof("Checking for issues with xfs_repair on disk: %s", source)
+
+	args := []string{source}
+	checkArgs := []string{"-n", source}
+
+	// check-only using "xfs_repair -n", if the exit status is not 0, perform a "xfs_repair"
+	_, err := mounter.Exec.Command("xfs_repair", checkArgs...).CombinedOutput()
+	if err != nil {
+		if err == utilexec.ErrExecutableNotFound {
+			klog.Warningf("'xfs_repair' not found on system; continuing mount without running 'xfs_repair'.")
+			return nil
+		} else {
+			klog.Warningf("Filesystem corruption was detected for %s, running xfs_repair to repair", source)
+			out, err := mounter.Exec.Command("xfs_repair", args...).CombinedOutput()
+			if err != nil {
+				return fmt.Errorf("'xfs_repair' found errors on device %s but could not correct them: %s\n", source, out)
+			} else {
+				klog.Infof("Device %s has errors which were corrected by xfs_repair.", source)
+				return nil
+			}
+		}
+	}
+	return nil
+}
+
 // formatAndMount uses unix utils to format and mount the given disk
 func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, fstype string, options []string) error {
 	readOnly := false
@@ -269,26 +317,6 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string,
 	options = append(options, "defaults")
 	var mountErrorValue MountErrorType
 
-	if !readOnly {
-		// Run fsck on the disk to fix repairable issues, only do this for volumes requested as rw.
-		klog.V(4).Infof("Checking for issues with fsck on disk: %s", source)
-		args := []string{"-a", source}
-		out, err := mounter.Exec.Command("fsck", args...).CombinedOutput()
-		if err != nil {
-			ee, isExitError := err.(utilexec.ExitError)
-			switch {
-			case err == utilexec.ErrExecutableNotFound:
-				klog.Warningf("'fsck' not found on system; continuing mount without running 'fsck'.")
-			case isExitError && ee.ExitStatus() == fsckErrorsCorrected:
-				klog.Infof("Device %s has errors which were corrected by fsck.", source)
-			case isExitError && ee.ExitStatus() == fsckErrorsUncorrected:
-				return NewMountError(HasFilesystemErrors, "'fsck' found errors on device %s but could not correct them: %s", source, string(out))
-			case isExitError && ee.ExitStatus() > fsckErrorsUncorrected:
-				klog.Infof("`fsck` error %s", string(out))
-			}
-		}
-	}
-
 	// Check if the disk is already formatted
 	existingFormat, err := mounter.GetDiskFormat(source)
 	if err != nil {
@@ -324,10 +352,27 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string,
 		}
 
 		klog.Infof("Disk successfully formatted (mkfs): %s - %s %s", fstype, source, target)
-	} else if fstype != existingFormat {
+	} else {
-		// Verify that the disk is formatted with filesystem type we are expecting
+		if fstype != existingFormat {
-		mountErrorValue = FilesystemMismatch
+			// Verify that the disk is formatted with filesystem type we are expecting
-		klog.Warningf("Configured to mount disk %s as %s but current format is %s, things might break", source, existingFormat, fstype)
+			mountErrorValue = FilesystemMismatch
+			klog.Warningf("Configured to mount disk %s as %s but current format is %s, things might break", source, existingFormat, fstype)
+		}
+
+		if !readOnly {
+			// Run check tools on the disk to fix repairable issues, only do this for formatted volumes requested as rw.
+			var err error
+			switch existingFormat {
+			case "xfs":
+				err = mounter.checkAndRepairXfsFilesystem(source)
+			default:
+				err = mounter.checkAndRepairFilesystem(source)
+			}
+
+			if err != nil {
+				return err
+			}
+		}
 	}
 
 	// Mount the disk

safe_format_and_mount_test.go

@@ -79,8 +79,8 @@ func TestSafeFormatAndMount(t *testing.T) {
 			description: "Test a normal mount of an already formatted device",
 			fstype:      "ext4",
 			execScripts: []ExecArgs{
-				{"fsck", []string{"-a", "/dev/foo"}, "", nil},
 				{"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil},
+				{"fsck", []string{"-a", "/dev/foo"}, "", nil},
 			},
 		},
 		{
@@ -96,7 +96,6 @@ func TestSafeFormatAndMount(t *testing.T) {
 			description: "Test a normal mount of unformatted device",
 			fstype:      "ext4",
 			execScripts: []ExecArgs{
-				{"fsck", []string{"-a", "/dev/foo"}, "", nil},
 				{"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}},
 				{"mkfs.ext4", []string{"-F", "-m0", "/dev/foo"}, "", nil},
 			},
@@ -105,8 +104,8 @@ func TestSafeFormatAndMount(t *testing.T) {
 			description: "Test 'fsck' fails with exit status 4",
 			fstype:      "ext4",
 			execScripts: []ExecArgs{
-				{"fsck", []string{"-a", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 4}},
 				{"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil},
+				{"fsck", []string{"-a", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 4}},
 			},
 			expectedError: fmt.Errorf("'fsck' found errors on device /dev/foo but could not correct them"),
 		},
@@ -114,16 +113,16 @@ func TestSafeFormatAndMount(t *testing.T) {
 			description: "Test 'fsck' fails with exit status 1 (errors found and corrected)",
 			fstype:      "ext4",
 			execScripts: []ExecArgs{
-				{"fsck", []string{"-a", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 1}},
 				{"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil},
+				{"fsck", []string{"-a", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 1}},
 			},
 		},
 		{
 			description: "Test 'fsck' fails with exit status other than 1 and 4 (likely unformatted device)",
 			fstype:      "ext4",
 			execScripts: []ExecArgs{
-				{"fsck", []string{"-a", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 8}},
 				{"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil},
+				{"fsck", []string{"-a", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 8}},
 			},
 		},
 		{
@@ -131,8 +130,8 @@ func TestSafeFormatAndMount(t *testing.T) {
 			fstype:      "ext4",
 			mountErrs:   []error{fmt.Errorf("unknown filesystem type '(null)'")},
 			execScripts: []ExecArgs{
-				{"fsck", []string{"-a", "/dev/foo"}, "", nil},
 				{"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nPTTYPE=dos\n", nil},
+				{"fsck", []string{"-a", "/dev/foo"}, "", nil},
 			},
 			expectedError: fmt.Errorf("unknown filesystem type '(null)'"),
 		},
@@ -141,7 +140,6 @@ func TestSafeFormatAndMount(t *testing.T) {
 			fstype:      "ext4",
 			mountErrs:   []error{fmt.Errorf("unknown filesystem type '(null)'")},
 			execScripts: []ExecArgs{
-				{"fsck", []string{"-a", "/dev/foo"}, "", nil},
 				{"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}},
 				{"mkfs.ext4", []string{"-F", "-m0", "/dev/foo"}, "", fmt.Errorf("formatting failed")},
 			},
@@ -152,7 +150,6 @@ func TestSafeFormatAndMount(t *testing.T) {
 			fstype:      "ext4",
 			mountErrs:   []error{fmt.Errorf("unknown filesystem type '(null)'")},
 			execScripts: []ExecArgs{
-				{"fsck", []string{"-a", "/dev/foo"}, "", nil},
 				{"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}},
 				{"mkfs.ext4", []string{"-F", "-m0", "/dev/foo"}, "", nil},
 			},
@@ -162,7 +159,6 @@ func TestSafeFormatAndMount(t *testing.T) {
 			description: "Test that 'blkid' is called and confirms unformatted disk, format passes, mount passes",
 			fstype:      "ext4",
 			execScripts: []ExecArgs{
-				{"fsck", []string{"-a", "/dev/foo"}, "", nil},
 				{"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}},
 				{"mkfs.ext4", []string{"-F", "-m0", "/dev/foo"}, "", nil},
 			},
@@ -172,7 +168,6 @@ func TestSafeFormatAndMount(t *testing.T) {
 			description: "Test that 'blkid' is called and confirms unformatted disk, format passes, mount passes with ext3",
 			fstype:      "ext3",
 			execScripts: []ExecArgs{
-				{"fsck", []string{"-a", "/dev/foo"}, "", nil},
 				{"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}},
 				{"mkfs.ext3", []string{"-F", "-m0", "/dev/foo"}, "", nil},
 			},
@@ -182,7 +177,6 @@ func TestSafeFormatAndMount(t *testing.T) {
 			description: "test that none ext4 fs does not get called with ext4 options.",
 			fstype:      "xfs",
 			execScripts: []ExecArgs{
-				{"fsck", []string{"-a", "/dev/foo"}, "", nil},
 				{"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}},
 				{"mkfs.xfs", []string{"/dev/foo"}, "", nil},
 			},
@@ -192,8 +186,8 @@ func TestSafeFormatAndMount(t *testing.T) {
 			description: "Test that 'blkid' is called and reports ext4 partition",
 			fstype:      "ext4",
 			execScripts: []ExecArgs{
-				{"fsck", []string{"-a", "/dev/foo"}, "", nil},
 				{"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil},
+				{"fsck", []string{"-a", "/dev/foo"}, "", nil},
 			},
 		},
 		{
@@ -201,12 +195,51 @@ func TestSafeFormatAndMount(t *testing.T) {
 			fstype:      "xfs",
 			mountErrs:   []error{fmt.Errorf("unknown filesystem type '(null)'"), nil},
 			execScripts: []ExecArgs{
-				{"fsck", []string{"-a", "/dev/foo"}, "", nil},
 				{"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 4}},
 				{"mkfs.xfs", []string{"/dev/foo"}, "", nil},
 			},
 			expectedError: fmt.Errorf("exit 4"),
 		},
+		{
+			description: "Test that 'xfs_repair' is called only once, no need to repair the filesystem",
+			fstype:      "xfs",
+			execScripts: []ExecArgs{
+				{"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=xfs\n", nil},
+				{"xfs_repair", []string{"-n", "/dev/foo"}, "", nil},
+			},
+			expectedError: nil,
+		},
+		{
+			description: "Test that 'xfs_repair' is called twice and repair the filesystem",
+			fstype:      "xfs",
+			execScripts: []ExecArgs{
+				{"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=xfs\n", nil},
+				{"xfs_repair", []string{"-n", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 1}},
+				{"xfs_repair", []string{"/dev/foo"}, "\ndone\n", nil},
+			},
+			expectedError: nil,
+		},
+		{
+			description: "Test that 'xfs_repair' is called twice and repair the filesystem, but mount failed",
+			fstype:      "xfs",
+			mountErrs:   []error{fmt.Errorf("unknown filesystem type '(null)'")},
+			execScripts: []ExecArgs{
+				{"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=xfs\n", nil},
+				{"xfs_repair", []string{"-n", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 1}},
+				{"xfs_repair", []string{"/dev/foo"}, "\ndone\n", nil},
+			},
+			expectedError: fmt.Errorf("unknown filesystem type '(null)'"),
+		},
+		{
+			description: "Test that 'xfs_repair' is called twice but could not repair the filesystem",
+			fstype:      "xfs",
+			execScripts: []ExecArgs{
+				{"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=xfs\n", nil},
+				{"xfs_repair", []string{"-n", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 1}},
+				{"xfs_repair", []string{"/dev/foo"}, "\nAn error occurred\n", &testingexec.FakeExitError{Status: 1}},
+			},
+			expectedError: fmt.Errorf("'xfs_repair' found errors on device %s but could not correct them: %v", "/dev/foo", "\nAn error occurred\n"),
+		},
 	}
 
 	for _, test := range tests {