Github user mike-tutkowski commented on the issue:

    https://github.com/apache/cloudstack/pull/1847
  
    Let me provide a bit of background on this and then we can decide which way 
we want to correct this side effect.
    
    Here is the PR that went in a while ago that enabled CloudStack to support 
volume snapshots that reside on primary storage:
    
    https://github.com/apache/cloudstack/pull/1403
    
    The idea being these types of snapshots are faster than the 
back-up-to-secondary-storage approach CloudStack does by default and they can 
be a lot more space efficient, as well.
    
    As part of this process, I went through and tried to identify all of the 
locations where we assumed a volume snapshot resided on secondary storage (and 
I put in code to see if it really resides there or, instead, if it's on primary 
storage).
    
    As we have noted, a couple places were missed and this PR (as wells as 
#1735) were opened to address those issues.
    
    The way this particular PR's code is written should work fine. In the case 
where the original primary storage has been removed, an exception will be 
thrown, caught, logged, and then we will default to returning secondary storage 
as the location (which it should be).
    
    Instead of the try/catch approach, though, it might be better if we see if 
dataStore is null.
    
    DataStore dataStore = dataStoreMgr.getDataStore(storagePoolId, 
DataStoreRole.Primary);
    
    If that comes back null, then we apparently have removed primary storage, 
which can only be done if your snapshots don't reside on it. If dataStore == 
null, return DataStoreRole.IMAGE.
    



---
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.
---

Reply via email to