[GitHub] cloudstack pull request #1749: Updates for SAN-assisted snapshots

[mike-tutkowski]

GitHub user mike-tutkowski opened a pull request:

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

    Updates for SAN-assisted snapshots

    This PR is to address a few issues in #1600 (which was recently merged to 
master for 4.10).
    
    In StorageSystemDataMotionStrategy.performCopyOfVdi we call 
getSnapshotDetails. In one such scenario, the source snapshot in question is 
coming from secondary storage (when we are creating a new volume on managed 
storage from a snapshot of ours that’s on secondary storage).
    
    This usually “worked” in the regression tests due to a bit of "luck": 
We retrieve the ID of the snapshot (which is on secondary storage) and then try 
to pull out its StorageVO object (which is for primary storage). If you happen 
to have a primary storage that matches the ID (which is the ID of a secondary 
storage), then getSnapshotDetails populates its Map<String, String> with 
inapplicable data (that is later ignored) and you don’t easily see a problem. 
However, if you don’t have a primary storage that matches that ID (which I 
didn’t today because I had removed that primary storage), then a 
NullPointerException is thrown.
    
    I have fixed that issue by skipping getSnapshotDetails if the source is 
coming from secondary storage.
    
    While fixing that, I noticed a couple more problems:
    
    1)       We can invoke grantAccess on a snapshot that’s actually on 
secondary storage (this doesn’t amount to much because the VolumeServiceImpl 
ignores the call when it’s not for a primary-storage driver).
    2)       We can invoke revokeAccess on a snapshot that’s actually on 
secondary storage (this doesn’t amount to much because the VolumeServiceImpl 
ignores the call when it’s not for a primary-storage driver).
    
    I have corrected those issues, as well.
    
    I then came across one more problem:
    ·         When using a SAN snapshot and copying it to secondary storage or 
creating a new managed-storage volume from a snapshot of ours on secondary 
storage, we attach to the SR in the XenServer code, but detach from it in the 
StorageSystemDataMotionStrategy code (by sending a message to the XenServer 
code to perform an SR detach). Since we know to detach from the SR after the 
copy is done, we should detach from the SR in the XenServer code (without that 
code having to be explicitly called from outside of the XenServer logic).
    
    I went ahead and changed that, as well.

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

    $ git pull https://github.com/mike-tutkowski/cloudstack archived_snapshots

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

    https://github.com/apache/cloudstack/pull/1749.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 #1749
    
----
commit 7ccb74b7ff500f1131e84023f47f65e0e08ac411
Author: Mike Tutkowski <mike.tutkow...@solidfire.com>
Date:   2016-11-06T18:22:11Z

    Updates for SAN-assisted snapshots

----


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---