From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id CC63D1FF13C for ; Thu, 25 Jun 2026 18:42:17 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 2921AD56; Thu, 25 Jun 2026 18:41:59 +0200 (CEST) From: "Max R. Carrara" To: pve-devel@lists.proxmox.com Subject: [PATCH pve-storage 6/9] luncmd: lio: modernize sub `create_lun()` Date: Thu, 25 Jun 2026 18:40:58 +0200 Message-ID: <20260625164110.706694-7-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: 1782405678457 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.070 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 POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_1 0.1 random spam to be learned in bayes POISEN_SPAM_PILL_3 0.1 random spam to be learned in bayes 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: DTRPLNMLYJSCCXJ6RCKCAT5QMOOGS3DF X-Message-ID-Hash: DTRPLNMLYJSCCXJ6RCKCAT5QMOOGS3DF 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 `create_lun()` a "proper" private subroutine instead of a lexically-scoped sub reference. Use the new `run_remote_command()` helper for slightly easier reading and better error messages. Signed-off-by: Max R. Carrara --- src/PVE/Storage/LunCmd/LIO.pm | 67 ++++++++++++++++++++++++----------- 1 file changed, 47 insertions(+), 20 deletions(-) diff --git a/src/PVE/Storage/LunCmd/LIO.pm b/src/PVE/Storage/LunCmd/LIO.pm index 483e2459..143b3fda 100644 --- a/src/PVE/Storage/LunCmd/LIO.pm +++ b/src/PVE/Storage/LunCmd/LIO.pm @@ -283,14 +283,15 @@ my sub list_lun { } # adds a new LUN to the target -my $create_lun = sub { +my sub create_lun { my ($scfg, $config, $timeout, $method, @params) = @_; + my $device = $params[0]; + if (list_lun($scfg, $config, $timeout, $method, @params)) { - die "$params[0]: LUN already exists!"; + die "$device: LUN already exists!"; } - my $device = $params[0]; my $volname = extract_volname($scfg, $config, $device); # Here we create a new device, so we didn't get the volname prefixed with the pool name # as extract_volname couldn't find a matching vol yet @@ -298,37 +299,63 @@ my $create_lun = sub { 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}; + my $backstore_res = run_remote_command( + $scfg, + [$targetcli, $BACKSTORE, 'create', "name=$volname", "dev=$device"], + timeout => $timeout, + ); + if (!$backstore_res->{result}) { + my $err_msg = + "Error while creating backstore for '$device':\n" . $backstore_res->{'merged-log'}; + die $err_msg; + } # step 2: enable unmap support on the backstore - @cliparams = ($BACKSTORE . '/' . $volname, 'set', 'attribute', 'emulate_tpu=1'); - $res = $execute_remote_command->($scfg, $timeout, $targetcli, @cliparams); - die $res->{msg} if !$res->{result}; + my $unmap_res = run_remote_command( + $scfg, + [$targetcli, "$BACKSTORE/$volname", 'set', 'attribute', 'emulate_tpu=1'], + timeout => $timeout, + ); + if (!$unmap_res->{result}) { + my $err_msg = + "Error while enabling unmap support on backstore for '$device':\n" + . $unmap_res->{'merged-log'}; + die $err_msg; + } # step 3: 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}; + my $register_res = run_remote_command( + $scfg, + [$targetcli, "/iscsi/$scfg->{target}/$tpg/luns", 'create', "$BACKSTORE/$volname"], + timeout => $timeout, + ); + if (!$register_res->{result}) { + my $err_msg = + "Error while registering LUN for '$device':\n" . $register_res->{'merged-log'}; + die $err_msg; + } # 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+)/) { + if ($register_res->{'out-log'} =~ /LUN (\d+)/) { $lun_idx = $1; } else { - die "unable to determine new LUN index: $res->{msg}"; + die "Unable to determine new LUN index: " . $register_res->{'merged-log'}; } - # step 3: unfortunately, targetcli doesn't always save changes, no matter + # step 4: 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'); + 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}; - return $res->{msg}; -}; + return $register_res->{'merged-log'}; +} my $delete_lun = sub { my ($scfg, $config, $timeout, $method, @params) = @_; @@ -370,7 +397,7 @@ my $delete_lun = sub { my $import_lun = sub { my ($scfg, $config, $timeout, $method, @params) = @_; - return $create_lun->($scfg, $config, $timeout, $method, @params); + return create_lun($scfg, $config, $timeout, $method, @params); }; # needed for example when the underlying ZFS volume has been resized @@ -387,7 +414,7 @@ my $add_view = sub { }; my %lun_cmd_map = ( - create_lu => $create_lun, + create_lu => sub { create_lun(@_) }, delete_lu => $delete_lun, import_lu => $import_lun, modify_lu => $modify_lun, -- 2.47.3