Repository navigation
FibreChannelAdaptor disconnectPhysicalDiskByPath should not always return true #8789
Description
ISSUE TYPE
- Bug Report
COMPONENT NAME
KVM-Storage
CLOUDSTACK VERSION
4.19.0
CONFIGURATION
OS / ENVIRONMENT
SUMMARY
I'm not pointing fingers as the Linstor driver also needs a fix for that(comming), but
disconnectPhysicalDiskByPath should only return true if it is able to handle that path.
Currently it tries to handle every path and if it is used as the first entry in the StorageadaptorMap it will always "win",
even if it isn't the driver device.
Also the formatting looks a bit off in that method.
STEPS TO REPRODUCE
Have a non FibreChannel primary storage and stop/move a VM to another host.
disconnectbyPath will be called and if FibreChannel adaptor is 1. in the Map it will take over the disconnect call.
EXPECTED RESULTS
FibreChannel driver only handling paths belonging to it
ACTUAL RESULTS
Metadata
Metadata
Assignees
Labels
No labels
Activity
rg9975 commentedon Mar 14, 2024
Thank you for flagging this, I will include a fix for this in a forthcoming 4.19.1 pull request.
harikrishna-patnala commentedon Jun 10, 2024
@rp- I see the following part has be already handled in the PR #8889
public boolean disconnectPhysicalDiskByPath(String localPath) {
I'm trying to fix the case of running into the loop of all adapters by somehow passing the storage pool type to that caller cleanup() method but this is touching all over the code (which I fear it creates other regressions), instead I feel we can keep it the current way only since Fibrechannel adapter has already fixed. Whats your thoughts @rp-
rp- commentedon Jun 10, 2024
For now, yes.
But I think a rework, and keeping track of the storage pool a path belongs too would be very good.
Otherwise with a new primary storage or a regression would trigger such a bug again.
harikrishna-patnala commentedon Jun 11, 2024
Thanks @rp- let me add a clear java doc on that method for that purpose and explaining the possible bug.
kvm: Update the java doc for the method disconnectPhysicalDiskByPath (#…
yadvr commentedon Jun 11, 2024
Javadoc addressed by #9210
kvm: Update the java doc for the method disconnectPhysicalDiskByPath (a…