NAS-133513 / 25.04 / Add iSCSI target parameters mechanism for Queued…

…Commands (#15362)

* Add iSCSI target parameters mechanism for QueuedCommands

* Add test__target_iscsi_parameters

src/middlewared/middlewared/alembic/versions/25.04/2025-01-09_15-46_iscsi_target_parameters.py

@@ -0,0 +1,25 @@
+"""iSCSI target parameters
+
+Revision ID: 899852cb2a92
+Revises: 83d9689fcbc8
+Create Date: 2025-01-09 15:46:35.550172+00:00
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = '899852cb2a92'
+down_revision = '83d9689fcbc8'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    with op.batch_alter_table('services_iscsitarget', schema=None) as batch_op:
+        batch_op.add_column(sa.Column('iscsi_target_iscsi_parameters', sa.TEXT(), nullable=True))
+
+def downgrade():
+    with op.batch_alter_table('services_iscsitarget', schema=None) as batch_op:
+        batch_op.drop_column('iscsi_target_iscsi_parameters')

src/middlewared/middlewared/api/v25_04_0/iscsi_target.py

@@ -31,6 +31,10 @@ class IscsiGroup(BaseModel):
     auth: int | None = None
 
 
+class IscsiTargetParameters(BaseModel):
+    QueuedCommands: Literal[32, 128] | None = None
+
+
 class IscsiTargetEntry(BaseModel):
     id: int
     name: Annotated[NonEmptyString,
@@ -46,6 +50,7 @@ class IscsiTargetEntry(BaseModel):
     groups: list[IscsiGroup] = []
     auth_networks: list[str] = []  # IPvAnyNetwork: "Object of type IPv4Network is not JSON serializable", etc
     rel_tgt_id: int
+    iscsi_parameters: IscsiTargetParameters | None = None
 
 
 class IscsiTargetValidateNameArgs(BaseModel):

src/middlewared/middlewared/etc_files/scst.conf.mako

@@ -8,6 +8,7 @@
 
     from middlewared.service import CallError
     from middlewared.plugins.fc.utils import is_fc_addr, str_to_naa, wwn_as_colon_hex
+    from middlewared.plugins.iscsi_.utils import ISCSI_TARGET_PARAMETERS, ISCSI_HA_TARGET_PARAMETERS
 
     REL_TGT_ID_NODEB_OFFSET = 32000
     REL_TGT_ID_FC_OFFSET = 5000
@@ -295,6 +296,11 @@
             if set(devices).issubset(clustered_extents):
                 return True
         return False
+
+    def option_value(v):
+        if isinstance(v, bool):
+            return "Yes" if v else "No"
+        return v
 %>\
 ##
 ## If we are on a HA system then write out a cluster name, we'll hard-code
@@ -523,6 +529,14 @@ TARGET_DRIVER iscsi {
 %   if mutual_chap:
         OutgoingUser "${mutual_chap}"
 %   endif
+##
+## Add target parameters (if not None value)
+##
+% for k,v in target.get('iscsi_parameters', {}).items():
+%   if k in ISCSI_TARGET_PARAMETERS and v is not None:
+        ${k} ${option_value(v)}
+%   endif
+% endfor
 
         GROUP security_group {
 %   for access_control in iscsi_initiator_portal_access:
@@ -568,6 +582,14 @@ ${retrieve_luns(target['id'], ' ' * 4)}\
         forward_dst 1
         aen_disabled 1
         forwarding 1
+##
+## Add target parameters (if not None value)
+##
+% for k,v in target.get('iscsi_parameters', {}).items():
+%   if k in ISCSI_HA_TARGET_PARAMETERS and v is not None:
+        ${k} ${option_value(v)}
+%   endif
+% endfor
 ${retrieve_luns(target['id'],'')}\
     }
 %     endfor

src/middlewared/middlewared/plugins/iscsi_/scst.py

@@ -2,6 +2,7 @@
 import pathlib
 
 from middlewared.service import Service
+from .utils import ISCSI_TARGET_PARAMETERS
 
 SCST_BASE = '/sys/kernel/scst_tgt'
 SCST_TARGETS_ISCSI_ENABLED_PATH = '/sys/kernel/scst_tgt/targets/iscsi/enabled'
@@ -144,3 +145,17 @@ def set_node_optimized(self, node):
         else:
             pathlib.Path(SCST_CONTROLLER_A_TARGET_GROUPS_STATE).write_text("nonoptimized\n")
             pathlib.Path(SCST_CONTROLLER_B_TARGET_GROUPS_STATE).write_text("active\n")
+
+    def reset_target_parameters(self, iqn, parameter_names):
+        """Reset the specified parameters to their default values."""
+        # Do some sanity checking
+        for param in parameter_names:
+            if param not in ISCSI_TARGET_PARAMETERS:
+                raise ValueError('Invalid parameter name supplied', param)
+        iqndir = pathlib.Path(f'{SCST_BASE}/targets/iscsi/{iqn}')
+        for param in parameter_names:
+            try:
+                (iqndir / pathlib.Path(param)).write_text(':default:\n')
+            except (FileNotFoundError, PermissionError):
+                # If we're not running, that's OK
+                pass

src/middlewared/middlewared/plugins/iscsi_/targets.py

@@ -30,6 +30,7 @@ class iSCSITargetModel(sa.Model):
     iscsi_target_mode = sa.Column(sa.String(20), default='iscsi')
     iscsi_target_auth_networks = sa.Column(sa.JSON(list))
     iscsi_target_rel_tgt_id = sa.Column(sa.Integer(), unique=True)
+    iscsi_target_iscsi_parameters = sa.Column(sa.JSON(), nullable=True)
 
 
 class iSCSITargetGroupModel(sa.Model):
@@ -323,9 +324,31 @@ async def do_update(self, audit_callback, id_, data):
         await self._service_change('iscsitarget', 'reload', options={'ha_propagate': False})
 
         # Then process the BACKUP config if we are HA and ALUA is enabled.
-        if await self.middleware.call("iscsi.global.alua_enabled") and await self.middleware.call('failover.remote_connected'):
+        alua_enabled = await self.middleware.call("iscsi.global.alua_enabled")
+        run_on_peer = alua_enabled and await self.middleware.call('failover.remote_connected')
+        if run_on_peer:
             await self.middleware.call('failover.call_remote', 'service.reload', ['iscsitarget'])
 
+        # NOTE: Any parameters whose keys are omitted will be removed from the config i.e. we
+        # will deliberately revert removed items to the SCST default value
+        old_params = set(old.get('iscsi_parameters', {}).keys())
+        if old_params:
+            new_params = set(new.get('iscsi_parameters', {}).keys())
+            reset_params = old_params - new_params
+            # Has the value just been set to None
+            for param in old_params & new_params:
+                if new['iscsi_parameters'][param] is None and old['iscsi_parameters'][param] is not None:
+                    reset_params.add(param)
+            if reset_params:
+                global_basename = (await self.middleware.call('iscsi.global.config'))['basename']
+                iqn = f'{global_basename}:{new["name"]}'
+                await self.middleware.call('iscsi.scst.reset_target_parameters', iqn, list(reset_params))
+                if alua_enabled:
+                    ha_iqn = f'{global_basename}:HA:{new["name"]}'
+                    await self.middleware.call('iscsi.scst.reset_target_parameters', ha_iqn, list(reset_params))
+                if run_on_peer:
+                    await self.middleware.call('failover.call_remote', 'iscsi.scst.reset_target_parameters', [iqn, list(reset_params)])
+
         return await self.get_instance(id_)
 
     @api_method(

src/middlewared/middlewared/plugins/iscsi_/utils.py

@@ -25,3 +25,7 @@ def inv(self):
 
 # Currently SCST has this limit (scst_vdisk_dev->name)
 MAX_EXTENT_NAME_LEN = 64
+
+# We deliberately only support a subset of target parameters
+ISCSI_TARGET_PARAMETERS = ['QueuedCommands']
+ISCSI_HA_TARGET_PARAMETERS = ['QueuedCommands']

tests/api2/test_261_iscsi_cmd.py

@@ -1,4 +1,5 @@
 import contextlib
+import copy
 import enum
 import errno
 import ipaddress
@@ -3049,3 +3050,39 @@ def test_empty(path, extent_size_mb=5):
                 s.write16(0, 1, zeros)
                 s.synchronizecache16(0, 1)
                 assert test_empty(test_path) is True, 'Expected zero data'
+
+
+def read_target_value(iqn, name):
+    return ssh(f'head -1 /sys/kernel/scst_tgt/targets/iscsi/{iqn}/{name}').strip()
+
+
+def test__target_iscsi_parameters(iscsi_running):
+    """Test iSCSI target parameters"""
+    def new_params_val(params, key, val):
+        new = copy.copy(params)
+        new[key] = val
+        return {'iscsi_parameters': new}
+
+    DEFAULT_QUEUED_COMMANDS = 32
+    iqn = f'{basename}:{target_name}'
+    with initiator_portal() as config:
+        with configured_target(config, target_name, 'VOLUME') as target_config:
+            target_id = target_config['target']['id']
+            params = target_config['target']['iscsi_parameters']
+            # QueuedCommands
+            # Check the default
+            assert params['QueuedCommands'] is None, params
+            assert int(read_target_value(iqn, 'QueuedCommands')) == DEFAULT_QUEUED_COMMANDS
+            # Set to 128
+            call('iscsi.target.update', target_id, new_params_val(params, 'QueuedCommands', 128))
+            assert int(read_target_value(iqn, 'QueuedCommands')) == 128
+            # Set to None and ensure it has the default
+            call('iscsi.target.update', target_id, new_params_val(params, 'QueuedCommands', None))
+            assert int(read_target_value(iqn, 'QueuedCommands')) == DEFAULT_QUEUED_COMMANDS
+            # Now we'll test removing it from the dict
+            call('iscsi.target.update', target_id, new_params_val(params, 'QueuedCommands', 128))
+            assert int(read_target_value(iqn, 'QueuedCommands')) == 128
+            new_params = copy.copy(params)
+            del new_params['QueuedCommands']
+            call('iscsi.target.update', target_id, {'iscsi_parameters': new_params})
+            assert int(read_target_value(iqn, 'QueuedCommands')) == DEFAULT_QUEUED_COMMANDS