From 4cf36b8b3946b0fd2abf5f651c3d4e232021eced Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Mon, 20 Mar 2017 13:08:13 +0100 Subject: [PATCH] Do not reformat devices with partitions lsblk reports FSTYPE of devices with partition tables as empty string "", which is indistinguishable from empty devices. We must look for dependent devices (i.e. partitions) to see that the device is really empty and report error otherwise. I checked that LVM, LUKS and MD RAID have their own FSTYPE in lsblk output, so it should be only a partition table that has empty FSTYPE. The main point of this patch is to run lsblk without "-n", i.e. print all dependent devices and check if they're there. --- pkg/util/mount/mount_linux.go | 28 ++++++++++++----- pkg/util/mount/safe_format_and_mount_test.go | 32 ++++++++++++++++---- 2 files changed, 47 insertions(+), 13 deletions(-) diff --git a/pkg/util/mount/mount_linux.go b/pkg/util/mount/mount_linux.go index 366101be22..1685ecb48f 100644 --- a/pkg/util/mount/mount_linux.go +++ b/pkg/util/mount/mount_linux.go @@ -386,7 +386,7 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, return mountErr } else { // Block device is formatted with unexpected filesystem, let the user know - return fmt.Errorf("failed to mount the volume as %q, it's already formatted with %q. Mount error: %v", fstype, existingFormat, mountErr) + return fmt.Errorf("failed to mount the volume as %q, it already contains %s. Mount error: %v", fstype, existingFormat, mountErr) } } } @@ -395,19 +395,33 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, // diskLooksUnformatted uses 'lsblk' to see if the given disk is unformated func (mounter *SafeFormatAndMount) getDiskFormat(disk string) (string, error) { - args := []string{"-nd", "-o", "FSTYPE", disk} + args := []string{"-n", "-o", "FSTYPE", disk} cmd := mounter.Runner.Command("lsblk", args...) glog.V(4).Infof("Attempting to determine if disk %q is formatted using lsblk with args: (%v)", disk, args) dataOut, err := cmd.CombinedOutput() - output := strings.TrimSpace(string(dataOut)) - - // TODO (#13212): check if this disk has partitions and return false, and - // an error if so. + output := string(dataOut) + glog.V(4).Infof("Output: %q", output) if err != nil { glog.Errorf("Could not determine if disk %q is formatted (%v)", disk, err) return "", err } - return strings.TrimSpace(output), nil + // Split lsblk output into lines. Unformatted devices should contain only + // "\n". Beware of "\n\n", that's a device with one empty partition. + output = strings.TrimSuffix(output, "\n") // Avoid last empty line + lines := strings.Split(output, "\n") + if lines[0] != "" { + // The device is formatted + return lines[0], nil + } + + if len(lines) == 1 { + // The device is unformatted and has no dependent devices + return "", nil + } + + // The device has dependent devices, most probably partitions (LVM, LUKS + // and MD RAID are reported as FSTYPE and caught above). + return "unknown data, probably partitions", nil } diff --git a/pkg/util/mount/safe_format_and_mount_test.go b/pkg/util/mount/safe_format_and_mount_test.go index a6669080e3..15f46f3e38 100644 --- a/pkg/util/mount/safe_format_and_mount_test.go +++ b/pkg/util/mount/safe_format_and_mount_test.go @@ -98,7 +98,7 @@ func TestSafeFormatAndMount(t *testing.T) { mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'")}, execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, - {"lsblk", []string{"-nd", "-o", "FSTYPE", "/dev/foo"}, "ext4", nil}, + {"lsblk", []string{"-n", "-o", "FSTYPE", "/dev/foo"}, "ext4\n", nil}, }, expectedError: fmt.Errorf("unknown filesystem type '(null)'"), }, @@ -108,7 +108,7 @@ func TestSafeFormatAndMount(t *testing.T) { mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'")}, execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, - {"lsblk", []string{"-nd", "-o", "FSTYPE", "/dev/foo"}, "", nil}, + {"lsblk", []string{"-n", "-o", "FSTYPE", "/dev/foo"}, "\n", nil}, {"mkfs.ext4", []string{"-F", "/dev/foo"}, "", fmt.Errorf("formatting failed")}, }, expectedError: fmt.Errorf("formatting failed"), @@ -119,7 +119,7 @@ func TestSafeFormatAndMount(t *testing.T) { mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), fmt.Errorf("Still cannot mount")}, execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, - {"lsblk", []string{"-nd", "-o", "FSTYPE", "/dev/foo"}, "", nil}, + {"lsblk", []string{"-n", "-o", "FSTYPE", "/dev/foo"}, "\n", nil}, {"mkfs.ext4", []string{"-F", "/dev/foo"}, "", nil}, }, expectedError: fmt.Errorf("Still cannot mount"), @@ -130,7 +130,7 @@ func TestSafeFormatAndMount(t *testing.T) { mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), nil}, execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, - {"lsblk", []string{"-nd", "-o", "FSTYPE", "/dev/foo"}, "", nil}, + {"lsblk", []string{"-n", "-o", "FSTYPE", "/dev/foo"}, "\n", nil}, {"mkfs.ext4", []string{"-F", "/dev/foo"}, "", nil}, }, expectedError: nil, @@ -141,7 +141,7 @@ func TestSafeFormatAndMount(t *testing.T) { mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), nil}, execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, - {"lsblk", []string{"-nd", "-o", "FSTYPE", "/dev/foo"}, "", nil}, + {"lsblk", []string{"-n", "-o", "FSTYPE", "/dev/foo"}, "\n", nil}, {"mkfs.ext3", []string{"-F", "/dev/foo"}, "", nil}, }, expectedError: nil, @@ -152,11 +152,31 @@ func TestSafeFormatAndMount(t *testing.T) { mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), nil}, execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, - {"lsblk", []string{"-nd", "-o", "FSTYPE", "/dev/foo"}, "", nil}, + {"lsblk", []string{"-n", "-o", "FSTYPE", "/dev/foo"}, "\n", nil}, {"mkfs.xfs", []string{"/dev/foo"}, "", nil}, }, expectedError: nil, }, + { + description: "Test that 'lsblk' is called and reports ext4 partition", + fstype: "ext3", + mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'")}, + execScripts: []ExecArgs{ + {"fsck", []string{"-a", "/dev/foo"}, "", nil}, + {"lsblk", []string{"-n", "-o", "FSTYPE", "/dev/foo"}, "\next4\n", nil}, + }, + expectedError: fmt.Errorf("failed to mount the volume as \"ext3\", it already contains unknown data, probably partitions. Mount error: unknown filesystem type '(null)'"), + }, + { + description: "Test that 'lsblk' is called and reports empty partition", + fstype: "ext3", + mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'")}, + execScripts: []ExecArgs{ + {"fsck", []string{"-a", "/dev/foo"}, "", nil}, + {"lsblk", []string{"-n", "-o", "FSTYPE", "/dev/foo"}, "\n\n", nil}, + }, + expectedError: fmt.Errorf("failed to mount the volume as \"ext3\", it already contains unknown data, probably partitions. Mount error: unknown filesystem type '(null)'"), + }, } for _, test := range tests {