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 797601FF146 for ; Tue, 26 May 2026 10:36:37 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id BC89716701; Tue, 26 May 2026 10:36:35 +0200 (CEST) Message-ID: <855f8336-66ff-4bd9-823f-3f3572c6dd96@proxmox.com> Date: Tue, 26 May 2026 10:36:30 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 storage 12/15] iscsi: add support for non-persistent discovery To: Thomas Lamprecht , Proxmox VE development discussion References: <20260430173220.441001-1-m.limbeck@proxmox.com> <20260430173220.441001-13-m.limbeck@proxmox.com> <20260523212856.2822353-9-t.lamprecht@proxmox.com> Content-Language: en-US From: Mira Limbeck In-Reply-To: <20260523212856.2822353-9-t.lamprecht@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1779784566113 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.353 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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [iscsiplugin.pm] Message-ID-Hash: POANJN7VJA7TS6YJTZNIOMCON2XALEQ6 X-Message-ID-Hash: POANJN7VJA7TS6YJTZNIOMCON2XALEQ6 X-MailFrom: m.limbeck@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: On 5/23/26 11:28 PM, Thomas Lamprecht wrote: > On Thu, 30 Apr 2026 19:27:10 +0200, Mira Limbeck wrote: >> diff --git a/src/PVE/Storage/ISCSIPlugin.pm b/src/PVE/Storage/ISCSIPlugin.pm >> @@ -226,7 +227,7 @@ sub iscsi_discovery { >> # In case of multipath we can stop after receiving targets from any available portal >> - last if scalar(keys %$res) > 0; >> + last if defined($target) && scalar(keys %$res) > 0; > > $target is declared with `my` inside the outfunc closure a few lines up, so > it is out of scope here - this doesn't compile under `use strict` ("Global > symbol $target requires explicit package name"). 13/15 rewrites the line to > use $target_found, so the final status is again fine, but this commit on its > own breaks the build. Worth folding that fix into this commit for > bisectability. > > btw. I like using rebase + exec for ensuring that every commit is clean > from code format and testing/build POV. It's slightly nicer in rust projects > where just cargo fmt / check / test calls can be used in a generic fashion > independent of the specific repo layout, but most of our perl repos are > rather uniform in its basic targets nowadays, so something like: > > git rebase --update-refs -x 'printf "\n-------\n"' \ > -x 'git show --no-patch --oneline --no-decorate HEAD' \ > -x 'make tidy' \ > -x 'make -C src test' \ > origin/master > > should do the trick (btw. --update-refs is normally not required, but > essential for stacked branches for (sub) series, and it normally doesn't > hurt to pass). > We might want to add something like that to the developer wiki, or maybe > even add a proper (e.g.) "dev-test" make target that combines the four -x > steps (for me my shell history got me covered ^^). Thank you for the tip! I've rebased it so many times, but didn't think of doing an exec and building every commit on its own.