Patch Submission: iscsi target with multiple subnets

[Sybaweb_Jay]

We had issues adding iscsi targets with multiple subnets where one of the subnets was not connectable from the proxmox server:

root@debian:~# iscsiadm --mode discovery --type sendtargets --portal 172.16.123.111
172.16.124.1:3260,1 iqn.2011-07.local.wpps:std.drbd0
172.16.123.111:3260,1 iqn.2011-07.local.wpps:std.drbd0

Getting the iscsi_discovery function to test for connectivity before adding it to the available targets fixed this for us:

+++ /usr/share/perl5/PVE/Storage.pm     2011-07-28 14:27:30.993054558 +0200
@@ -16,6 +16,7 @@
 use Getopt::Long qw(GetOptionsFromArray);
 use Socket;
 use Digest::SHA1;
+use Net::Ping;

 my $ISCSIADM = '/usr/bin/iscsiadm';
 my $UDEVADM = '/sbin/udevadm';
@@ -1139,15 +1140,19 @@
     my $res = {};
     run_command ($cmd, outfunc => sub {
        my $line = shift;
-
        if ($line =~ m/^(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}:\d+)\,\S+\s+(\S+)\s*$/) {
-           my $portal = $1;
-           my $target = $2;
-           # one target can have more than one portal (multipath).
-           push @{$res->{$target}}, $portal;
+           my $p = Net::Ping->new("tcp", 2);
+           my @ip = split(':',$1);
+           $p->port_number($ip[1]);
+           if ($p->ping($ip[0])){
+               my $portal = $1;
+               my $target = $2;
+               # one target can have more than one portal (multipath).
+               push @{$res->{$target}}, $portal;
+           }
+           $p->close();
        }
     });
-
     return $res;
 }

 

[dietmar]

I do not think that patch makes sense. I think it is better to modify iscsi_login() to tolerate errors:

sub iscsi_login {
    my ($target, $portal_in) = @_;

    check_iscsi_support ();

    my $res = iscsi_discovery ($portal_in);

    my $success = 0;
    foreach my $portal (@{$res->{$target}}) {
    my $cmd = [$ISCSIADM, '--mode', 'node', '--portal', $portal, 
           '--targetname',  $target, '--login'];
    eval { run_command ($cmd); $success = 1; };
    warn $@ if $@;
    }
    die "iscsi login failed\n" if !$success;
}

Does that work for you?

 

[Sybaweb_Jay]

Maybe I'm missing something, but your suggestion seems to still result in the same errors on our test platform.

When a new iscsi storage location is saved, the web interface hangs for a while and then gives the alert: "500 read timeout".

Even though adding the location seems to have failed, proxmox has already saved the storage location to /etc/pve/storage.conf . Once this has happened you cannot access the storage page in the web interface until you manually delete the failed storage location from storage.conf .

I agree the patch I propose doesn't seem like the most rational approach, with my limited knowledge of proxmox internals (and perl) I couldn't find another workable solution.

 

[dietmar]

I agree the patch I propose doesn't seem like the most rational approach, with my limited knowledge of proxmox internals (and perl) I couldn't find another workable solution.

So I suppose the iscsi login command hangs for a long time, so we run into a timeout. I wonder if there is a timeout option for iscsiadm?

 

[Sybaweb_Jay]

This shows current login timeout:

iscsiadm -m node -p 172.16.123.111:3260 | grep 'login_timeout'

This sets the timeout:

iscsiadm -m node -p 172.16.123.111:3260 --op=update --name=node.conn[0].timeo.login_timeout --value=5

So perhaps setting the timeout lower before attempting login will help, I'm not convinced that fiddling this setting is more sane than checking whether the tcp port is connectable during discovery

 

[dietmar]

I'm not convinced that fiddling this setting is more sane than checking whether the tcp port is connectable during discovery

I am sure it is. Because a simple ping only tests if the 'echo' service works. But there are 1000 other things which can fail. if we run iscsiadm we test all those conditions. Or is there and reason why ping is better than iscsiadm?

Oh, just noticed that you test the correct iscsi port using Net:ing - will try to use that.

 

[dietmar]

OK, here is another try. I do not modify iscsi_discovery(), so that function still returns all portals. Instead I
use your ping test in iscsi_login()

sub iscsi_login {
    my ($target, $portal_in) = @_;

    check_iscsi_support ();

    my $res = iscsi_discovery ($portal_in);

    my $success = 0;
    foreach my $portal (@{$res->{$target}}) {
    my ($server, $port) = split(':', $portal);
    my $cmd = [$ISCSIADM, '--mode', 'node', '--portal', $portal, 
           '--targetname',  $target, '--login'];
    my $p = Net::Ping->new("tcp", 2);
    $p->port_number($port);
    if ($p->ping($server)) {
        eval { run_command ($cmd); $success = 1; };
        warn $@ if $@;
    }
    }
    die "iscsi login failed\n" if !$success;
}

Does that work as expected?

 

[dietmar]

Beside, that whole thing looks like a design bug. We want to login to all portals? if one portal is not available, we want some kind of delayed loging (login as soon as it becomes available). So can we use 'discovery' mode with option 'login' instead? Or does that fail also when one node is not available?

 

[dietmar]

Please can you test if the following works:

sub iscsi_login {
    my ($target, $portal_in) = @_;

    check_iscsi_support ();

    eval { iscsi_discovery ($portal_in); };
    warn $@ if $@;

    my $cmd = [$ISCSIADM, '--mode', 'node', '--targetname',  $target, '--login'];
    run_command ($cmd);
}

 

[Sybaweb_Jay]

Beside, that whole thing looks like a design bug. We want to login to all portals? if one portal is not available, we want some kind of delayed loging (login as soon as it becomes available).

Agreed, I will test as soon as I have a moment.

 

[Vasily Poterjyko]

i fixed this problem with this patch:

ibp-proxmox:~# diff /usr/share/perl5/PVE/Storage.pm.dist /usr/share/perl5/PVE/Storage.pm
1143c1143,1144
<       if ($line =~ m/^(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}:\d+)\,\S+\s+(\S+)\s*$/) {
---
>       #if ($line =~ m/^(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}:\d+)\,\S+\s+(\S+)\s*$/) {
>       if ($line =~ m/^($portal:\d+)\,\S+\s+(\S+)\s*$/) {

 

[Sybaweb_Jay]

Please can you test if the following works:

sub iscsi_login {
    my ($target, $portal_in) = @_;

    check_iscsi_support ();

    eval { iscsi_discovery ($portal_in); };
    warn $@ if $@;

    my $cmd = [$ISCSIADM, '--mode', 'node', '--targetname',  $target, '--login'];
    run_command ($cmd);
}

This seems to work. Thanks alot for all your feedback on this.

 

[dietmar]

This seems to work. Thanks alot for all your feedback on this.

OK, just uploaded a fixed version to pvetest repository. Please can you test:

ftp://download.proxmox.com/debian/d...nary-amd64/libpve-storage-perl_1.0-18_all.deb