Skip to content

Commit acae5c5

Browse files
kvm: Update the java doc for the method disconnectPhysicalDiskByPath (#9210)
This PR addresses the issue #8789 The original issue is disconnectPhysicalDiskByPath() implementation in FibreChannelAdaptor always returns true irrespective of the success of the operation. This was already fixed in the PR #8889 . Ideally this method has to be called after choosing the right adapter based on the storage pool type of the volume path, but currently it is just called in a loop. https://github.com/shapeblue/cloudstack/blob/05b9b6e2e77b9b633f0fe22aff5f9f3c047b2303/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java#L200-L212 while 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. In this PR I've added the java doc explaining the method and situation.
1 parent 43ab8a9 commit acae5c5

File tree

1 file changed

+11
-2
lines changed

1 file changed

+11
-2
lines changed

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,17 @@ public KVMPhysicalDisk createPhysicalDisk(String name, KVMStoragePool pool,
5050

5151
public boolean disconnectPhysicalDisk(Map<String, String> volumeToDisconnect);
5252

53-
// given local path to file/device (per Libvirt XML), 1) check that device is
54-
// handled by your adaptor, return false if not. 2) clean up device, return true
53+
/**
54+
* Given local path to file/device (per Libvirt XML),
55+
* 1) Make sure to check that device is handled by your adaptor, return false if not.
56+
* 2) clean up device, return true
57+
* 3) if clean up fails, then return false
58+
*
59+
* If the method wrongly returns true, then there are chances that disconnect will not reach the right storage adapter
60+
*
61+
* @param localPath path for the file/device from the disk definition per Libvirt XML.
62+
* @return true if the operation is successful; false if the operation fails or the adapter fails to handle the path.
63+
*/
5564
public boolean disconnectPhysicalDiskByPath(String localPath);
5665

5766
public boolean deletePhysicalDisk(String uuid, KVMStoragePool pool, Storage.ImageFormat format);

0 commit comments

Comments
 (0)