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

scst_lib,scst_sysfs: Add aen_disabled setting #141

Merged
merged 1 commit into from
Apr 17, 2023

Conversation

bmeagherix
Copy link
Contributor

@bmeagherix bmeagherix commented Mar 23, 2023

Add a setting to scst_tgt that can prevent scst_gen_aen_or_ua from generating an AEN, even if the underlying transport is capable of transmitting them. It will instead generate a UA.

This could prove useful in different situations, including when the target port is also a forward_dst.

(The new sysfs interface was modeled on the code for forward_dst.)

@lnocturno
Copy link
Contributor

Hi Brian,

Sorry for the long response, but I still need more time to review your patch.
I'm trying to get it done by the end of this week.

Thanks,
Gleb

@bmeagherix
Copy link
Contributor Author

bmeagherix commented Apr 11, 2023

Sorry for the long response, but I still need more time to review your patch.
I'm trying to get it done by the end of this week.

Hi Gleb,

No problem, I understand and appreciate all your effort & consideration!

FWIW, as you may notice the new checkpatch requirement means that some code doesn't match the rest of the codebase, e.g.

WARNING:SYMBOLIC_PERMS: Symbolic permissions 'S_IRUGO | S_IWUSR' are not preferred. Consider using octal permissions '0644'.
#192: FILE: scst/src/scst_sysfs.c:2694:
+       __ATTR(aen_disabled, S_IRUGO | S_IWUSR, scst_tgt_aen_disabled_show,

so now this PR is using 0644. Please let me know if that should have been handled differently.

Best regards,
-Brian.

@lnocturno
Copy link
Contributor

Hi Brian,

FWIW, as you may notice the new checkpatch requirement means that some code doesn't match the rest of the codebase

The idea behind the new checkpatch requirement is to check new patches for errors/warnings. The current codebase still contains a lot of checkpatch errors/warnings, but I think I will fix them step by step wherever possible.

Not every error/warning needs to be fixed, I have already added an ignore rule for some of them and will continue to do so, depending on the type. In your case, you did everything right:

https://lkml.org/lkml/2021/9/4/55
https://lore.kernel.org/lkml/CA+55aFw5v23T-zvDZp-MmD_EYxF8WbafwwB59934FV7g21uMGQ@mail.gmail.com/

scst/src/scst_sysfs.c Show resolved Hide resolved
scst/src/scst_sysfs.c Outdated Show resolved Hide resolved
Add a setting to scst_tgt that can prevent scst_gen_aen_or_ua from
generating an AEN, even if the underlying transport is capable of
transmitting them.  It will instead generate a UA.
@bmeagherix
Copy link
Contributor Author

BTW, this PR deliberately does not include suppressing AENs from scst_report_luns_changed_sess ... less obviously useful to do so there.

@lnocturno lnocturno merged commit c326e96 into SCST-project:master Apr 17, 2023
@lnocturno
Copy link
Contributor

Hi Brian,

Thank you for the patch!

Thanks,
Gleb

@bmeagherix bmeagherix deleted the tgt_dev_aen_disabled branch April 17, 2023 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants