Skip to content

CA-411122: do not call set-iscsi-initiator with an empty string for IQN #6474

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

Merged
merged 1 commit into from
May 19, 2025

Conversation

edwintorok
Copy link
Contributor

Back in 2018 a4a94b3 rejected empty IQNs in set_iscsi_iqn API calls. However hosts are created with an empty IQN, and if this code runs too early then it will attempt to call set-iscsi-initiator with an empty string for the IQN:

/opt/xensource/libexec/set-iscsi-initiator  myhost

About a second later the script is called again with the correct value. This could potentially result in the iscsid service being restarted multiple times (and if a restart is still pending when restart is called a 2nd time I'm not sure it'll take effect, so we might be left with an empty initiator, I have also seen a GFS2 SR plug failure following this).

It is best to avoid setting empty initiators. The exception would be raised and ignore due to the log_and_ignore in the caller.

Also log wherever the IQN is set using %S, so that we notice if it ends up containing some extra whitespace characters.

Back in 2018 a4a94b3 rejected empty IQNs in set_iscsi_iqn API calls.
However hosts are created with an empty IQN, and if this code runs too early then it will attempt to call `set-iscsi-initiator` with an empty string for the IQN:
```
/opt/xensource/libexec/set-iscsi-initiator  myhost
```

About a second later the script is called again with the correct value.
This could potentially result in the iscsid service being restarted multiple times (and if a restart is still pending when restart is called a 2nd time I'm not sure it'll take effect, so we might be left with an empty initiator).

It is best to avoid setting empty initiators. The exception would be raised and ignore due to the log_and_ignore in the caller.

Also log wherever the IQN is set using %S, so that we notice if it ends up containing some extra whitespace characters.

Signed-off-by: Edwin Török <[email protected]>
@edwintorok edwintorok enabled auto-merge May 19, 2025 13:59
Copy link
Contributor

@Vincent-lau Vincent-lau left a comment

Choose a reason for hiding this comment

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

Will this fail existing callers that pass an empty IQN?

@edwintorok edwintorok added this pull request to the merge queue May 19, 2025
Merged via the queue into xapi-project:master with commit 38f23f9 May 19, 2025
17 checks passed
@edwintorok
Copy link
Contributor Author

Will this fail existing callers that pass an empty IQN?

Yes, but there are only 2 such callers: set_iscsi_iqn has already checked that it is empty and raised an exception, and the one in sync_config_files uses log_and_ignore_exn.

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.

4 participants