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-128574 / 24.10 / Allow HeaderDigest and DataDigest to be configured for iSCSI targets #13624

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

bmeagherix
Copy link
Contributor

@bmeagherix bmeagherix commented Apr 26, 2024

Enhance iSCSI APIs to allow some target options to be specified

@bmeagherix bmeagherix added the WIP label Apr 26, 2024
@bmeagherix bmeagherix self-assigned this Apr 26, 2024
@bugclerk bugclerk changed the title Nas 128574 NAS-128574 / 24.10 / Nas 128574 Apr 26, 2024
@bugclerk
Copy link
Contributor

@bmeagherix bmeagherix changed the title NAS-128574 / 24.10 / Nas 128574 NAS-128574 / 24.10 / Allow HeaderDigest and DataDigest to be configured for iSCSI targets Apr 26, 2024
@bmeagherix bmeagherix removed the WIP label May 1, 2024
@bmeagherix bmeagherix requested a review from a team May 1, 2024 17:53
@@ -97,6 +127,18 @@ async def extend(self, data):
),
]),
List('auth_networks', items=[IPAddr('ip', network=True)]),
Dict(
'options',
Str('DataDigest', enum=DigestEnum.choices()),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we typically have camelcase parameters. Convention is all lower-case with underscores as-needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand, but we're exposing raw iSCSI settings, so IMO this is clearer.

e.g. https://www.rfc-editor.org/rfc/rfc7143#section-13.1

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yocalebo do you have strong opinion on this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants