[pve-devel] [PATCH storage v2 1/2] add Diskmanage Utilities

Wolfgang Bumiller w.bumiller at proxmox.com
Mon Jun 6 13:34:23 CEST 2016


On Mon, Jun 06, 2016 at 10:46:02AM +0200, Dominik Csapak wrote:
> this adds the functions for listing the disks (mostly copied from
> the ceph code), checking if a disk is a valid blockdevice, if it
> is used/in a zfs pool/as an lvm pv, and an init function (just to add a gpt header;
> this is important if one wants to use a fresh disk for ceph journals)
> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>  PVE/Diskmanage.pm | 372 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  PVE/Makefile      |   1 +
>  2 files changed, 373 insertions(+)
>  create mode 100644 PVE/Diskmanage.pm
> 
> diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
> new file mode 100644
> index 0000000..6e18113
> --- /dev/null
> +++ b/PVE/Diskmanage.pm
> @@ -0,0 +1,372 @@
> +package PVE::Diskmanage;
> +
> +use strict;
> +use warnings;
> +use PVE::ProcFSTools;
> +use Data::Dumper;
> +use Cwd qw(abs_path);
> +
> +use PVE::Tools qw(extract_param run_command file_get_contents file_read_firstline dir_glob_regex dir_glob_foreach trim);
> +
> +my $SMARTCTL = "/usr/sbin/smartctl";
> +my $ZPOOL = "/sbin/zpool";
> +my $SGDISK = "/sbin/sgdisk";
> +my $PVS = "/sbin/pvs";
> +my $UDEVADM = "/bin/udevadm";
> +
> +sub verify_blockdev_path {
> +    my ($rel_path) = @_;
> +
> +    die "missing path" if !$rel_path;
> +    my $path = abs_path($rel_path);
> +    die "failed to get absolute path to $rel_path\n" if !$path;
> +
> +    die "got unusual device path '$path'\n" if $path !~  m|^/dev/(.*)$|;
> +
> +    $path = "/dev/$1"; # untaint
> +
> +    die "no such block device '$path'\n"
> +	if ! -b $path;

"Not a block device" might make more sense, since abs_path() says the
path at least exists.

> +
> +    return $path;
> +};
> +
> +sub init_disk {
> +    my ($disk, $uuid) = @_;
> +
> +    die "not a valid disk" if $disk !~ m|^/dev/| ||  ! -b $disk;

It might be worth factorizing out this check considering it's done below
again with differently worded error messages. Would improve consistency.

> +    # we should already have checked if it is in use in the api call
> +    # but we check again for safety
> +    die "disk $disk is already in use\n" if disk_is_used($disk);
> +
> +    my $id = $uuid || 'R';
> +    eval {
> +	run_command([$SGDISK, $disk, '-U', $id]);
> +    };
> +    die "ERROR: $@" if $@;
> +}
> +
> +sub disk_is_used {
> +    my ($disk) = @_;
> +
> +    my $dev = $disk;
> +    $dev =~ s|^/dev/||;
> +
> +    my $disklist = get_disks($dev);
> +
> +    die "'$disk' is not a valid local disk\n" if !defined($disklist->{$dev});
> +    return 1 if $disklist->{$dev}->{used};
> +
> +    return 0;
> +}
> +
> +sub get_smart_data {
> +    my ($disk) = @_;
> +
> +    die "not a device" if $disk !~ m|^/dev/|;

Should this contain a -b check as well? If so, it can use the
factorized version as suggested above. Note the wording used in this
error.

> +    my $smartdata = {};
> +    my $datastarted = 0;
> +
> +    eval {
> +	run_command([$SMARTCTL, '-a', '-f', 'brief', $disk], outfunc => sub{
> +	    my ($line) = @_;
> +
> +	    if ($datastarted && $line =~ m/^[ \d]{2}\d/) {
> +		$line = trim($line);
> +		my @data = split /\s+/, $line;
> +		my $entry = {};
> +		$entry->{name} = $data[1];
> +		$entry->{flags} = $data[2];
> +		$entry->{value} = $data[3];
> +		$entry->{worst} = $data[4];
> +		$entry->{threshold} = $data[5];
> +		$entry->{fail} = $data[6];
> +		$entry->{raw} = $data[7];
> +		$entry->{id} = $data[0];
> +		push @{$smartdata->{attributes}}, $entry;
> +	    } elsif ($line =~ m/self\-assessment test result: (.*)$/) {
> +		$smartdata->{health} = $1;
> +	    } elsif ($line =~ m/Vendor Specific SMART Attributes with Thresholds:/) {
> +		$datastarted = 1;
> +	    }
> +	});
> +    };
> +    die "Error getting S.M.A.R.T. data: $@\n" if $@;
> +    $smartdata->{health} = 'UNKOWN' if !defined $smartdata->{health};
> +    return $smartdata;
> +}
> +
> +sub get_smart_health {
> +    my ($disk) = @_;
> +
> +    return "NOT A DEVICE" if $disk !~ m|^/dev/| || ! -b $disk;

This is the same check as the second one I marked and uses a different
and capitalized error message. So again, an assert_blockdev($) function
with a sane/consistent error message would be useful here.


> +    my $message = "UNKOWN";
> +
> +    eval {
> +	run_command([$SMARTCTL, '-H', $disk], outfunc => sub{
> +	    my ($line) = @_;
> +
> +	    if ($line =~ m/test result: (.*)$/) {
> +		$message = $1;
> +	    } elsif ($line =~ m/open device: (.*) failed: (.*)$/) {
> +		$message = "FAILED TO OPEN";
> +	    } elsif ($line =~ m/^SMART Disabled/) {
> +		$message = "SMART DISABLED";
> +	    }
> +	});
> +    };
> +
> +    return $message;
> +}
> +
> +sub get_zfs_devices {
> +    my $list = {};
> +    eval {
> +	run_command([$ZPOOL, 'list', '-HLv'], outfunc => sub {
> +	     my ($line) = @_;
> +
> +	     if ($line =~ m|^\t([^\t]+)\t|) {
> +		$list->{$1} = 1;
> +	     }
> +	});
> +    };
> +    return $list;
> +}
> +
> +sub get_lvm_devices {
> +    my $list = {};
> +    eval {
> +	run_command([$PVS, '--noheadings', '--readonly', '-o', 'pv_name'], outfunc => sub{
> +	    my ($line) = @_;
> +	    $line = trim($line);
> +	    if ($line =~ m|^/dev/|) {
> +		$list->{$line} = 1;
> +	    }
> +	});
> +    };
> +    return $list;
> +}
> +
> +sub get_disks {
> +    my ($disk) = @_;
> +    my $disklist = {};
> +
> +    my $fd = IO::File->new("/proc/mounts", "r") ||
> +	die "unable to open /proc/mounts - $!\n";

Looks like left-over code. You use parse_proc_mounts() below and this
$fd is not used at all anymore.

> +
> +    my $mounted = {};
> +
> +    my $mounts = PVE::ProcFSTools::parse_proc_mounts();
> +
> +    foreach my $mount (@$mounts) {
> +	next if $mount->[0] !~ m|^/dev/|;
> +	$mounted->{abs_path($mount->[0])} = $mount->[1];
> +    };
> +
> +    my $dev_is_mounted = sub {
> +	my ($dev) = @_;
> +	return $mounted->{$dev};
> +    };
> +
> +    my $dir_is_empty = sub {
> +	my ($dir) = @_;
> +
> +	my $dh = IO::Dir->new ($dir);
> +	return 1 if !$dh;
> +
> +	while (defined(my $tmp = $dh->read)) {
> +	    next if $tmp eq '.' || $tmp eq '..';
> +	    $dh->close;
> +	    return 0;
> +	}
> +	$dh->close;
> +	return 1;
> +    };
> +
> +    my $journal_uuid = '45b0969e-9b03-4f30-b4c6-b4b80ceff106';
> +
> +    my $journalhash = {};
> +    dir_glob_foreach('/dev/disk/by-parttypeuuid', "$journal_uuid\..+", sub {
> +	my ($entry) = @_;
> +	my $real_dev = abs_path("/dev/disk/by-parttypeuuid/$entry");
> +	$journalhash->{$real_dev} = 1;
> +    });
> +
> +    my $zfslist = get_zfs_devices();
> +
> +    my $lvmlist = get_lvm_devices();
> +
> +    dir_glob_foreach('/sys/block', '.*', sub {
> +	my ($dev) = @_;
> +	return if defined($disk) && $disk ne $dev;
> +	# whitelisting following devices
> +	# hdX: ide block device
> +	# sdX: sd block device
> +	# vdX: virtual block device
> +	# xvdX: xen virtual block device
> +	# nvmeXnY: nvme devices
> +	# cXnY: cciss devices
> +	return if $dev !~ m@^(h|s|x?v)d[a-z]+$@ &&
> +		  $dev !~ m@^nvme\d+n\d+$@ &&
> +		  $dev !~ m@^c\d+d\d+$@;
> +
> +	my $sysdir = "/sys/block/$dev";
> +
> +	return if ! -d "$sysdir/device";
> +
> +	# we do not want iscsi devices
> +	return if readlink($sysdir) =~ m|host[^/]*/session[^/]*|;
> +
> +	my $size = file_read_firstline("$sysdir/size");
> +	return if !$size;
> +
> +	$size = $size * 512;
> +
> +	my $info = "";
> +	eval {
> +	    run_command([$UDEVADM, 'info', '-n', $dev, '--query', 'all'], outfunc => sub {
> +		my ($line) = @_;
> +		$info .= "$line\n";
> +	    });
> +	};
> +	warn $@ if $@;
> +	return if !$info;
> +
> +	return if $info !~ m/^E: DEVTYPE=disk$/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
> +	my $devpath;
> +	if ($info =~ m/^E: DEVNAME=(\S+)$/m) {
> +	    $devpath = $1;
> +	}
> +	return if !defined($devpath);
> +
> +	my $serial = 'unknown';
> +	if ($info =~ m/^E: ID_SERIAL_SHORT=(\S+)$/m) {
> +	    $serial = $1;
> +	}
> +
> +	my $gpt = 0;
> +	if ($info =~ m/^E: ID_PART_TABLE_TYPE=gpt$/m) {
> +	    $gpt = 1;
> +	}
> +
> +	# detect SSD
> +	my $rpm = -1;
> +	if ($info =~ m/^E: ID_ATA_ROTATION_RATE_RPM=(\d+)$/m) {
> +	    $rpm = $1;
> +	}
> +
> +	# dir/queue/rotational should be 1 for hdd, 0 for ssd
> +	my $type = 'unknown';
> +	my $rotational = file_read_firstline("$sysdir/queue/rotational");
> +
> +	if ($rotational == 0) {
> +	    $type = 'ssd';
> +	    $rpm = 0;
> +	} elsif ($rotational == 1) {
> +	    if ($rpm != -1) {
> +		$type = 'hdd';
> +	    } elsif ($info =~ m/^E: ID_BUS=usb/) {
> +		$type = 'usb';
> +		$rpm = 0;
> +	    }
> +	}
> +
> +	my $wwn = 'unknown';
> +	if ($info =~ m/^E: ID_WWN=(.*)$/m) {
> +	    $wwn = $1;
> +	}
> +
> +	my $vendor = file_read_firstline("$sysdir/device/vendor") || 'unknown';
> +	my $model = file_read_firstline("$sysdir/device/model") || 'unknown';
> +
> +	# we do not need smart data if we check a single disk
> +	# because this functionality is only for disk_is_used
> +	my $health = get_smart_health($devpath) if !defined($disk);
> +
> +	my $used;
> +
> +	$used = 'LVM' if $lvmlist->{$dev};
> +
> +	$used = 'mounted' if &$dev_is_mounted($devpath);
> +
> +	$used = 'ZFS' if $zfslist->{$dev};
> +
> +	$disklist->{$dev} = {
> +	    vendor => $vendor,
> +	    model => $model,
> +	    size => $size,
> +	    serial => $serial,
> +	    gpt => $gpt,
> +	    rpm => $rpm,
> +	    type =>  $type,
> +	    wwn => $wwn,
> +	    health => $health,
> +	    devpath => $devpath,
> +	};
> +
> +	my $osdid = -1;
> +
> +	my $journal_count = 0;
> +
> +	my $found_partitions;
> +	my $found_lvm;
> +	my $found_mountpoints;
> +	my $found_zfs;
> +	my $found_dm;
> +	my $partpath = $devpath;
> +	$partpath =~ s/\/[^\/]+$//;
> +
> +	dir_glob_foreach("$sysdir", "$dev.+", sub {
> +	    my ($part) = @_;
> +
> +	    $found_partitions = 1;
> +
> +	    if (my $mp = &$dev_is_mounted("$partpath/$part")) {
> +		$found_mountpoints = 1;
> +		if ($mp =~ m|^/var/lib/ceph/osd/ceph-(\d+)$|) {
> +		    $osdid = $1;
> +		}
> +	    }
> +
> +	    if ($lvmlist->{"$partpath/$part"}) {
> +		$found_lvm = 1;
> +	    }
> +
> +	    if ($zfslist->{$part}) {
> +		$found_zfs = 1;
> +	    }
> +
> +	    $journal_count++ if $journalhash->{"$partpath/$part"};
> +
> +	    if (!&$dir_is_empty("$sysdir/$part/holders") && !$found_lvm)  {
> +		$found_dm = 1;
> +	    }
> +	});
> +
> +	$used = 'mounted' if $found_mountpoints && !$used;
> +	$used = 'LVM' if $found_lvm && !$used;
> +	$used = 'ZFS' if $found_zfs && !$used;
> +	$used = 'Device Mapper' if $found_dm && !$used;
> +	$used = 'partitions' if $found_partitions && !$used;
> +
> +	# multipath, software raid, etc. 
> +	# this check comes in last, to show more specific info
> +	# if we have it
> +	$used = 'Device Mapper' if !$used && !&$dir_is_empty("$sysdir/holders");
> +
> +	$disklist->{$dev}->{used} = $used if $used;
> +	$disklist->{$dev}->{osdid} = $osdid;
> +	$disklist->{$dev}->{journals} = $journal_count;
> +    });
> +
> +    return $disklist;
> +
> +}
> +
> +1;
> diff --git a/PVE/Makefile b/PVE/Makefile
> index 655cad8..ae2bd35 100644
> --- a/PVE/Makefile
> +++ b/PVE/Makefile
> @@ -3,6 +3,7 @@
>  .PHONY: install
>  install:
>  	install -D -m 0644 Storage.pm ${DESTDIR}${PERLDIR}/PVE/Storage.pm
> +	install -D -m 0644 Diskmanage.pm ${DESTDIR}${PERLDIR}/PVE/Diskmanage.pm
>  	make -C Storage install
>  	make -C API2 install
>  	make -C CLI install
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 



More information about the pve-devel mailing list