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