HawkForge opened a new pull request, #14686:
URL: https://github.com/apache/cloudstack/pull/14686
### Description
`IscsiAdmStorageAdaptor.disconnectPhysicalDiskByPath` only checked that the
path contains `-iscsi-` before parsing it as
`/dev/disk/by-path/ip-<host>:<port>-iscsi-<iqn>-lun-<n>`. KVMStoragePoolManager
passes the local path of every disk to every adaptor, so a SharedMountPoint
disk such as `/mnt/fa-iscsi-primary/<uuid>` passed the check, `indexOf(":")`
returned -1 and the first Stop of the VM failed with `Range [21, -1) out of
bounds for length 58`.
The method now requires the `/dev/disk/by-path/ip-` prefix and checks the
parsed positions before calling `substring`, returning false for paths that are
not iSCSI by-path entries.
Root cause analysis and the controlled reproduction (same storage mounted at
a directory without `-iscsi-` stops fine) by @famerico in the issue.
Fixes: #14664
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
- [ ] Build/CI
- [ ] Test (unit or integration test code)
### Feature/Enhancement Scale or Bug Severity
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [x] Minor
- [ ] Trivial
### How Has This Been Tested?
Added `IscsiAdmStorageAdaptorTest` covering a SharedMountPoint path
containing `-iscsi-`, a path without it, and a by-path style string missing the
port. Two of the three fail with `StringIndexOutOfBoundsException` (including
the exact `begin 21, end -1, length 58` from the issue) without the fix.
`mvn -pl plugins/hypervisors/kvm test -Dtest=IscsiAdmStorageAdaptorTest`: 3
tests, 0 failures (checkstyle passes). That run was on main; this branch is the
same commit cherry-picked cleanly onto 4.22 (the method is identical there),
not re-run on 4.22 locally. Not tested against a real iSCSI target.
#### How did you try to break this feature and the system with this change?
Well-formed by-path strings are parsed exactly as before. Other callers of
cleanupDisk (migration, detach) go through the same method so they get the same
guard.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
