Move wipe_disks from PVE::Ceph::Tools to PVE::Diskmanage and improve it by
- Handling invalid parameters
- Adding options for wiping
- Making names clearer
- Adding tests
Relies on the corresponding patch in pve-manager.
Signed-off-by: Dominic Jäger <d.jae...@proxmox.com>
---
v2->v3:
- Relative instead of full paths
- Use run_command for system calls
- Die when a device is invalid instead of skipping
- More concise syntax
- More tests (with fake blockdevice path)
v1->v2:
- Fix typo
- Add improvements
@Dominik: Thank you for the feedback!
PVE/Diskmanage.pm | 58 ++++++++++++++++++++
test/disklist_test.pm | 125 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 183 insertions(+)
diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index abb90a7..448f753 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -761,4 +761,62 @@ sub append_partition {
return $partition;
}
+# Return undef on error, the size of the blockdev otherwise
+sub get_blockdev_size {
+ my ($dev) = @_;
+ if (!defined($dev)) {
+ warn "Undefined parameter";
+ return undef;
+ }
+ if (!assert_blockdev($dev, 1)) {
+ warn "$dev is not a blockdevice";
+ return undef;
+ }
+ my $size;
+ my $exitcode = eval { run_command(
+ ['blockdev', '--getsize64', $dev],
+ (outfunc => sub { $size = shift })
+ )};
+ warn "run_command for \'blockdev\' syscall failed: $@" if $@;
+ 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(dd if=/dev/zero iflag=count_bytes conv=fdatasync);
+ my $max_amount = $options->{amount} // 209715200;
+ die "$max_amount is not a natural number and thus an invalid value for
max_amount."
+ if ($max_amount !~ /^[0-9]+$/);
+
+ my %toDelete = ();
+ foreach my $dev (@$devices) {
+ my $device_size = get_blockdev_size($dev);
+ die "Not wiping anything because could not get size of $dev." if
!defined($device_size);
+ my $amount = ($device_size < $max_amount) ? $device_size : $max_amount;
+ $toDelete{$dev} = $amount;
+ }
+
+ foreach my $dev (keys %toDelete) {
+ print "Wiping device: $dev\n";
+ if ($options->{wipefs}) {
+ eval { run_command(['wipefs', '-a', $dev]) };
+ warn $@ if $@;
+ }
+ # 1M seems reasonable for this case
+ eval { run_command([@dd_cmd, "count=$toDelete{$dev}", "bs=1M",
"of=$dev"]) };
+ warn $@ if $@;
+ }
+}
+
1;
diff --git a/test/disklist_test.pm b/test/disklist_test.pm
index 9cb6763..e5d339b 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,128 @@ while (readdir $dh) {
test_disk_list($dir);
}
+subtest 'get_blockdev_size' => sub {
+ plan tests => 8;
+
+ my $blockdevice = '/fake/block/device';
+ my $msg = "Test";
+ $diskmanage_module->mock('assert_blockdev' => sub { return 1 });
+ {
+ $diskmanage_module->mock('run_command' => sub { die });
+ my (undef, $stderr, $exitcode) = Capture::Tiny::capture {
PVE::Diskmanage::get_blockdev_size(undef) };
+ is ($exitcode, undef, 'Return undef if parameter is undefined.');
+ like ($stderr, qr!Undefined parameter!, 'Print warning for undefined
parameter.');
+ }
+ {
+ $diskmanage_module->mock('assert_blockdev' => sub { return undef });
+ my (undef, $stderr, $exitcode) = Capture::Tiny::capture {
PVE::Diskmanage::get_blockdev_size($blockdevice) };
+ is ($exitcode, undef, 'Return undef if device is not a blockdevice.');
+ like ($stderr, qr!not a blockdev!, 'Print warning for invalid
blockdevices.');
+ $diskmanage_module->mock('assert_blockdev' => sub { return 1 });
+ }
+ {
+ $diskmanage_module->mock('run_command' => sub { return 0 });
+ my (undef, $stderr, $exitcode) = Capture::Tiny::capture {
PVE::Diskmanage::get_blockdev_size($blockdevice) };
+ is ($exitcode, undef, 'Return undef when reading size from stdout
failed.');
+ }
+ {
+ $diskmanage_module->mock('run_command' => sub { die });
+ my (undef, $stderr, $exitcode) = Capture::Tiny::capture {
PVE::Diskmanage::get_blockdev_size($blockdevice) };
+ is ($exitcode, undef, 'Return undef on error without message.');
+ }
+ {
+ $diskmanage_module->mock('run_command' => sub { die $msg});
+ my (undef, $stderr, $exitcode) = Capture::Tiny::capture {
PVE::Diskmanage::get_blockdev_size($blockdevice) };
+ is ($exitcode, undef, 'Return undef on error with message.');
+ like ($stderr, qr!${msg}!, 'Print error message from run_command as
warning.');
+ }
+ $testcount++;
+};
+
+subtest 'wipe_blockdevices' => sub {
+ plan tests => 12;
+
+ our $blockdevice = '/fake/block/device';
+ sub wipe_helper {
+ my ($options, $devices) = @_;
+ $devices //= [$blockdevice];
+ return Capture::Tiny::capture {
+ PVE::Diskmanage::wipe_blockdevices($devices, $options);
+ };
+ }
+
+ my $device_size = 0;
+ $diskmanage_module->mock('run_command' => sub { });
+ $diskmanage_module->mock('get_blockdev_size' => sub { return $device_size
});
+ {
+ eval { PVE::Diskmanage::wipe_blockdevices([$blockdevice],
{amount=>-3})};
+ like ($@, qr/-3 is not/, 'Die with negative amounts');
+ }
+ {
+ $diskmanage_module->mock('get_blockdev_size' => sub { return undef });
+ eval { PVE::Diskmanage::wipe_blockdevices([$blockdevice])};
+ like ($@, qr!Not wiping anything!, 'Die when helper returns undef');
+ $diskmanage_module->mock('get_blockdev_size' => sub { return
$device_size });
+ }
+ {
+ $diskmanage_module->unmock('get_blockdev_size');
+ $diskmanage_module->mock('assert_blockdev' => sub { return undef });
+ my (undef, $stderr, $exitcode) = Capture::Tiny::capture {
+ eval { PVE::Diskmanage::wipe_blockdevices([$blockdevice]) }
+ };
+ like ($@, qr!Not wiping anything!, 'Die if helper finds an invalid
device');
+ like ($stderr, qr!is not a blockdev!, 'Helper prints a reason for
dying');
+ $diskmanage_module->mock('get_blockdev_size' => sub { return
$device_size });
+ }
+ {
+ $diskmanage_module->mock('get_blockdev_size' => sub { return undef });
+ eval { PVE::Diskmanage::wipe_blockdevices([$blockdevice])};
+ like ($@, qr!Not wiping anything!, 'Die with invalid devices');
+ $diskmanage_module->mock('get_blockdev_size' => sub { return
$device_size });
+ }
+ {
+ my ($stdout, undef, undef) = wipe_helper();
+ like ($stdout, qr!Wiping device: $blockdevice\n!, 'Invoking with a
disk');
+ }
+ {
+ my ($stdout, undef, undef) = wipe_helper(undef, ["${blockdevice}1"]);
+ like ($stdout, qr!Wiping device: ${blockdevice}1\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
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
