From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 5B24D1FF13C for ; Thu, 25 Jun 2026 18:42:25 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 62EBBE9B; Thu, 25 Jun 2026 18:42:03 +0200 (CEST) From: "Max R. Carrara" To: pve-devel@lists.proxmox.com Subject: [PATCH pve-storage 7/9] luncmd: lio: modernize sub `delete_lun()` Date: Thu, 25 Jun 2026 18:40:59 +0200 Message-ID: <20260625164110.706694-8-m.carrara@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260625164110.706694-1-m.carrara@proxmox.com> References: <20260625164110.706694-1-m.carrara@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1782405680547 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.080 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: HNLLITMO6I4HUQIVE5BC6F5QISWMZXPA X-Message-ID-Hash: HNLLITMO6I4HUQIVE5BC6F5QISWMZXPA X-MailFrom: m.carrara@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 --- 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