From aa83e53ebbab1606c3bcb8d34cbe5be81c40a017 Mon Sep 17 00:00:00 2001 From: Lou Date: Wed, 4 Dec 2019 16:51:52 +0800 Subject: [PATCH 1/2] use xfs_repair to check and repair xfs filesystem Signed-off-by: Lou --- mount/mount_linux.go | 93 +++++++++++++++++++++-------- mount/safe_format_and_mount_test.go | 59 ++++++++++++++---- 2 files changed, 115 insertions(+), 37 deletions(-) diff --git a/mount/mount_linux.go b/mount/mount_linux.go index 77d01e78..88febbb0 100644 --- a/mount/mount_linux.go +++ b/mount/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 { - // Verify that the disk is formatted with filesystem type we are expecting - mountErrorValue = FilesystemMismatch - klog.Warningf("Configured to mount disk %s as %s but current format is %s, things might break", source, existingFormat, fstype) + } else { + if fstype != existingFormat { + // Verify that the disk is formatted with filesystem type we are expecting + 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 diff --git a/mount/safe_format_and_mount_test.go b/mount/safe_format_and_mount_test.go index 17de98b1..5a872713 100644 --- a/mount/safe_format_and_mount_test.go +++ b/mount/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 { From 08f269abf66d473682af7cdddfdfb0a3c8bb279d Mon Sep 17 00:00:00 2001 From: Lou Date: Thu, 9 Jan 2020 20:21:21 +0800 Subject: [PATCH 2/2] update after review Signed-off-by: Lou --- mount/mount_linux.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mount/mount_linux.go b/mount/mount_linux.go index 88febbb0..88a46424 100644 --- a/mount/mount_linux.go +++ b/mount/mount_linux.go @@ -294,7 +294,7 @@ func (mounter *SafeFormatAndMount) checkAndRepairXfsFilesystem(source string) er 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) + return NewMountError(HasFilesystemErrors, "'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