From: Avan Thakkar Date: Thu, 29 Jan 2026 07:18:59 +0000 (+0530) Subject: mgr/smb: QoS bandwidth pass-through and burst_mult parameter X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=b09c5f41096ae9c9b2ff1fc7bcd451e13254d704;p=ceph.git mgr/smb: QoS bandwidth pass-through and burst_mult parameter Replace delay_max with burst_mult and add human-readable bandwidth format support for QoS configuration. Signed-off-by: Avan Thakkar --- diff --git a/src/pybind/mgr/smb/handler.py b/src/pybind/mgr/smb/handler.py index 612d5b8b6d8..1ae364cd1a9 100644 --- a/src/pybind/mgr/smb/handler.py +++ b/src/pybind/mgr/smb/handler.py @@ -852,10 +852,10 @@ def _generate_share(conf: _ShareConf) -> Dict[str, Dict[str, str]]: for field in ( "read_iops_limit", "read_bw_limit", - "read_delay_max", + "read_burst_mult", "write_iops_limit", "write_bw_limit", - "write_delay_max", + "write_burst_mult", ): if value := getattr(qos, field): opts[f"{vfs_rl}:{field}"] = str(value) diff --git a/src/pybind/mgr/smb/module.py b/src/pybind/mgr/smb/module.py index 70f4e3aea2d..53b28006950 100644 --- a/src/pybind/mgr/smb/module.py +++ b/src/pybind/mgr/smb/module.py @@ -396,10 +396,10 @@ class Module(orchestrator.OrchestratorClientMixin, MgrModule): share_id: str, read_iops_limit: Optional[int] = None, write_iops_limit: Optional[int] = None, - read_bw_limit: Optional[int] = None, - write_bw_limit: Optional[int] = None, - read_delay_max: Optional[int] = 30, - write_delay_max: Optional[int] = 30, + read_bw_limit: Optional[str] = None, + write_bw_limit: Optional[str] = None, + read_burst_mult: Optional[int] = None, + write_burst_mult: Optional[int] = None, ) -> results.Result: """Update QoS settings for a CephFS share""" try: @@ -418,8 +418,8 @@ class Module(orchestrator.OrchestratorClientMixin, MgrModule): write_iops_limit=write_iops_limit, read_bw_limit=read_bw_limit, write_bw_limit=write_bw_limit, - read_delay_max=read_delay_max, - write_delay_max=write_delay_max, + read_burst_mult=read_burst_mult, + write_burst_mult=write_burst_mult, ) updated_share = replace(share, cephfs=updated_cephfs) diff --git a/src/pybind/mgr/smb/resources.py b/src/pybind/mgr/smb/resources.py index e72a5f82cc6..19f0f9ae754 100644 --- a/src/pybind/mgr/smb/resources.py +++ b/src/pybind/mgr/smb/resources.py @@ -14,7 +14,14 @@ from ceph.deployment.service_spec import ( SMBClusterPublicIPSpec, SpecValidationError, ) -from ceph.smb.constants import REMOTE_CONTROL, REMOTE_CONTROL_LOCAL +from ceph.smb.constants import ( + BURST_MULT_MAX, + BURST_MULT_MIN, + BYTES_LIMIT_MAX, + IOPS_LIMIT_MAX, + REMOTE_CONTROL, + REMOTE_CONTROL_LOCAL, +) from ceph.smb.network import to_network from object_format import ErrorResponseBase @@ -141,10 +148,10 @@ class QoSConfig(_RBase): read_iops_limit: Optional[int] = None write_iops_limit: Optional[int] = None - read_bw_limit: Optional[int] = None - write_bw_limit: Optional[int] = None - read_delay_max: Optional[int] = 30 - write_delay_max: Optional[int] = 30 + read_bw_limit: Optional[str] = None + write_bw_limit: Optional[str] = None + read_burst_mult: Optional[int] = 15 + write_burst_mult: Optional[int] = 15 @resourcelib.component() @@ -157,11 +164,6 @@ class CephFSStorage(_RBase): subvolume: str = '' provider: CephFSStorageProvider = CephFSStorageProvider.SAMBA_VFS qos: Optional[QoSConfig] = None - DELAY_MAX_LIMIT = 300 - # Maximal value for iops_limit - IOPS_LIMIT_MAX = 1_000_000 - # Maximal value for bw_limit (1 << 40 = 1 TB) - BYTES_LIMIT_MAX = 1 << 40 def __post_init__(self) -> None: # Allow a shortcut form of / in the subvolume @@ -198,47 +200,83 @@ class CephFSStorage(_RBase): rc.qos.quiet = True return rc + @staticmethod + def _apply_limit( + qos_updates: Dict[str, Union[int, str, None]], + key: str, + value: Union[int, str, None], + maximum: int, + ) -> None: + """Apply a QoS limit to the updates dict, handling disable (0) and capping at maximum.""" + if value is None: + return + if value == 0 or value == "0": + qos_updates[key] = None + return + if isinstance(value, str): + if value.isdigit(): + qos_updates[key] = str(min(int(value), maximum)) + else: + qos_updates[key] = value + else: + qos_updates[key] = min(value, maximum) + def update_qos( self, *, read_iops_limit: Optional[int] = None, write_iops_limit: Optional[int] = None, - read_bw_limit: Optional[int] = None, - write_bw_limit: Optional[int] = None, - read_delay_max: Optional[int] = 30, - write_delay_max: Optional[int] = 30, + read_bw_limit: Optional[str] = None, + write_bw_limit: Optional[str] = None, + read_burst_mult: Optional[int] = None, + write_burst_mult: Optional[int] = None, ) -> Self: """Return a new CephFSStorage instance with updated QoS values.""" - - qos_updates = {} + qos_updates: Dict[str, Union[int, str, None]] = {} new_qos: Optional[QoSConfig] = None - if read_iops_limit is not None and read_iops_limit > 0: - qos_updates["read_iops_limit"] = min( - read_iops_limit, self.IOPS_LIMIT_MAX - ) - if write_iops_limit is not None and write_iops_limit > 0: - qos_updates["write_iops_limit"] = min( - write_iops_limit, self.IOPS_LIMIT_MAX - ) - if read_bw_limit is not None and read_bw_limit > 0: - qos_updates["read_bw_limit"] = min( - read_bw_limit, self.BYTES_LIMIT_MAX - ) - if write_bw_limit is not None and write_bw_limit > 0: - qos_updates["write_bw_limit"] = min( - write_bw_limit, self.BYTES_LIMIT_MAX - ) - if read_delay_max is not None and read_delay_max > 0: - qos_updates["read_delay_max"] = min( - read_delay_max, self.DELAY_MAX_LIMIT + + self._apply_limit( + qos_updates, "read_iops_limit", read_iops_limit, IOPS_LIMIT_MAX + ) + self._apply_limit( + qos_updates, "write_iops_limit", write_iops_limit, IOPS_LIMIT_MAX + ) + self._apply_limit( + qos_updates, "read_bw_limit", read_bw_limit, BYTES_LIMIT_MAX + ) + self._apply_limit( + qos_updates, "write_bw_limit", write_bw_limit, BYTES_LIMIT_MAX + ) + + if read_burst_mult is not None: + if read_burst_mult < BURST_MULT_MIN: + raise ValueError( + f"read_burst_mult must be at least {BURST_MULT_MIN} (got {read_burst_mult})" + ) + qos_updates["read_burst_mult"] = min( + read_burst_mult, BURST_MULT_MAX ) - if write_delay_max is not None and write_delay_max > 0: - qos_updates["write_delay_max"] = min( - write_delay_max, self.DELAY_MAX_LIMIT + if write_burst_mult is not None: + if write_burst_mult < BURST_MULT_MIN: + raise ValueError( + f"write_burst_mult must be at least {BURST_MULT_MIN} (got {write_burst_mult})" + ) + qos_updates["write_burst_mult"] = min( + write_burst_mult, BURST_MULT_MAX ) if qos_updates: - new_qos = replace(self.qos or QoSConfig(), **qos_updates) + new_qos = replace(self.qos or QoSConfig(), **qos_updates) # type: ignore + + if ( + new_qos.read_iops_limit is None + and new_qos.write_iops_limit is None + and new_qos.read_bw_limit is None + and new_qos.write_bw_limit is None + ): + new_qos = None + else: + new_qos = self.qos return replace(self, qos=new_qos) diff --git a/src/pybind/mgr/smb/tests/test_handler.py b/src/pybind/mgr/smb/tests/test_handler.py index eb85cfa70a8..868958999f2 100644 --- a/src/pybind/mgr/smb/tests/test_handler.py +++ b/src/pybind/mgr/smb/tests/test_handler.py @@ -1794,10 +1794,10 @@ def test_apply_share_with_qos(thandler): qos=smb.resources.QoSConfig( read_iops_limit=100, write_iops_limit=200, - read_bw_limit=1048576, - write_bw_limit=2097152, - read_delay_max=20, - write_delay_max=30, + read_bw_limit="1048576", + write_bw_limit="2097152", + read_burst_mult=20, + write_burst_mult=15, ), ), ) @@ -1810,7 +1810,7 @@ def test_apply_share_with_qos(thandler): ] assert share_dict['cephfs']['qos']['read_iops_limit'] == 100 assert share_dict['cephfs']['qos']['write_iops_limit'] == 200 - assert share_dict['cephfs']['qos']['read_bw_limit'] == 1048576 - assert share_dict['cephfs']['qos']['write_bw_limit'] == 2097152 - assert share_dict['cephfs']['qos']['read_delay_max'] == 20 - assert share_dict['cephfs']['qos']['write_delay_max'] == 30 + assert share_dict['cephfs']['qos']['read_bw_limit'] == "1048576" + assert share_dict['cephfs']['qos']['write_bw_limit'] == "2097152" + assert share_dict['cephfs']['qos']['read_burst_mult'] == 20 + assert share_dict['cephfs']['qos']['write_burst_mult'] == 15 diff --git a/src/pybind/mgr/smb/tests/test_resources.py b/src/pybind/mgr/smb/tests/test_resources.py index fe61f37a3e8..cb90dd47fba 100644 --- a/src/pybind/mgr/smb/tests/test_resources.py +++ b/src/pybind/mgr/smb/tests/test_resources.py @@ -971,10 +971,10 @@ cephfs: qos: read_iops_limit: 100 write_iops_limit: 200 - read_bw_limit: 1048576 - write_bw_limit: 2097152 - read_delay_max: 20 - write_delay_max: 30 + read_bw_limit: "1048576" + write_bw_limit: "2097152" + read_burst_mult: 20 + write_burst_mult: 15 """ data = yaml.safe_load_all(yaml_str) loaded = smb.resources.load(data) @@ -984,10 +984,10 @@ cephfs: assert share.cephfs.qos is not None assert share.cephfs.qos.read_iops_limit == 100 assert share.cephfs.qos.write_iops_limit == 200 - assert share.cephfs.qos.read_bw_limit == 1048576 - assert share.cephfs.qos.write_bw_limit == 2097152 - assert share.cephfs.qos.read_delay_max == 20 - assert share.cephfs.qos.write_delay_max == 30 + assert share.cephfs.qos.read_bw_limit == "1048576" + assert share.cephfs.qos.write_bw_limit == "2097152" + assert share.cephfs.qos.read_burst_mult == 20 + assert share.cephfs.qos.write_burst_mult == 15 def test_share_update_qos(): @@ -1001,34 +1001,34 @@ def test_share_update_qos(): qos=smb.resources.QoSConfig( read_iops_limit=100, write_iops_limit=200, - read_delay_max=5, - write_delay_max=30, + read_burst_mult=20, + write_burst_mult=15, ), ), ) # Update with new QoS values updated_cephfs = share.cephfs.update_qos( - read_bw_limit=1048576, - write_bw_limit=2097152, + read_bw_limit="1048576", + write_bw_limit="2M", read_iops_limit=300, - read_delay_max=15, + read_burst_mult=25, ) assert updated_cephfs.qos is not None assert updated_cephfs.qos.read_iops_limit == 300 # new value assert updated_cephfs.qos.write_iops_limit == 200 # preserved original - assert updated_cephfs.qos.read_bw_limit == 1048576 # new value - assert updated_cephfs.qos.write_bw_limit == 2097152 # new value - assert updated_cephfs.qos.read_delay_max == 15 # new value - assert updated_cephfs.qos.write_delay_max == 30 # preserved original + assert updated_cephfs.qos.read_bw_limit == "1048576" # new value + assert updated_cephfs.qos.write_bw_limit == "2M" # new value + assert updated_cephfs.qos.read_burst_mult == 25 # new value + assert updated_cephfs.qos.write_burst_mult == 15 # preserved original # Verify share with updated QoS works data = share.to_simplified() data.pop("resource_type", None) updated_share = smb.resources.Share(**{**data, 'cephfs': updated_cephfs}) - assert updated_share.cephfs.qos.read_bw_limit == 1048576 - assert updated_share.cephfs.qos.read_delay_max == 15 + assert updated_share.cephfs.qos.read_bw_limit == "1048576" + assert updated_share.cephfs.qos.read_burst_mult == 25 def test_share_qos_remove(): @@ -1042,8 +1042,8 @@ def test_share_qos_remove(): qos=smb.resources.QoSConfig( read_iops_limit=100, write_iops_limit=200, - read_delay_max=5, - write_delay_max=30, + read_burst_mult=20, + write_burst_mult=15, ), ), ) @@ -1052,18 +1052,16 @@ def test_share_qos_remove(): updated_cephfs = share.cephfs.update_qos( read_iops_limit=0, write_iops_limit=0, - read_bw_limit=0, - write_bw_limit=0, - read_delay_max=0, - write_delay_max=0, + read_bw_limit="0", + write_bw_limit="0", ) # Verify QoS is completely removed assert updated_cephfs.qos is None -def test_share_qos_default_delay(): - """Test that delay_max defaults to 30 when not specified""" +def test_share_qos_default_burst_mult(): + """Test that burst_mult defaults to 15 when not specified""" share = smb.resources.Share( cluster_id='qoscluster', share_id='qostest', @@ -1072,20 +1070,47 @@ def test_share_qos_default_delay(): volume='myvol', path='/qos', qos=smb.resources.QoSConfig( - read_iops_limit=100, - write_iops_limit=200 - # delay_max not specified - should use defaults + read_iops_limit=100, write_iops_limit=200 ), ), ) assert share.cephfs.qos is not None - assert share.cephfs.qos.read_delay_max == 30 # Default value - assert share.cephfs.qos.write_delay_max == 30 # Default value + assert share.cephfs.qos.read_burst_mult == 15 # Default value + assert share.cephfs.qos.write_burst_mult == 15 # Default value -def test_share_qos_max_allowed_delay(): - """Test that delay_max values exceeding 300 will be capped to 300""" +def test_share_qos_max_allowed_iops_and_bandwidth(): + """Test that IOPS and bandwidth values exceeding limits will be capped""" + IOPS_LIMIT_MAX = 1_000_000 + BYTES_LIMIT_MAX = 1 << 40 # 1 TB + + share = smb.resources.Share( + cluster_id="qoscluster", + share_id="qostest", + name="QoS Test Share", + cephfs=smb.resources.CephFSStorage( + volume="myvol", + path="/qos", + qos=smb.resources.QoSConfig( + read_iops_limit=100, + write_iops_limit=200, + ), + ), + ) + + updated_cephfs = share.cephfs.update_qos( + read_iops_limit=1_500_000_000, # way above limit + write_bw_limit="2000000000000", # ~2 TB as string, way above limit + ) + + assert updated_cephfs.qos is not None + assert updated_cephfs.qos.read_iops_limit == IOPS_LIMIT_MAX # capped + assert updated_cephfs.qos.write_bw_limit == str(BYTES_LIMIT_MAX) # capped + + +def test_share_update_qos_human_readable(): + """Test update_qos with human-readable bandwidth limits (pass-through to Samba).""" share = smb.resources.Share( cluster_id='qoscluster', share_id='qostest', @@ -1093,47 +1118,85 @@ def test_share_qos_max_allowed_delay(): cephfs=smb.resources.CephFSStorage( volume='myvol', path='/qos', - qos=smb.resources.QoSConfig( - read_iops_limit=100, - write_iops_limit=200, - read_delay_max=30, - write_delay_max=30, - ), ), ) - updated_cephfs = share.cephfs.update_qos(read_delay_max=350) + # Update with human-readable format (stored as-is) + updated_cephfs = share.cephfs.update_qos( + read_bw_limit="10M", + write_bw_limit="1G", + read_iops_limit=100, + read_burst_mult=20, + ) assert updated_cephfs.qos is not None - assert updated_cephfs.qos.read_delay_max == 300 # Capped value + assert updated_cephfs.qos.read_bw_limit == "10M" + assert updated_cephfs.qos.write_bw_limit == "1G" + assert updated_cephfs.qos.read_iops_limit == 100 + assert updated_cephfs.qos.read_burst_mult == 20 + + # Update with byte values (also strings) + updated_cephfs2 = share.cephfs.update_qos( + read_bw_limit="50M", + write_bw_limit="52428800", + ) + assert updated_cephfs2.qos.read_bw_limit == "50M" + assert updated_cephfs2.qos.write_bw_limit == "52428800" -def test_share_qos_max_allowed_iops_and_bandwidth(): - """Test that iops and bandwidth values exceeding limits will be capped to IOPS_LIMIT_MAX and BYTES_LIMIT_MAX""" - IOPS_LIMIT_MAX = 1_000_000 - BYTES_LIMIT_MAX = 1 << 40 # 1 TB +def test_share_qos_backward_compat_integers(): + """Test backward compatibility with integer bandwidth values from YAML.""" + import yaml + + # Old-style YAML with integer bandwidth values (pre-human-readable support) + yaml_str = """ +resource_type: ceph.smb.share +cluster_id: qoscluster +share_id: oldstyle +name: Old Style Share +cephfs: + volume: myvol + path: /old + qos: + read_iops_limit: 100 + write_iops_limit: 200 + read_bw_limit: 1048576 + write_bw_limit: 2097152 +""" + data = yaml.safe_load_all(yaml_str) + loaded = smb.resources.load(data) + assert loaded + + share = loaded[0] + assert share.cephfs.qos is not None + assert share.cephfs.qos.read_bw_limit == "1048576" + assert share.cephfs.qos.write_bw_limit == "2097152" + + +def test_share_qos_remove_individual_limit(): + """Test removing individual limits while keeping others.""" share = smb.resources.Share( - cluster_id="qoscluster", - share_id="qostest", - name="QoS Test Share", + cluster_id='qoscluster', + share_id='qostest', + name='QoS Test Share', cephfs=smb.resources.CephFSStorage( - volume="myvol", - path="/qos", + volume='myvol', + path='/qos', qos=smb.resources.QoSConfig( read_iops_limit=100, write_iops_limit=200, - read_delay_max=30, - write_delay_max=30, + read_bw_limit="10M", + write_bw_limit="20M", ), ), ) - updated_cephfs = share.cephfs.update_qos( - read_iops_limit=1_500_000_000, # way above limit - write_bw_limit=2_000_000_000_000, # ~2 TB, way above limit - ) + # Remove only read IOPS limit + updated_cephfs = share.cephfs.update_qos(read_iops_limit=0) assert updated_cephfs.qos is not None - assert updated_cephfs.qos.read_iops_limit == IOPS_LIMIT_MAX # capped - assert updated_cephfs.qos.write_bw_limit == BYTES_LIMIT_MAX # capped + assert updated_cephfs.qos.read_iops_limit is None # Removed + assert updated_cephfs.qos.write_iops_limit == 200 # Preserved + assert updated_cephfs.qos.read_bw_limit == "10M" # Preserved + assert updated_cephfs.qos.write_bw_limit == "20M" # Preserved diff --git a/src/pybind/mgr/smb/tests/test_smb.py b/src/pybind/mgr/smb/tests/test_smb.py index 51bf1099493..5ad2214f1a4 100644 --- a/src/pybind/mgr/smb/tests/test_smb.py +++ b/src/pybind/mgr/smb/tests/test_smb.py @@ -959,10 +959,10 @@ def test_cmd_share_update_qos(tmodule): share_id='qostest', read_iops_limit=100, write_iops_limit=200, - read_bw_limit=1048576, - write_bw_limit=2097152, - read_delay_max=20, - write_delay_max=30, + read_bw_limit="1048576", + write_bw_limit="2097152", + read_burst_mult=20, + write_burst_mult=15, ) assert res == 0 bdata = json.loads(body) @@ -978,21 +978,19 @@ def test_cmd_share_update_qos(tmodule): assert updated_share.cephfs.qos is not None assert updated_share.cephfs.qos.read_iops_limit == 100 assert updated_share.cephfs.qos.write_iops_limit == 200 - assert updated_share.cephfs.qos.read_bw_limit == 1048576 - assert updated_share.cephfs.qos.write_bw_limit == 2097152 - assert updated_share.cephfs.qos.read_delay_max == 20 - assert updated_share.cephfs.qos.write_delay_max == 30 + assert updated_share.cephfs.qos.read_bw_limit == "1048576" + assert updated_share.cephfs.qos.write_bw_limit == "2097152" + assert updated_share.cephfs.qos.read_burst_mult == 20 + assert updated_share.cephfs.qos.write_burst_mult == 15 - # Test updating with None values (should remove QoS) + # Test updating with 0 values (should remove QoS) res, body, status = tmodule.share_update_qos.command( cluster_id='qoscluster', share_id='qostest', read_iops_limit=0, write_iops_limit=0, - read_bw_limit=0, - write_bw_limit=0, - read_delay_max=0, - write_delay_max=0, + read_bw_limit="0", + write_bw_limit="0", ) assert res == 0 bdata = json.loads(body) @@ -1011,7 +1009,7 @@ def test_cmd_share_update_qos(tmodule): cluster_id='qoscluster', share_id='qostest', read_iops_limit=500, - write_bw_limit=524288, + write_bw_limit="524288", ) assert res == 0 bdata = json.loads(body) @@ -1027,6 +1025,6 @@ def test_cmd_share_update_qos(tmodule): assert updated_share.cephfs.qos.read_iops_limit == 500 assert updated_share.cephfs.qos.write_iops_limit is None assert updated_share.cephfs.qos.read_bw_limit is None - assert updated_share.cephfs.qos.write_bw_limit == 524288 - assert updated_share.cephfs.qos.read_delay_max == 30 - assert updated_share.cephfs.qos.write_delay_max == 30 + assert updated_share.cephfs.qos.write_bw_limit == "524288" + assert updated_share.cephfs.qos.read_burst_mult == 15 # Default + assert updated_share.cephfs.qos.write_burst_mult == 15 # Default diff --git a/src/python-common/ceph/smb/constants.py b/src/python-common/ceph/smb/constants.py index 1f473dfec21..90fb24aad48 100644 --- a/src/python-common/ceph/smb/constants.py +++ b/src/python-common/ceph/smb/constants.py @@ -60,3 +60,15 @@ DEBUG_LEVEL_TIERS = [ ("DEBUG", 9, 10), ] DEBUG_LEVEL_TERMS = {t[0] for t in DEBUG_LEVEL_TIERS} + +# Maximum value for iops_limit +IOPS_LIMIT_MAX = 1_000_000 + +# Maximum value for bandwidth limit (1 << 40 = 1 TB) +BYTES_LIMIT_MAX = 1 << 40 + +# Minimum value for burst multiplier +BURST_MULT_MIN = 10 + +# Maximum value for burst multiplier +BURST_MULT_MAX = 100