yesBad/pve-storage
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

disk manage: module wide code/style cleanup

Browse Source 
fixing some issues reported by perlcritic along the way.

cutting down 70 lines, often with even improving readability.
Tried to recheck and be conservative, so there shouldn't be any
regression, but it's still perl after all...

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
This commit is contained in:
Thomas Lamprecht
2022-09-23 11:54:41 +02:00
parent 4c86c7111c
commit 9aff3f3d8e
1 changed files with 71 additions and 142 deletions
Download Patch File Download Diff File Expand all files Collapse all files

213
PVE/Diskmanage.pm
View File

@ -51,7 +51,7 @@ sub assert_blockdev {
my ($dev, $noerr) = @_; my ($dev, $noerr) = @_;
if ($dev !~ m|^/dev/| || !(-b $dev)) { if ($dev !~ m|^/dev/| || !(-b $dev)) {
return undef if $noerr; return if $noerr;
die "not a valid block device\n"; die "not a valid block device\n";
} }
@ -93,8 +93,6 @@ sub get_smart_data {
my $smartdata = {}; my $smartdata = {};
my $type; my $type;
my $returncode = 0;
if ($disk =~ m!^/dev/(nvme\d+n\d+)$!) { if ($disk =~ m!^/dev/(nvme\d+n\d+)$!) {
my $info = get_sysdir_info("/sys/block/$1"); my $info = get_sysdir_info("/sys/block/$1");
$disk = "/dev/".($info->{device} $disk = "/dev/".($info->{device}
@ -105,8 +103,8 @@ sub get_smart_data {
push @$cmd, '-A', '-f', 'brief' if !$healthonly; push @$cmd, '-A', '-f', 'brief' if !$healthonly;
push @$cmd, $disk; push @$cmd, $disk;
eval { my $returncode = eval {
$returncode = run_command($cmd, noerr => 1, outfunc => sub{ run_command($cmd, noerr => 1, outfunc => sub {
my ($line) = @_; my ($line) = @_;
# ATA SMART attributes, e.g.: # ATA SMART attributes, e.g.:
@ -154,13 +152,11 @@ sub get_smart_data {
} elsif ($line =~ m/SMART Disabled/) { } elsif ($line =~ m/SMART Disabled/) {
$smartdata->{health} = "SMART Disabled"; $smartdata->{health} = "SMART Disabled";
} }
}); })
}; };
my $err = $@; my $err = $@;
# bit 0 and 1 mark an severe smartctl error # bit 0 and 1 mark a fatal error, other bits are for disk status -> ignore (see man 8 smartctl)
# all others are for disk status, so ignore them
# see smartctl(8)
if ((defined($returncode) && ($returncode & 0b00000011)) || $err) { if ((defined($returncode) && ($returncode & 0b00000011)) || $err) {
die "Error getting S.M.A.R.T. data: Exit code: $returncode\n"; die "Error getting S.M.A.R.T. data: Exit code: $returncode\n";
} }
@ -170,34 +166,28 @@ sub get_smart_data {
return $smartdata; return $smartdata;
} }
sub get_lsblk_info() { sub get_lsblk_info {
my $cmd = [$LSBLK, '--json', '-o', 'path,parttype,fstype']; my $cmd = [$LSBLK, '--json', '-o', 'path,parttype,fstype'];
my $output = ""; my $output = "";
my $res = {}; eval { run_command($cmd, outfunc => sub { $output .= "$_[0]\n"; }) };
eval {
run_command($cmd, outfunc => sub {
my ($line) = @_;
$output .= "$line\n";
});
};
warn "$@\n" if $@; warn "$@\n" if $@;
return $res if $output eq ''; return {} if $output eq '';
my $parsed = eval { decode_json($output) }; my $parsed = eval { decode_json($output) } // {};
warn "$@\n" if $@; warn "$@\n" if $@;
my $list = $parsed->{blockdevices} // []; my $list = $parsed->{blockdevices} // [];
$res = { map { return {
$_->{path} => { map {
parttype => $_->{parttype}, $_->{path} => {
fstype => $_->{fstype} parttype => $_->{parttype},
} fstype => $_->{fstype}
} @{$list} }; }
} @{$list}
return $res; };
} }
my $get_devices_by_partuuid = sub { my sub get_devices_by_partuuid {
my ($lsblk_info, $uuids, $res) = @_; my ($lsblk_info, $uuids, $res) = @_;
$res = {} if !defined($res); $res = {} if !defined($res);
@ -209,7 +199,7 @@ my $get_devices_by_partuuid = sub {
} }
return $res; return $res;
}; }
sub get_zfs_devices { sub get_zfs_devices {
my ($lsblk_info) = @_; my ($lsblk_info) = @_;
@ -217,21 +207,17 @@ sub get_zfs_devices {
return {} if !check_bin($ZPOOL); return {} if !check_bin($ZPOOL);
# use zpool and parttype uuid, # use zpool and parttype uuid, because log and cache do not have zfs type uuid
# because log and cache do not have
# zfs type uuid
eval { eval {
run_command([$ZPOOL, 'list', '-HPLv'], outfunc => sub { run_command([$ZPOOL, 'list', '-HPLv'], outfunc => sub {
my ($line) = @_; my ($line) = @_;
if ($line =~ m|^\t([^\t]+)\t|) { if ($line =~ m|^\t([^\t]+)\t|) {
$res->{$1} = 1; $res->{$1} = 1;
} }
}); });
}; };
# only warn here, # only warn here, because maybe zfs tools are not installed
# because maybe zfs tools are not installed
warn "$@\n" if $@; warn "$@\n" if $@;
my $uuids = { my $uuids = {
@ -240,7 +226,7 @@ sub get_zfs_devices {
}; };
$res = $get_devices_by_partuuid->($lsblk_info, $uuids, $res); $res = get_devices_by_partuuid($lsblk_info, $uuids, $res);
return $res; return $res;
} }
@ -258,15 +244,14 @@ sub get_lvm_devices {
}); });
}; };
# if something goes wrong, we do not want # if something goes wrong, we do not want to give up, but indicate an error has occurred
# to give up, but indicate an error has occurred
warn "$@\n" if $@; warn "$@\n" if $@;
my $uuids = { my $uuids = {
"e6d6d379-f507-44c2-a23c-238f2a3df928" => 1, "e6d6d379-f507-44c2-a23c-238f2a3df928" => 1,
}; };
$res = $get_devices_by_partuuid->($lsblk_info, $uuids, $res); $res = get_devices_by_partuuid($lsblk_info, $uuids, $res);
return $res; return $res;
} }
@ -282,7 +267,7 @@ sub get_ceph_journals {
'cafecafe-9b03-4f30-b4c6-b4b80ceff106' => 4, # block 'cafecafe-9b03-4f30-b4c6-b4b80ceff106' => 4, # block
}; };
$res = $get_devices_by_partuuid->($lsblk_info, $uuids, $res); $res = get_devices_by_partuuid($lsblk_info, $uuids, $res);
return $res; return $res;
} }
@ -333,46 +318,31 @@ sub get_udev_info {
}); });
}; };
warn $@ if $@; warn $@ if $@;
return undef if !$info; return if !$info;
return undef if $info !~ m/^E: DEVTYPE=(disk|partition)$/m; return if $info !~ m/^E: DEVTYPE=(disk|partition)$/m;
return undef if $info =~ m/^E: ID_CDROM/m; return if $info =~ m/^E: ID_CDROM/m;
# we use this, because some disks are not simply in /dev # we use this, because some disks are not simply in /dev e.g. /dev/cciss/c0d0
# e.g. /dev/cciss/c0d0
if ($info =~ m/^E: DEVNAME=(\S+)$/m) { if ($info =~ m/^E: DEVNAME=(\S+)$/m) {
$data->{devpath} = $1; $data->{devpath} = $1;
} }
return if !defined($data->{devpath}); return if !defined($data->{devpath});
$data->{serial} = 'unknown'; $data->{serial} = 'unknown';
if ($info =~ m/^E: ID_SERIAL_SHORT=(\S+)$/m) { $data->{serial} = $1 if $info =~ m/^E: ID_SERIAL_SHORT=(\S+)$/m;
$data->{serial} = $1;
}
$data->{gpt} = 0; $data->{gpt} = $info =~ m/^E: ID_PART_TABLE_TYPE=gpt$/m ? 1 : 0;
if ($info =~ m/^E: ID_PART_TABLE_TYPE=gpt$/m) {
$data->{gpt} = 1;
}
# detect SSD
$data->{rpm} = -1; $data->{rpm} = -1;
if ($info =~ m/^E: ID_ATA_ROTATION_RATE_RPM=(\d+)$/m) { $data->{rpm} = $1 if $info =~ m/^E: ID_ATA_ROTATION_RATE_RPM=(\d+)$/m; # detects SSD implicit
$data->{rpm} = $1;
}
if ($info =~ m/^E: ID_BUS=usb$/m) { $data->{usb} = 1 if $info =~ m/^E: ID_BUS=usb$/m;
$data->{usb} = 1;
}
if ($info =~ m/^E: ID_MODEL=(.+)$/m) { $data->{model} = $1 if $info =~ m/^E: ID_MODEL=(.+)$/m;
$data->{model} = $1;
}
$data->{wwn} = 'unknown'; $data->{wwn} = 'unknown';
if ($info =~ m/^E: ID_WWN=(.*)$/m) { $data->{wwn} = $1 if $info =~ m/^E: ID_WWN=(.*)$/m;
$data->{wwn} = $1;
}
if ($info =~ m/^E: DEVLINKS=(.+)$/m) { if ($info =~ m/^E: DEVLINKS=(.+)$/m) {
my @devlinks = grep(m#^/dev/disk/by-id/(ata|scsi|nvme(?!-eui))#, split (/ /, $1)); my @devlinks = grep(m#^/dev/disk/by-id/(ata|scsi|nvme(?!-eui))#, split (/ /, $1));
@ -388,15 +358,14 @@ sub get_sysdir_size {
my $size = file_read_firstline("$sysdir/size"); my $size = file_read_firstline("$sysdir/size");
return if !$size; return if !$size;
# linux always considers sectors to be 512 bytes, # linux always considers sectors to be 512 bytes, independently of real block size
# independently of real block size
return $size * 512; return $size * 512;
} }
sub get_sysdir_info { sub get_sysdir_info {
my ($sysdir) = @_; my ($sysdir) = @_;
return undef if ! -d "$sysdir/device"; return if ! -d "$sysdir/device";
my $data = {}; my $data = {};
@ -409,8 +378,7 @@ sub get_sysdir_info {
$data->{model} = file_read_firstline("$sysdir/device/model") || 'unknown'; $data->{model} = file_read_firstline("$sysdir/device/model") || 'unknown';
if (defined(my $device = readlink("$sysdir/device"))) { if (defined(my $device = readlink("$sysdir/device"))) {
# strip directory and untaint: ($data->{device}) = $device =~ m!([^/]+)$!; # strip directory and untaint
($data->{device}) = $device =~ m!([^/]+)$!;
} }
return $data; return $data;
@ -426,9 +394,8 @@ sub get_wear_leveling_info {
my $wearout; my $wearout;
# Common register names that represent percentage values of potential # Common register names that represent percentage values of potential failure indicators used
# failure indicators used in drivedb.h of smartmontool's. Order matters, # in drivedb.h of smartmontool's. Order matters, as some drives may have multiple definitions
# as some drives may have multiple definitions
my @wearoutregisters = ( my @wearoutregisters = (
"Media_Wearout_Indicator", "Media_Wearout_Indicator",
"SSD_Life_Left", "SSD_Life_Left",
@ -553,18 +520,17 @@ sub get_disks {
dir_glob_foreach('/sys/block', $disk_regex, sub { dir_glob_foreach('/sys/block', $disk_regex, sub {
my ($dev) = @_; my ($dev) = @_;
# whitelisting following devices # whitelisting following devices
# hdX: ide block device # - hdX ide block device
# sdX: sd block device # - sdX scsi/sata block device
# vdX: virtual block device # - vdX virtIO block device
# xvdX: xen virtual block device # - xvdX: xen virtual block device
# nvmeXnY: nvme devices # - nvmeXnY: nvme devices
# cciss!cXnY: cciss devices # - cciss!cXnY cciss devices
return if $dev !~ m/^(h|s|x?v)d[a-z]+$/ && return if $dev !~ m/^(h|s|x?v)d[a-z]+$/ &&
$dev !~ m/^nvme\d+n\d+$/ && $dev !~ m/^nvme\d+n\d+$/ &&
$dev !~ m/^cciss\!c\d+d\d+$/; $dev !~ m/^cciss\!c\d+d\d+$/;
my $data = get_udev_info("/sys/block/$dev"); my $data = get_udev_info("/sys/block/$dev") // return;
return if !defined($data);
my $devpath = $data->{devpath}; my $devpath = $data->{devpath};
my $sysdir = "/sys/block/$dev"; my $sysdir = "/sys/block/$dev";
@ -590,26 +556,21 @@ sub get_disks {
} }
} }
my $health = 'UNKNOWN'; my ($health, $wearout) = ('UNKNOWN', 'N/A');
my $wearout = 'N/A';
if (!$nosmart) { if (!$nosmart) {
eval { eval {
my $smartdata = get_smart_data($devpath, !is_ssdlike($type)); my $smartdata = get_smart_data($devpath, !is_ssdlike($type));
$health = $smartdata->{health} if $smartdata->{health}; $health = $smartdata->{health} if $smartdata->{health};
if (is_ssdlike($type)) { if (is_ssdlike($type)) { # if we have an ssd we try to get the wearout indicator
# if we have an ssd we try to get the wearout indicator my $wear_level = get_wear_leveling_info($smartdata);
my $wearval = get_wear_leveling_info($smartdata); $wearout = $wear_level if defined($wear_level);
$wearout = $wearval if defined($wearval);
} }
}; };
} }
# we replaced cciss/ with cciss! above # we replaced cciss/ with cciss! above, but in the result we need cciss/ again because the
# but in the result we need cciss/ again # caller might want to check the result again with the original parameter
# because the caller might want to check the
# result again with the original parameter
if ($dev =~ m|^cciss!|) { if ($dev =~ m|^cciss!|) {
$dev =~ s|^cciss!|cciss/|; $dev =~ s|^cciss!|cciss/|;
} }
@ -632,19 +593,11 @@ sub get_disks {
my $by_id_link = $data->{by_id_link}; my $by_id_link = $data->{by_id_link};
$disklist->{$dev}->{by_id_link} = $by_id_link if defined($by_id_link); $disklist->{$dev}->{by_id_link} = $by_id_link if defined($by_id_link);
my $osdid = -1; my ($osdid, $bluestore, $osdencrypted) = (-1, 0, 0);
my $bluestore = 0; my ($journal_count, $db_count, $wal_count) = (0, 0, 0);
my $osdencrypted = 0;
my $journal_count = 0;
my $db_count = 0;
my $wal_count = 0;
my $partpath = $devpath; my $partpath = $devpath;
# remove trailing part to get the partition base path, e.g. /dev/cciss/c0d0 -> /dev/cciss
# remove part after last / to
# get the base path for the partitions
# e.g. from /dev/cciss/c0d0 get /dev/cciss
$partpath =~ s/\/[^\/]+$//; $partpath =~ s/\/[^\/]+$//;
my $determine_usage = sub { my $determine_usage = sub {
@ -655,18 +608,13 @@ sub get_disks {
my $info = $lsblk_info->{$devpath} // {}; my $info = $lsblk_info->{$devpath} // {};
my $parttype = $info->{parttype}; if (defined(my $parttype = $info->{parttype})) {
if (defined($parttype)) { return 'BIOS boot'if $parttype eq '21686148-6449-6e6f-744e-656564454649';
return 'BIOS boot' return 'EFI' if $parttype eq 'c12a7328-f81f-11d2-ba4b-00a0c93ec93b';
if $parttype eq '21686148-6449-6e6f-744e-656564454649'; return 'ZFS reserved' if $parttype eq '6a945a3b-1dd2-11b2-99a6-080020736631';
return 'EFI'
if $parttype eq 'c12a7328-f81f-11d2-ba4b-00a0c93ec93b';
return 'ZFS reserved'
if $parttype eq '6a945a3b-1dd2-11b2-99a6-080020736631';
} }
my $fstype = $info->{fstype}; return "$info->{fstype}" if defined($info->{fstype});
return "${fstype}" if defined($fstype);
return 'mounted' if $mounted->{$devpath}; return 'mounted' if $mounted->{$devpath};
return if !$is_partition; return if !$is_partition;
@ -691,13 +639,11 @@ sub get_disks {
} }
my $result = { %{$ceph_volume} }; my $result = { %{$ceph_volume} };
$result->{journals} = delete $result->{journal} $result->{journals} = delete $result->{journal} if $result->{journal};
if $result->{journal};
return $result; return $result;
}; };
my $partitions = {}; my $partitions = {};
dir_glob_foreach("$sysdir", "$dev.+", sub { dir_glob_foreach("$sysdir", "$dev.+", sub {
my ($part) = @_; my ($part) = @_;
@ -709,18 +655,14 @@ sub get_disks {
$partitions->{$part}->{mounted} = 1 if exists $mounted->{"$partpath/$part"}; $partitions->{$part}->{mounted} = 1 if exists $mounted->{"$partpath/$part"};
$partitions->{$part}->{gpt} = $data->{gpt}; $partitions->{$part}->{gpt} = $data->{gpt};
$partitions->{$part}->{type} = 'partition'; $partitions->{$part}->{type} = 'partition';
$partitions->{$part}->{size} = $partitions->{$part}->{size} = get_sysdir_size("$sysdir/$part") // 0;
get_sysdir_size("$sysdir/$part") // 0; $partitions->{$part}->{used} = $determine_usage->("$partpath/$part", "$sysdir/$part", 1);
$partitions->{$part}->{used} =
$determine_usage->("$partpath/$part", "$sysdir/$part", 1);
$partitions->{$part}->{osdid} //= -1; $partitions->{$part}->{osdid} //= -1;
# Avoid counting twice (e.g. partition on which the LVM for the # avoid counting twice (e.g. partition with the LVM for the DB OSD is in $journalhash)
# DB OSD resides is present in the $journalhash)
return if $lvm_based_osd; return if $lvm_based_osd;
# Legacy handling for non-LVM based OSDs # Legacy handling for non-LVM based OSDs
if (my $mp = $mounted->{"$partpath/$part"}) { if (my $mp = $mounted->{"$partpath/$part"}) {
if ($mp =~ m|^/var/lib/ceph/osd/ceph-(\d+)$|) { if ($mp =~ m|^/var/lib/ceph/osd/ceph-(\d+)$|) {
$osdid = $1; $osdid = $1;
@ -768,14 +710,11 @@ sub get_disks {
$disklist->{$dev}->{wal} = $wal_count if $wal_count; $disklist->{$dev}->{wal} = $wal_count if $wal_count;
if ($include_partitions) { if ($include_partitions) {
foreach my $part (keys %{$partitions}) { $disklist->{$_} = $partitions->{$_} for keys %{$partitions};
$disklist->{$part} = $partitions->{$part};
}
} }
}); });
return $disklist; return $disklist;
} }
sub get_partnum { sub get_partnum {
@ -790,19 +729,11 @@ sub get_partnum {
my $minor = PVE::Tools::dev_t_minor($st->rdev); my $minor = PVE::Tools::dev_t_minor($st->rdev);
my $partnum_path = "/sys/dev/block/$major:$minor/"; my $partnum_path = "/sys/dev/block/$major:$minor/";
my $partnum; my $partnum = file_read_firstline("${partnum_path}partition");
$partnum = file_read_firstline("${partnum_path}partition");
die "Partition does not exist\n" if !defined($partnum); die "Partition does not exist\n" if !defined($partnum);
die "Failed to get partition number\n" if $partnum !~ m/(\d+)/; # untaint
#untaint and ensure it is a int $partnum = $1;
if ($partnum =~ m/(\d+)/) { die "Partition number $partnum is invalid\n" if $partnum > 128;
$partnum = $1;
die "Partition number $partnum is invalid\n" if $partnum > 128;
} else {
die "Failed to get partition number\n";
}
return $partnum; return $partnum;
} }
@ -841,10 +772,8 @@ sub locked_disk_action {
sub assert_disk_unused { sub assert_disk_unused {
my ($dev) = @_; my ($dev) = @_;
die "device '$dev' is already in use\n" if disk_is_used($dev); die "device '$dev' is already in use\n" if disk_is_used($dev);
return;
return undef;
} }
sub append_partition { sub append_partition {