[pve-devel] [PATCH] Add Multipath direct storage plugin and enable it.

Dmitry Petuhov mityapetuhov at gmail.com
Wed Jun 29 16:59:14 CEST 2016


See comments below inline.

29.06.2016 16:34, Wolfgang Bumiller wrote:
> The iSCSI plugin *should* notice multipath devices and make use of them
> if I'm not mistaken here.
> I wonder if it might be better to do the same in the iSCSI-direct
> plugin?
Yes, looks so. But iSCSI-direct currently doesn't use kernel interface 
to access LUNs, so we have to convert its code from libiscsi to 
open-iscsi (and probably drop libiscsi dependency in qemu) by the way.

> Because currently there's one major problem with your current code: it
> seems to include more than just multipath devices, iow. all disks on my
> machine here.
Yes, it may pass almost any disk, visible to host, to VM. Even if disk 
does not have own WWN ([s]ata/virtual/etc disks), Linux generates it 
using some unique disk identifiers, like SN. This feature is sometimes 
useful too. In worst case, there's some host-unique ids like 
wwn-0xQEMU_QEMU_HARDDISK_drive-scsi0.

Real problem is that we cannot distinguish between local-only and shared 
disks. That's actually OK for single-host setups, but is dangerous in 
clusters. I'll try to find out how to manage this.

Maybe we could share which WWN is visible by which host via pmxcfs? 
Also, we could store here which WWN belongs to which VM, preventing 
accidental lun sharing.

Thanks for code notes, I'll use it.

> I wonder if this is maybe a too general approach to what you're really
> after?
General approaches are cool. We will be able to use many 
enterprise-grade storages with single universal plugin. Also later we'll 
be able to control them via LunCmd/* specialised modules.

> A few inline comments follow:
>
> On Wed, Jun 29, 2016 at 02:00:42PM +0300, Dmitry Petuhov wrote:
>> Signed-off-by: Dmitry Petuhov <mityapetuhov at gmail.com>
>> ---
>>   PVE/Storage.pm                |   2 +
>>   PVE/Storage/MPDirectPlugin.pm | 208 ++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 210 insertions(+)
>>   create mode 100644 PVE/Storage/MPDirectPlugin.pm
>>
>> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
>> index 011c4f3..ddf9ffe 100755
>> --- a/PVE/Storage.pm
>> +++ b/PVE/Storage.pm
>> @@ -32,6 +32,7 @@ use PVE::Storage::GlusterfsPlugin;
>>   use PVE::Storage::ZFSPoolPlugin;
>>   use PVE::Storage::ZFSPlugin;
>>   use PVE::Storage::DRBDPlugin;
>> +use PVE::Storage::MPDirectPlugin;
>>   
>>   # load and initialize all plugins
>>   PVE::Storage::DirPlugin->register();
>> @@ -46,6 +47,7 @@ PVE::Storage::GlusterfsPlugin->register();
>>   PVE::Storage::ZFSPoolPlugin->register();
>>   PVE::Storage::ZFSPlugin->register();
>>   PVE::Storage::DRBDPlugin->register();
>> +PVE::Storage::MPDirectPlugin->register();
>>   PVE::Storage::Plugin->init();
>>   
>>   my $UDEVADM = '/sbin/udevadm';
>> diff --git a/PVE/Storage/MPDirectPlugin.pm b/PVE/Storage/MPDirectPlugin.pm
>> new file mode 100644
>> index 0000000..76ced94
>> --- /dev/null
>> +++ b/PVE/Storage/MPDirectPlugin.pm
>> @@ -0,0 +1,208 @@
>> +package PVE::Storage::MPDirectPlugin;
>> +
>> +use strict;
>> +use warnings;
>> +use Data::Dumper;
>> +use IO::File;
>> +use PVE::Tools qw(run_command trim);
>> +use PVE::Storage::Plugin;
>> +use PVE::JSONSchema qw(get_standard_option);
>> +
>> +use base qw(PVE::Storage::Plugin);
>> +
>> +# Configuration
>> +
>> +sub type {
>> +    return 'mpdirect';
>> +}
>> +
>> +sub plugindata {
>> +    return {
>> +	content => [ {images => 1, rootdir => 1, none => 1}, { images => 1 }],
>> +    };
>> +}
>> +
>> +sub properties {
>> +    return {
>> +    };
>> +}
>> +
>> +sub options {
>> +    return {
>> +	content => { optional => 1 },
>> +    };
>> +}
>> +
>> +# Storage implementation
>> +
>> +sub parse_volname {
>> +    my ($class, $volname) = @_;
>> +
>> +    return ('images', $volname, undef, undef, undef, undef, 'raw');
>> +}
>> +
>> +sub filesystem_path {
>> +    my ($class, $scfg, $volname, $snapname) = @_;
>> +
>> +    die "Direct attached device snapshot is not implemented" if defined($snapname);
>> +
>> +    my ($vtype, $name, $vmid) = $class->parse_volname($volname);
>> +
>> +    my $path = "/dev/disk/by-id/wwn-0x$name";
>> +
>> +    return wantarray ? ($path, $vmid, $vtype) : $path;
>> +}
>> +
>> +sub create_base {
>> +    my ($class, $storeid, $scfg, $volname) = @_;
>> +
>> +    die "Cannot create physical device";
>> +}
>> +
>> +sub clone_image {
>> +    my ($class, $scfg, $storeid, $volname, $vmid, $snap) = @_;
>> +
>> +    die "Cannot create physical device";
>> +}
>> +
>> +sub alloc_image {
>> +    my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size) = @_;
>> +
>> +    die "Cannot create physical device";
>> +}
>> +
>> +sub free_image {
>> +    my ($class, $storeid, $scfg, $volname, $isBase) = @_;
>> +
>> +    die "Cannot create physical device";
>> +}
>> +
>> +sub list_images {
>> +    my ($class, $storeid, $scfg, $vmid, $vollist, $cache) = @_;
>> +
>> +    my $vgname = $scfg->{vgname};
>> +
>> +    my $res = [];
>> +
>> +    opendir my $dir, '/dev/disk/by-id' || die "Cannot list disks";
>> +
>> +    while (my $disk = readdir $dir) {
>> +	# We don't want partitions here
>> +	next if $disk =~ m/-part\d+$/;
>> +
>> +	next if $disk !~ m/^wwn-0x(.+)$/;
>> +	my $wwid = $1;
> This includes all disks, not just multipath devices.
>
> # multipath -ll
> # ls -l /dev/disk/by-id |grep wwn- |grep -v -- -part
> lrwxrwxrwx 1 root root  9 Jun 29 09:09 wwn-0x5000c50064bbed7d -> ../../sdd
> lrwxrwxrwx 1 root root  9 Jun 29 09:09 wwn-0x5002538d409de6ec -> ../../sdb
> lrwxrwxrwx 1 root root  9 Jun 29 09:09 wwn-0x500a07510e6af4b5 -> ../../sdc
> lrwxrwxrwx 1 root root  9 Jun 29 09:09 wwn-0x55cd2e404c608ca2 -> ../../sda
> lrwxrwxrwx 1 root root  9 Jun 29 09:11 wwn-0x60014059c03c50c2e8b4d99972140221 -> ../../sdf
>
>> +
>> +	my $volid = "$storeid:$wwid";
>> +
>> +	if ($vollist) {
>> +	    my $found = grep { $_ eq $volid } @$vollist;
>> +	    next if !$found;
>> +	} else {
>> +	    next if defined($vmid);
>> +	}
>> +
>> +	my $actualdev = readlink "/dev/disk/by-id/$disk";
>> +	$actualdev =~ s/..\/..\///;
> would be easier to read with a different regex symbol, eg: s at ../../@@;
>
>> +
>> +	my $size;
>> +	if (open SZ, "/sys/block/$actualdev/size" ) {
> We have a helper for this:
>    file_read_firstline("/sys/block/$actualdev/size");
>
> For future reference: for new code our prefered style there should use
> variables and include the open-mode:
>    open my $sz, '<', "/sys/block/$actualdev/size"
>
>> +	    $size = <SZ>;
>> +	    $size *= 512;
>> +	    close SZ;
>> +	} else {
>> +	    $size = undef;
>> +	}
>> +
>> +	push @$res, {
>> +	    volid => $volid, format => 'raw', size => $size,
>> +	};
>> +    }
>> +    closedir $dir;
>> +
>> +    return $res;
>> +}
>> +
>> +sub status {
>> +    my ($class, $storeid, $scfg, $cache) = @_;
>> +
>> +    # We cannot know status of undefined set of devices
>> +    return undef;
>> +}
>> +
>> +sub activate_storage {
>> +    my ($class, $storeid, $scfg, $cache) = @_;
>> +
>> +    # Server's SCSI subsystem is always up, so there's nothing to do
>> +    # Maybe we could control selected controllers/ports in future.
>> +    return 1;
>> +}
>> +
>> +sub deactivate_storage {
>> +    my ($class, $storeid, $scfg, $cache) = @_;
>> +
>> +    return 1;
>> +}
>> +
>> +sub activate_volume {
>> +    my ($class, $storeid, $scfg, $volname, $snapname, $cache) = @_;
>> +
>> +    die "volume snapshot is not possible on direct-attached storage device" if $snapname;
>> +
>> +    return 1;
>> +}
>> +
>> +sub deactivate_volume {
>> +    my ($class, $storeid, $scfg, $volname, $snapname, $cache) = @_;
>> +
>> +    die "volume snapshot is not possible on direct-attached storage  device" if $snapname;
>> +
>> +    return 1;
>> +}
>> +
>> +sub volume_resize {
>> +    my ($class, $scfg, $storeid, $volname, $size, $running) = @_;
>> +
>> +    die "Direct-attached storage device cannot be resized";
>> +}
>> +
>> +sub volume_snapshot {
>> +    my ($class, $scfg, $storeid, $volname, $snap) = @_;
>> +
>> +    die "Direct-attached storage snapshot is not implemented";
>> +}
>> +
>> +sub volume_snapshot_rollback {
>> +    my ($class, $scfg, $storeid, $volname, $snap) = @_;
>> +
>> +    die "Direct-attached storage snapshot rollback is not implemented";
>> +}
>> +
>> +sub volume_snapshot_delete {
>> +    my ($class, $scfg, $storeid, $volname, $snap) = @_;
>> +
>> +    die "Direct-attached storage snapshot delete is not implemented";
>> +}
>> +
>> +sub volume_has_feature {
>> +    my ($class, $scfg, $feature, $storeid, $volname, $snapname, $running) = @_;
>> +
>> +    my $features = {
>> +	copy => { base => 1, current => 1},
>> +    };
>> +
>> +    my ($vtype, $name, $vmid, $basename, $basevmid, $isBase) =
>> +	$class->parse_volname($volname);
>> +
>> +    my $key = undef;
>> +    if($snapname){
>> +	$key = 'snap';
>> +    }else{
>> +	$key =  $isBase ? 'base' : 'current';
>> +    }
>> +    return 1 if $features->{$feature}->{$key};
>> +
>> +    return undef;
>> +}
>> +
>> +1;
>> -- 
>> 2.1.4



More information about the pve-devel mailing list