[pve-devel] [PATCH v2 storage 2/3] Incorporate wipe_disks from PVE::Ceph::Tools

Dominic Jäger d.jaeger at proxmox.com
Tue Feb 25 11:28:36 CET 2020


Move wipe_disks from PVE::Ceph::Tools to PVE::Diskmanage and improve it by
 - Handling invalid parameters
 - Adding options for wiping
 - Making names clearer, especially the sub name to indicate that it works for
   partitions, too.
 - Use `blockdev` system call because it is easier to understand
 - Adding tests

Relies on the corresponding patch in pve-manager.

Signed-off-by: Dominic Jäger <d.jaeger at proxmox.com>
---
v1->v2:
 - Fix typo
 - Not only move the function but also improve it

 PVE/Diskmanage.pm     | 63 ++++++++++++++++++++++++++++++++++++++
 test/disklist_test.pm | 71 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 134 insertions(+)

diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index abb90a7..09d89a0 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -761,4 +761,67 @@ sub append_partition {
     return $partition;
 }
 
+sub get_blockdev_size {
+    my ($dev) = @_;
+    my $size = `blockdev --getsize64 $dev`;
+    chomp $size;
+    return $size
+}
+
+# Wipe the first part of a device
+#
+# Calls dd and optionally wipefs. This is useful to clear off leftovers from
+# previous use as it avoids failing OSD and directory creation.
+#
+# $devices array of blockdevices (including partitions), e.g. ['/dev/sda3']
+# $options hash with
+#   - {amount} number of bytes that will be deleted. Will wipe at most the
+#	device size. Default: 209715200.
+#   - {wipefs} additionally erase all available signatures from all devices.
+#	Default: false
+sub wipe_blockdevices {
+    my ($devices, $options) = @_;
+
+    my @dd_cmd = qw(/bin/dd if=/dev/zero iflag=count_bytes conv=fdatasync);
+
+    my $max_amount = $options->{amount};
+    if (!defined $max_amount) {
+	$max_amount = 209715200;
+    } elsif ($max_amount !~ /^\d+$/) {
+	die "Dying because $max_amount is not a natural number ".
+	    "and thus not a valid value for max_amount.";
+    }
+
+    foreach my $dev (@$devices) {
+	eval { assert_blockdev($dev) };
+	if ($@) {
+	    warn "Skipping $dev because it is not a blockdevice";
+	    next;
+	}
+
+	my $device_size = get_blockdev_size($dev);
+	my $amount;
+	if (defined $device_size) {
+	    $amount = ($device_size < $max_amount) ? $device_size : $max_amount;
+	} else {
+	    warn "Could not get size of $dev. Wiping nonetheless but dd might throw errors.";
+	    $amount = $max_amount;
+	}
+
+	print "Wiping device: $dev\n";
+	if ($options->{wipefs}) {
+	    # wipefs location:
+	    #  - Debian  9: /sbin/wipefs
+	    #  - Debian 10: /usr/sbin/wipefs
+	    # The symlink from /sbin to /usr/sbin makes this work for fresh PVE
+	    # 6 as well as upgraded ones
+	    eval { run_command(['/sbin/wipefs', '-a', $dev]) };
+	    warn $@ if $@;
+	}
+	# 1M seems reasonable for this case
+	eval { run_command([@dd_cmd, "count=$amount", "bs=1M", "of=${dev}"]) };
+	warn $@ if $@;
+    }
+}
+
 1;
diff --git a/test/disklist_test.pm b/test/disklist_test.pm
index 9cb6763..280ea41 100644
--- a/test/disklist_test.pm
+++ b/test/disklist_test.pm
@@ -10,6 +10,7 @@ use PVE::Tools;
 
 use Test::MockModule;
 use Test::More;
+use Capture::Tiny;
 use JSON;
 use Data::Dumper;
 
@@ -248,4 +249,74 @@ while (readdir $dh) {
     test_disk_list($dir);
 }
 
+
+subtest 'wipe_blockdevices' => sub {
+
+    plan tests => 9;
+    sub wipe_helper {
+	my ($options, $devices) = @_;
+	$devices //= ['/dev/sdb'];
+	return Capture::Tiny::capture {
+	    PVE::Diskmanage::wipe_blockdevices($devices, $options);
+	};
+    }
+
+    $diskmanage_module->mock('run_command' => sub { });
+    my $device_size = 0;
+    $diskmanage_module->mock('get_blockdev_size' => sub { return $device_size });
+    {
+	eval { PVE::Diskmanage::wipe_blockdevices(['/dev/sdb'], {amount=>-3})};
+	like ($@, qr/-3 is not/, 'Die with negative amounts');
+    }
+    {
+	$diskmanage_module->mock('assert_blockdev' => sub { die });
+	my (undef, $stderr, undef) = wipe_helper();
+	like ($stderr, qr!Skipping /dev/sdb!, 'Skip invalid devices');
+	$diskmanage_module->mock('assert_blockdev' => sub { });
+    }
+    {
+	my ($stdout, undef, undef) = wipe_helper();
+	like ($stdout, qr!Wiping device: /dev/sdb\n!, 'Invoking with a disk');
+    }
+    {
+	my ($stdout, undef, undef) = wipe_helper(undef, ['/dev/sdb1']);
+	like ($stdout, qr!Wiping device: /dev/sdb1\n!, 'Invoking with a partition');
+    }
+
+    $diskmanage_module->mock('run_command' => sub { print Dumper(@_) });
+    my $default_max_wipe_amount = 209715200;
+    {
+	$device_size = $default_max_wipe_amount-1;
+	my ($stdout, undef, undef) = wipe_helper();
+	like ($stdout, qr!count=$device_size!,
+		'Device size is smaller than default maximum wipe amount');
+    }
+    {
+	$device_size = $default_max_wipe_amount+1;
+	my ($stdout, undef, undef) = wipe_helper();
+	like ($stdout, qr!count=$default_max_wipe_amount!,
+		'Device size is bigger than default maximum wipe amount');
+    }
+
+    my $changed_max_wipe_amount = 50000;
+    {
+	$device_size = $changed_max_wipe_amount-1;
+	my ($stdout, undef, undef) = wipe_helper({amount=>$changed_max_wipe_amount});
+	like ($stdout, qr!count=$device_size!,
+		'Device size is smaller than changed maximum wipe amount');
+    }
+    {
+	$device_size = $changed_max_wipe_amount+1;
+	my ($stdout, undef, undef) = wipe_helper({amount=>$changed_max_wipe_amount});
+	like ($stdout, qr!count=$changed_max_wipe_amount!,
+		'Device size is bigger than changed maximum wipe amount');
+    }
+    {
+	$device_size = 0;
+	my ($stdout, $stderr, undef) = wipe_helper({wipefs=>1});
+	like ($stdout, qr!/wipefs!, 'Call wipefs');
+    }
+    $testcount++;
+};
+
 done_testing($testcount);
-- 
2.20.1



More information about the pve-devel mailing list