[PATCH pve-storage 7/9] luncmd: lio: modernize sub `delete_lun()`

["Max R. Carrara" <m.carrara@proxmox.com>]

As before, make `delete_lun()` a "proper" private subroutine instead
of a lexically-scoped sub reference.

Use the new `run_remote_command()` helper for easier reading and
better error messages. Clean up the code style along the way a little.

Remove the old `$execute_remote_command` helper, as it is not used
anymore now.

Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
---
 src/PVE/Storage/LunCmd/LIO.pm | 96 +++++++++++++----------------------
 1 file changed, 35 insertions(+), 61 deletions(-)

diff --git a/src/PVE/Storage/LunCmd/LIO.pm b/src/PVE/Storage/LunCmd/LIO.pm
index 143b3fda..f4ad2ac8 100644
--- a/src/PVE/Storage/LunCmd/LIO.pm
+++ b/src/PVE/Storage/LunCmd/LIO.pm
@@ -166,47 +166,6 @@ my sub parse_remote_config {
     return $parsed_config;
 }
 
-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 { $msg .= "$_[0]\n" };
-    my $errfunc = sub { $err .= "$_[0]\n" };
-
-    $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;
-};
-
 my sub get_backstore_prefix {
     my ($scfg) = @_;
     my $pool = $scfg->{pool};
@@ -357,10 +316,8 @@ my sub create_lun {
     return $register_res->{'merged-log'};
 }
 
-my $delete_lun = sub {
+my sub delete_lun {
     my ($scfg, $config, $timeout, $method, @params) = @_;
-    my $res = { msg => undef };
-
     my $tpg = $scfg->{lio_tpg} || die "Target Portal Group not set, aborting!\n";
 
     my $path = $params[0];
@@ -368,31 +325,48 @@ my $delete_lun = sub {
     my $id = "$scfg->{portal}.$scfg->{target}";
     my $target = $config->{$id};
 
-    foreach my $lun (@{ $target->{luns} }) {
+    for my $lun ($target->{luns}->@*) {
         next if $lun->{storage_object} ne "$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};
+        my $lun_path = "/iscsi/$scfg->{target}/$tpg/luns/";
+        my $lun_id = 'lun' . $lun->{index};
+        my $lun_delete_res = run_remote_command(
+            $scfg,
+            [$targetcli, $lun_path, 'delete', $lun_id],
+            timeout => $timeout,
+        );
+        if (!$lun_delete_res->{result}) {
+            my $err_msg = "Failed to delete LUN '$lun_id' in '$lun_path':\n"
+                . $lun_delete_res->{'merged-log'};
+            die $err_msg;
+        }
 
         # step 2: delete the backstore
-        @cliparams = ($BACKSTORE, 'delete', $volname);
-        $res = $execute_remote_command->($scfg, $timeout, $targetcli, @cliparams);
-        do {
-            die $res->{msg};
-        } unless $res->{result};
+        my $backstore_delete_res = run_remote_command(
+            $scfg,
+            [$targetcli, $BACKSTORE, 'delete', $volname],
+            timeout => $timeout,
+        );
+        if (!$backstore_delete_res->{result}) {
+            my $err_msg = "Failed to delete volume '$volname' from backstore '$BACKSTORE':\n"
+                . $backstore_delete_res->{'merged-log'};
+            die $err_msg;
+        }
 
-        # step 3: save to be safe ...
-        $execute_remote_command->($scfg, $timeout, $targetcli, 'saveconfig');
+        # 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 ...
+        my $saveconfig_res = run_remote_command(
+            $scfg, [$targetcli, 'saveconfig'], timeout => $timeout,
+        );
+        warn("Could not save LUN config: " . $saveconfig_res->{'merged-log'} . "\n")
+            if !$saveconfig_res->{result};
 
-        last;
+        return $backstore_delete_res->{'out-log'};
     }
 
-    return $res->{msg};
-};
+    die "Failed to delete LUN '$path'";
+}
 
 my $import_lun = sub {
     my ($scfg, $config, $timeout, $method, @params) = @_;
@@ -415,7 +389,7 @@ my $add_view = sub {
 
 my %lun_cmd_map = (
     create_lu => sub { create_lun(@_) },
-    delete_lu => $delete_lun,
+    delete_lu => sub { delete_lun(@_) },
     import_lu => $import_lun,
     modify_lu => $modify_lun,
     add_view => $add_view,
-- 
2.47.3