Merge pull request #55 from amulet1/cleanup3

Cleanup

PureStoragePlugin.pm

@@ -11,7 +11,7 @@ use File::Path ();
 
 use PVE::JSONSchema      ();
 use PVE::Network         ();
-use PVE::Tools           qw( run_command );
+use PVE::Tools           qw( file_read_firstline run_command );
 use PVE::INotify         ();
 use PVE::Storage::Plugin ();
 
@@ -31,17 +31,13 @@ $Data::Dumper::Terse  = 1;    # Removes `$VAR1 =` in output
 $Data::Dumper::Indent = 1;    # Outputs everything in one line
 $Data::Dumper::Useqq  = 1;    # Uses quotes for strings
 
-my $purestorage_wwn_prefix = "624a9370";
+my $PSFA_API               = '2.26';
+my $purestorage_wwn_prefix = '3624a9370';
 my $default_hgsuffix       = "";
+my $default_protocol       = 'iscsi';
 
 my $DEBUG = 0;
 
-my $cmd = {
-  iscsiadm  => '/usr/bin/iscsiadm',
-  multipath => '/sbin/multipath',
-  blockdev  => '/usr/sbin/blockdev'
-};
-
 ### BLOCK: Configuration
 sub api {
 
@@ -75,29 +71,34 @@ sub properties {
   return {
     hgsuffix => {
       description => "Host group suffx.",
-      type        => "string",
+      type        => 'string',
       default     => $default_hgsuffix
     },
     address => {
       description => "PureStorage Management IP address or DNS name.",
-      type        => "string"
+      type        => 'string'
     },
     token => {
       description => "Storage API token.",
-      type        => "string"
+      type        => 'string'
     },
     podname => {
-      description => 'PureStorage pod name',
+      description => "PureStorage pod name",
       type        => 'string'
     },
     vnprefix => {
-      description => 'Prefix to add to volume name before sending it to PureStorage array',
+      description => "Prefix to add to volume name before sending it to PureStorage array",
       type        => 'string'
     },
     check_ssl => {
       description => "Verify the server's TLS certificate",
-      type        => "boolean",
-      default     => "no"
+      type        => 'boolean',
+      default     => 'no'
+    },
+    protocol => {
+      description => "Set storage protocol ( iscsi | fc | nvme )",
+      type        => 'string',
+      default     => $default_protocol
     },
   };
 }
@@ -112,6 +113,7 @@ sub options {
     podname   => { optional => 1 },
     vnprefix  => { optional => 1 },
     check_ssl => { optional => 1 },
+    protocol  => { optional => 1 },
     nodes     => { optional => 1 },
     disable   => { optional => 1 },
     content   => { optional => 1 },
@@ -121,17 +123,110 @@ sub options {
 
 ### BLOCK: Supporting functions
 
+my $cmd = {
+
+  #  fuser      => '/usr/bin/fuser',
+  multipath  => '/sbin/multipath',
+  multipathd => '/sbin/multipathd',
+  blockdev   => '/usr/sbin/blockdev'
+};
+
 sub exec_command {
-  my ( $command, $die, %param ) = @_;
+  my ( $command, $dm, %param ) = @_;
+
+  $dm //= 1;
+
+  my $fc = $cmd->{ $command->[0] };
+  $command->[0] = $fc if defined $fc;
 
   print "Debug :: execute '" . join( ' ', @$command ) . "'\n" if $DEBUG >= 2;
+
+  if ( $DEBUG < 3 ) {
+    $param{ 'quiet' } = 1 unless exists $param{ 'quiet' };
+  }
+
   eval { run_command( $command, %param ) };
   if ( $@ ) {
-    my $error = " :: Cannot execute '" . join( ' ', @$command ) . "'. Error :: $@\n";
-    die 'Error' . $error if $die;
+    my $error = " :: Cannot execute '" . join( ' ', @$command ) . "'\n  ==> Error :: $@\n";
+    die 'Error' . $error if $dm > 0;
 
-    warn 'Warning' . $error;
+    warn 'Warning' . $error unless $dm < 0;
+    return $dm < 0;
   }
+
+  return $dm >= 0;
+}
+
+sub scsi_scan_new {
+  my ( $protocol ) = @_;
+
+  print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::scsi_scan_new\n" if $DEBUG;
+
+  my $path = '/sys/class/' . $protocol . '_host';
+  opendir( my $dh, $path ) or die "Cannot open directory: $!";
+  my @hosts = grep { !/^\.\.?$/ } readdir( $dh );
+  closedir( $dh );
+
+  my $count = 0;
+  foreach my $host ( @hosts ) {
+    next unless $host =~ /^(\w+)$/;
+    $path = '/sys/class/scsi_host/' . $1;    # untaint
+    if ( -d $path ) {
+      device_op( $path, 'scan', '- - -' );
+      ++$count;
+    } else {
+      warn "Warning :: SCSI host path $path does not exist.\n";
+    }
+  }
+
+  die "Error :: Did not find hosts to scan.\n" unless $count > 0;
+
+  print "Debug :: Scanned $count host" . ( $count > 1 ? 's' : '' ) . " for new devices\n" if $DEBUG;
+}
+
+sub multipath_check {
+  my ( $wwid ) = @_;
+
+  # TODO: Find a better check
+  # TODO: Support non-multipath mode
+  my $output = `$cmd->{ multipath } -l $wwid`;
+
+  return $output ne '';
+}
+
+sub wait_for {
+  my ( $success, $message, $timeout, $delay ) = @_;
+
+  my $debug = 'Debug :: Waiting for ' . $message;
+
+  $timeout //= 5;
+  $delay   //= 0.1;
+
+  # Wait for the device size to update
+  my $time = 0;
+  while ( $time < $timeout ) {
+    if ( &$success() ) {
+      if ( $DEBUG && $time > 0 ) {
+        print $debug if $DEBUG >= 2;
+        print ": done in $time sec\n";
+      }
+      return 1;
+    }
+
+    if ( $DEBUG && $time == 0 ) {
+      print $debug;
+      print "\n" if $DEBUG >= 2;
+    }
+
+    select( undef, undef, undef, $delay );
+
+    $time += $delay;
+  }
+
+  print $debug                        if $DEBUG >= 2;
+  print ": timeout after $time sec\n" if $DEBUG;
+
+  die "Error :: Timeout while waiting for $message\n";
 }
 
 sub prepare_api_params {
@@ -197,7 +292,7 @@ sub purestorage_name_prefix {
     $value = $scfg->{ $pkey };
     if ( defined( $value ) ) {
       $prefix .= $value;
-      die "Error:: Invalid \"$pkey\" parameter value \"$value\"\n" if $prefix !~ m/^\w([\w-]*\w)?((\/|::)(\w[\w-]*)?)?$/;
+      die "Error :: Invalid \"$pkey\" parameter value \"$value\"\n" if $prefix !~ m/^\w([\w-]*\w)?((\/|::)(\w[\w-]*)?)?$/;
     }
 
     $scfg->{ $ckey } = $prefix;
@@ -211,10 +306,11 @@ sub purestorage_name {
 
   my $name = length( $volname ) ? purestorage_name_prefix( $scfg ) . $volname : '';
   if ( length( $snapname ) ) {
-
-    # $snapname =~ s/_/-/g;
+    my $snap = $snapname;
+    $snap =~ s/^(veeam_)/veeam-/;    # s/_/-/g;
+    $snap = 'snap-' . $snap unless defined $1;
     $name .= '.' if $name ne '';
-    $name .= 'snap-' . $snapname;
+    $name .= $snap;
   }
 
   print 'Debug :: purestorage_name ::',
@@ -224,84 +320,226 @@ sub purestorage_name {
   return $name;
 }
 
+sub get_device_path_wwn {
+  my ( $serial ) = @_;
+
+  die 'Error :: Volume serial is missing' unless length( $serial );
+
+  # Construct the WWN path
+  my $wwn  = lc( $purestorage_wwn_prefix . $serial );
+  my $path = '/dev/disk/by-id/wwn-0x' . substr( $wwn, -32 );
+  return ( $path, $wwn );
+}
+
+sub get_device_size {
+  my ( $device ) = @_;
+  print "Debug :: get_device_size($device)\n" if $DEBUG;
+
+  my $path = '/sys/block/' . basename( $device ) . '/size';
+  my $size = file_read_firstline( $path ) << 9;
+
+  print "Debug :: Device \"$device\" size is $size bytes\n" if $DEBUG;
+  return $size;
+}
+
+sub device_op {
+  my ( $device_path, $op, $value ) = @_;
+
+  open( my $fh, '>', $device_path . '/' . $op ) or die "Error :: Could not open file \"$device_path/$op\" for writing.\n";
+  print $fh $value;
+  close( $fh );
+}
+
+sub block_device_action {
+  my ( $action, @devices ) = @_;
+  print "Debug :: block_device_action($action,@devices)\n" if $DEBUG;
+
+  foreach my $device ( @devices ) {
+    if ( $device !~ /^(sd[a-z]+)$/ ) {
+      warn "Warning :: Unexpected device name in block_device_action() => $action $device)\n";
+      next;
+    }
+    $device = $1;    # untaint
+    my $device_path = '/sys/block/' . $device . '/device';
+    if ( $action eq 'remove' ) {
+      print "Debug :: Removing device: $device\n" if $DEBUG;
+      exec_command( [ 'blockdev', '--flushbufs', '/dev/' . $device ] );
+      device_op( $device_path, 'state',  'offline' );
+      device_op( $device_path, 'delete', '1' );
+    } elsif ( $action eq 'rescan' ) {
+      print "Debug :: Rescanning: $device\n" if $DEBUG;
+      device_op( $device_path, 'rescan', '1' );
+    } else {
+      die "Error :: Unsuported acitonin block_device_action() => $action\n";
+    }
+  }
+}
+
+sub block_device_slaves {
+  my ( $path ) = @_;
+
+  my $device_path = abs_path( $path );
+  die "Error :: Can't resolve device path for $path\n" unless $device_path =~ /^([\/a-zA-Z0-9_\-\.]+)$/;
+  $device_path = $1;    # untaint
+
+  print "Debug :: Device path resolved to \"$device_path\".\n" if $DEBUG;
+
+  my $device_name = basename( $device_path );
+  my $slaves_path = '/sys/block/' . $device_name . '/slaves';
+
+  my @slaves;
+  if ( -d $slaves_path ) {
+    opendir( my $dh, $slaves_path ) or die "Cannot open directory: $!";
+    @slaves = grep { !/^\.\.?$/ } readdir( $dh );
+    closedir( $dh );
+  }
+  if ( @slaves ) {
+    print "Debug :: Disk \"$device_name\" slaves: " . join( ', ', @slaves ) . "\n" if $DEBUG;
+  } else {
+    warn "Warning :: Disk \"$device_name\" has no slaves.\n";
+    push @slaves, $device_name;
+  }
+  return $device_path, @slaves;
+}
+
 ### BLOCK: Local multipath => PVE::Storage::Custom::PureStoragePlugin::sub::s
 
-my $psfa_api = "2.26";
-
 sub purestorage_api_request {
-  my ( $scfg, $action ) = @_;
+  my ( $scfg, $action, $all ) = @_;
   print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::purestorage_api_request\n" if $DEBUG;
 
-  my $url = $scfg->{ address } or die "Error :: Pure Storage host address is not defined.\n";
+  $all //= 0;
 
-  my $type = $action->{ type };
-  $url .= '/api/' . $psfa_api . '/' . $type;
-
-  my $params = prepare_api_params( $action->{ params } );
-  $url .= "?$params" if length( $params );
-
-  my $headers = HTTP::Headers->new( 'Content-Type' => 'application/json' );
-
-  my $login = $type eq 'login';
-  if ( $login ) {
-    $headers->header( 'api-token' => $scfg->{ token } );
-  } else {
-    if ( $scfg->{ x_auth_token } ) {
-      print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::purestorage_get_auth_token::cached\n" if $DEBUG;
-    } else {
-      print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::purestorage_get_auth_token\n" if $DEBUG;
-      purestorage_api_request( $scfg, { name => 'Authentication', type => 'login', method => 'POST' } );
-    }
-    $headers->header( 'x-auth-token' => $scfg->{ x_auth_token } );
-  }
-  $headers->header( 'X-Request-ID' => $scfg->{ x_request_id } ) if $scfg->{ x_request_id };
-
-  my $ua = LWP::UserAgent->new;
+  my $ua = LWP::UserAgent->new( timeout => 15 );
   $ua->ssl_opts(
     verify_hostname => 0,
     SSL_verify_mode => 0x00
   ) unless $scfg->{ check_ssl };
 
-  my $request      = HTTP::Request->new( $action->{ method }, $url, $headers, $action->{ body } ? encode_json( $action->{ body } ) : undef );
-  my $response     = $ua->request( $request );
-  my $content_type = $response->header( "Content-Type" );
-  my $content =
-    defined $content_type && $content_type =~ /application\/json/ && $response->content ne ''
-    ? decode_json( $response->content )
-    : $response->decoded_content;
+  my $type  = $action->{ type };
+  my $login = $type eq 'login' ? 1 : 0;
 
-  $content = {} if $content eq '';
+  my $params = prepare_api_params( $action->{ params } );
+  my $path   = $type;
+  $path .= '?' . $params if length( $params );
 
-  my $action_name = $action->{ name } || "Action '$type' (method '" . $action->{ method } . "')";
-  my $success     = $response->is_success;
-  if ( $success ) {
-    if ( $login ) {
-      $headers                = $response->headers;
-      $scfg->{ x_auth_token } = $headers->header( 'x-auth-token' ) || die "Error :: Header 'x-auth-token' is missing.";
-      $scfg->{ x_request_id } = $headers->header( 'x-request-id' );
-    }
-  } else {
-    my $ignore_errors = $action->{ ignore };
-    if ( defined( $ignore_errors ) ) {
-      $ignore_errors = [$ignore_errors] if ref( $ignore_errors ) eq '';
-      my $first = $content->{ errors }->[0]->{ message };
-      $success = 1 if grep { $_ eq $first } @$ignore_errors;
+  my $method = $action->{ method };
+
+  my $body = $action->{ body };
+
+  my $error;
+  my $content;
+  my $url;
+
+  my @urls   = split( ',', $scfg->{ address } // '' );
+  my @tokens = split( ',', $scfg->{ token }   // '' );
+
+  foreach my $i ( 0, 1 ) {
+    $url = $urls[$i] // '';
+    my $token = $tokens[$i] // '';
+    next if $i && $url eq '' && $token eq '';
+
+    my $cf = $url eq '' ? 'address' : $token eq '' ? 'token' : '';
+    die "Error :: Pure Storage \"$cf\" parameter" . ( $i == 0 ? '' : ' for second array' ) . " is not defined.\n" unless $cf eq '';
+
+    my $config = {
+      ua         => $ua,
+      url        => $url,
+      token      => $token,
+      auth_token => $scfg->{ '_auth_token' . $i },
+      request_id => $scfg->{ '_request_id' . $i }
+    };
+
+    ( $error, $content ) = purestorage_api_request1( $config, $path, $method, $login, $body );
+    if ( $error == -1 ) {
+      $scfg->{ '_auth_token' . $i } = $config->{ auth_token };
+      $scfg->{ '_request_id' . $i } = $config->{ request_id };
+    } elsif ( $error == 1 ) {
+      my $ignore = $action->{ ignore };
+      if ( defined( $ignore ) ) {
+        $ignore = [$ignore] if ref( $ignore ) eq '';
+        my $first = $content->{ errors }->[0]->{ message };
+        $error = 0 if grep { $_ eq $first } @$ignore;
+      }
     }
 
-    if ( !$success ) {
-      my $message = substr( $action_name, 0, 1 );
-      $message = $message eq uc( $message ) ? $action_name . ' failed' : 'Failed to ' . $action_name;
-      die "Error :: PureStorage API :: $message.\n"
-        . "=> Trace:\n"
-        . "==> Code: "
-        . $response->code . "\n"
-        . ( $content ? "==> Message: " . Dumper( $content ) : '' );
-    }
+    last if $error == 1 || $error <= 0 && !$all;
+  }
+
+  if ( $error > 0 ) {
+    my $message = $error == 3 ? 'Authentication' : $action->{ name } || "Action '$type' (method '$method')";
+    $message = substr( $message, 0, 1 ) eq uc( substr( $message, 0, 1 ) ) ? $message . ' failed' : 'Failed to ' . $message;
+    $message = 'PureStorage API :: ' . $message if $error == 1;
+    die "Error :: $message.\n" . "=> Trace:\n" . "==> address: " . $url . "\n" . ( $content ? "==> Message: " . Dumper( $content ) : '' );
   }
 
   return $content;
 }
 
+sub purestorage_api_request1 {
+  my ( $config, $path, $method, $login, $body ) = @_;
+
+  my $headers = HTTP::Headers->new( 'Content-Type' => 'application/json' );
+
+  my $token_state;
+  if ( $login ) {
+    $token_state = 0;    # login request
+    $headers->header( 'api-token' => $config->{ token } );
+  } elsif ( $config->{ auth_token } ) {
+    $token_state = 2;    # have cached token
+  } else {
+    $token_state = 1;    # need token
+  }
+
+  my $error;
+  my $response;
+  my $content;
+  while ( 1 ) {
+    if ( $token_state > 0 ) {
+      if ( $token_state == 1 ) {
+        print "Debug :: Requesting new session token\n" if $DEBUG;
+        ( $error, $content ) = purestorage_api_request1( $config, 'login', 'POST', 1 );
+        return ( $error, $content ) if $error > 0;
+      } else {
+        print "Debug :: Using existing session token\n" if $DEBUG;
+      }
+      $headers->header( 'x-auth-token' => $config->{ auth_token } );
+    }
+    $headers->header( 'X-Request-ID' => $config->{ request_id } ) if $config->{ request_id };
+
+    my $request = HTTP::Request->new( $method, $config->{ url } . '/api/' . $PSFA_API . '/' . $path, $headers, length( $body ) ? encode_json( $body ) : undef );
+    $response = $config->{ ua }->request( $request );
+
+    $error = $response->is_success ? 0 : 1;
+    if ( $error && $token_state == 2 && $response->code == 401 ) {
+      print "Debug :: Session token expired\n";
+      $token_state = 1;
+      next;
+    }
+    last;
+  }
+
+  $headers = $response->headers;
+  if ( $error == 0 ) {
+    if ( $token_state == 0 ) {
+      $config->{ auth_token } = $headers->header( 'x-auth-token' ) or die "Error :: PureStorage API :: Header 'x-auth-token' is missing.\n";
+      $config->{ request_id } = $headers->header( 'x-request-id' );
+    }
+    $error = -1 if $token_state < 2;    # auth_token was updated
+  }
+
+  $content = $response->decoded_content;
+  my $content_type = $headers->header( 'Content-Type' ) // '';
+  if ( $content_type =~ /application\/json/ ) {
+    $content = decode_json( $content );
+  } else {
+    $error   = $login ? 3 : 2 if $error == 1;    # non-API error (connectivity, etc.)
+    $content = { response => $content };
+  }
+
+  return ( $error, $content );
+}
+
 sub purestorage_list_volumes {
   my ( $class, $scfg, $vmid, $storeid, $destroyed ) = @_;
   print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::purestorage_list_volumes\n" if $DEBUG;
@@ -372,58 +610,12 @@ sub purestorage_get_wwn {
   print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::purestorage_get_wwn\n" if $DEBUG;
 
   my $volume = $class->purestorage_get_existing_volume_info( $scfg, $volname );
-  if ( $volume ) {
+  return get_device_path_wwn( $volume->{ serial } ) if $volume;
 
-    # Construct the WWN path
-    my $path = lc( "/dev/disk/by-id/wwn-0x" . $purestorage_wwn_prefix . $volume->{ serial } );
-    my $wwn  = lc( "3" . $purestorage_wwn_prefix . $volume->{ serial } );
-    return ( $path, $wwn );
-  }
+  warn "Warning :: Can't get volume \"$volname\" info\n";
   return ( '', '' );
 }
 
-sub purestorage_unmap_disk {
-  my ( $class, $disk_name ) = @_;
-  print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::purestorage_unmap_disk\n" if $DEBUG;
-
-  if ( $disk_name =~ m|^(sd[a-z]+)$| ) {
-    $disk_name = $1;    # untaint;
-    my $sysfs_path = "/sys/block/$disk_name";
-    my $disk_path  = "/dev/$disk_name";
-
-    if ( -e $disk_path ) {
-      exec_command( [ $cmd->{ blockdev }, '--flushbufs', $disk_path ] );
-    }
-
-    my $fh;
-    open( $fh, ">", $sysfs_path . "/device/state" ) or die "Could not open file \"$sysfs_path/device/state\" for writing.\n";
-    print $fh "offline";
-    close( $fh );
-
-    open( $fh, ">", $sysfs_path . "/device/delete" ) or die "Could not open file \"$sysfs_path/device/delete\" for writing.\n";
-    print $fh "1";
-    close( $fh );
-  }
-  return 1;
-}
-
-sub purestorage_cleanup_diskmap {
-  my ( $class ) = @_;
-  print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::purestorage_cleanup_diskmap\n" if $DEBUG;
-
-  my @disks = `lsblk -o NAME,TYPE,SIZE -nr`;
-
-  foreach my $disk_name ( @disks ) {
-    my ( $name, $type, $size ) = split( /\s+/, $disk_name );
-
-    if ( $type eq 'disk' && $size eq '0B' ) {
-      $class->purestorage_unmap_disk( $name );
-    }
-  }
-
-  return 1;
-}
-
 sub purestorage_volume_connection {
   my ( $class, $scfg, $volname, $mode ) = @_;
 
@@ -455,7 +647,7 @@ sub purestorage_volume_connection {
     }
   };
 
-  my $response = purestorage_api_request( $scfg, $action );
+  my $response = purestorage_api_request( $scfg, $action, 1 );
 
   my $message = ( $response->{ errors } ? 'already ' : '' ) . ( $mode ? 'connected to' : 'disconnected from' );
   print "Info :: Volume \"$volname\" is $message host \"$hname\".\n";
@@ -523,24 +715,6 @@ sub purestorage_remove_volume {
   return 1;
 }
 
-sub purestorage_get_device_size {
-  my ( $class, $path ) = @_;
-  print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::purestorage_get_device_size\n" if $DEBUG;
-  my $size = 0;
-
-  exec_command(
-    [ $cmd->{ blockdev }, '--getsize64', $path ],
-    1,
-    outfunc => sub {
-      $size = $_[0];
-      chomp $size;
-    }
-  );
-
-  print "Debug :: Detected size: $size\n" if $DEBUG;
-  return $size;
-}
-
 sub purestorage_resize_volume {
   my ( $class, $scfg, $volname, $size ) = @_;
   print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::purestorage_resize_volume\n" if $DEBUG;
@@ -553,39 +727,41 @@ sub purestorage_resize_volume {
     body   => { provisioned => $size }
   };
 
-  purestorage_api_request( $scfg, $action );
+  my $response = purestorage_api_request( $scfg, $action );
 
-  print "Info :: Volume \"$volname\" is resized.\n";
+  my $serial = $response->{ items }->[0]->{ serial } or die "Error :: Failed to retrieve volume serial";
 
-  my ( $path, $wwid ) = $class->purestorage_get_wwn( $scfg, $volname );
+  my ( $path, $wwid ) = get_device_path_wwn( $serial );
 
-  exec_command( [ $cmd->{ iscsiadm }, '--mode', 'node', '--rescan' ], 1 );
+  # return early if the volume is not mapped (normally should not happen)
+  return $size unless $path ne '' && -b $path;
 
-  # FIXME: wwid is probably ignored
-  exec_command( [ $cmd->{ multipath }, '-r', $wwid ], 1 );
+  my ( $device_path, @slaves ) = block_device_slaves( $path );
 
-  # Wait for the device size to update
-  my $iteration    = 0;
-  my $max_attempts = 15;    # Max iter count
-  my $interval     = 1;     # Interval for checking in seconds
-  my $new_size     = 0;
+  # Iterate through slaves and rescan each device
+  block_device_action( 'rescan', @slaves );
+
+  if ( multipath_check( $wwid ) ) {
+    print "Debug :: Device \"$wwid\" is a multipath device. Proceeding with resizing.\n" if $DEBUG;
+    exec_command( [ 'multipathd', 'resize', 'map', $wwid ] );
+  }
 
   print "Debug :: Expected size = $size\n" if $DEBUG;
 
-  while ( $iteration < $max_attempts ) {
-    print "Info :: Waiting (" . $iteration . "s) for size update for volume \"$volname\"...\n";
+  my $new_size;
+  my $updated_size = sub {
+    $new_size = get_device_size( $device_path );
+    return $new_size >= $size;
+  };
 
-    $new_size = $class->purestorage_get_device_size( $path );
-    if ( $new_size >= $size ) {
-      print "Info :: New size detected for volume \"$volname\": $new_size bytes.\n";
-      return $new_size;
-    }
+  # FIXME: With the current implementation we may not need to wait
+  wait_for( $updated_size, "volume \"$volname\" size update" );
 
-    sleep $interval;
-    ++$iteration;
-  }
+  print "Debug :: New size detected for volume \"$volname\": $new_size bytes.\n" if $DEBUG;
 
-  die "Error :: Timeout while waiting for updated size of volume \"$volname\".\n";
+  print "Info :: Volume \"$volname\" is resized.\n";
+
+  return $new_size;
 }
 
 sub purestorage_rename_volume {
@@ -798,58 +974,38 @@ sub free_image {
 
 sub list_images {
   my ( $class, $storeid, $scfg, $vmid, $vollist, $cache ) = @_;
+  print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::list_images\n" if $DEBUG;
 
-  my $key = type() . ':' . $storeid;
-  if ( $cache->{ $key } ) {
-    print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::list_images::cached\n" if $DEBUG;
-  } else {
-    print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::list_images\n" if $DEBUG;
-    $cache->{ $key } = $class->purestorage_list_volumes( $scfg, $vmid, $storeid, 0 );
-  }
-
-  return $cache->{ $key };
+  return $class->purestorage_list_volumes( $scfg, $vmid, $storeid, 0 );
 }
 
 sub status {
   my ( $class, $storeid, $scfg, $cache ) = @_;
+  print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::status\n" if $DEBUG;
 
-  $cache = $cache->{ type() . ':' . $storeid } //= {};
-  $cache->{ last_update } //= 0;
+  my $response = purestorage_api_request( $scfg, { name => 'get array space', type => 'arrays/space', method => 'GET' } );
 
-  my $current_time = gettimeofday();
-  if ( $current_time - $cache->{ last_update } >= 60 ) {
-    print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::status\n" if $DEBUG;
+  # Get storage capacity and used space from the response
+  my $array = $response->{ items }->[0];
+  my $total = $array->{ capacity };
+  my $used  = $array->{ space }->{ total_physical };
 
-    my $response = purestorage_api_request( $scfg, { name => 'get array space', type => 'arrays/space', method => 'GET' } );
-
-    # Get storage capacity and used space from the response
-    $cache->{ total } = $response->{ items }->[0]->{ capacity };
-    $cache->{ used }  = $response->{ items }->[0]->{ space }->{ total_physical };
-
-    # $cache->{ used } = $response->{ items }->[0]->{ space }->{ total_used }; # Do not know what is correct
-
-    $cache->{ last_update } = $current_time;
-  } else {
-    print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::status::cached\n" if $DEBUG;
-  }
+  # my $used = $array->{ space }->{ total_used }; # Do not know what is correct
 
   # Calculate free space
-  my $free = $cache->{ total } - $cache->{ used };
+  my $free = $total - $used;
 
   # Mark storage as active
   my $active = 1;
 
   # Return total, free, used space and the active status
-  return ( $cache->{ total }, $free, $cache->{ used }, $active );
+  return ( $total, $free, $used, $active );
 }
 
 sub activate_storage {
   my ( $class, $storeid, $scfg, $cache ) = @_;
   print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::activate_storage\n" if $DEBUG;
 
-  #FIXME: Why is this needed?
-  $class->purestorage_cleanup_diskmap();
-
   return 1;
 }
 
@@ -881,84 +1037,60 @@ sub map_volume {
 
   print "Debug :: Mapping volume \"$volname\" with WWN: " . uc( $wwid ) . ".\n" if $DEBUG;
 
-  exec_command( [ $cmd->{ multipath }, '-a', $wwid ], 1 );
-
-  exec_command( [ $cmd->{ iscsiadm }, '--mode', 'session', '--rescan' ], 1 );
-
-  # Wait for the device to apear
-  my $iteration    = 0;
-  my $max_attempts = 15;
-  my $interval     = 1;
-
-  while ( $iteration < $max_attempts ) {
-    print "Info :: Waiting (" . $iteration . "s) for map volume \"$volname\"...\n";
-    $iteration++;
-    if ( -e $path ) {
-      return $path;
-    }
-    sleep $interval;
+  my $protocol = $scfg->{ protocol } // $default_protocol;
+  if ( $protocol eq 'iscsi' || $protocol eq 'fc' ) {
+    scsi_scan_new( $protocol );
+  } elsif ( $protocol eq 'nvme' ) {
+    die "Error :: Protocol: \"$protocol\" isn't implemented yet.\n";
+  } else {
+    die "Error :: Protocol: \"$protocol\" isn't a valid protocol.\n";
   }
 
-  die "Error :: Local path \"$path\" does not exist.\n";
+  my $path_exists = sub {
+    return -e $path;
+  };
+
+  # Wait for the device to appear
+  wait_for( $path_exists, "volume \"$volname\" to map" );
+
+  # we might end up with operational disk but without multipathing, e.g.
+  # if unmapping was interrupted ('remove map' was already done, but slaves were not removed)
+  exec_command( [ 'multipathd', 'add', 'map', $wwid ] ) unless multipath_check( $wwid );
+
+  return $path;
 }
 
 sub unmap_volume {
   my ( $class, $storeid, $scfg, $volname, $snapname ) = @_;
   print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::unmap_volume\n" if $DEBUG;
 
-  my ( $path, undef, undef, $wwid ) = $class->filesystem_path( $scfg, $volname );
+  my ( $path, $wwid ) = $class->purestorage_get_wwn( $scfg, $volname );
+  return 0 unless $path ne '' && -b $path;
 
-  if ( $path && -b $path ) {
-    my $device_path = abs_path( $path );
-    if ( defined( $device_path ) ) {
-      print "Info :: Device path resolved to \"$device_path\".\n";
-    } else {
-      die "Error :: unable to get device path for $path - $!.\n";
-    }
+  my ( $device_path, @slaves ) = block_device_slaves( $path );
 
-    exec_command( [ $cmd->{ blockdev }, '--flushbufs', $path ] );
+  exec_command( [ 'blockdev', '--flushbufs', $device_path ] );
 
-    my $device_name = basename( $device_path );
-    my $slaves_path = "/sys/block/$device_name/slaves";
+  # this may help if there is a write-back cache (see issue #47)
+  ## my $fuser = sub {
+  ##   return exec_command( [ 'fuser', '-s', $device_path ], -1 );
+  ## };
+  ## wait_for( $fuser, 'device cache flush', 30, 0.5 );
 
-    my @slaves = ();
-    if ( -d $slaves_path ) {
-      opendir( my $dh, $slaves_path ) or die "Cannot open directory: $!";
-      @slaves = grep { !/^\.\.?$/ } readdir( $dh );
-      closedir( $dh );
-      print "Info :: Disk \"$device_name\" slaves: " . join( ', ', @slaves ) . "\n" if $DEBUG;
-    } elsif ( $device_name =~ m|^(sd[a-z]+)$| ) {
-      warn "Warning :: Disk \"$device_name\" has no slaves.\n";
-      push @slaves, $1;
-    }
+  if ( multipath_check( $wwid ) ) {
+    print "Debug :: Device \"$wwid\" is a multipath device. Proceeding with multipath removal.\n" if $DEBUG;
 
-    my $multipath_check = `$cmd->{ "multipath" } -l $wwid`;
-    if ( $multipath_check ) {
-      print "Info :: Device \"$device_path\" is a multipath device. Proceeding with multipath removal.\n";
-      exec_command( [ $cmd->{ multipath }, '-w', $wwid ] );
-
-      # remove the link
-      exec_command( [ $cmd->{ multipath }, '-f', $wwid ] );
-    } else {
-      print "Info :: Device \"$wwid\" is not a multipath device. Skipping multipath removal.\n";
-    }
-
-    # Iterate through slaves and delete each device
-    foreach my $slave_name ( @slaves ) {
-      print "Info :: Remove slave: $slave_name\n" if $DEBUG;
-      if ( $slave_name =~ m|^(sd[a-z]+)$| ) {
-        $slave_name = $1;    # untaint;
-        $class->purestorage_unmap_disk( $slave_name );
-      } else {
-        die "Error :: Invalid disk name \"$slave_name\".";
-      }
-    }
-
-    print "Info :: Device \"$device_name\" removed from system.\n";
-    return 1;
+    # remove the link
+    exec_command( [ 'multipathd', 'remove', 'map', $wwid ] );
+  } else {
+    print "Debug :: Device \"$wwid\" is not a multipath device. Skipping multipath removal.\n" if $DEBUG;
   }
 
-  return 0;
+  # Iterate through slaves and remove each device
+  block_device_action( 'remove', @slaves );
+
+  print "Debug :: Device \"$wwid\" is removed.\n" if $DEBUG;
+  return 1;
 }
 
 sub activate_volume {

README.md

@@ -73,7 +73,7 @@ To manually install the plugin, follow these steps:
 
 ```bash
 # Clone repository
-git clone git@github.com:kolesa-team/pve-purestorage.git
+git clone https://github.com/kolesa-team/pve-purestorage-plugin.git
 # Navigate to the Plugin Directory
 cd pve-purestorage
 # Create the custom plugin directory if it does not already exist
@@ -102,7 +102,7 @@ After installing the plugin, you need to configure Proxmox VE to use it. Since P
 pvesm add purestorage <storage_id> \
    --nodes <proxmox_node_list> \
    --address https://<purestorage_fqdn_or_ip> \
-   --token token <purestorage_api_token> \
+   --token <purestorage_api_token> \
    --vgname <purestorage_volume_group_name> \
    --hgsuffix <purestorage_host_suffix>
    --content images
@@ -131,6 +131,7 @@ purestorage: <storage_id>
 | vnprefix | (`optional`) The prefix to prepend to name of virtual disks. |
 | hgsuffix | (`optional`) A suffix that is appended to the hostname when the plugin interacts with the Pure Storage array. This can help differentiate hosts if necessary. |
 | content | Specifies the types of content that can be stored. For virtual machine disk images, use images. |
+| protocol | (`optional`, default is `iscsi`) Specifies the storage protocol (iscsi, fc) |
 
 > **_NOTE:_** Ensure that the token and other sensitive information are kept secure and not exposed publicly.