[pve-devel] [PATCH v4 storage 1/1] Linux LIO/targetcli support

Thomas Lamprecht t.lamprecht at proxmox.com
Thu Aug 2 09:24:51 CEST 2018


Am 08/01/2018 um 03:24 PM schrieb Stoiko Ivanov:
> Hi,
> 
> Both Dominik and I gave your code another spin - and it worked as
> expected. 
> Additionally we both looked over the code and it looks far better than
> the last version.
> 

Much thanks to you two for testing/review!

> We discussed a further refactoring of the LunCmd modules, but that's a
> different task altogether.
> 
> Thank you for your time and contribution!
> 
> Tested-by: Stoiko Ivanov <s.ivanov at proxmox.com>
> 

OK, I'd apply and push this out, a single formal issue is stopping me.

@Udo: your patch is missing your developer certificate of origin (DCO),
i.e., a Signed-Off-By line. You do not need to resend the patch with it,
just replying here (or better to your patches mail itself) with this
line, which would be formatted as Stoiko's Tested-By line above, would
be enough - just that we have it on the record :)

cheers,
Thomas

> 
> On Thu, 26 Jul 2018 23:26:27 +0200
> Udo Rader <udo.rader at bestsolution.at> wrote:
> 
>> Introducing LIO/targetcli support allowing to use recent linux
>> distributions as iSCSI targets for ZFS volumes.
>>
>> In order for this to work, two preconditions have to be met:
>>
>> 1. the portal has to be set up correctly using targetcli
>> 2. the initiator has to be authorized to connect to the target
>>    based on the initiator's InitiatorName
>>
>> When adding a LIO iSCSI target, a new "LIO target portal group" field
>> needs to be correctly populated in the "Add: ZFS over iSCSI" popup,
>> containing the fitting "LIO target portal group" name (typically
>> something like 'tpg1'). ---
>>  PVE/Storage/LunCmd/LIO.pm   | 387
>> ++++++++++++++++++++++++++++++++++++ PVE/Storage/LunCmd/Makefile |
>> 2 +- PVE/Storage/ZFSPlugin.pm    |  12 +-
>>  3 files changed, 399 insertions(+), 2 deletions(-)
>>  create mode 100644 PVE/Storage/LunCmd/LIO.pm
>>
>> diff --git a/PVE/Storage/LunCmd/LIO.pm b/PVE/Storage/LunCmd/LIO.pm
>> new file mode 100644
>> index 0000000..a2050d4
>> --- /dev/null
>> +++ b/PVE/Storage/LunCmd/LIO.pm
>> @@ -0,0 +1,387 @@
>> +package PVE::Storage::LunCmd::LIO;
>> +
>> +# lightly based on code from Iet.pm 
>> +#
>> +# additional changes:
>> +# -----------------------------------------------------------------
>> +# Copyright (c) 2018 BestSolution.at EDV Systemhaus GmbH
>> +# All Rights Reserved.
>> +#
>> +# This software is released under the terms of the
>> +#
>> +#            "GNU Affero General Public License"
>> +# 
>> +# and may only be distributed and used under the terms of the
>> +# mentioned license. You should have received a copy of the license
>> +# along with this software product, if not you can download it from
>> +# https://www.gnu.org/licenses/agpl-3.0.en.html
>> +# 
>> +# Author: udo.rader at bestsolution.at
>> +# -----------------------------------------------------------------
>> +
>> +use strict;
>> +use warnings;
>> +use PVE::Tools qw(run_command);
>> +use JSON;
>> +
>> +sub get_base;
>> +
>> +# targetcli constants
>> +# config file location differs from distro to distro
>> +my @CONFIG_FILES = (
>> +	'/etc/rtslib-fb-target/saveconfig.json',	# Debian 9.x
>> et al	
>> +	'/etc/target/saveconfig.json' ,			#
>> ArchLinux, CentOS +);
>> +my $BACKSTORE = '/backstores/block';
>> +
>> +my $SETTINGS = undef;
>> +my $SETTINGS_TIMESTAMP = 0;
>> +my $SETTINGS_MAXAGE = 15; # in seconds
>> +
>> +my @ssh_opts = ('-o', 'BatchMode=yes');
>> +my @ssh_cmd = ('/usr/bin/ssh', @ssh_opts);
>> +my $id_rsa_path = '/etc/pve/priv/zfs';
>> +my $targetcli = '/usr/bin/targetcli';
>> +
>> +my $execute_remote_command = sub {
>> +    my ($scfg, $timeout, $remote_command, @params) = @_;
>> +
>> +    my $msg = '';
>> +    my $err = undef;
>> +    my $target;
>> +    my $cmd;
>> +    my $res = ();
>> +
>> +    $timeout = 10 if !$timeout;
>> +
>> +    my $output = sub {
>> +        my $line = shift;
>> +        $msg .= "$line\n";
>> +    };
>> +
>> +    my $errfunc = sub {
>> +        my $line = shift;
>> +        $err .= "$line";
>> +    };
>> +
>> +    $target = 'root@' . $scfg->{portal};
>> +    $cmd = [@ssh_cmd, '-i', "$id_rsa_path/$scfg->{portal}_id_rsa",
>> $target, '--', $remote_command, @params]; +
>> +    eval {
>> +        run_command($cmd, outfunc => $output, errfunc => $errfunc,
>> timeout => $timeout);
>> +    };
>> +    if ($@) {
>> +        $res = {
>> +            result => 0,
>> +            msg => $err,
>> +        }
>> +    } else {
>> +        $res = {
>> +            result => 1,
>> +            msg => $msg,
>> +        }
>> +    }
>> +
>> +    return $res;
>> +};
>> +
>> +# fetch targetcli configuration from the portal
>> +my $read_config = sub {
>> +    my ($scfg, $timeout) = @_;
>> +
>> +    my $msg = '';
>> +    my $err = undef;
>> +    my $luncmd = 'cat';
>> +    my $target;
>> +    my $retry = 1;
>> +
>> +    $timeout = 10 if !$timeout;
>> +
>> +    my $output = sub {
>> +        my $line = shift;
>> +        $msg .= "$line\n";
>> +    };
>> +
>> +    my $errfunc = sub {
>> +        my $line = shift;
>> +        $err .= "$line";
>> +    };
>> +
>> +    $target = 'root@' . $scfg->{portal};
>> +
>> +    foreach my $oneFile (@CONFIG_FILES) {
>> +        my $cmd = [@ssh_cmd, '-i',
>> "$id_rsa_path/$scfg->{portal}_id_rsa", $target, $luncmd, $oneFile];
>> +        eval {
>> +            run_command($cmd, outfunc => $output, errfunc =>
>> $errfunc, timeout => $timeout);
>> +        };
>> +        if ($@) {
>> +            die $err if ($err !~ /No such file or directory/);
>> +        }
>> +        return $msg if $msg ne '';
>> +    }
>> +
>> +    die "No configuration found. Install targetcli on
>> $scfg->{portal}\n" if $msg eq ''; +
>> +    return $msg;
>> +};
>> +
>> +my $get_config = sub {
>> +    my ($scfg) = @_;
>> +    my @conf = undef;
>> +
>> +    my $config = $read_config->($scfg, undef);
>> +    die "Missing config file" unless $config;
>> +
>> +    return $config;
>> +};
>> +
>> +# fetches and parses targetcli config from the portal
>> +my $parser = sub {
>> +    my ($scfg) = @_;
>> +    my $tpg = $scfg->{lio_tpg} || die "Target Portal Group not set,
>> aborting!\n";
>> +    my $tpg_tag;
>> +
>> +    if ($tpg =~ /^tpg(\d+)$/) {
>> +	$tpg_tag = $1;
>> +    } else {
>> +	die "Target Portal Group has invalid value, must contain
>> string 'tpg' and a suffix number, eg 'tpg17'\n";
>> +    }
>> +
>> +    my $base = get_base;
>> +
>> +    my $config = $get_config->($scfg);
>> +    my $jsonconfig = JSON->new->utf8->decode($config);
>> +
>> +    my $haveTarget = 0;
>> +    foreach my $oneTarget (@{$jsonconfig->{targets}}) {
>> +	# only interested in iSCSI targets
>> +        if ($oneTarget->{fabric} eq 'iscsi' && $oneTarget->{wwn} eq
>> $scfg->{target}) {
>> +	    # find correct TPG
>> +	    foreach my $oneTpg (@{$oneTarget->{tpgs}}) {
>> +                if ($oneTpg->{tag} == $tpg_tag) {
>> +                    $SETTINGS->{target} = $oneTpg;
>> +                    $haveTarget = 1;
>> +                    last;
>> +		}
>> +	    }
>> +	}
>> +    }
>> +
>> +    # seriously unhappy if the target server lacks iSCSI target
>> configuration ...
>> +    if (!$haveTarget) {
>> +	die "target portal group tpg$tpg_tag not found!\n";
>> +    }
>> +};
>> +
>> +# removes the given lu_name from the local list of luns
>> +my $free_lu_name = sub {
>> +    my ($lu_name) = @_;
>> +    my $new;
>> +
>> +    foreach my $lun (@{$SETTINGS->{target}->{luns}}) {
>> +        if ($lun->{storage_object} ne "$BACKSTORE/$lu_name") {
>> +            push @$new, $lun;
>> +        }
>> +    }
>> +
>> +    $SETTINGS->{target}->{luns} = $new;
>> +};
>> +
>> +# locally registers a new lun 
>> +my $register_lun = sub {
>> +    my ($scfg, $idx, $volname) = @_;
>> +
>> +    my $conf = {
>> +        index => $idx,
>> +        storage_object => "$BACKSTORE/$volname",
>> +        is_new => 1,
>> +    };
>> +    push @{$SETTINGS->{target}->{luns}}, $conf;
>> +
>> +    return $conf;
>> +};
>> +
>> +# extracts the ZFS volume name from a device path
>> +my $extract_volname = sub {
>> +    my ($scfg, $lunpath) = @_;
>> +    my $volname = undef;
>> +
>> +    my $base = get_base;
>> +    if ($lunpath =~ /^$base\/$scfg->{pool}\/([\w\-]+)$/) {
>> +	$volname = $1;
>> +    }
>> +
>> +    return $volname;
>> +};
>> +
>> +# retrieves the LUN index for a particular object
>> +my $list_view = sub {
>> +    my ($scfg, $timeout, $method, @params) = @_;
>> +    my $lun = undef;
>> +
>> +    my $object = $params[0];
>> +    my $volname = $extract_volname->($scfg, $params[0]);
>> +
>> +    foreach my $lun (@{$SETTINGS->{target}->{luns}}) {
>> +        if ($lun->{storage_object} eq "$BACKSTORE/$volname") {
>> +            return $lun->{index};
>> +        }
>> +    }
>> +
>> +    return $lun;
>> +};
>> +
>> +# determines, if the given object exists on the portal
>> +my $list_lun = sub {
>> +    my ($scfg, $timeout, $method, @params) = @_;
>> +    my $name = undef;
>> +
>> +    my $object = $params[0];
>> +    my $volname = $extract_volname->($scfg, $params[0]);
>> +
>> +    foreach my $lun (@{$SETTINGS->{target}->{luns}}) {
>> +        if ($lun->{storage_object} eq "$BACKSTORE/$volname") {
>> +            return $object;
>> +        }
>> +    }
>> +
>> +    return $name;
>> +};
>> +
>> +# adds a new LUN to the target
>> +my $create_lun = sub {
>> +    my ($scfg, $timeout, $method, @params) = @_;
>> +
>> +    if ($list_lun->($scfg, $timeout, $method, @params)) {
>> +        die "$params[0]: LUN already exists!";
>> +    }
>> +
>> +    my $device = $params[0];
>> +    my $volname = $extract_volname->($scfg, $device);
>> +    my $tpg = $scfg->{lio_tpg} || die "Target Portal Group not set,
>> aborting!\n"; +
>> +    # step 1: create backstore for device
>> +    my @cliparams = ($BACKSTORE, 'create', "name=$volname",
>> "dev=$device" );
>> +    my $res = $execute_remote_command->($scfg, $timeout, $targetcli,
>> @cliparams);
>> +    die $res->{msg} if !$res->{result};
>> +
>> +    # step 2: register lun with target
>> +    #
>> targetcli /iscsi/iqn.2018-04.at.bestsolution.somehost:target/tpg1/luns/
>> create /backstores/block/foobar
>> +    @cliparams = ("/iscsi/$scfg->{target}/$tpg/luns/", 'create',
>> "$BACKSTORE/$volname" );
>> +    $res = $execute_remote_command->($scfg, $timeout, $targetcli,
>> @cliparams);
>> +    die $res->{msg} if !$res->{result};
>> +
>> +    # targetcli responds with "Created LUN 99"
>> +    # not calculating the index ourselves, because the index at the
>> portal might have
>> +    # changed without our knowledge, so relying on the number that
>> targetcli returns
>> +    my $lun_idx;
>> +    if ($res->{msg} =~ /LUN (\d+)/) {
>> +        $lun_idx = $1;
>> +    } else {
>> +        die "unable to determine new LUN index: $res->{msg}";
>> +    }
>> +
>> +    $register_lun->($scfg, $lun_idx, $volname);
>> +
>> +    # step 3: unfortunately, targetcli doesn't always save changes,
>> no matter 
>> +    #         if auto_save_on_exit is true or not. So saving to be
>> safe ...
>> +    $execute_remote_command->($scfg, $timeout, $targetcli,
>> 'saveconfig'); +
>> +    return $res->{msg};
>> +};
>> +
>> +my $delete_lun = sub {
>> +    my ($scfg, $timeout, $method, @params) = @_;
>> +    my $res = {msg => undef};
>> +
>> +    my $tpg = $scfg->{lio_tpg} || die "Target Portal Group not set,
>> aborting!\n"; +
>> +    my $path = $params[0];
>> +    my $volname = $extract_volname->($scfg, $params[0]);
>> +
>> +    foreach my $lun (@{$SETTINGS->{target}->{luns}}) {
>> +        if ($lun->{storage_object} eq "$BACKSTORE/$volname") {
>> +            # step 1: delete the lun
>> +            my @cliparams = ("/iscsi/$scfg->{target}/$tpg/luns/",
>> 'delete', "lun$lun->{index}" );
>> +            my $res = $execute_remote_command->($scfg, $timeout,
>> $targetcli, @cliparams);
>> +            do {
>> +                die $res->{msg};
>> +            } unless $res->{result};
>> +
>> +            # step 2: delete the backstore
>> +            @cliparams = ($BACKSTORE, 'delete', $volname);
>> +            $res = $execute_remote_command->($scfg, $timeout,
>> $targetcli, @cliparams);
>> +            do {
>> +                die $res->{msg};
>> +            } unless $res->{result};
>> +
>> +            # step 3: save to be safe ...
>> +            $execute_remote_command->($scfg, $timeout, $targetcli,
>> 'saveconfig'); +
>> +            # update interal cache
>> +            $free_lu_name->($volname);
>> +
>> +            last;
>> +        }
>> +    }
>> +
>> +    return $res->{msg};
>> +};
>> +
>> +my $import_lun = sub {
>> +    my ($scfg, $timeout, $method, @params) = @_;
>> +
>> +    return $create_lun->($scfg, $timeout, $method, @params);
>> +};
>> +
>> +# needed for example when the underlying ZFS volume has been resized
>> +my $modify_lun = sub {
>> +    my ($scfg, $timeout, $method, @params) = @_;
>> +    my $msg;
>> +
>> +    $msg = $delete_lun->($scfg, $timeout, $method, @params);
>> +    if ($msg) {
>> +        $msg = $create_lun->($scfg, $timeout, $method, @params);
>> +    }
>> +
>> +    return $msg;
>> +};
>> +
>> +my $add_view = sub {
>> +    my ($scfg, $timeout, $method, @params) = @_;
>> +
>> +    return '';
>> +};
>> +
>> +my %lun_cmd_map = (
>> +    create_lu   =>  $create_lun,
>> +    delete_lu   =>  $delete_lun,
>> +    import_lu   =>  $import_lun,
>> +    modify_lu   =>  $modify_lun,
>> +    add_view    =>  $add_view,
>> +    list_view   =>  $list_view,
>> +    list_lu     =>  $list_lun,
>> +);
>> +
>> +sub run_lun_command {
>> +    my ($scfg, $timeout, $method, @params) = @_;
>> +
>> +    # fetch configuration from target if we haven't yet
>> +    # or if our configuration is stale
>> +    my $timediff = time - $SETTINGS_TIMESTAMP;
>> +    if ( ! $SETTINGS || $timediff > $SETTINGS_MAXAGE ) {
>> +        $SETTINGS_TIMESTAMP = time;
>> +        $parser->($scfg);
>> +    }
>> +
>> +    die "unknown command '$method'" unless exists
>> $lun_cmd_map{$method};
>> +    my $msg = $lun_cmd_map{$method}->($scfg, $timeout, $method,
>> @params); +
>> +    return $msg;
>> +}
>> +
>> +sub get_base {
>> +    return '/dev';
>> +}
>> +
>> +1;
>> diff --git a/PVE/Storage/LunCmd/Makefile b/PVE/Storage/LunCmd/Makefile
>> index b959255..a7209d1 100644
>> --- a/PVE/Storage/LunCmd/Makefile
>> +++ b/PVE/Storage/LunCmd/Makefile
>> @@ -1,4 +1,4 @@
>> -SOURCES=Comstar.pm Istgt.pm Iet.pm
>> +SOURCES=Comstar.pm Istgt.pm Iet.pm LIO.pm
>>  
>>  .PHONY: install
>>  install:
>> diff --git a/PVE/Storage/ZFSPlugin.pm b/PVE/Storage/ZFSPlugin.pm
>> index f88fe94..1fac811 100644
>> --- a/PVE/Storage/ZFSPlugin.pm
>> +++ b/PVE/Storage/ZFSPlugin.pm
>> @@ -12,6 +12,7 @@ use base qw(PVE::Storage::ZFSPoolPlugin);
>>  use PVE::Storage::LunCmd::Comstar;
>>  use PVE::Storage::LunCmd::Istgt;
>>  use PVE::Storage::LunCmd::Iet;
>> +use PVE::Storage::LunCmd::LIO;
>>  
>>  
>>  my @ssh_opts = ('-o', 'BatchMode=yes');
>> @@ -31,7 +32,7 @@ my $lun_cmds = {
>>  my $zfs_unknown_scsi_provider = sub {
>>      my ($provider) = @_;
>>  
>> -    die "$provider: unknown iscsi provider. Available [comstar,
>> istgt, iet]";
>> +    die "$provider: unknown iscsi provider. Available [comstar,
>> istgt, iet, LIO]"; };
>>  
>>  my $zfs_get_base = sub {
>> @@ -43,6 +44,8 @@ my $zfs_get_base = sub {
>>          return PVE::Storage::LunCmd::Istgt::get_base;
>>      } elsif ($scfg->{iscsiprovider} eq 'iet') {
>>          return PVE::Storage::LunCmd::Iet::get_base;
>> +    } elsif ($scfg->{iscsiprovider} eq 'LIO') {
>> +        return PVE::Storage::LunCmd::LIO::get_base;
>>      } else {
>>          $zfs_unknown_scsi_provider->($scfg->{iscsiprovider});
>>      }
>> @@ -63,6 +66,8 @@ sub zfs_request {
>>              $msg =
>> PVE::Storage::LunCmd::Istgt::run_lun_command($scfg, $timeout,
>> $method, @params); } elsif ($scfg->{iscsiprovider} eq 'iet') { $msg =
>> PVE::Storage::LunCmd::Iet::run_lun_command($scfg, $timeout, $method,
>> @params);
>> +        } elsif ($scfg->{iscsiprovider} eq 'LIO') {
>> +            $msg = PVE::Storage::LunCmd::LIO::run_lun_command($scfg,
>> $timeout, $method, @params); } else {
>>              $zfs_unknown_scsi_provider->($scfg->{iscsiprovider});
>>          }
>> @@ -188,6 +193,10 @@ sub properties {
>>  	    description => "host group for comstar views",
>>  	    type => 'string',
>>  	},
>> +	lio_tpg => {
>> +	    description => "target portal group for Linux LIO
>> targets",
>> +	    type => 'string',
>> +	},
>>      };
>>  }
>>  
>> @@ -204,6 +213,7 @@ sub options {
>>  	sparse => { optional => 1 },
>>  	comstar_hg => { optional => 1 },
>>  	comstar_tg => { optional => 1 },
>> +	lio_tpg => { optional => 1 },
>>  	content => { optional => 1 },
>>  	bwlimit => { optional => 1 },
>>      };
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 





More information about the pve-devel mailing list