[GitHub] cloudstack pull request #1780: Fix the KVM hypervisor's handling of not-real...

[ppenchev-storpool <gi...@git.apache.org>]

GitHub user ppenchev-storpool opened a pull request:

    https://github.com/apache/cloudstack/pull/1780

    Fix the KVM hypervisor's handling of not-really-SCSI devices.

    Hi,
    
    Thanks for developing and maintaining CloudStack!
    
    What do you think about the attached trivial patch that lets the KVM storage pool use more than one type of storage adaptor?  If a disk is handled by another adaptor and a request to disconnect it comes in, the KVM storage pool might hand it off to the iSCSI storage adaptor first.  With the current iSCSI adaptor code, it will recognize that the disk is not one of its own and return "true", signalling to the storage pool that the disk has been successfully disconnected.  The patch makes it return "false" so that the storage pool will pass the request to the *next* storage adaptor in the pool which might very well actually handle it.
    
    Thanks in advance for your time and attention, and keep up the great work!
    
    Best regards,
    Peter


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/storpool/cloudstack pp-scsi-pass

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cloudstack/pull/1780.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1780
    
----
commit 847897a64662654cd1d59aae8531bdd157230d50
Author: Peter Pentchev <pp...@storpool.com>
Date:   2016-06-14T14:36:21Z

    Fix the KVM hypervisor's handling of not-really-SCSI devices.
    
    Submitted by:		Gencho Zhilkov

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1780: Fix the KVM hypervisor's handling of not-really-SCSI...

[wido <gi...@git.apache.org>]

Github user wido commented on the issue:

    https://github.com/apache/cloudstack/pull/1780
  
    These small fixes *can* be very dangerous. So wondering how we can test this easily.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1780: Fix the KVM hypervisor's handling of not-really-SCSI...

[syed <gi...@git.apache.org>]

Github user syed commented on the issue:

    https://github.com/apache/cloudstack/pull/1780
  
    @ppenchev-storpool I haven't worked with a non-iscsi adaptor for KVM but wouldn't it change the default behaviour now for non-iscsi disks? What if the other adaptors also ignore this? Can we add a default? Also is there a way of adding another adopter to test this from an integration test?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1780: Fix the KVM hypervisor's handling of not-really-SCSI...

[ppenchev-storpool <gi...@git.apache.org>]

Github user ppenchev-storpool commented on the issue:

    https://github.com/apache/cloudstack/pull/1780
  
    Hi,
    
    Thanks for looking at this!
    
    Testing this could simply be a matter of adding more than one storage adaptor to a KVM storage pool, then connecting and disconnecting physical disks.  The problem will only manifest itself when disconnecting a disk that is not handled by the iSCSI provider if the loop in KVMStoragePoolManager.disconnectPhysicalDiskByPath() (file plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java around line 161) finds the iSCSI adaptor first.  Well, yeah, making sure the loop finds the iSCSI adaptor first may not be easy if Java orders the map keys almost randomly, but with some luck and/or some changes to the loop itself it could be arranged (change only the order in which the entries are walked, not the processing of the entries themselves).
    
    BTW, this is actually the *only* place where the adaptor's disconnectPhysicalDiskByPath() method is invoked, so just analyzing the code there should show that this change ought to be safe.
    
    Best regards,
    Peter



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---