Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NAS-130103 / 24.10 / Allow extent to be deleted even if underlying storage is missing #14067

Merged
merged 3 commits into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/middlewared/middlewared/plugins/iscsi_/extents.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,9 @@ async def do_delete(self, audit_callback, id_, remove, force):
# This change is being made in conjunction with threads_num being specified in scst.conf
if data['type'] == 'DISK' and data['path'].startswith('zvol/'):
zvolname = zvol_path_to_name(os.path.join('/dev', data['path']))
await self.middleware.call('zfs.dataset.update', zvolname, {'properties': {'volthreading': {'value': 'on'}}})
# Only try to set volthreading if the volume still exists.
if await self.middleware.call('pool.dataset.query', [['name', '=', zvolname], ['type', '=', 'VOLUME']]):
await self.middleware.call('zfs.dataset.update', zvolname, {'properties': {'volthreading': {'value': 'on'}}})

try:
return await self.middleware.call(
Expand Down Expand Up @@ -474,6 +476,8 @@ async def remove_extent_file(self, data):
if data['type'] == 'FILE':
try:
os.unlink(data['path'])
except FileNotFoundError:
pass
except Exception as e:
return e

Expand Down
54 changes: 49 additions & 5 deletions tests/api2/test_261_iscsi_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@
portal, read_capacity16, target,
target_extent_associate, verify_capacity,
verify_luns)
from middlewared.service_exception import ValidationError, ValidationErrors
from middlewared.service_exception import InstanceNotFound, ValidationError, ValidationErrors
from middlewared.test.integration.assets.iscsi import target_login_test
from middlewared.test.integration.assets.pool import dataset, snapshot
from middlewared.test.integration.utils import call
from middlewared.test.integration.utils import call, ssh
from middlewared.test.integration.utils.client import truenas_server
from pyscsi.pyscsi.scsi_sense import sense_ascq_dict
from pytest_dependency import depends
Expand Down Expand Up @@ -156,7 +156,7 @@ def file_extent(pool_name, dataset_name, file_name, filesize=MB_512, extent_name


@contextlib.contextmanager
def zvol_dataset(zvol, volsize=MB_512):
def zvol_dataset(zvol, volsize=MB_512, recursive=False, force=False):
payload = {
'name': zvol,
'type': 'VOLUME',
Expand All @@ -168,7 +168,10 @@ def zvol_dataset(zvol, volsize=MB_512):
try:
yield dataset_config
finally:
call('pool.dataset.delete', dataset_config['id'])
try:
call('pool.dataset.delete', dataset_config['id'], {'recursive': recursive, 'force': force})
except InstanceNotFound:
pass


def modify_extent(ident, payload):
Expand Down Expand Up @@ -211,6 +214,10 @@ def get_client_count():
return call('iscsi.global.client_count')


def get_volthreading(zvolid):
return call('zfs.dataset.query', [['id', '=', zvolid]], {'get': True})['properties']['volthreading']['value']


def verify_client_count(count, retries=10):
"""Verify that the client count is the expected value, but include some
retries to allow things to settle if necessary."""
Expand All @@ -236,7 +243,10 @@ def zvol_extent(zvol, extent_name='zvol_extent'):
try:
yield extent_config
finally:
call('iscsi.extent.delete', extent_config['id'], True, True)
try:
call('iscsi.extent.delete', extent_config['id'], True, True)
except InstanceNotFound:
pass


@contextlib.contextmanager
Expand Down Expand Up @@ -2602,6 +2612,40 @@ def test_33_no_lun_zero():
verify_luns(s, [100, 101])


def test_34_zvol_extent_volthreading():
"""
Ensure that volthreading is on for regular zvols and off when they are being
used an iSCSI extent.
"""
zvol_name = f"zvol_volthreading_test{digit}"
zvol = f'{pool_name}/{zvol_name}'
with zvol_dataset(zvol, MB_100, True, True):
assert get_volthreading(zvol) == 'on'
with zvol_extent(zvol, extent_name='zvolextent1'):
assert get_volthreading(zvol) == 'off'
assert get_volthreading(zvol) == 'on'


@pytest.mark.parametrize('extent_type', ["FILE", "VOLUME"])
def test_35_delete_extent_no_dataset(extent_type):
"""
Verify that even if a dataset that contains an extent has been deleted from
the command line, can still use the webui/API to delete the extent.
"""
dataset_name = f'iscsids_{extent_type}_{digit}'
with dataset(dataset_name) as dspath:
DESTROY_CMD = f'zfs destroy -r {dspath}'
match extent_type:
case 'FILE':
with file_extent(pool_name, dataset_name, 'testfile', extent_name='fileextent1'):
ssh(DESTROY_CMD)
case 'VOLUME':
zvol = f'{dspath}/zvol{digit}'
with zvol_dataset(zvol, MB_100, True, True):
with zvol_extent(zvol, extent_name='zvolextent1'):
ssh(DESTROY_CMD)


def test_99_teardown(request):
# Disable iSCSI service
depends(request, ["iscsi_cmd_00"])
Expand Down
Loading