[pve-devel] applied: [PATCH storage v2] iscsi: disable Open-iSCSI login retries

[Thomas Lamprecht]

 Am 11/10/2024 um 14:58 schrieb Friedrich Weber:
> Since 90c1b10 ("fix #254: iscsi: add support for multipath targets"),
> iSCSI storage activation checks whether a session exists for each
> discovered portal. If there is a discovered portal without a session,
> it performs a discovery and login in the hope of establishing a
> session to the portal. If the portal is unreachable when trying to log
> in, Open-iSCSI's default behavior is to retry for up to 2 minutes, as
> explained in /etc/iscsi/iscid.conf:
> 
>> # The default node.session.initial_login_retry_max is 8 and
>> # node.conn[0].timeo.login_timeout is 15 so we have:
>> #
>> # node.conn[0].timeo.login_timeout * \
>> node.session.initial_login_retry_max = 120s
> 
> If pvestatd is activating the storage, it will be blocked during that
> time, which is undesirable. This is particularly unfortunate if the
> target announces portals that the host permanently cannot reach. In
> that case, every pvestatd iteration will take 2 minutes. While it can
> be argued that such setups are misconfigured, it is still desirable to
> keep the fallout of that misconfiguration as low as possible.
> 
> In order to reduce the time Open-iSCSI tries to log in, instruct
> Open-ISCSI to not perform login retries for that target. For this, set
> node.session.initial_login_retry_max for the target to 0. This setting
> is stored in Open-iSCSI's records under /etc/iscsi/nodes. As these
> records are overwritten with the defaults from /etc/iscsi/iscsid.conf
> on discovery, the setting needs to be applied after discovery.
> 
> With this setting, one login attempt should take at most 15 seconds.
> This is still higher than pvestatd's iteration time of 10 seconds, but
> more tolerable. Logins will still be continuously retried by pvestatd
> in every iteration until there is a session to each discovered portal.
> 
> Signed-off-by: Friedrich Weber <f.weber at proxmox.com>
> Tested-by: Mira Limbeck <m.limbeck at proxmox.com>
> Reviewed-by: Mira Limbeck <m.limbeck at proxmox.com>
> ---
> 
> Notes:
>     changes since v1:
>     - dropped "to avoid blocking pvestatd" from commit title, as pvestatd
>       will still be blocked, just for a shorter time
>     - changed formatting
>     - added Mira's R-b and T-b (thx!). Took the liberty to include them
>       although the code, strictly speaking, changed
>     
>     v1: https://lore.proxmox.com/pve-devel/20241011093744.25545-1-f.weber@proxmox.com/
> 
>  src/PVE/Storage/ISCSIPlugin.pm | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
>

applied, thanks!

FWIW, for such run command call where the CMD does not need to get arguments
added on some conditional check you could avoid the intermediate variable, i.e.:

run_command([
    $ISCSIADM,
    ...,
]);

but no hard feelings, I kept it as is, this just stuck out to me.