[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/1403 --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-219702269 this one is ready... --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-219702190 ### CI RESULTS ``` Tests Run: 85 Skipped: 0 Failed: 0 Errors: 0 Duration: 4h 20m 17s ``` **Associated Uploads** **`/tmp/MarvinLogs/DeployDataCenter__May_17_2016_00_15_14_6DMLAP:`** * [dc_entries.obj](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1403/tmp/MarvinLogs/DeployDataCenter__May_17_2016_00_15_14_6DMLAP/dc_entries.obj) * [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1403/tmp/MarvinLogs/DeployDataCenter__May_17_2016_00_15_14_6DMLAP/failed_plus_exceptions.txt) * [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1403/tmp/MarvinLogs/DeployDataCenter__May_17_2016_00_15_14_6DMLAP/runinfo.txt) **`/tmp/MarvinLogs/test_network_CRQ5C2:`** * [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1403/tmp/MarvinLogs/test_network_CRQ5C2/failed_plus_exceptions.txt) * [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1403/tmp/MarvinLogs/test_network_CRQ5C2/results.txt) * [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1403/tmp/MarvinLogs/test_network_CRQ5C2/runinfo.txt) **`/tmp/MarvinLogs/test_vpc_routers_KUJ7NL:`** * [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1403/tmp/MarvinLogs/test_vpc_routers_KUJ7NL/failed_plus_exceptions.txt) * [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1403/tmp/MarvinLogs/test_vpc_routers_KUJ7NL/results.txt) * [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1403/tmp/MarvinLogs/test_vpc_routers_KUJ7NL/runinfo.txt) Uploads will be available until `2016-07-17 02:00:00 +0200 CEST` *Comment created by [`upr comment`](https://github.com/cloudops/upr).* --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user jburwell commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-219148309 @DaanHoogland in my experience, stack traces are a critical piece of information for operationally debugging CloudStack. Unfortunately, our logging lacks the clarity and specify to use the error message alone. I hope our logging improves overtime to omit them from ``INFO``, ``WARN``, and ``ERROR``. However, today, this information is vital to resolving issues. --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-219146275 @jburwell @DaanHoogland I have a solution for quickly looking up if a cluster supports resigning that I think we'll all be happy with. Upon a host connecting to the management server is when I check to see if the host supports resigning (and update the host_details table). I added logic to this connection code to not only update the host_details table with info about resigning, but also the cluster_details table about resigning. A host connecting to the management server should not be a frequent occurrence, so I believe it's OK at this point to run through the list of hosts in the cluster of the connecting host and see if all of those hosts support resigning. If they do, then I update the cluster_details table that the cluster in question supports resigning. I also changed the logic to not bother to add a row to either the host_details table or the cluster_details table if the "supportsResign" property would be false (only a true value is stored now). We can then save space by understanding that a missing "supportsResign" property for a host or cluster indicates false (I was proceeding under that assumption anyways). When the time comes to ask the cluster if it supports resigning, it's a simple matter of looking for the "supportsResign" property in the cluster_details table. --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63140242
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -361,59 +600,205 @@ private String getProperty(long snapshotId, String
property) {
}
private Map getVolumeDetails(VolumeInfo volumeInfo) {
-Map sourceDetails = new HashMap();
+Map volumeDetails = new HashMap();
VolumeVO volumeVO = _volumeDao.findById(volumeInfo.getId());
long storagePoolId = volumeVO.getPoolId();
StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
-sourceDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
-sourceDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
-sourceDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
+volumeDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+volumeDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+volumeDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
ChapInfo chapInfo = _volumeService.getChapInfo(volumeInfo,
volumeInfo.getDataStore());
if (chapInfo != null) {
-sourceDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
-sourceDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
-sourceDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
-sourceDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
+volumeDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
+volumeDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
}
-return sourceDetails;
+return volumeDetails;
}
-public HostVO getHost(long dataStoreId) {
-StoragePoolVO storagePoolVO =
_storagePoolDao.findById(dataStoreId);
+private Map getSnapshotDetails(SnapshotInfo
snapshotInfo) {
+Map snapshotDetails = new HashMap();
+
+long storagePoolId = snapshotInfo.getDataStore().getId();
+StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
+
+snapshotDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+snapshotDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+
+long snapshotId = snapshotInfo.getId();
+
+snapshotDetails.put(DiskTO.IQN, getProperty(snapshotId,
DiskTO.IQN));
-List clusters =
_mgr.searchForClusters(storagePoolVO.getDataCenterId(), new Long(0),
Long.MAX_VALUE, HypervisorType.XenServer.toString());
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_SECRET));
+snapshotDetails.put(DiskTO.CHAP_TARGET_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_TARGET_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_TARGET_SECRET,
getProperty(snapshotId, DiskTO.CHAP_TARGET_SECRET));
-if (clusters == null) {
-throw new CloudRuntimeException("Unable to locate an
applicable cluster");
+return snapshotDetails;
+}
+
+private HostVO getHost(SnapshotInfo snapshotInfo) {
+HostVO hostVO = getHost(snapshotInfo.getDataCenterId(), true);
+
+if (hostVO == null) {
+hostVO = getHost(snapshotInfo.getDataCenterId(), false);
+
+if (hostVO == null) {
+throw new CloudRuntimeException("Unable to locate an
applicable host in data center with ID = " + snapshotInfo.getDataCenterId());
+}
}
-for (Cluster cluster : clusters) {
-if (cluster.getAllocationState() == AllocationState.Enabled) {
-List hosts =
_hostDao.findByClusterId(cluster.getId());
+return hostVO;
+}
+
+private HostVO getHost(Long zoneId, boolean
computeClusterMustSupportResign) {
+Preconditions.checkArgument(zoneId != null, "Zone ID cannot be
null.");
-if (hosts != null) {
-
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63139458
--- Diff: engine/schema/src/com/cloud/dc/dao/ClusterDaoImpl.java ---
@@ -260,4 +268,41 @@ public boolean remove(Long id) {
sc.setParameters("dataCenterId", zoneId);
return customSearch(sc, null);
}
+
+@Override
+public boolean computeWhetherClusterSupportsResigning(long clusterId) {
+ClusterVO cluster = findById(clusterId);
+
+if (cluster == null || cluster.getAllocationState() !=
Grouping.AllocationState.Enabled) {
+return false;
+}
+
+List hosts = hostDao.findByClusterId(clusterId);
+
+if (hosts == null) {
+return false;
+}
+
+for (HostVO host : hosts) {
+if (host == null) {
+return false;
+}
+
+DetailVO hostDetail = hostDetailsDao.findDetail(host.getId(),
"supportsResign");
+
+if (hostDetail == null) {
+return false;
+}
+
+String value = hostDetail.getValue();
+
+Boolean booleanValue = Boolean.valueOf(value);
+
+if (booleanValue == false) {
+return false;
+}
+}
+
+return true;
+}
--- End diff --
Actually, I think we just need to join the host and the host_details tables
(because cluster ID should be in the host table).
We'd want all of the hosts with a particular cluster ID selected from the
host table and then we'd want to join on host ID in the host_details table and
further refine the returned hosts by those that have a "supportsResign" value
in the name field.
Do you happen to know of any code of ours that allows you to SQL filter the
returned values based on values that exist in not just one, but two tables?
From what I've seen, we seem to be pretty heavily filtering only on data
present in one table (even when we join with another table).
If this were just raw SQL, it would be pretty easy, but we've got that Java
DB layer that this needs to fit within.
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63138267
--- Diff: engine/schema/src/com/cloud/dc/dao/ClusterDaoImpl.java ---
@@ -260,4 +268,41 @@ public boolean remove(Long id) {
sc.setParameters("dataCenterId", zoneId);
return customSearch(sc, null);
}
+
+@Override
+public boolean computeWhetherClusterSupportsResigning(long clusterId) {
+ClusterVO cluster = findById(clusterId);
+
+if (cluster == null || cluster.getAllocationState() !=
Grouping.AllocationState.Enabled) {
+return false;
+}
+
+List hosts = hostDao.findByClusterId(clusterId);
+
+if (hosts == null) {
+return false;
+}
+
+for (HostVO host : hosts) {
+if (host == null) {
+return false;
+}
+
+DetailVO hostDetail = hostDetailsDao.findDetail(host.getId(),
"supportsResign");
+
+if (hostDetail == null) {
+return false;
+}
+
+String value = hostDetail.getValue();
+
+Boolean booleanValue = Boolean.valueOf(value);
+
+if (booleanValue == false) {
+return false;
+}
+}
+
+return true;
+}
--- End diff --
See what you think...I just added a method on the HostDetailsDao to return
a Map where the key is the ID of the host and the value is true or false (you
pass in the "name" field you are interested in).
This way we just do one query and then look up the results in the map each
time through the "for" loop.
If we want to do a join of the host, host_details, and cluster tables
(which, I believe, would be necessary to reduce the results coming back from
the DB), perhaps you know of somewhere in our codebase where we do some similar
action? Thanks
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63136000
--- Diff: engine/schema/src/com/cloud/dc/dao/ClusterDaoImpl.java ---
@@ -260,4 +268,41 @@ public boolean remove(Long id) {
sc.setParameters("dataCenterId", zoneId);
return customSearch(sc, null);
}
+
+@Override
+public boolean computeWhetherClusterSupportsResigning(long clusterId) {
+ClusterVO cluster = findById(clusterId);
+
+if (cluster == null || cluster.getAllocationState() !=
Grouping.AllocationState.Enabled) {
+return false;
+}
+
+List hosts = hostDao.findByClusterId(clusterId);
+
+if (hosts == null) {
+return false;
+}
+
+for (HostVO host : hosts) {
+if (host == null) {
+return false;
+}
+
+DetailVO hostDetail = hostDetailsDao.findDetail(host.getId(),
"supportsResign");
+
+if (hostDetail == null) {
+return false;
+}
+
+String value = hostDetail.getValue();
+
+Boolean booleanValue = Boolean.valueOf(value);
+
+if (booleanValue == false) {
+return false;
+}
+}
+
+return true;
+}
--- End diff --
Isn't there a limit of like 16 - 32 hosts in a cluster (therefore only 16 -
32 DB calls in the for loop)?
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63133626
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -361,59 +600,205 @@ private String getProperty(long snapshotId, String
property) {
}
private Map getVolumeDetails(VolumeInfo volumeInfo) {
-Map sourceDetails = new HashMap();
+Map volumeDetails = new HashMap();
VolumeVO volumeVO = _volumeDao.findById(volumeInfo.getId());
long storagePoolId = volumeVO.getPoolId();
StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
-sourceDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
-sourceDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
-sourceDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
+volumeDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+volumeDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+volumeDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
ChapInfo chapInfo = _volumeService.getChapInfo(volumeInfo,
volumeInfo.getDataStore());
if (chapInfo != null) {
-sourceDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
-sourceDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
-sourceDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
-sourceDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
+volumeDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
+volumeDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
}
-return sourceDetails;
+return volumeDetails;
}
-public HostVO getHost(long dataStoreId) {
-StoragePoolVO storagePoolVO =
_storagePoolDao.findById(dataStoreId);
+private Map getSnapshotDetails(SnapshotInfo
snapshotInfo) {
+Map snapshotDetails = new HashMap();
+
+long storagePoolId = snapshotInfo.getDataStore().getId();
+StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
+
+snapshotDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+snapshotDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+
+long snapshotId = snapshotInfo.getId();
+
+snapshotDetails.put(DiskTO.IQN, getProperty(snapshotId,
DiskTO.IQN));
-List clusters =
_mgr.searchForClusters(storagePoolVO.getDataCenterId(), new Long(0),
Long.MAX_VALUE, HypervisorType.XenServer.toString());
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_SECRET));
+snapshotDetails.put(DiskTO.CHAP_TARGET_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_TARGET_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_TARGET_SECRET,
getProperty(snapshotId, DiskTO.CHAP_TARGET_SECRET));
-if (clusters == null) {
-throw new CloudRuntimeException("Unable to locate an
applicable cluster");
+return snapshotDetails;
+}
+
+private HostVO getHost(SnapshotInfo snapshotInfo) {
+HostVO hostVO = getHost(snapshotInfo.getDataCenterId(), true);
+
+if (hostVO == null) {
+hostVO = getHost(snapshotInfo.getDataCenterId(), false);
+
+if (hostVO == null) {
+throw new CloudRuntimeException("Unable to locate an
applicable host in data center with ID = " + snapshotInfo.getDataCenterId());
+}
}
-for (Cluster cluster : clusters) {
-if (cluster.getAllocationState() == AllocationState.Enabled) {
-List hosts =
_hostDao.findByClusterId(cluster.getId());
+return hostVO;
+}
+
+private HostVO getHost(Long zoneId, boolean
computeClusterMustSupportResign) {
+Preconditions.checkArgument(zoneId != null, "Zone ID cannot be
null.");
-if (hosts != null) {
-
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63132551
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -361,59 +600,205 @@ private String getProperty(long snapshotId, String
property) {
}
private Map getVolumeDetails(VolumeInfo volumeInfo) {
-Map sourceDetails = new HashMap();
+Map volumeDetails = new HashMap();
VolumeVO volumeVO = _volumeDao.findById(volumeInfo.getId());
long storagePoolId = volumeVO.getPoolId();
StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
-sourceDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
-sourceDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
-sourceDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
+volumeDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+volumeDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+volumeDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
ChapInfo chapInfo = _volumeService.getChapInfo(volumeInfo,
volumeInfo.getDataStore());
if (chapInfo != null) {
-sourceDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
-sourceDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
-sourceDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
-sourceDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
+volumeDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
+volumeDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
}
-return sourceDetails;
+return volumeDetails;
}
-public HostVO getHost(long dataStoreId) {
-StoragePoolVO storagePoolVO =
_storagePoolDao.findById(dataStoreId);
+private Map getSnapshotDetails(SnapshotInfo
snapshotInfo) {
+Map snapshotDetails = new HashMap();
+
+long storagePoolId = snapshotInfo.getDataStore().getId();
+StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
+
+snapshotDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+snapshotDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+
+long snapshotId = snapshotInfo.getId();
+
+snapshotDetails.put(DiskTO.IQN, getProperty(snapshotId,
DiskTO.IQN));
-List clusters =
_mgr.searchForClusters(storagePoolVO.getDataCenterId(), new Long(0),
Long.MAX_VALUE, HypervisorType.XenServer.toString());
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_SECRET));
+snapshotDetails.put(DiskTO.CHAP_TARGET_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_TARGET_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_TARGET_SECRET,
getProperty(snapshotId, DiskTO.CHAP_TARGET_SECRET));
-if (clusters == null) {
-throw new CloudRuntimeException("Unable to locate an
applicable cluster");
+return snapshotDetails;
+}
+
+private HostVO getHost(SnapshotInfo snapshotInfo) {
+HostVO hostVO = getHost(snapshotInfo.getDataCenterId(), true);
+
+if (hostVO == null) {
+hostVO = getHost(snapshotInfo.getDataCenterId(), false);
+
+if (hostVO == null) {
+throw new CloudRuntimeException("Unable to locate an
applicable host in data center with ID = " + snapshotInfo.getDataCenterId());
+}
}
-for (Cluster cluster : clusters) {
-if (cluster.getAllocationState() == AllocationState.Enabled) {
-List hosts =
_hostDao.findByClusterId(cluster.getId());
+return hostVO;
+}
+
+private HostVO getHost(Long zoneId, boolean
computeClusterMustSupportResign) {
+Preconditions.checkArgument(zoneId != null, "Zone ID cannot be
null.");
-if (hosts != null) {
-
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63132440
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -172,78 +258,207 @@ private void validate(SnapshotInfo snapshotInfo) {
}
}
-private Void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
+private boolean usingBackendSnapshotFor(SnapshotInfo snapshotInfo) {
+String property = getProperty(snapshotInfo.getId(),
"takeSnapshot");
+
+return Boolean.parseBoolean(property);
+}
+
+private void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
try {
snapshotInfo.processEvent(Event.CopyingRequested);
}
catch (Exception ex) {
throw new CloudRuntimeException("This snapshot is not
currently in a state where it can be used to create a template.");
}
-HostVO hostVO = getHost(snapshotInfo.getDataStore().getId());
-DataStore srcDataStore = snapshotInfo.getDataStore();
+HostVO hostVO = getHost(snapshotInfo);
-String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
-int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
-CopyCommand copyCommand = new CopyCommand(snapshotInfo.getTO(),
templateInfo.getTO(), primaryStorageDownloadWait,
VirtualMachineManager.ExecuteInSequence.value());
+boolean usingBackendSnapshot =
usingBackendSnapshotFor(snapshotInfo);
+boolean computeClusterSupportsResign =
clusterDao.computeWhetherClusterSupportsResigning(hostVO.getClusterId());
-String errMsg = null;
-
-CopyCmdAnswer copyCmdAnswer = null;
+if (usingBackendSnapshot && !computeClusterSupportsResign) {
+throw new CloudRuntimeException("Unable to locate an
applicable host with which to perform a resignature operation");
+}
try {
-_volumeService.grantAccess(snapshotInfo, hostVO, srcDataStore);
+if (usingBackendSnapshot) {
+createVolumeFromSnapshot(hostVO, snapshotInfo, true);
+}
-Map srcDetails =
getSnapshotDetails(_storagePoolDao.findById(srcDataStore.getId()),
snapshotInfo);
+DataStore srcDataStore = snapshotInfo.getDataStore();
-copyCommand.setOptions(srcDetails);
+String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
+int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
+CopyCommand copyCommand = new
CopyCommand(snapshotInfo.getTO(), templateInfo.getTO(),
primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value());
+
+String errMsg = null;
+
+CopyCmdAnswer copyCmdAnswer = null;
-copyCmdAnswer = (CopyCmdAnswer)_agentMgr.send(hostVO.getId(),
copyCommand);
-}
-catch (Exception ex) {
-throw new CloudRuntimeException(ex.getMessage());
-}
-finally {
try {
-_volumeService.revokeAccess(snapshotInfo, hostVO,
srcDataStore);
+// If we are using a back-end snapshot, then we should
still have access to it from the hosts in the cluster that hostVO is in
+// (because we passed in true as the third parameter to
createVolumeFromSnapshot above).
+if (usingBackendSnapshot == false) {
+_volumeService.grantAccess(snapshotInfo, hostVO,
srcDataStore);
+}
+
+Map srcDetails =
getSnapshotDetails(snapshotInfo);
+
+copyCommand.setOptions(srcDetails);
+
+copyCmdAnswer =
(CopyCmdAnswer)_agentMgr.send(hostVO.getId(), copyCommand);
}
-catch (Exception ex) {
-s_logger.debug(ex.getMessage(), ex);
+catch (AgentUnavailableException | OperationTimedoutException
ex) {
+throw new CloudRuntimeException("Failed to create template
from snapshot : " + ex.getMessage());
}
+finally {
+try {
+_volumeService.revokeAccess(snapshotInfo, hostVO,
srcDataStore);
+}
+
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63132242
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -172,78 +258,207 @@ private void validate(SnapshotInfo snapshotInfo) {
}
}
-private Void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
+private boolean usingBackendSnapshotFor(SnapshotInfo snapshotInfo) {
+String property = getProperty(snapshotInfo.getId(),
"takeSnapshot");
+
+return Boolean.parseBoolean(property);
+}
+
+private void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
try {
snapshotInfo.processEvent(Event.CopyingRequested);
}
catch (Exception ex) {
throw new CloudRuntimeException("This snapshot is not
currently in a state where it can be used to create a template.");
}
-HostVO hostVO = getHost(snapshotInfo.getDataStore().getId());
-DataStore srcDataStore = snapshotInfo.getDataStore();
+HostVO hostVO = getHost(snapshotInfo);
-String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
-int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
-CopyCommand copyCommand = new CopyCommand(snapshotInfo.getTO(),
templateInfo.getTO(), primaryStorageDownloadWait,
VirtualMachineManager.ExecuteInSequence.value());
+boolean usingBackendSnapshot =
usingBackendSnapshotFor(snapshotInfo);
+boolean computeClusterSupportsResign =
clusterDao.computeWhetherClusterSupportsResigning(hostVO.getClusterId());
-String errMsg = null;
-
-CopyCmdAnswer copyCmdAnswer = null;
+if (usingBackendSnapshot && !computeClusterSupportsResign) {
+throw new CloudRuntimeException("Unable to locate an
applicable host with which to perform a resignature operation");
+}
try {
-_volumeService.grantAccess(snapshotInfo, hostVO, srcDataStore);
+if (usingBackendSnapshot) {
+createVolumeFromSnapshot(hostVO, snapshotInfo, true);
+}
-Map srcDetails =
getSnapshotDetails(_storagePoolDao.findById(srcDataStore.getId()),
snapshotInfo);
+DataStore srcDataStore = snapshotInfo.getDataStore();
-copyCommand.setOptions(srcDetails);
+String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
+int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
+CopyCommand copyCommand = new
CopyCommand(snapshotInfo.getTO(), templateInfo.getTO(),
primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value());
+
+String errMsg = null;
+
+CopyCmdAnswer copyCmdAnswer = null;
-copyCmdAnswer = (CopyCmdAnswer)_agentMgr.send(hostVO.getId(),
copyCommand);
-}
-catch (Exception ex) {
-throw new CloudRuntimeException(ex.getMessage());
-}
-finally {
try {
-_volumeService.revokeAccess(snapshotInfo, hostVO,
srcDataStore);
+// If we are using a back-end snapshot, then we should
still have access to it from the hosts in the cluster that hostVO is in
+// (because we passed in true as the third parameter to
createVolumeFromSnapshot above).
+if (usingBackendSnapshot == false) {
+_volumeService.grantAccess(snapshotInfo, hostVO,
srcDataStore);
+}
+
+Map srcDetails =
getSnapshotDetails(snapshotInfo);
+
+copyCommand.setOptions(srcDetails);
+
+copyCmdAnswer =
(CopyCmdAnswer)_agentMgr.send(hostVO.getId(), copyCommand);
}
-catch (Exception ex) {
-s_logger.debug(ex.getMessage(), ex);
+catch (AgentUnavailableException | OperationTimedoutException
ex) {
+throw new CloudRuntimeException("Failed to create template
from snapshot : " + ex.getMessage());
}
+finally {
+try {
+_volumeService.revokeAccess(snapshotInfo, hostVO,
srcDataStore);
+}
+
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63132177
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -172,78 +258,207 @@ private void validate(SnapshotInfo snapshotInfo) {
}
}
-private Void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
+private boolean usingBackendSnapshotFor(SnapshotInfo snapshotInfo) {
+String property = getProperty(snapshotInfo.getId(),
"takeSnapshot");
+
+return Boolean.parseBoolean(property);
+}
+
+private void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
try {
snapshotInfo.processEvent(Event.CopyingRequested);
}
catch (Exception ex) {
throw new CloudRuntimeException("This snapshot is not
currently in a state where it can be used to create a template.");
}
-HostVO hostVO = getHost(snapshotInfo.getDataStore().getId());
-DataStore srcDataStore = snapshotInfo.getDataStore();
+HostVO hostVO = getHost(snapshotInfo);
-String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
-int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
-CopyCommand copyCommand = new CopyCommand(snapshotInfo.getTO(),
templateInfo.getTO(), primaryStorageDownloadWait,
VirtualMachineManager.ExecuteInSequence.value());
+boolean usingBackendSnapshot =
usingBackendSnapshotFor(snapshotInfo);
+boolean computeClusterSupportsResign =
clusterDao.computeWhetherClusterSupportsResigning(hostVO.getClusterId());
-String errMsg = null;
-
-CopyCmdAnswer copyCmdAnswer = null;
+if (usingBackendSnapshot && !computeClusterSupportsResign) {
+throw new CloudRuntimeException("Unable to locate an
applicable host with which to perform a resignature operation");
+}
try {
-_volumeService.grantAccess(snapshotInfo, hostVO, srcDataStore);
+if (usingBackendSnapshot) {
+createVolumeFromSnapshot(hostVO, snapshotInfo, true);
+}
-Map srcDetails =
getSnapshotDetails(_storagePoolDao.findById(srcDataStore.getId()),
snapshotInfo);
+DataStore srcDataStore = snapshotInfo.getDataStore();
-copyCommand.setOptions(srcDetails);
+String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
+int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
+CopyCommand copyCommand = new
CopyCommand(snapshotInfo.getTO(), templateInfo.getTO(),
primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value());
+
+String errMsg = null;
+
+CopyCmdAnswer copyCmdAnswer = null;
-copyCmdAnswer = (CopyCmdAnswer)_agentMgr.send(hostVO.getId(),
copyCommand);
-}
-catch (Exception ex) {
-throw new CloudRuntimeException(ex.getMessage());
-}
-finally {
try {
-_volumeService.revokeAccess(snapshotInfo, hostVO,
srcDataStore);
+// If we are using a back-end snapshot, then we should
still have access to it from the hosts in the cluster that hostVO is in
+// (because we passed in true as the third parameter to
createVolumeFromSnapshot above).
+if (usingBackendSnapshot == false) {
+_volumeService.grantAccess(snapshotInfo, hostVO,
srcDataStore);
+}
+
+Map srcDetails =
getSnapshotDetails(snapshotInfo);
+
+copyCommand.setOptions(srcDetails);
+
+copyCmdAnswer =
(CopyCmdAnswer)_agentMgr.send(hostVO.getId(), copyCommand);
}
-catch (Exception ex) {
-s_logger.debug(ex.getMessage(), ex);
+catch (AgentUnavailableException | OperationTimedoutException
ex) {
+throw new CloudRuntimeException("Failed to create template
from snapshot : " + ex.getMessage());
--- End diff --
Done
---
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
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63132007
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -172,78 +258,207 @@ private void validate(SnapshotInfo snapshotInfo) {
}
}
-private Void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
+private boolean usingBackendSnapshotFor(SnapshotInfo snapshotInfo) {
+String property = getProperty(snapshotInfo.getId(),
"takeSnapshot");
+
+return Boolean.parseBoolean(property);
+}
+
+private void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
try {
snapshotInfo.processEvent(Event.CopyingRequested);
}
catch (Exception ex) {
throw new CloudRuntimeException("This snapshot is not
currently in a state where it can be used to create a template.");
}
-HostVO hostVO = getHost(snapshotInfo.getDataStore().getId());
-DataStore srcDataStore = snapshotInfo.getDataStore();
+HostVO hostVO = getHost(snapshotInfo);
-String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
-int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
-CopyCommand copyCommand = new CopyCommand(snapshotInfo.getTO(),
templateInfo.getTO(), primaryStorageDownloadWait,
VirtualMachineManager.ExecuteInSequence.value());
+boolean usingBackendSnapshot =
usingBackendSnapshotFor(snapshotInfo);
+boolean computeClusterSupportsResign =
clusterDao.computeWhetherClusterSupportsResigning(hostVO.getClusterId());
-String errMsg = null;
-
-CopyCmdAnswer copyCmdAnswer = null;
+if (usingBackendSnapshot && !computeClusterSupportsResign) {
+throw new CloudRuntimeException("Unable to locate an
applicable host with which to perform a resignature operation");
--- End diff --
Done
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63131882
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -55,65 +68,98 @@
import com.cloud.host.Host;
import com.cloud.host.HostVO;
import com.cloud.host.dao.HostDao;
+import com.cloud.host.dao.HostDetailsDao;
import com.cloud.hypervisor.Hypervisor.HypervisorType;
-import com.cloud.org.Cluster;
-import com.cloud.org.Grouping.AllocationState;
-import com.cloud.resource.ResourceState;
import com.cloud.server.ManagementService;
import com.cloud.storage.DataStoreRole;
import com.cloud.storage.DiskOfferingVO;
import com.cloud.storage.SnapshotVO;
import com.cloud.storage.Storage.ImageFormat;
+import com.cloud.storage.VolumeDetailVO;
import com.cloud.storage.VolumeVO;
import com.cloud.storage.dao.DiskOfferingDao;
import com.cloud.storage.dao.SnapshotDao;
import com.cloud.storage.dao.SnapshotDetailsDao;
import com.cloud.storage.dao.SnapshotDetailsVO;
import com.cloud.storage.dao.VolumeDao;
+import com.cloud.storage.dao.VolumeDetailsDao;
import com.cloud.utils.NumbersUtil;
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.vm.VirtualMachineManager;
+import com.google.common.base.Preconditions;
+
@Component
public class StorageSystemDataMotionStrategy implements DataMotionStrategy
{
private static final Logger s_logger =
Logger.getLogger(StorageSystemDataMotionStrategy.class);
+private static final Random random = new Random(System.nanoTime());
@Inject private AgentManager _agentMgr;
@Inject private ConfigurationDao _configDao;
+@Inject private DataStoreManager dataStoreMgr;
@Inject private DiskOfferingDao _diskOfferingDao;
+@Inject private ClusterDao clusterDao;
@Inject private HostDao _hostDao;
+@Inject private HostDetailsDao hostDetailsDao;
@Inject private ManagementService _mgr;
@Inject private PrimaryDataStoreDao _storagePoolDao;
@Inject private SnapshotDao _snapshotDao;
@Inject private SnapshotDetailsDao _snapshotDetailsDao;
@Inject private VolumeDao _volumeDao;
@Inject private VolumeDataFactory _volumeDataFactory;
+@Inject private VolumeDetailsDao volumeDetailsDao;
@Inject private VolumeService _volumeService;
@Override
public StrategyPriority canHandle(DataObject srcData, DataObject
destData) {
if (srcData instanceof SnapshotInfo) {
-if (canHandle(srcData.getDataStore()) ||
canHandle(destData.getDataStore())) {
+if (canHandle(srcData) || canHandle(destData)) {
return StrategyPriority.HIGHEST;
}
}
+if (srcData instanceof TemplateInfo && destData instanceof
VolumeInfo &&
+(srcData.getDataStore().getId() ==
destData.getDataStore().getId()) &&
+(canHandle(srcData) || canHandle(destData))) {
+// Both source and dest are on the same storage, so just clone
them.
+return StrategyPriority.HIGHEST;
+}
+
return StrategyPriority.CANT_HANDLE;
}
-private boolean canHandle(DataStore dataStore) {
+private boolean canHandle(DataObject dataObject) {
+Preconditions.checkArgument(dataObject != null, "Passing 'null' to
dataObject of canHandle(DataObject) is not supported.");
+
+DataStore dataStore = dataObject.getDataStore();
+
if (dataStore.getRole() == DataStoreRole.Primary) {
Map mapCapabilities =
dataStore.getDriver().getCapabilities();
-if (mapCapabilities != null) {
+if (mapCapabilities == null) {
+return false;
+}
+
+if (dataObject instanceof VolumeInfo || dataObject instanceof
SnapshotInfo) {
String value =
mapCapabilities.get(DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT.toString());
-Boolean supportsStorageSystemSnapshots = new
Boolean(value);
+Boolean supportsStorageSystemSnapshots =
Boolean.valueOf(value);
if (supportsStorageSystemSnapshots) {
s_logger.info("Using
'StorageSystemDataMotionStrategy'");
return true;
}
+} else if (dataObject instanceof TemplateInfo) {
+// If the storage system can clone volumes, we can cache
templates on it.
+String value =
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63131753
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -361,59 +608,225 @@ private String getProperty(long snapshotId, String
property) {
}
private Map getVolumeDetails(VolumeInfo volumeInfo) {
-Map sourceDetails = new HashMap();
+Map volumeDetails = new HashMap();
VolumeVO volumeVO = _volumeDao.findById(volumeInfo.getId());
long storagePoolId = volumeVO.getPoolId();
StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
-sourceDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
-sourceDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
-sourceDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
+volumeDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+volumeDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+volumeDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
ChapInfo chapInfo = _volumeService.getChapInfo(volumeInfo,
volumeInfo.getDataStore());
if (chapInfo != null) {
-sourceDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
-sourceDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
-sourceDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
-sourceDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
+volumeDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
+volumeDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
+}
+
+return volumeDetails;
+}
+
+private Map getSnapshotDetails(SnapshotInfo
snapshotInfo) {
+Map snapshotDetails = new HashMap();
+
+long storagePoolId = snapshotInfo.getDataStore().getId();
+StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
+
+snapshotDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+snapshotDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+
+long snapshotId = snapshotInfo.getId();
+
+snapshotDetails.put(DiskTO.IQN, getProperty(snapshotId,
DiskTO.IQN));
+
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_SECRET));
+snapshotDetails.put(DiskTO.CHAP_TARGET_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_TARGET_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_TARGET_SECRET,
getProperty(snapshotId, DiskTO.CHAP_TARGET_SECRET));
+
+return snapshotDetails;
+}
+
+private HostVO getHost(SnapshotInfo snapshotInfo) {
+HostVO hostVO = getHost(snapshotInfo.getDataCenterId(), true);
+
+if (hostVO == null) {
+hostVO = getHost(snapshotInfo.getDataCenterId(), false);
+
+if (hostVO == null) {
+throw new CloudRuntimeException("Unable to locate an
applicable host");
+}
}
-return sourceDetails;
+return hostVO;
}
-public HostVO getHost(long dataStoreId) {
-StoragePoolVO storagePoolVO =
_storagePoolDao.findById(dataStoreId);
+private HostVO getHost(Long zoneId, boolean
computeClusterMustSupportResign) {
+if (zoneId == null) {
+throw new CloudRuntimeException("Zone ID cannot be null.");
+}
-List clusters =
_mgr.searchForClusters(storagePoolVO.getDataCenterId(), new Long(0),
Long.MAX_VALUE, HypervisorType.XenServer.toString());
+List clusters = _mgr.searchForClusters(zoneId,
new Long(0), Long.MAX_VALUE, HypervisorType.XenServer.toString());
if (clusters == null) {
-throw new CloudRuntimeException("Unable to locate an
applicable cluster");
+clusters = new ArrayList<>();
}
+Collections.shuffle(clusters, new Random(System.nanoTime()));
+
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63131338
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -255,99 +448,153 @@ private Void
handleCreateVolumeFromSnapshotBothOnStorageSystem(SnapshotInfo snap
VolumeApiResult result = future.get();
+if (volumeDetail != null) {
+_volumeDetailsDao.remove(volumeDetail.getId());
+}
+
if (result.isFailed()) {
s_logger.debug("Failed to create a volume: " +
result.getResult());
throw new CloudRuntimeException(result.getResult());
}
-}
-catch (Exception ex) {
-throw new CloudRuntimeException(ex.getMessage());
-}
-volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(),
volumeInfo.getDataStore());
+volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(),
volumeInfo.getDataStore());
-volumeInfo.processEvent(Event.MigrationRequested);
+volumeInfo.processEvent(Event.MigrationRequested);
-volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(),
volumeInfo.getDataStore());
+volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(),
volumeInfo.getDataStore());
-HostVO hostVO = getHost(snapshotInfo.getDataStore().getId());
+if (useCloning) {
+copyCmdAnswer = performResignature(volumeInfo, hostVO);
+}
+else {
+// asking for a XenServer host here so we don't always
prefer to use XenServer hosts that support resigning
+// even when we don't need those hosts to do this kind of
copy work
+hostVO = getHost(snapshotInfo.getDataCenterId(), false);
-String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
-int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
-CopyCommand copyCommand = new CopyCommand(snapshotInfo.getTO(),
volumeInfo.getTO(), primaryStorageDownloadWait,
VirtualMachineManager.ExecuteInSequence.value());
+copyCmdAnswer = performCopyOfVdi(volumeInfo, snapshotInfo,
hostVO);
+}
-CopyCmdAnswer copyCmdAnswer = null;
+if (copyCmdAnswer == null || !copyCmdAnswer.getResult()) {
+if (copyCmdAnswer != null &&
!StringUtils.isEmpty(copyCmdAnswer.getDetails())) {
+errMsg = copyCmdAnswer.getDetails();
+}
+else {
+errMsg = "Unable to perform host-side operation";
+}
+}
+}
+catch (Exception ex) {
+errMsg = ex.getMessage() != null ? ex.getMessage() : "Copy
operation failed";
--- End diff --
Done
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63131303
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -132,31 +171,52 @@ public void copyAsync(DataObject srcData, DataObject
destData, Host destHost, As
validate(snapshotInfo);
-boolean canHandleSrc = canHandle(srcData.getDataStore());
+boolean canHandleSrc = canHandle(srcData);
if (canHandleSrc && destData instanceof TemplateInfo &&
(destData.getDataStore().getRole() ==
DataStoreRole.Image || destData.getDataStore().getRole() ==
DataStoreRole.ImageCache)) {
handleCreateTemplateFromSnapshot(snapshotInfo,
(TemplateInfo)destData, callback);
+
return;
}
if (destData instanceof VolumeInfo) {
VolumeInfo volumeInfo = (VolumeInfo)destData;
-boolean canHandleDest = canHandle(destData.getDataStore());
+
+boolean canHandleDest = canHandle(destData);
if (canHandleSrc && canHandleDest) {
-
handleCreateVolumeFromSnapshotBothOnStorageSystem(snapshotInfo, volumeInfo,
callback);
-return;
+if (snapshotInfo.getDataStore().getId() ==
volumeInfo.getDataStore().getId()) {
+
handleCreateVolumeFromSnapshotBothOnStorageSystem(snapshotInfo, volumeInfo,
callback);
+return;
+}
+else {
+throw new UnsupportedOperationException("This
operation is not supported (DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT " +
+"not supported by source or destination
storage plug-in).");
+}
}
+
if (canHandleSrc) {
throw new UnsupportedOperationException("This
operation is not supported (DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT " +
"not supported by destination storage
plug-in).");
}
+
if (canHandleDest) {
throw new UnsupportedOperationException("This
operation is not supported (DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT " +
"not supported by source storage plug-in).");
}
}
+} else if (srcData instanceof TemplateInfo && destData instanceof
VolumeInfo) {
+boolean canHandleSrc = canHandle(srcData);
+
+if (!canHandleSrc) {
+throw new UnsupportedOperationException("This operation is
not supported (DataStoreCapabilities.STORAGE_CAN_CREATE_VOLUME_FROM_VOLUME " +
--- End diff --
Done
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63131310
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -132,31 +171,52 @@ public void copyAsync(DataObject srcData, DataObject
destData, Host destHost, As
validate(snapshotInfo);
-boolean canHandleSrc = canHandle(srcData.getDataStore());
+boolean canHandleSrc = canHandle(srcData);
if (canHandleSrc && destData instanceof TemplateInfo &&
(destData.getDataStore().getRole() ==
DataStoreRole.Image || destData.getDataStore().getRole() ==
DataStoreRole.ImageCache)) {
handleCreateTemplateFromSnapshot(snapshotInfo,
(TemplateInfo)destData, callback);
+
return;
}
if (destData instanceof VolumeInfo) {
VolumeInfo volumeInfo = (VolumeInfo)destData;
-boolean canHandleDest = canHandle(destData.getDataStore());
+
+boolean canHandleDest = canHandle(destData);
if (canHandleSrc && canHandleDest) {
-
handleCreateVolumeFromSnapshotBothOnStorageSystem(snapshotInfo, volumeInfo,
callback);
-return;
+if (snapshotInfo.getDataStore().getId() ==
volumeInfo.getDataStore().getId()) {
+
handleCreateVolumeFromSnapshotBothOnStorageSystem(snapshotInfo, volumeInfo,
callback);
+return;
+}
+else {
+throw new UnsupportedOperationException("This
operation is not supported (DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT " +
--- End diff --
Done
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63112842
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -361,59 +608,225 @@ private String getProperty(long snapshotId, String
property) {
}
private Map getVolumeDetails(VolumeInfo volumeInfo) {
-Map sourceDetails = new HashMap();
+Map volumeDetails = new HashMap();
VolumeVO volumeVO = _volumeDao.findById(volumeInfo.getId());
long storagePoolId = volumeVO.getPoolId();
StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
-sourceDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
-sourceDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
-sourceDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
+volumeDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+volumeDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+volumeDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
ChapInfo chapInfo = _volumeService.getChapInfo(volumeInfo,
volumeInfo.getDataStore());
if (chapInfo != null) {
-sourceDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
-sourceDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
-sourceDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
-sourceDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
+volumeDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
+volumeDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
+}
+
+return volumeDetails;
+}
+
+private Map getSnapshotDetails(SnapshotInfo
snapshotInfo) {
+Map snapshotDetails = new HashMap();
+
+long storagePoolId = snapshotInfo.getDataStore().getId();
+StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
+
+snapshotDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+snapshotDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+
+long snapshotId = snapshotInfo.getId();
+
+snapshotDetails.put(DiskTO.IQN, getProperty(snapshotId,
DiskTO.IQN));
+
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_SECRET));
+snapshotDetails.put(DiskTO.CHAP_TARGET_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_TARGET_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_TARGET_SECRET,
getProperty(snapshotId, DiskTO.CHAP_TARGET_SECRET));
+
+return snapshotDetails;
+}
+
+private HostVO getHost(SnapshotInfo snapshotInfo) {
+HostVO hostVO = getHost(snapshotInfo.getDataCenterId(), true);
+
+if (hostVO == null) {
+hostVO = getHost(snapshotInfo.getDataCenterId(), false);
+
+if (hostVO == null) {
+throw new CloudRuntimeException("Unable to locate an
applicable host");
+}
}
-return sourceDetails;
+return hostVO;
}
-public HostVO getHost(long dataStoreId) {
-StoragePoolVO storagePoolVO =
_storagePoolDao.findById(dataStoreId);
+private HostVO getHost(Long zoneId, boolean
computeClusterMustSupportResign) {
+if (zoneId == null) {
+throw new CloudRuntimeException("Zone ID cannot be null.");
+}
-List clusters =
_mgr.searchForClusters(storagePoolVO.getDataCenterId(), new Long(0),
Long.MAX_VALUE, HypervisorType.XenServer.toString());
+List clusters = _mgr.searchForClusters(zoneId,
new Long(0), Long.MAX_VALUE, HypervisorType.XenServer.toString());
if (clusters == null) {
-throw new CloudRuntimeException("Unable to locate an
applicable cluster");
+clusters = new ArrayList<>();
}
+Collections.shuffle(clusters, new Random(System.nanoTime()));
+
+
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63112811
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -361,59 +600,205 @@ private String getProperty(long snapshotId, String
property) {
}
private Map getVolumeDetails(VolumeInfo volumeInfo) {
-Map sourceDetails = new HashMap();
+Map volumeDetails = new HashMap();
VolumeVO volumeVO = _volumeDao.findById(volumeInfo.getId());
long storagePoolId = volumeVO.getPoolId();
StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
-sourceDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
-sourceDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
-sourceDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
+volumeDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+volumeDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+volumeDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
ChapInfo chapInfo = _volumeService.getChapInfo(volumeInfo,
volumeInfo.getDataStore());
if (chapInfo != null) {
-sourceDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
-sourceDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
-sourceDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
-sourceDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
+volumeDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
+volumeDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
}
-return sourceDetails;
+return volumeDetails;
}
-public HostVO getHost(long dataStoreId) {
-StoragePoolVO storagePoolVO =
_storagePoolDao.findById(dataStoreId);
+private Map getSnapshotDetails(SnapshotInfo
snapshotInfo) {
+Map snapshotDetails = new HashMap();
+
+long storagePoolId = snapshotInfo.getDataStore().getId();
+StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
+
+snapshotDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+snapshotDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+
+long snapshotId = snapshotInfo.getId();
+
+snapshotDetails.put(DiskTO.IQN, getProperty(snapshotId,
DiskTO.IQN));
-List clusters =
_mgr.searchForClusters(storagePoolVO.getDataCenterId(), new Long(0),
Long.MAX_VALUE, HypervisorType.XenServer.toString());
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_SECRET));
+snapshotDetails.put(DiskTO.CHAP_TARGET_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_TARGET_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_TARGET_SECRET,
getProperty(snapshotId, DiskTO.CHAP_TARGET_SECRET));
-if (clusters == null) {
-throw new CloudRuntimeException("Unable to locate an
applicable cluster");
+return snapshotDetails;
+}
+
+private HostVO getHost(SnapshotInfo snapshotInfo) {
+HostVO hostVO = getHost(snapshotInfo.getDataCenterId(), true);
+
+if (hostVO == null) {
+hostVO = getHost(snapshotInfo.getDataCenterId(), false);
+
+if (hostVO == null) {
+throw new CloudRuntimeException("Unable to locate an
applicable host in data center with ID = " + snapshotInfo.getDataCenterId());
+}
}
-for (Cluster cluster : clusters) {
-if (cluster.getAllocationState() == AllocationState.Enabled) {
-List hosts =
_hostDao.findByClusterId(cluster.getId());
+return hostVO;
+}
+
+private HostVO getHost(Long zoneId, boolean
computeClusterMustSupportResign) {
+Preconditions.checkArgument(zoneId != null, "Zone ID cannot be
null.");
-if (hosts != null) {
-
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63112751
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -361,59 +600,205 @@ private String getProperty(long snapshotId, String
property) {
}
private Map getVolumeDetails(VolumeInfo volumeInfo) {
-Map sourceDetails = new HashMap();
+Map volumeDetails = new HashMap();
VolumeVO volumeVO = _volumeDao.findById(volumeInfo.getId());
long storagePoolId = volumeVO.getPoolId();
StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
-sourceDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
-sourceDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
-sourceDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
+volumeDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+volumeDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+volumeDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
ChapInfo chapInfo = _volumeService.getChapInfo(volumeInfo,
volumeInfo.getDataStore());
if (chapInfo != null) {
-sourceDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
-sourceDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
-sourceDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
-sourceDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
+volumeDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
+volumeDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
}
-return sourceDetails;
+return volumeDetails;
}
-public HostVO getHost(long dataStoreId) {
-StoragePoolVO storagePoolVO =
_storagePoolDao.findById(dataStoreId);
+private Map getSnapshotDetails(SnapshotInfo
snapshotInfo) {
+Map snapshotDetails = new HashMap();
+
+long storagePoolId = snapshotInfo.getDataStore().getId();
+StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
+
+snapshotDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+snapshotDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+
+long snapshotId = snapshotInfo.getId();
+
+snapshotDetails.put(DiskTO.IQN, getProperty(snapshotId,
DiskTO.IQN));
-List clusters =
_mgr.searchForClusters(storagePoolVO.getDataCenterId(), new Long(0),
Long.MAX_VALUE, HypervisorType.XenServer.toString());
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_SECRET));
+snapshotDetails.put(DiskTO.CHAP_TARGET_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_TARGET_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_TARGET_SECRET,
getProperty(snapshotId, DiskTO.CHAP_TARGET_SECRET));
-if (clusters == null) {
-throw new CloudRuntimeException("Unable to locate an
applicable cluster");
+return snapshotDetails;
+}
+
+private HostVO getHost(SnapshotInfo snapshotInfo) {
+HostVO hostVO = getHost(snapshotInfo.getDataCenterId(), true);
+
+if (hostVO == null) {
+hostVO = getHost(snapshotInfo.getDataCenterId(), false);
+
+if (hostVO == null) {
+throw new CloudRuntimeException("Unable to locate an
applicable host in data center with ID = " + snapshotInfo.getDataCenterId());
+}
}
-for (Cluster cluster : clusters) {
-if (cluster.getAllocationState() == AllocationState.Enabled) {
-List hosts =
_hostDao.findByClusterId(cluster.getId());
+return hostVO;
+}
+
+private HostVO getHost(Long zoneId, boolean
computeClusterMustSupportResign) {
+Preconditions.checkArgument(zoneId != null, "Zone ID cannot be
null.");
-if (hosts != null) {
-
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63112646
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -361,59 +608,225 @@ private String getProperty(long snapshotId, String
property) {
}
private Map getVolumeDetails(VolumeInfo volumeInfo) {
-Map sourceDetails = new HashMap();
+Map volumeDetails = new HashMap();
VolumeVO volumeVO = _volumeDao.findById(volumeInfo.getId());
long storagePoolId = volumeVO.getPoolId();
StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
-sourceDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
-sourceDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
-sourceDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
+volumeDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+volumeDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+volumeDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
ChapInfo chapInfo = _volumeService.getChapInfo(volumeInfo,
volumeInfo.getDataStore());
if (chapInfo != null) {
-sourceDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
-sourceDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
-sourceDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
-sourceDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
+volumeDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
+volumeDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
+}
+
+return volumeDetails;
+}
+
+private Map getSnapshotDetails(SnapshotInfo
snapshotInfo) {
+Map snapshotDetails = new HashMap();
+
+long storagePoolId = snapshotInfo.getDataStore().getId();
+StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
+
+snapshotDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+snapshotDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+
+long snapshotId = snapshotInfo.getId();
+
+snapshotDetails.put(DiskTO.IQN, getProperty(snapshotId,
DiskTO.IQN));
+
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_SECRET));
+snapshotDetails.put(DiskTO.CHAP_TARGET_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_TARGET_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_TARGET_SECRET,
getProperty(snapshotId, DiskTO.CHAP_TARGET_SECRET));
+
+return snapshotDetails;
+}
+
+private HostVO getHost(SnapshotInfo snapshotInfo) {
+HostVO hostVO = getHost(snapshotInfo.getDataCenterId(), true);
+
+if (hostVO == null) {
+hostVO = getHost(snapshotInfo.getDataCenterId(), false);
+
+if (hostVO == null) {
+throw new CloudRuntimeException("Unable to locate an
applicable host");
+}
}
-return sourceDetails;
+return hostVO;
}
-public HostVO getHost(long dataStoreId) {
-StoragePoolVO storagePoolVO =
_storagePoolDao.findById(dataStoreId);
+private HostVO getHost(Long zoneId, boolean
computeClusterMustSupportResign) {
+if (zoneId == null) {
+throw new CloudRuntimeException("Zone ID cannot be null.");
+}
-List clusters =
_mgr.searchForClusters(storagePoolVO.getDataCenterId(), new Long(0),
Long.MAX_VALUE, HypervisorType.XenServer.toString());
+List clusters = _mgr.searchForClusters(zoneId,
new Long(0), Long.MAX_VALUE, HypervisorType.XenServer.toString());
if (clusters == null) {
-throw new CloudRuntimeException("Unable to locate an
applicable cluster");
+clusters = new ArrayList<>();
}
+Collections.shuffle(clusters, new Random(System.nanoTime()));
+
+
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63111964
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -255,99 +448,153 @@ private Void
handleCreateVolumeFromSnapshotBothOnStorageSystem(SnapshotInfo snap
VolumeApiResult result = future.get();
+if (volumeDetail != null) {
+_volumeDetailsDao.remove(volumeDetail.getId());
+}
+
if (result.isFailed()) {
s_logger.debug("Failed to create a volume: " +
result.getResult());
throw new CloudRuntimeException(result.getResult());
}
-}
-catch (Exception ex) {
-throw new CloudRuntimeException(ex.getMessage());
-}
-volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(),
volumeInfo.getDataStore());
+volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(),
volumeInfo.getDataStore());
-volumeInfo.processEvent(Event.MigrationRequested);
+volumeInfo.processEvent(Event.MigrationRequested);
-volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(),
volumeInfo.getDataStore());
+volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(),
volumeInfo.getDataStore());
-HostVO hostVO = getHost(snapshotInfo.getDataStore().getId());
+if (useCloning) {
+copyCmdAnswer = performResignature(volumeInfo, hostVO);
+}
+else {
+// asking for a XenServer host here so we don't always
prefer to use XenServer hosts that support resigning
+// even when we don't need those hosts to do this kind of
copy work
+hostVO = getHost(snapshotInfo.getDataCenterId(), false);
-String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
-int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
-CopyCommand copyCommand = new CopyCommand(snapshotInfo.getTO(),
volumeInfo.getTO(), primaryStorageDownloadWait,
VirtualMachineManager.ExecuteInSequence.value());
+copyCmdAnswer = performCopyOfVdi(volumeInfo, snapshotInfo,
hostVO);
+}
-CopyCmdAnswer copyCmdAnswer = null;
+if (copyCmdAnswer == null || !copyCmdAnswer.getResult()) {
+if (copyCmdAnswer != null &&
!StringUtils.isEmpty(copyCmdAnswer.getDetails())) {
+errMsg = copyCmdAnswer.getDetails();
+}
+else {
+errMsg = "Unable to perform host-side operation";
+}
+}
+}
+catch (Exception ex) {
+errMsg = ex.getMessage() != null ? ex.getMessage() : "Copy
operation failed";
--- End diff --
@mike-tutkowski would it be possible to add more context information to
this error message?
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63111842
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -172,78 +258,207 @@ private void validate(SnapshotInfo snapshotInfo) {
}
}
-private Void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
+private boolean usingBackendSnapshotFor(SnapshotInfo snapshotInfo) {
+String property = getProperty(snapshotInfo.getId(),
"takeSnapshot");
+
+return Boolean.parseBoolean(property);
+}
+
+private void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
try {
snapshotInfo.processEvent(Event.CopyingRequested);
}
catch (Exception ex) {
throw new CloudRuntimeException("This snapshot is not
currently in a state where it can be used to create a template.");
}
-HostVO hostVO = getHost(snapshotInfo.getDataStore().getId());
-DataStore srcDataStore = snapshotInfo.getDataStore();
+HostVO hostVO = getHost(snapshotInfo);
-String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
-int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
-CopyCommand copyCommand = new CopyCommand(snapshotInfo.getTO(),
templateInfo.getTO(), primaryStorageDownloadWait,
VirtualMachineManager.ExecuteInSequence.value());
+boolean usingBackendSnapshot =
usingBackendSnapshotFor(snapshotInfo);
+boolean computeClusterSupportsResign =
clusterDao.computeWhetherClusterSupportsResigning(hostVO.getClusterId());
-String errMsg = null;
-
-CopyCmdAnswer copyCmdAnswer = null;
+if (usingBackendSnapshot && !computeClusterSupportsResign) {
+throw new CloudRuntimeException("Unable to locate an
applicable host with which to perform a resignature operation");
+}
try {
-_volumeService.grantAccess(snapshotInfo, hostVO, srcDataStore);
+if (usingBackendSnapshot) {
+createVolumeFromSnapshot(hostVO, snapshotInfo, true);
+}
-Map srcDetails =
getSnapshotDetails(_storagePoolDao.findById(srcDataStore.getId()),
snapshotInfo);
+DataStore srcDataStore = snapshotInfo.getDataStore();
-copyCommand.setOptions(srcDetails);
+String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
+int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
+CopyCommand copyCommand = new
CopyCommand(snapshotInfo.getTO(), templateInfo.getTO(),
primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value());
+
+String errMsg = null;
+
+CopyCmdAnswer copyCmdAnswer = null;
-copyCmdAnswer = (CopyCmdAnswer)_agentMgr.send(hostVO.getId(),
copyCommand);
-}
-catch (Exception ex) {
-throw new CloudRuntimeException(ex.getMessage());
-}
-finally {
try {
-_volumeService.revokeAccess(snapshotInfo, hostVO,
srcDataStore);
+// If we are using a back-end snapshot, then we should
still have access to it from the hosts in the cluster that hostVO is in
+// (because we passed in true as the third parameter to
createVolumeFromSnapshot above).
+if (usingBackendSnapshot == false) {
+_volumeService.grantAccess(snapshotInfo, hostVO,
srcDataStore);
+}
+
+Map srcDetails =
getSnapshotDetails(snapshotInfo);
+
+copyCommand.setOptions(srcDetails);
+
+copyCmdAnswer =
(CopyCmdAnswer)_agentMgr.send(hostVO.getId(), copyCommand);
}
-catch (Exception ex) {
-s_logger.debug(ex.getMessage(), ex);
+catch (AgentUnavailableException | OperationTimedoutException
ex) {
+throw new CloudRuntimeException("Failed to create template
from snapshot : " + ex.getMessage());
}
+finally {
+try {
+_volumeService.revokeAccess(snapshotInfo, hostVO,
srcDataStore);
+}
+
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63111740
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -172,78 +258,207 @@ private void validate(SnapshotInfo snapshotInfo) {
}
}
-private Void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
+private boolean usingBackendSnapshotFor(SnapshotInfo snapshotInfo) {
+String property = getProperty(snapshotInfo.getId(),
"takeSnapshot");
+
+return Boolean.parseBoolean(property);
+}
+
+private void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
try {
snapshotInfo.processEvent(Event.CopyingRequested);
}
catch (Exception ex) {
throw new CloudRuntimeException("This snapshot is not
currently in a state where it can be used to create a template.");
}
-HostVO hostVO = getHost(snapshotInfo.getDataStore().getId());
-DataStore srcDataStore = snapshotInfo.getDataStore();
+HostVO hostVO = getHost(snapshotInfo);
-String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
-int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
-CopyCommand copyCommand = new CopyCommand(snapshotInfo.getTO(),
templateInfo.getTO(), primaryStorageDownloadWait,
VirtualMachineManager.ExecuteInSequence.value());
+boolean usingBackendSnapshot =
usingBackendSnapshotFor(snapshotInfo);
+boolean computeClusterSupportsResign =
clusterDao.computeWhetherClusterSupportsResigning(hostVO.getClusterId());
-String errMsg = null;
-
-CopyCmdAnswer copyCmdAnswer = null;
+if (usingBackendSnapshot && !computeClusterSupportsResign) {
+throw new CloudRuntimeException("Unable to locate an
applicable host with which to perform a resignature operation");
+}
try {
-_volumeService.grantAccess(snapshotInfo, hostVO, srcDataStore);
+if (usingBackendSnapshot) {
+createVolumeFromSnapshot(hostVO, snapshotInfo, true);
+}
-Map srcDetails =
getSnapshotDetails(_storagePoolDao.findById(srcDataStore.getId()),
snapshotInfo);
+DataStore srcDataStore = snapshotInfo.getDataStore();
-copyCommand.setOptions(srcDetails);
+String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
+int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
+CopyCommand copyCommand = new
CopyCommand(snapshotInfo.getTO(), templateInfo.getTO(),
primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value());
+
+String errMsg = null;
+
+CopyCmdAnswer copyCmdAnswer = null;
-copyCmdAnswer = (CopyCmdAnswer)_agentMgr.send(hostVO.getId(),
copyCommand);
-}
-catch (Exception ex) {
-throw new CloudRuntimeException(ex.getMessage());
-}
-finally {
try {
-_volumeService.revokeAccess(snapshotInfo, hostVO,
srcDataStore);
+// If we are using a back-end snapshot, then we should
still have access to it from the hosts in the cluster that hostVO is in
+// (because we passed in true as the third parameter to
createVolumeFromSnapshot above).
+if (usingBackendSnapshot == false) {
+_volumeService.grantAccess(snapshotInfo, hostVO,
srcDataStore);
+}
+
+Map srcDetails =
getSnapshotDetails(snapshotInfo);
+
+copyCommand.setOptions(srcDetails);
+
+copyCmdAnswer =
(CopyCmdAnswer)_agentMgr.send(hostVO.getId(), copyCommand);
}
-catch (Exception ex) {
-s_logger.debug(ex.getMessage(), ex);
+catch (AgentUnavailableException | OperationTimedoutException
ex) {
+throw new CloudRuntimeException("Failed to create template
from snapshot : " + ex.getMessage());
}
+finally {
+try {
+_volumeService.revokeAccess(snapshotInfo, hostVO,
srcDataStore);
+}
+
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63111670
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -172,78 +258,207 @@ private void validate(SnapshotInfo snapshotInfo) {
}
}
-private Void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
+private boolean usingBackendSnapshotFor(SnapshotInfo snapshotInfo) {
+String property = getProperty(snapshotInfo.getId(),
"takeSnapshot");
+
+return Boolean.parseBoolean(property);
+}
+
+private void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
try {
snapshotInfo.processEvent(Event.CopyingRequested);
}
catch (Exception ex) {
throw new CloudRuntimeException("This snapshot is not
currently in a state where it can be used to create a template.");
}
-HostVO hostVO = getHost(snapshotInfo.getDataStore().getId());
-DataStore srcDataStore = snapshotInfo.getDataStore();
+HostVO hostVO = getHost(snapshotInfo);
-String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
-int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
-CopyCommand copyCommand = new CopyCommand(snapshotInfo.getTO(),
templateInfo.getTO(), primaryStorageDownloadWait,
VirtualMachineManager.ExecuteInSequence.value());
+boolean usingBackendSnapshot =
usingBackendSnapshotFor(snapshotInfo);
+boolean computeClusterSupportsResign =
clusterDao.computeWhetherClusterSupportsResigning(hostVO.getClusterId());
-String errMsg = null;
-
-CopyCmdAnswer copyCmdAnswer = null;
+if (usingBackendSnapshot && !computeClusterSupportsResign) {
+throw new CloudRuntimeException("Unable to locate an
applicable host with which to perform a resignature operation");
+}
try {
-_volumeService.grantAccess(snapshotInfo, hostVO, srcDataStore);
+if (usingBackendSnapshot) {
+createVolumeFromSnapshot(hostVO, snapshotInfo, true);
+}
-Map srcDetails =
getSnapshotDetails(_storagePoolDao.findById(srcDataStore.getId()),
snapshotInfo);
+DataStore srcDataStore = snapshotInfo.getDataStore();
-copyCommand.setOptions(srcDetails);
+String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
+int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
+CopyCommand copyCommand = new
CopyCommand(snapshotInfo.getTO(), templateInfo.getTO(),
primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value());
+
+String errMsg = null;
+
+CopyCmdAnswer copyCmdAnswer = null;
-copyCmdAnswer = (CopyCmdAnswer)_agentMgr.send(hostVO.getId(),
copyCommand);
-}
-catch (Exception ex) {
-throw new CloudRuntimeException(ex.getMessage());
-}
-finally {
try {
-_volumeService.revokeAccess(snapshotInfo, hostVO,
srcDataStore);
+// If we are using a back-end snapshot, then we should
still have access to it from the hosts in the cluster that hostVO is in
+// (because we passed in true as the third parameter to
createVolumeFromSnapshot above).
+if (usingBackendSnapshot == false) {
+_volumeService.grantAccess(snapshotInfo, hostVO,
srcDataStore);
+}
+
+Map srcDetails =
getSnapshotDetails(snapshotInfo);
+
+copyCommand.setOptions(srcDetails);
+
+copyCmdAnswer =
(CopyCmdAnswer)_agentMgr.send(hostVO.getId(), copyCommand);
}
-catch (Exception ex) {
-s_logger.debug(ex.getMessage(), ex);
+catch (AgentUnavailableException | OperationTimedoutException
ex) {
+throw new CloudRuntimeException("Failed to create template
from snapshot : " + ex.getMessage());
--- End diff --
Please add the snapshotId to the error message to assist operational
debugging.
---
If your project is set up for it, you can reply to this email and have your
reply appear on
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63111572
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -172,78 +258,207 @@ private void validate(SnapshotInfo snapshotInfo) {
}
}
-private Void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
+private boolean usingBackendSnapshotFor(SnapshotInfo snapshotInfo) {
+String property = getProperty(snapshotInfo.getId(),
"takeSnapshot");
+
+return Boolean.parseBoolean(property);
+}
+
+private void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
try {
snapshotInfo.processEvent(Event.CopyingRequested);
}
catch (Exception ex) {
throw new CloudRuntimeException("This snapshot is not
currently in a state where it can be used to create a template.");
}
-HostVO hostVO = getHost(snapshotInfo.getDataStore().getId());
-DataStore srcDataStore = snapshotInfo.getDataStore();
+HostVO hostVO = getHost(snapshotInfo);
-String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
-int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
-CopyCommand copyCommand = new CopyCommand(snapshotInfo.getTO(),
templateInfo.getTO(), primaryStorageDownloadWait,
VirtualMachineManager.ExecuteInSequence.value());
+boolean usingBackendSnapshot =
usingBackendSnapshotFor(snapshotInfo);
+boolean computeClusterSupportsResign =
clusterDao.computeWhetherClusterSupportsResigning(hostVO.getClusterId());
-String errMsg = null;
-
-CopyCmdAnswer copyCmdAnswer = null;
+if (usingBackendSnapshot && !computeClusterSupportsResign) {
+throw new CloudRuntimeException("Unable to locate an
applicable host with which to perform a resignature operation");
--- End diff --
Please include the cluster id in the error message to assist operational
debugging.
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63110910
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -132,31 +171,52 @@ public void copyAsync(DataObject srcData, DataObject
destData, Host destHost, As
validate(snapshotInfo);
-boolean canHandleSrc = canHandle(srcData.getDataStore());
+boolean canHandleSrc = canHandle(srcData);
if (canHandleSrc && destData instanceof TemplateInfo &&
(destData.getDataStore().getRole() ==
DataStoreRole.Image || destData.getDataStore().getRole() ==
DataStoreRole.ImageCache)) {
handleCreateTemplateFromSnapshot(snapshotInfo,
(TemplateInfo)destData, callback);
+
return;
}
if (destData instanceof VolumeInfo) {
VolumeInfo volumeInfo = (VolumeInfo)destData;
-boolean canHandleDest = canHandle(destData.getDataStore());
+
+boolean canHandleDest = canHandle(destData);
if (canHandleSrc && canHandleDest) {
-
handleCreateVolumeFromSnapshotBothOnStorageSystem(snapshotInfo, volumeInfo,
callback);
-return;
+if (snapshotInfo.getDataStore().getId() ==
volumeInfo.getDataStore().getId()) {
+
handleCreateVolumeFromSnapshotBothOnStorageSystem(snapshotInfo, volumeInfo,
callback);
+return;
+}
+else {
+throw new UnsupportedOperationException("This
operation is not supported (DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT " +
--- End diff --
@mike-tutkowski is it possible to add this information to log message?
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63110957
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -132,31 +171,52 @@ public void copyAsync(DataObject srcData, DataObject
destData, Host destHost, As
validate(snapshotInfo);
-boolean canHandleSrc = canHandle(srcData.getDataStore());
+boolean canHandleSrc = canHandle(srcData);
if (canHandleSrc && destData instanceof TemplateInfo &&
(destData.getDataStore().getRole() ==
DataStoreRole.Image || destData.getDataStore().getRole() ==
DataStoreRole.ImageCache)) {
handleCreateTemplateFromSnapshot(snapshotInfo,
(TemplateInfo)destData, callback);
+
return;
}
if (destData instanceof VolumeInfo) {
VolumeInfo volumeInfo = (VolumeInfo)destData;
-boolean canHandleDest = canHandle(destData.getDataStore());
+
+boolean canHandleDest = canHandle(destData);
if (canHandleSrc && canHandleDest) {
-
handleCreateVolumeFromSnapshotBothOnStorageSystem(snapshotInfo, volumeInfo,
callback);
-return;
+if (snapshotInfo.getDataStore().getId() ==
volumeInfo.getDataStore().getId()) {
+
handleCreateVolumeFromSnapshotBothOnStorageSystem(snapshotInfo, volumeInfo,
callback);
+return;
+}
+else {
+throw new UnsupportedOperationException("This
operation is not supported (DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT " +
+"not supported by source or destination
storage plug-in).");
+}
}
+
if (canHandleSrc) {
throw new UnsupportedOperationException("This
operation is not supported (DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT " +
"not supported by destination storage
plug-in).");
}
+
if (canHandleDest) {
throw new UnsupportedOperationException("This
operation is not supported (DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT " +
"not supported by source storage plug-in).");
}
}
+} else if (srcData instanceof TemplateInfo && destData instanceof
VolumeInfo) {
+boolean canHandleSrc = canHandle(srcData);
+
+if (!canHandleSrc) {
+throw new UnsupportedOperationException("This operation is
not supported (DataStoreCapabilities.STORAGE_CAN_CREATE_VOLUME_FROM_VOLUME " +
--- End diff --
@mike-tutkowski is it possible to add this information to the log message?
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63110833
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -55,65 +68,98 @@
import com.cloud.host.Host;
import com.cloud.host.HostVO;
import com.cloud.host.dao.HostDao;
+import com.cloud.host.dao.HostDetailsDao;
import com.cloud.hypervisor.Hypervisor.HypervisorType;
-import com.cloud.org.Cluster;
-import com.cloud.org.Grouping.AllocationState;
-import com.cloud.resource.ResourceState;
import com.cloud.server.ManagementService;
import com.cloud.storage.DataStoreRole;
import com.cloud.storage.DiskOfferingVO;
import com.cloud.storage.SnapshotVO;
import com.cloud.storage.Storage.ImageFormat;
+import com.cloud.storage.VolumeDetailVO;
import com.cloud.storage.VolumeVO;
import com.cloud.storage.dao.DiskOfferingDao;
import com.cloud.storage.dao.SnapshotDao;
import com.cloud.storage.dao.SnapshotDetailsDao;
import com.cloud.storage.dao.SnapshotDetailsVO;
import com.cloud.storage.dao.VolumeDao;
+import com.cloud.storage.dao.VolumeDetailsDao;
import com.cloud.utils.NumbersUtil;
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.vm.VirtualMachineManager;
+import com.google.common.base.Preconditions;
+
@Component
public class StorageSystemDataMotionStrategy implements DataMotionStrategy
{
private static final Logger s_logger =
Logger.getLogger(StorageSystemDataMotionStrategy.class);
+private static final Random random = new Random(System.nanoTime());
@Inject private AgentManager _agentMgr;
@Inject private ConfigurationDao _configDao;
+@Inject private DataStoreManager dataStoreMgr;
@Inject private DiskOfferingDao _diskOfferingDao;
+@Inject private ClusterDao clusterDao;
@Inject private HostDao _hostDao;
+@Inject private HostDetailsDao hostDetailsDao;
@Inject private ManagementService _mgr;
@Inject private PrimaryDataStoreDao _storagePoolDao;
@Inject private SnapshotDao _snapshotDao;
@Inject private SnapshotDetailsDao _snapshotDetailsDao;
@Inject private VolumeDao _volumeDao;
@Inject private VolumeDataFactory _volumeDataFactory;
+@Inject private VolumeDetailsDao volumeDetailsDao;
@Inject private VolumeService _volumeService;
@Override
public StrategyPriority canHandle(DataObject srcData, DataObject
destData) {
if (srcData instanceof SnapshotInfo) {
-if (canHandle(srcData.getDataStore()) ||
canHandle(destData.getDataStore())) {
+if (canHandle(srcData) || canHandle(destData)) {
return StrategyPriority.HIGHEST;
}
}
+if (srcData instanceof TemplateInfo && destData instanceof
VolumeInfo &&
+(srcData.getDataStore().getId() ==
destData.getDataStore().getId()) &&
+(canHandle(srcData) || canHandle(destData))) {
+// Both source and dest are on the same storage, so just clone
them.
+return StrategyPriority.HIGHEST;
+}
+
return StrategyPriority.CANT_HANDLE;
}
-private boolean canHandle(DataStore dataStore) {
+private boolean canHandle(DataObject dataObject) {
+Preconditions.checkArgument(dataObject != null, "Passing 'null' to
dataObject of canHandle(DataObject) is not supported.");
+
+DataStore dataStore = dataObject.getDataStore();
+
if (dataStore.getRole() == DataStoreRole.Primary) {
Map mapCapabilities =
dataStore.getDriver().getCapabilities();
-if (mapCapabilities != null) {
+if (mapCapabilities == null) {
+return false;
+}
+
+if (dataObject instanceof VolumeInfo || dataObject instanceof
SnapshotInfo) {
String value =
mapCapabilities.get(DataStoreCapabilities.STORAGE_SYSTEM_SNAPSHOT.toString());
-Boolean supportsStorageSystemSnapshots = new
Boolean(value);
+Boolean supportsStorageSystemSnapshots =
Boolean.valueOf(value);
if (supportsStorageSystemSnapshots) {
s_logger.info("Using
'StorageSystemDataMotionStrategy'");
return true;
}
+} else if (dataObject instanceof TemplateInfo) {
+// If the storage system can clone volumes, we can cache
templates on it.
+String value =
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63108616
--- Diff: engine/schema/src/com/cloud/dc/dao/ClusterDaoImpl.java ---
@@ -260,4 +268,41 @@ public boolean remove(Long id) {
sc.setParameters("dataCenterId", zoneId);
return customSearch(sc, null);
}
+
+@Override
+public boolean computeWhetherClusterSupportsResigning(long clusterId) {
+ClusterVO cluster = findById(clusterId);
+
+if (cluster == null || cluster.getAllocationState() !=
Grouping.AllocationState.Enabled) {
+return false;
+}
+
+List hosts = hostDao.findByClusterId(clusterId);
+
+if (hosts == null) {
+return false;
+}
+
+for (HostVO host : hosts) {
+if (host == null) {
+return false;
+}
+
+DetailVO hostDetail = hostDetailsDao.findDetail(host.getId(),
"supportsResign");
+
+if (hostDetail == null) {
+return false;
+}
+
+String value = hostDetail.getValue();
+
+Boolean booleanValue = Boolean.valueOf(value);
+
+if (booleanValue == false) {
+return false;
+}
+}
+
+return true;
+}
--- End diff --
The DAO calls in the ``for`` loop are concerning from a performance
perspective (both for the speed of this call and the load placed on the
database). Ideally, it would be calculated in a single join. Would it be
possible to craft a query across the details for all hosts in a cluster where
the detail key = "supportsResign" and the value = "true"?
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-218874072 @DaanHoogland I had to put the "Void" return types back. It is used for AOP and won't compile with "void" for those two methods. --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-218869567 @DaanHoogland I think I addressed all your concerns. I plan to rebuild this locally, then push to GitHub in a bit. --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-218869277 @DaanHoogland I agree we have some strange patterns in the codebase. For this feature, I simply followed those patterns when I saw them because I wasn't sure why they were coded that way (maybe for AOP?) and was afraid to deviate from it unless I better understood why it was the way it was in the first place (for example, some "Void" return types instead of "void" return types). I did make the changes you suggested, though, Daan. Hopefully there wasn't any good reason in the first place why some of those Void return types were in there. --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63086591
--- Diff:
engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
---
@@ -554,6 +574,51 @@ protected Void
managedCopyBaseImageCallback(AsyncCallbackDispatcher callback, CreateVolumeContext context) {
+CreateCmdResult result = callback.getResult();
+VolumeApiResult res = new VolumeApiResult(null);
+
+res.setResult(result.getResult());
+
+AsyncCallFuture future = context.getFuture();
+DataObject templateOnPrimaryStoreObj = context.getVolume();
+
+if (result.isSuccess()) {
+
((TemplateObject)templateOnPrimaryStoreObj).setInstallPath(result.getPath());
+
templateOnPrimaryStoreObj.processEvent(Event.OperationSuccessed,
result.getAnswer());
+}
+else {
+templateOnPrimaryStoreObj.processEvent(Event.OperationFailed);
+}
+
+future.complete(res);
+
+return null;
+}
+
+protected Void
copyManagedTemplateCallback(AsyncCallbackDispatcher callback, CreateBaseImageContext context) {
--- End diff --
@DaanHoogland I changed them back to the more traditional "void" return
type. I'm not sure why "Void" is being used for those kinds of methods in this
class (I was thinking maybe for AOP).
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63086019
--- Diff:
engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java
---
@@ -347,38 +395,87 @@ private String getProperty(long snapshotId, String
property) {
return null;
}
-private HostVO getHost(Long hostId, VolumeVO volumeVO) {
-HostVO hostVO = _hostDao.findById(hostId);
+private HostVO getHost(long volumeId) {
+VolumeVO volumeVO = _volumeDao.findById(volumeId);
+
+Long vmInstanceId = volumeVO.getInstanceId();
+VMInstanceVO vmInstanceVO = _vmInstanceDao.findById(vmInstanceId);
+
+Long hostId = null;
+
+// if the volume to snapshot is associated with a VM
+if (vmInstanceVO != null) {
+hostId = vmInstanceVO.getHostId();
+
+// if the VM is not associated with a host
+if (hostId == null) {
+hostId = vmInstanceVO.getLastHostId();
+}
+}
+
+return getHost(volumeVO.getDataCenterId(), hostId);
+}
+
+private HostVO getHost(long zoneId, Long hostId) {
+HostVO hostVO = getHost(zoneId, true);
if (hostVO != null) {
return hostVO;
}
-// pick a host in any XenServer cluster that's in the applicable
zone
+hostVO = _hostDao.findById(hostId);
-long zoneId = volumeVO.getDataCenterId();
+if (hostVO != null) {
+return hostVO;
+}
+
+hostVO = getHost(zoneId, false);
-List clusters = _mgr.searchForClusters(zoneId,
new Long(0), Long.MAX_VALUE, HypervisorType.XenServer.toString());
+if (hostVO != null) {
+return hostVO;
+}
+
+throw new CloudRuntimeException("Unable to locate an applicable
host");
+}
+
+private HostVO getHost(long zoneId, boolean
computeClusterMustSupportResign) {
+List clusters = _mgr.searchForClusters(zoneId,
0L, Long.MAX_VALUE, HypervisorType.XenServer.toString());
if (clusters == null) {
-throw new CloudRuntimeException("Unable to locate an
applicable cluster");
+clusters = new ArrayList<>();
}
+Collections.shuffle(clusters, new Random(System.nanoTime()));
+
+clusters:
for (Cluster cluster : clusters) {
if (cluster.getAllocationState() == AllocationState.Enabled) {
List hosts =
_hostDao.findByClusterId(cluster.getId());
if (hosts != null) {
+Collections.shuffle(hosts, new
Random(System.nanoTime()));
+
for (HostVO host : hosts) {
if (host.getResourceState() ==
ResourceState.Enabled) {
-return host;
+if (computeClusterMustSupportResign) {
+if
(_clusterDao.computeClusterSupportsResign(cluster.getId())) {
+return host;
+}
+else {
+// no other host in the cluster in
question should be able to satisfy our requirements here, so move on to the
next cluster
+continue clusters;
+}
+}
+else {
+return host;
+}
}
}
}
}
}
-throw new CloudRuntimeException("Unable to locate an applicable
cluster");
+return null;
--- End diff --
@DaanHoogland I changed this to make use of Guava's Optional class.
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63082964
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -255,99 +473,123 @@ private Void
handleCreateVolumeFromSnapshotBothOnStorageSystem(SnapshotInfo snap
VolumeApiResult result = future.get();
+if (volumeDetail != null) {
+_volumeDetailsDao.remove(volumeDetail.getId());
+}
+
if (result.isFailed()) {
-s_logger.debug("Failed to create a volume: " +
result.getResult());
+s_logger.warn("Failed to create a volume: " +
result.getResult());
throw new CloudRuntimeException(result.getResult());
}
-}
-catch (Exception ex) {
-throw new CloudRuntimeException(ex.getMessage());
-}
-volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(),
volumeInfo.getDataStore());
+volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(),
volumeInfo.getDataStore());
-volumeInfo.processEvent(Event.MigrationRequested);
+volumeInfo.processEvent(Event.MigrationRequested);
-volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(),
volumeInfo.getDataStore());
+volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(),
volumeInfo.getDataStore());
-HostVO hostVO = getHost(snapshotInfo.getDataStore().getId());
-
-String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
-int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
-CopyCommand copyCommand = new CopyCommand(snapshotInfo.getTO(),
volumeInfo.getTO(), primaryStorageDownloadWait,
VirtualMachineManager.ExecuteInSequence.value());
+if (useCloning) {
+copyCmdAnswer = performResignature(volumeInfo, hostVO);
+}
+else {
+// asking for a XenServer host here so we don't always
prefer to use XenServer hosts that support resigning
+// even when we don't need those hosts to do this kind of
copy work
+hostVO = getHost(snapshotInfo.getDataCenterId(), false);
-CopyCmdAnswer copyCmdAnswer = null;
+copyCmdAnswer = performCopyOfVdi(volumeInfo, snapshotInfo,
hostVO);
+}
-try {
-_volumeService.grantAccess(snapshotInfo, hostVO,
snapshotInfo.getDataStore());
-_volumeService.grantAccess(volumeInfo, hostVO,
volumeInfo.getDataStore());
+if (copyCmdAnswer == null || !copyCmdAnswer.getResult()) {
+if (copyCmdAnswer != null &&
!StringUtils.isEmpty(copyCmdAnswer.getDetails())) {
+errMsg = copyCmdAnswer.getDetails();
+}
+else {
+errMsg = "Unable to create volume from snapshot";
+}
+}
+}
+catch (Exception ex) {
+errMsg = ex.getMessage() != null ? ex.getMessage() : "Copy
operation failed";
+}
-Map srcDetails =
getSnapshotDetails(_storagePoolDao.findById(snapshotInfo.getDataStore().getId()),
snapshotInfo);
+CopyCommandResult result = new CopyCommandResult(null,
copyCmdAnswer);
-copyCommand.setOptions(srcDetails);
+result.setResult(errMsg);
-Map destDetails = getVolumeDetails(volumeInfo);
+callback.complete(result);
+}
-copyCommand.setOptions2(destDetails);
+/**
+ * If the underlying storage system is making use of read-only
snapshots, this gives the storage system the opportunity to
+ * create a volume from the snapshot so that we can copy the VHD file
that should be inside of the snapshot to secondary storage.
+ *
+ * The resultant volume must be writable because we need to resign the
SR and the VDI that should be inside of it before we copy
+ * the VHD file to secondary storage.
+ *
+ * If the storage system is using writable snapshots, then nothing
need be done by that storage system here because we can just
+ * resign the SR and the VDI that should be inside of the snapshot
before copying the VHD file to secondary storage.
+ */
+private void createVolumeFromSnapshot(HostVO hostVO, SnapshotInfo
snapshotInfo, boolean keepGrantedAccess) {
+SnapshotDetailsVO snapshotDetails
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-218865225 LGTM with one extra comment: This is adding a much wanted feature and not worsening the code base we have but some bad patterns are being maintained that we should divert from. Hope you agree @mike-tutkowski --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63083577
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -255,99 +473,123 @@ private Void
handleCreateVolumeFromSnapshotBothOnStorageSystem(SnapshotInfo snap
VolumeApiResult result = future.get();
+if (volumeDetail != null) {
+_volumeDetailsDao.remove(volumeDetail.getId());
+}
+
if (result.isFailed()) {
-s_logger.debug("Failed to create a volume: " +
result.getResult());
+s_logger.warn("Failed to create a volume: " +
result.getResult());
throw new CloudRuntimeException(result.getResult());
}
-}
-catch (Exception ex) {
-throw new CloudRuntimeException(ex.getMessage());
-}
-volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(),
volumeInfo.getDataStore());
+volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(),
volumeInfo.getDataStore());
-volumeInfo.processEvent(Event.MigrationRequested);
+volumeInfo.processEvent(Event.MigrationRequested);
-volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(),
volumeInfo.getDataStore());
+volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(),
volumeInfo.getDataStore());
-HostVO hostVO = getHost(snapshotInfo.getDataStore().getId());
-
-String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
-int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
-CopyCommand copyCommand = new CopyCommand(snapshotInfo.getTO(),
volumeInfo.getTO(), primaryStorageDownloadWait,
VirtualMachineManager.ExecuteInSequence.value());
+if (useCloning) {
+copyCmdAnswer = performResignature(volumeInfo, hostVO);
+}
+else {
+// asking for a XenServer host here so we don't always
prefer to use XenServer hosts that support resigning
+// even when we don't need those hosts to do this kind of
copy work
+hostVO = getHost(snapshotInfo.getDataCenterId(), false);
-CopyCmdAnswer copyCmdAnswer = null;
+copyCmdAnswer = performCopyOfVdi(volumeInfo, snapshotInfo,
hostVO);
+}
-try {
-_volumeService.grantAccess(snapshotInfo, hostVO,
snapshotInfo.getDataStore());
-_volumeService.grantAccess(volumeInfo, hostVO,
volumeInfo.getDataStore());
+if (copyCmdAnswer == null || !copyCmdAnswer.getResult()) {
+if (copyCmdAnswer != null &&
!StringUtils.isEmpty(copyCmdAnswer.getDetails())) {
+errMsg = copyCmdAnswer.getDetails();
+}
+else {
+errMsg = "Unable to create volume from snapshot";
+}
+}
+}
+catch (Exception ex) {
+errMsg = ex.getMessage() != null ? ex.getMessage() : "Copy
operation failed";
+}
-Map srcDetails =
getSnapshotDetails(_storagePoolDao.findById(snapshotInfo.getDataStore().getId()),
snapshotInfo);
+CopyCommandResult result = new CopyCommandResult(null,
copyCmdAnswer);
-copyCommand.setOptions(srcDetails);
+result.setResult(errMsg);
-Map destDetails = getVolumeDetails(volumeInfo);
+callback.complete(result);
+}
-copyCommand.setOptions2(destDetails);
+/**
+ * If the underlying storage system is making use of read-only
snapshots, this gives the storage system the opportunity to
+ * create a volume from the snapshot so that we can copy the VHD file
that should be inside of the snapshot to secondary storage.
+ *
+ * The resultant volume must be writable because we need to resign the
SR and the VDI that should be inside of it before we copy
+ * the VHD file to secondary storage.
+ *
+ * If the storage system is using writable snapshots, then nothing
need be done by that storage system here because we can just
+ * resign the SR and the VDI that should be inside of the snapshot
before copying the VHD file to secondary storage.
+ */
+private void createVolumeFromSnapshot(HostVO hostVO, SnapshotInfo
snapshotInfo, boolean keepGrantedAccess) {
+SnapshotDetailsVO snapshotDetails =
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63083834
--- Diff:
engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java
---
@@ -289,7 +291,7 @@ public boolean deleteSnapshot(Long snapshotId) {
@Override
public boolean revertSnapshot(SnapshotInfo snapshot) {
if (canHandle(snapshot,SnapshotOperation.REVERT) ==
StrategyPriority.CANT_HANDLE) {
-throw new UnsupportedOperationException("Reverting not
supported. Create a template or volume based on the snapshot instead.");
+throw new CloudRuntimeException("Reverting not supported.
Create a template or volume based on the snapshot instead.");
--- End diff --
@jburwell want to discuss?
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63083633
--- Diff:
engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java
---
@@ -289,7 +291,7 @@ public boolean deleteSnapshot(Long snapshotId) {
@Override
public boolean revertSnapshot(SnapshotInfo snapshot) {
if (canHandle(snapshot,SnapshotOperation.REVERT) ==
StrategyPriority.CANT_HANDLE) {
-throw new UnsupportedOperationException("Reverting not
supported. Create a template or volume based on the snapshot instead.");
+throw new CloudRuntimeException("Reverting not supported.
Create a template or volume based on the snapshot instead.");
--- End diff --
@DaanHoogland It was per John's suggestion.
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63083245
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -361,59 +603,221 @@ private String getProperty(long snapshotId, String
property) {
}
private Map getVolumeDetails(VolumeInfo volumeInfo) {
-Map sourceDetails = new HashMap();
+Map volumeDetails = new HashMap();
VolumeVO volumeVO = _volumeDao.findById(volumeInfo.getId());
long storagePoolId = volumeVO.getPoolId();
StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
-sourceDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
-sourceDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
-sourceDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
+volumeDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+volumeDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+volumeDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
ChapInfo chapInfo = _volumeService.getChapInfo(volumeInfo,
volumeInfo.getDataStore());
if (chapInfo != null) {
-sourceDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
-sourceDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
-sourceDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
-sourceDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
+volumeDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
+volumeDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
}
-return sourceDetails;
+return volumeDetails;
+}
+
+private Map getSnapshotDetails(SnapshotInfo
snapshotInfo) {
+Map snapshotDetails = new HashMap();
+
+long storagePoolId = snapshotInfo.getDataStore().getId();
+StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
+
+snapshotDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+snapshotDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+
+long snapshotId = snapshotInfo.getId();
+
+snapshotDetails.put(DiskTO.IQN, getProperty(snapshotId,
DiskTO.IQN));
+
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_SECRET));
+snapshotDetails.put(DiskTO.CHAP_TARGET_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_TARGET_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_TARGET_SECRET,
getProperty(snapshotId, DiskTO.CHAP_TARGET_SECRET));
+
+return snapshotDetails;
}
-public HostVO getHost(long dataStoreId) {
-StoragePoolVO storagePoolVO =
_storagePoolDao.findById(dataStoreId);
+private HostVO getHost(SnapshotInfo snapshotInfo) {
+HostVO hostVO = getHost(snapshotInfo.getDataCenterId(), true);
-List clusters =
_mgr.searchForClusters(storagePoolVO.getDataCenterId(), new Long(0),
Long.MAX_VALUE, HypervisorType.XenServer.toString());
+if (hostVO == null) {
+hostVO = getHost(snapshotInfo.getDataCenterId(), false);
-if (clusters == null) {
-throw new CloudRuntimeException("Unable to locate an
applicable cluster");
+if (hostVO == null) {
+throw new CloudRuntimeException("Unable to locate an
applicable host in data center with ID = " + snapshotInfo.getDataCenterId());
+}
}
-for (Cluster cluster : clusters) {
-if (cluster.getAllocationState() == AllocationState.Enabled) {
-List hosts =
_hostDao.findByClusterId(cluster.getId());
+return hostVO;
+}
-if (hosts != null) {
-for (HostVO host : hosts) {
-if (host.getResourceState() ==
ResourceState.Enabled) {
-return host;
-
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63083184
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -361,59 +603,221 @@ private String getProperty(long snapshotId, String
property) {
}
private Map getVolumeDetails(VolumeInfo volumeInfo) {
-Map sourceDetails = new HashMap();
+Map volumeDetails = new HashMap();
VolumeVO volumeVO = _volumeDao.findById(volumeInfo.getId());
long storagePoolId = volumeVO.getPoolId();
StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
-sourceDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
-sourceDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
-sourceDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
+volumeDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+volumeDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+volumeDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
ChapInfo chapInfo = _volumeService.getChapInfo(volumeInfo,
volumeInfo.getDataStore());
if (chapInfo != null) {
-sourceDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
-sourceDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
-sourceDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
-sourceDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
+volumeDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
+volumeDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
}
-return sourceDetails;
+return volumeDetails;
+}
+
+private Map getSnapshotDetails(SnapshotInfo
snapshotInfo) {
+Map snapshotDetails = new HashMap();
+
+long storagePoolId = snapshotInfo.getDataStore().getId();
+StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
+
+snapshotDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+snapshotDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+
+long snapshotId = snapshotInfo.getId();
+
+snapshotDetails.put(DiskTO.IQN, getProperty(snapshotId,
DiskTO.IQN));
+
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_SECRET));
+snapshotDetails.put(DiskTO.CHAP_TARGET_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_TARGET_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_TARGET_SECRET,
getProperty(snapshotId, DiskTO.CHAP_TARGET_SECRET));
+
+return snapshotDetails;
}
-public HostVO getHost(long dataStoreId) {
-StoragePoolVO storagePoolVO =
_storagePoolDao.findById(dataStoreId);
+private HostVO getHost(SnapshotInfo snapshotInfo) {
+HostVO hostVO = getHost(snapshotInfo.getDataCenterId(), true);
-List clusters =
_mgr.searchForClusters(storagePoolVO.getDataCenterId(), new Long(0),
Long.MAX_VALUE, HypervisorType.XenServer.toString());
+if (hostVO == null) {
+hostVO = getHost(snapshotInfo.getDataCenterId(), false);
-if (clusters == null) {
-throw new CloudRuntimeException("Unable to locate an
applicable cluster");
+if (hostVO == null) {
+throw new CloudRuntimeException("Unable to locate an
applicable host in data center with ID = " + snapshotInfo.getDataCenterId());
+}
}
-for (Cluster cluster : clusters) {
-if (cluster.getAllocationState() == AllocationState.Enabled) {
-List hosts =
_hostDao.findByClusterId(cluster.getId());
+return hostVO;
+}
-if (hosts != null) {
-for (HostVO host : hosts) {
-if (host.getResourceState() ==
ResourceState.Enabled) {
-return host;
-
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63082184
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -172,78 +232,211 @@ private void validate(SnapshotInfo snapshotInfo) {
}
}
-private Void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
+private boolean usingBackendSnapshotFor(SnapshotInfo snapshotInfo) {
+String property = getProperty(snapshotInfo.getId(),
"takeSnapshot");
+
+return Boolean.parseBoolean(property);
+}
+
+private void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
try {
snapshotInfo.processEvent(Event.CopyingRequested);
}
catch (Exception ex) {
throw new CloudRuntimeException("This snapshot is not
currently in a state where it can be used to create a template.");
}
-HostVO hostVO = getHost(snapshotInfo.getDataStore().getId());
-DataStore srcDataStore = snapshotInfo.getDataStore();
-
-String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
-int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
-CopyCommand copyCommand = new CopyCommand(snapshotInfo.getTO(),
templateInfo.getTO(), primaryStorageDownloadWait,
VirtualMachineManager.ExecuteInSequence.value());
+HostVO hostVO = getHost(snapshotInfo);
-String errMsg = null;
+boolean usingBackendSnapshot =
usingBackendSnapshotFor(snapshotInfo);
+boolean computeClusterSupportsResign =
computeClusterSupportsResign(hostVO.getClusterId());
-CopyCmdAnswer copyCmdAnswer = null;
+if (usingBackendSnapshot && !computeClusterSupportsResign) {
+throw new CloudRuntimeException("Unable to locate an
applicable host with which to perform a resignature operation");
+}
try {
-_volumeService.grantAccess(snapshotInfo, hostVO, srcDataStore);
+if (usingBackendSnapshot) {
+createVolumeFromSnapshot(hostVO, snapshotInfo, true);
+}
-Map srcDetails =
getSnapshotDetails(_storagePoolDao.findById(srcDataStore.getId()),
snapshotInfo);
+DataStore srcDataStore = snapshotInfo.getDataStore();
-copyCommand.setOptions(srcDetails);
+String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
+int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
+CopyCommand copyCommand = new
CopyCommand(snapshotInfo.getTO(), templateInfo.getTO(),
primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value());
+
+String errMsg = null;
+
+CopyCmdAnswer copyCmdAnswer = null;
-copyCmdAnswer = (CopyCmdAnswer)_agentMgr.send(hostVO.getId(),
copyCommand);
-}
-catch (Exception ex) {
-throw new CloudRuntimeException(ex.getMessage());
-}
-finally {
try {
-_volumeService.revokeAccess(snapshotInfo, hostVO,
srcDataStore);
+// If we are using a back-end snapshot, then we should
still have access to it from the hosts in the cluster that hostVO is in
+// (because we passed in true as the third parameter to
createVolumeFromSnapshot above).
+if (usingBackendSnapshot == false) {
+_volumeService.grantAccess(snapshotInfo, hostVO,
srcDataStore);
+}
+
+Map srcDetails =
getSnapshotDetails(snapshotInfo);
+
+copyCommand.setOptions(srcDetails);
+
+copyCmdAnswer =
(CopyCmdAnswer)_agentMgr.send(hostVO.getId(), copyCommand);
}
catch (Exception ex) {
-s_logger.debug(ex.getMessage(), ex);
+throw new CloudRuntimeException(ex.getMessage());
}
+finally {
+try {
+_volumeService.revokeAccess(snapshotInfo, hostVO,
srcDataStore);
+}
+catch (Exception ex) {
+s_logger.debug(ex.getMessage(), ex);
+}
-if (copyCmdAnswer ==
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63082200
--- Diff: engine/schema/src/com/cloud/dc/dao/ClusterDaoImpl.java ---
@@ -260,4 +268,41 @@ public boolean remove(Long id) {
sc.setParameters("dataCenterId", zoneId);
return customSearch(sc, null);
}
+
+@Override
+public boolean computeClusterSupportsResign(long clusterId) {
--- End diff --
@DaanHoogland Done
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63080850
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -172,78 +232,211 @@ private void validate(SnapshotInfo snapshotInfo) {
}
}
-private Void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
+private boolean usingBackendSnapshotFor(SnapshotInfo snapshotInfo) {
+String property = getProperty(snapshotInfo.getId(),
"takeSnapshot");
+
+return Boolean.parseBoolean(property);
+}
+
+private void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
try {
snapshotInfo.processEvent(Event.CopyingRequested);
}
catch (Exception ex) {
throw new CloudRuntimeException("This snapshot is not
currently in a state where it can be used to create a template.");
}
-HostVO hostVO = getHost(snapshotInfo.getDataStore().getId());
-DataStore srcDataStore = snapshotInfo.getDataStore();
-
-String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
-int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
-CopyCommand copyCommand = new CopyCommand(snapshotInfo.getTO(),
templateInfo.getTO(), primaryStorageDownloadWait,
VirtualMachineManager.ExecuteInSequence.value());
+HostVO hostVO = getHost(snapshotInfo);
-String errMsg = null;
+boolean usingBackendSnapshot =
usingBackendSnapshotFor(snapshotInfo);
+boolean computeClusterSupportsResign =
computeClusterSupportsResign(hostVO.getClusterId());
-CopyCmdAnswer copyCmdAnswer = null;
+if (usingBackendSnapshot && !computeClusterSupportsResign) {
+throw new CloudRuntimeException("Unable to locate an
applicable host with which to perform a resignature operation");
+}
try {
-_volumeService.grantAccess(snapshotInfo, hostVO, srcDataStore);
+if (usingBackendSnapshot) {
+createVolumeFromSnapshot(hostVO, snapshotInfo, true);
+}
-Map srcDetails =
getSnapshotDetails(_storagePoolDao.findById(srcDataStore.getId()),
snapshotInfo);
+DataStore srcDataStore = snapshotInfo.getDataStore();
-copyCommand.setOptions(srcDetails);
+String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
+int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
+CopyCommand copyCommand = new
CopyCommand(snapshotInfo.getTO(), templateInfo.getTO(),
primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value());
+
+String errMsg = null;
+
+CopyCmdAnswer copyCmdAnswer = null;
-copyCmdAnswer = (CopyCmdAnswer)_agentMgr.send(hostVO.getId(),
copyCommand);
-}
-catch (Exception ex) {
-throw new CloudRuntimeException(ex.getMessage());
-}
-finally {
try {
-_volumeService.revokeAccess(snapshotInfo, hostVO,
srcDataStore);
+// If we are using a back-end snapshot, then we should
still have access to it from the hosts in the cluster that hostVO is in
+// (because we passed in true as the third parameter to
createVolumeFromSnapshot above).
+if (usingBackendSnapshot == false) {
+_volumeService.grantAccess(snapshotInfo, hostVO,
srcDataStore);
+}
+
+Map srcDetails =
getSnapshotDetails(snapshotInfo);
+
+copyCommand.setOptions(srcDetails);
+
+copyCmdAnswer =
(CopyCmdAnswer)_agentMgr.send(hostVO.getId(), copyCommand);
}
catch (Exception ex) {
-s_logger.debug(ex.getMessage(), ex);
+throw new CloudRuntimeException(ex.getMessage());
}
+finally {
+try {
+_volumeService.revokeAccess(snapshotInfo, hostVO,
srcDataStore);
+}
+catch (Exception ex) {
+s_logger.debug(ex.getMessage(), ex);
+}
-if (copyCmdAnswer ==
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63079362
--- Diff:
engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
---
@@ -554,6 +574,51 @@ protected Void
managedCopyBaseImageCallback(AsyncCallbackDispatcher callback, CreateVolumeContext context) {
+CreateCmdResult result = callback.getResult();
+VolumeApiResult res = new VolumeApiResult(null);
+
+res.setResult(result.getResult());
+
+AsyncCallFuture future = context.getFuture();
+DataObject templateOnPrimaryStoreObj = context.getVolume();
+
+if (result.isSuccess()) {
+
((TemplateObject)templateOnPrimaryStoreObj).setInstallPath(result.getPath());
+
templateOnPrimaryStoreObj.processEvent(Event.OperationSuccessed,
result.getAnswer());
+}
+else {
+templateOnPrimaryStoreObj.processEvent(Event.OperationFailed);
+}
+
+future.complete(res);
+
+return null;
+}
+
+protected Void
copyManagedTemplateCallback(AsyncCallbackDispatcher callback, CreateBaseImageContext context) {
--- End diff --
Actually, @DaanHoogland, I was thinking of a different time when I
corrected an existing "Void" problem like this.
The reason I wrote it this way was to conform to existing code style in the
file (which was using Void (instead of void) for these types of methods and I
wasn't sure why, so I followed the pattern to be on the safe side).
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r63078617
--- Diff:
engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
---
@@ -554,6 +574,51 @@ protected Void
managedCopyBaseImageCallback(AsyncCallbackDispatcher callback, CreateVolumeContext context) {
+CreateCmdResult result = callback.getResult();
+VolumeApiResult res = new VolumeApiResult(null);
+
+res.setResult(result.getResult());
+
+AsyncCallFuture future = context.getFuture();
+DataObject templateOnPrimaryStoreObj = context.getVolume();
+
+if (result.isSuccess()) {
+
((TemplateObject)templateOnPrimaryStoreObj).setInstallPath(result.getPath());
+
templateOnPrimaryStoreObj.processEvent(Event.OperationSuccessed,
result.getAnswer());
+}
+else {
+templateOnPrimaryStoreObj.processEvent(Event.OperationFailed);
+}
+
+future.complete(res);
+
+return null;
+}
+
+protected Void
copyManagedTemplateCallback(AsyncCallbackDispatcher callback, CreateBaseImageContext context) {
--- End diff --
Hi @DaanHoogland This is a legacy problem and I corrected it in #816. Now
that #816 has been merged and I rebased against master, I have the updated code.
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-218851513 @mike-tutkowski I don't mind ignoring most of the remaining comments I made except for - the ones about returning null. - the renamed exception, the old exception type seems more appropriate --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1403#discussion_r63071868 --- Diff: server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java --- @@ -144,6 +145,8 @@ @Inject SnapshotDataStoreDao _snapshotStoreDao; @Inject +private DataStoreManager _dataStoreMgr; --- End diff -- Thanks for finding that, @DaanHoogland! I have corrected it and I will rebase against master now for @swill. --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-218805790 @mike-tutkowski can you rebase this PR as we now have merge conflicts. Can you review the recent comments and I will get this queued up to get CI run against it as soon as you have an updated version to make sure we are still good. Thanks... --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-218540891 Did a review, I have some remarks, some questions as well. --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1403#discussion_r62892495 --- Diff: server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java --- @@ -144,6 +145,8 @@ @Inject SnapshotDataStoreDao _snapshotStoreDao; @Inject +private DataStoreManager _dataStoreMgr; --- End diff -- there is already a dataStoreMgr. why are two needed? (also underscore names are against naming convention) --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user jburwell commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1403#discussion_r62892410 --- Diff: core/src/com/cloud/agent/api/StartupRoutingCommand.java --- @@ -35,7 +35,7 @@ long memory; long dom0MinMemory; boolean poolSync; - +private boolean _supportsClonedVolumes; --- End diff -- +1 as well. Ridding of odd Hungarian notation and "_" would be a very things for the codebase. --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r62892193
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -172,78 +232,211 @@ private void validate(SnapshotInfo snapshotInfo) {
}
}
-private Void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
+private boolean usingBackendSnapshotFor(SnapshotInfo snapshotInfo) {
+String property = getProperty(snapshotInfo.getId(),
"takeSnapshot");
+
+return Boolean.parseBoolean(property);
+}
+
+private void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
try {
snapshotInfo.processEvent(Event.CopyingRequested);
}
catch (Exception ex) {
throw new CloudRuntimeException("This snapshot is not
currently in a state where it can be used to create a template.");
}
-HostVO hostVO = getHost(snapshotInfo.getDataStore().getId());
-DataStore srcDataStore = snapshotInfo.getDataStore();
-
-String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
-int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
-CopyCommand copyCommand = new CopyCommand(snapshotInfo.getTO(),
templateInfo.getTO(), primaryStorageDownloadWait,
VirtualMachineManager.ExecuteInSequence.value());
+HostVO hostVO = getHost(snapshotInfo);
-String errMsg = null;
+boolean usingBackendSnapshot =
usingBackendSnapshotFor(snapshotInfo);
+boolean computeClusterSupportsResign =
computeClusterSupportsResign(hostVO.getClusterId());
-CopyCmdAnswer copyCmdAnswer = null;
+if (usingBackendSnapshot && !computeClusterSupportsResign) {
+throw new CloudRuntimeException("Unable to locate an
applicable host with which to perform a resignature operation");
+}
try {
-_volumeService.grantAccess(snapshotInfo, hostVO, srcDataStore);
+if (usingBackendSnapshot) {
+createVolumeFromSnapshot(hostVO, snapshotInfo, true);
+}
-Map srcDetails =
getSnapshotDetails(_storagePoolDao.findById(srcDataStore.getId()),
snapshotInfo);
+DataStore srcDataStore = snapshotInfo.getDataStore();
-copyCommand.setOptions(srcDetails);
+String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
+int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
+CopyCommand copyCommand = new
CopyCommand(snapshotInfo.getTO(), templateInfo.getTO(),
primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value());
+
+String errMsg = null;
+
+CopyCmdAnswer copyCmdAnswer = null;
-copyCmdAnswer = (CopyCmdAnswer)_agentMgr.send(hostVO.getId(),
copyCommand);
-}
-catch (Exception ex) {
-throw new CloudRuntimeException(ex.getMessage());
-}
-finally {
try {
-_volumeService.revokeAccess(snapshotInfo, hostVO,
srcDataStore);
+// If we are using a back-end snapshot, then we should
still have access to it from the hosts in the cluster that hostVO is in
+// (because we passed in true as the third parameter to
createVolumeFromSnapshot above).
+if (usingBackendSnapshot == false) {
+_volumeService.grantAccess(snapshotInfo, hostVO,
srcDataStore);
+}
+
+Map srcDetails =
getSnapshotDetails(snapshotInfo);
+
+copyCommand.setOptions(srcDetails);
+
+copyCmdAnswer =
(CopyCmdAnswer)_agentMgr.send(hostVO.getId(), copyCommand);
}
catch (Exception ex) {
-s_logger.debug(ex.getMessage(), ex);
+throw new CloudRuntimeException(ex.getMessage());
}
+finally {
+try {
+_volumeService.revokeAccess(snapshotInfo, hostVO,
srcDataStore);
+}
+catch (Exception ex) {
+s_logger.debug(ex.getMessage(), ex);
+}
-if (copyCmdAnswer == null
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1403#discussion_r62891804 --- Diff: server/src/com/cloud/storage/StorageManagerImpl.java --- @@ -209,6 +211,8 @@ @Inject protected HostDao _hostDao; @Inject +private HostDetailsDao _hostDetailsDao; --- End diff -- unused --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r62890825
--- Diff:
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidFirePrimaryDataStoreDriver.java
---
@@ -692,12 +1145,38 @@ private String deleteSnapshot(SnapshotInfo
snapshotInfo, long storagePoolId) {
_storagePoolDao.update(storagePoolId, storagePool);
}
catch (Exception ex) {
-s_logger.debug(SolidFireUtil.LOG_PREFIX + "Failed to delete
SolidFire volume. CloudStack snapshot ID: " + snapshotId, ex);
+s_logger.debug(SolidFireUtil.LOG_PREFIX + "Issue in
'deleteSnapshot(SnapshotInfo, long)'. CloudStack snapshot ID: " + csSnapshotId,
ex);
-errMsg = ex.getMessage();
+throw ex;
}
+}
+
+private void deleteTemplate(TemplateInfo template, long storagePoolId)
{
+try {
+SolidFireUtil.SolidFireConnection sfConnection =
SolidFireUtil.getSolidFireConnection(storagePoolId, _storagePoolDetailsDao);
+
+long sfTemplateVolumeId =
getVolumeIdFrom_iScsiPath(template.getInstallPath());
-return errMsg;
+SolidFireUtil.deleteSolidFireVolume(sfConnection,
sfTemplateVolumeId);
+
+VMTemplateStoragePoolVO templatePoolRef =
_tmpltPoolDao.findByPoolTemplate(storagePoolId, template.getId());
+
+_tmpltPoolDao.remove(templatePoolRef.getId());
+
+StoragePoolVO storagePool =
_storagePoolDao.findById(storagePoolId);
+
+// getUsedBytes(StoragePool) will not include the template to
delete because the "template_spool_ref" table has already been updated by this
point
+long usedBytes = getUsedBytes(storagePool);
+
+storagePool.setUsedBytes(usedBytes < 0 ? 0 : usedBytes);
+
+_storagePoolDao.update(storagePoolId, storagePool);
+}
+catch (Exception ex) {
--- End diff --
???
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r62890766
--- Diff:
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidFirePrimaryDataStoreDriver.java
---
@@ -665,22 +950,190 @@ private void updateSnapshotDetails(long
csSnapshotId, long sfNewVolumeId, long s
_snapshotDetailsDao.persist(snapshotDetail);
}
-// return null for no error message
-private String deleteSnapshot(SnapshotInfo snapshotInfo, long
storagePoolId) {
-String errMsg = null;
+private String createVolume(VolumeInfo volumeInfo, long storagePoolId)
{
+verifySufficientBytesForStoragePool(volumeInfo, storagePoolId);
+verifySufficientIopsForStoragePool(volumeInfo.getMinIops() != null
? volumeInfo.getMinIops() : getDefaultMinIops(storagePoolId), storagePoolId);
+
+SolidFireUtil.SolidFireConnection sfConnection =
SolidFireUtil.getSolidFireConnection(storagePoolId, _storagePoolDetailsDao);
+
+long sfAccountId = getCreateSolidFireAccountId(sfConnection,
volumeInfo.getAccountId(), storagePoolId);
+
+long csSnapshotId = getCsIdForCloning(volumeInfo.getId(),
"cloneOfSnapshot");
+long csTemplateId = getCsIdForCloning(volumeInfo.getId(),
"cloneOfTemplate");
+
+SolidFireUtil.SolidFireVolume sfVolume;
+
+if (csSnapshotId > 0) {
+// We are supposed to create a clone of the underlying volume
or snapshot that supports the CloudStack snapshot.
+sfVolume = createClone(sfConnection, csSnapshotId, volumeInfo,
sfAccountId, storagePoolId, DataObjectType.SNAPSHOT);
+} else if (csTemplateId > 0) {
+// Clone from template.
+sfVolume = createClone(sfConnection, csTemplateId, volumeInfo,
sfAccountId, storagePoolId, DataObjectType.TEMPLATE);
+
+long volumeSize =
getDataObjectSizeIncludingHypervisorSnapshotReserve(volumeInfo,
_storagePoolDao.findById(storagePoolId));
+
+if (volumeSize > sfVolume.getTotalSize()) {
+// Expand the volume to include HSR.
+SolidFireUtil.modifySolidFireVolume(sfConnection,
sfVolume.getId(), volumeSize, getVolumeAttributes(volumeInfo),
+sfVolume.getMinIops(), sfVolume.getMaxIops(),
sfVolume.getBurstIops());
+
+// Get the SolidFire volume from the SAN again because we
just updated its size.
+sfVolume = SolidFireUtil.getSolidFireVolume(sfConnection,
sfVolume.getId());
+}
+}
+else {
+sfVolume = createSolidFireVolume(sfConnection, volumeInfo,
sfAccountId);
+}
+
+String iqn = sfVolume.getIqn();
+
+VolumeVO volume = _volumeDao.findById(volumeInfo.getId());
+
+volume.set_iScsiName(iqn);
+volume.setFolder(String.valueOf(sfVolume.getId()));
+volume.setPoolType(StoragePoolType.IscsiLUN);
+volume.setPoolId(storagePoolId);
+
+_volumeDao.update(volume.getId(), volume);
+
+updateVolumeDetails(volume.getId(), sfVolume.getTotalSize());
+
+StoragePoolVO storagePool =
_storagePoolDao.findById(storagePoolId);
+
+long capacityBytes = storagePool.getCapacityBytes();
+// getUsedBytes(StoragePool) will include the bytes of the newly
created volume because
+// updateVolumeDetails(long, long) has already been called for
this volume
+long usedBytes = getUsedBytes(storagePool);
+
+storagePool.setUsedBytes(usedBytes > capacityBytes ? capacityBytes
: usedBytes);
+
+_storagePoolDao.update(storagePoolId, storagePool);
+
+return iqn;
+}
+
+private void createTempVolume(SnapshotInfo snapshotInfo, long
storagePoolId) {
+long csSnapshotId = snapshotInfo.getId();
+
+SnapshotDetailsVO snapshotDetails =
_snapshotDetailsDao.findDetail(csSnapshotId, SolidFireUtil.SNAPSHOT_ID);
+
+if (snapshotDetails == null || snapshotDetails.getValue() == null)
{
+throw new
CloudRuntimeException("'createTempVolume(SnapshotInfo, long)' should not be
invoked unless " + SolidFireUtil.SNAPSHOT_ID + " exists.");
+}
+
+SolidFireUtil.SolidFireConnection sfConnection =
SolidFireUtil.getSolidFireConnection(storagePoolId, _storagePoolDetailsDao);
+
+snapshotDetails = _snapshotDetailsDao.findDetail(csSnapshotId,
"tempVolume");
+
+if (snapshotDetails != null && snapshotDetails.getValue() != null
&& snapshotDetails.getValue().equalsIgnoreCase("create")) {
+long sfAccountId =
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r62886608
--- Diff:
plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java
---
@@ -2351,17 +2376,52 @@ public SR getIscsiSR(final Connection conn, final
String srNameLabel, final Stri
throw new CloudRuntimeException(msg, e);
}
}
+
deviceConfig.put("SCSIid", scsiid);
-final String result = SR.probe(conn, host, deviceConfig,
type, smConfig);
+String result = SR.probe(conn, host, deviceConfig, type,
smConfig);
+
String pooluuid = null;
+
if (result.indexOf("") != -1) {
pooluuid = result.substring(result.indexOf("") +
6, result.indexOf("")).trim();
}
if (pooluuid == null || pooluuid.length() != 36) {
sr = SR.create(conn, host, deviceConfig, new Long(0),
srNameLabel, srNameLabel, type, "user", true, smConfig);
-} else {
+}
+else {
+if (resignature) {
--- End diff --
maybe a method createIfResignable()? thsi file is a blob of blobs, let's
get to reducing the problem called CitrixResourceBase
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r62886063
--- Diff:
plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java
---
@@ -1792,10 +1796,26 @@ protected void fillHostInfo(final Connection conn,
final StartupRoutingCommand c
cmd.setPod(_pod);
cmd.setVersion(CitrixResourceBase.class.getPackage().getImplementationVersion());
+try {
+final String cmdLine = "xe sm-list | grep \"resigning of
duplicates\"";
+
+final XenServerUtilitiesHelper xenServerUtilitiesHelper =
getXenServerUtilitiesHelper();
+
+Pair result =
xenServerUtilitiesHelper.executeSshWrapper(_host.getIp(), 22, _username, null,
getPwdFromQueue(), cmdLine);
+
+boolean supportsClonedVolumes = result != null &&
result.first() != null && result.first() &&
+result.second() != null &&
result.second().length() > 0;
+
+cmd.setSupportsClonedVolumes(supportsClonedVolumes);
+} catch (Exception ex) {
--- End diff --
no checked exceptions are being thrown, why a catch all? is there a
specific exception that might threaten us?
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r62885313
--- Diff:
plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java
---
@@ -164,9 +165,12 @@
*
*/
public abstract class CitrixResourceBase implements ServerResource,
HypervisorResource, VirtualRouterDeployer {
-
+/**
+ * RELVMOISCSI = used for resigning metadata (like SR UUID and VDI
UUID when a
+ * particular storage manager is installed on a XenServer host (for
back-end snapshots to work))
+ */
--- End diff --
I would put the value javadoc on the value and describe only the whole enum
here. (PR coming to that end on another enum, shortly)
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r62881271
--- Diff:
engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
---
@@ -554,6 +574,51 @@ protected Void
managedCopyBaseImageCallback(AsyncCallbackDispatcher callback, CreateVolumeContext context) {
+CreateCmdResult result = callback.getResult();
+VolumeApiResult res = new VolumeApiResult(null);
+
+res.setResult(result.getResult());
+
+AsyncCallFuture future = context.getFuture();
+DataObject templateOnPrimaryStoreObj = context.getVolume();
+
+if (result.isSuccess()) {
+
((TemplateObject)templateOnPrimaryStoreObj).setInstallPath(result.getPath());
+
templateOnPrimaryStoreObj.processEvent(Event.OperationSuccessed,
result.getAnswer());
+}
+else {
+templateOnPrimaryStoreObj.processEvent(Event.OperationFailed);
+}
+
+future.complete(res);
+
+return null;
+}
+
+protected Void
copyManagedTemplateCallback(AsyncCallbackDispatcher callback, CreateBaseImageContext context) {
--- End diff --
same here, why Void and returning a null? returning null is a bad smell to
me.
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r62881152
--- Diff:
engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
---
@@ -554,6 +574,51 @@ protected Void
managedCopyBaseImageCallback(AsyncCallbackDispatcher callback, CreateVolumeContext context) {
+CreateCmdResult result = callback.getResult();
+VolumeApiResult res = new VolumeApiResult(null);
+
+res.setResult(result.getResult());
+
+AsyncCallFuture future = context.getFuture();
+DataObject templateOnPrimaryStoreObj = context.getVolume();
+
+if (result.isSuccess()) {
+
((TemplateObject)templateOnPrimaryStoreObj).setInstallPath(result.getPath());
+
templateOnPrimaryStoreObj.processEvent(Event.OperationSuccessed,
result.getAnswer());
+}
+else {
+templateOnPrimaryStoreObj.processEvent(Event.OperationFailed);
+}
+
+future.complete(res);
+
+return null;
--- End diff --
why Void and return null instead of void and return?
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1403#discussion_r62880956 --- Diff: engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java --- @@ -134,6 +146,14 @@ EndPointSelector _epSelector; @Inject HostDao _hostDao; +@Inject +private PrimaryDataStoreDao _storagePoolDao; --- End diff -- not used --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1403#discussion_r62880949 --- Diff: engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java --- @@ -134,6 +146,14 @@ EndPointSelector _epSelector; @Inject HostDao _hostDao; +@Inject +private PrimaryDataStoreDao _storagePoolDao; +@Inject +private HostDetailsDao _hostDetailsDao; --- End diff -- not used --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r62880464
--- Diff:
engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java
---
@@ -289,7 +291,7 @@ public boolean deleteSnapshot(Long snapshotId) {
@Override
public boolean revertSnapshot(SnapshotInfo snapshot) {
if (canHandle(snapshot,SnapshotOperation.REVERT) ==
StrategyPriority.CANT_HANDLE) {
-throw new UnsupportedOperationException("Reverting not
supported. Create a template or volume based on the snapshot instead.");
+throw new CloudRuntimeException("Reverting not supported.
Create a template or volume based on the snapshot instead.");
--- End diff --
why change the type of the exception?
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r62880347
--- Diff:
engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java
---
@@ -347,38 +395,87 @@ private String getProperty(long snapshotId, String
property) {
return null;
}
-private HostVO getHost(Long hostId, VolumeVO volumeVO) {
-HostVO hostVO = _hostDao.findById(hostId);
+private HostVO getHost(long volumeId) {
+VolumeVO volumeVO = _volumeDao.findById(volumeId);
+
+Long vmInstanceId = volumeVO.getInstanceId();
+VMInstanceVO vmInstanceVO = _vmInstanceDao.findById(vmInstanceId);
+
+Long hostId = null;
+
+// if the volume to snapshot is associated with a VM
+if (vmInstanceVO != null) {
+hostId = vmInstanceVO.getHostId();
+
+// if the VM is not associated with a host
+if (hostId == null) {
+hostId = vmInstanceVO.getLastHostId();
+}
+}
+
+return getHost(volumeVO.getDataCenterId(), hostId);
+}
+
+private HostVO getHost(long zoneId, Long hostId) {
+HostVO hostVO = getHost(zoneId, true);
if (hostVO != null) {
return hostVO;
}
-// pick a host in any XenServer cluster that's in the applicable
zone
+hostVO = _hostDao.findById(hostId);
-long zoneId = volumeVO.getDataCenterId();
+if (hostVO != null) {
+return hostVO;
+}
+
+hostVO = getHost(zoneId, false);
-List clusters = _mgr.searchForClusters(zoneId,
new Long(0), Long.MAX_VALUE, HypervisorType.XenServer.toString());
+if (hostVO != null) {
+return hostVO;
+}
+
+throw new CloudRuntimeException("Unable to locate an applicable
host");
+}
+
+private HostVO getHost(long zoneId, boolean
computeClusterMustSupportResign) {
+List clusters = _mgr.searchForClusters(zoneId,
0L, Long.MAX_VALUE, HypervisorType.XenServer.toString());
if (clusters == null) {
-throw new CloudRuntimeException("Unable to locate an
applicable cluster");
+clusters = new ArrayList<>();
}
+Collections.shuffle(clusters, new Random(System.nanoTime()));
+
+clusters:
for (Cluster cluster : clusters) {
if (cluster.getAllocationState() == AllocationState.Enabled) {
List hosts =
_hostDao.findByClusterId(cluster.getId());
if (hosts != null) {
+Collections.shuffle(hosts, new
Random(System.nanoTime()));
+
for (HostVO host : hosts) {
if (host.getResourceState() ==
ResourceState.Enabled) {
-return host;
+if (computeClusterMustSupportResign) {
+if
(_clusterDao.computeClusterSupportsResign(cluster.getId())) {
+return host;
+}
+else {
+// no other host in the cluster in
question should be able to satisfy our requirements here, so move on to the
next cluster
+continue clusters;
+}
+}
+else {
+return host;
+}
}
}
}
}
}
-throw new CloudRuntimeException("Unable to locate an applicable
cluster");
+return null;
--- End diff --
me no like, maybe a local private checked exception to have it handled in
the top getHost method?
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r62879575
--- Diff:
engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java
---
@@ -347,38 +395,87 @@ private String getProperty(long snapshotId, String
property) {
return null;
}
-private HostVO getHost(Long hostId, VolumeVO volumeVO) {
-HostVO hostVO = _hostDao.findById(hostId);
+private HostVO getHost(long volumeId) {
+VolumeVO volumeVO = _volumeDao.findById(volumeId);
+
+Long vmInstanceId = volumeVO.getInstanceId();
+VMInstanceVO vmInstanceVO = _vmInstanceDao.findById(vmInstanceId);
+
+Long hostId = null;
+
+// if the volume to snapshot is associated with a VM
+if (vmInstanceVO != null) {
+hostId = vmInstanceVO.getHostId();
+
+// if the VM is not associated with a host
+if (hostId == null) {
+hostId = vmInstanceVO.getLastHostId();
+}
+}
+
+return getHost(volumeVO.getDataCenterId(), hostId);
+}
+
+private HostVO getHost(long zoneId, Long hostId) {
+HostVO hostVO = getHost(zoneId, true);
if (hostVO != null) {
return hostVO;
}
-// pick a host in any XenServer cluster that's in the applicable
zone
+hostVO = _hostDao.findById(hostId);
-long zoneId = volumeVO.getDataCenterId();
+if (hostVO != null) {
+return hostVO;
+}
+
+hostVO = getHost(zoneId, false);
-List clusters = _mgr.searchForClusters(zoneId,
new Long(0), Long.MAX_VALUE, HypervisorType.XenServer.toString());
+if (hostVO != null) {
+return hostVO;
+}
+
+throw new CloudRuntimeException("Unable to locate an applicable
host");
+}
+
+private HostVO getHost(long zoneId, boolean
computeClusterMustSupportResign) {
+List clusters = _mgr.searchForClusters(zoneId,
0L, Long.MAX_VALUE, HypervisorType.XenServer.toString());
if (clusters == null) {
-throw new CloudRuntimeException("Unable to locate an
applicable cluster");
+clusters = new ArrayList<>();
}
+Collections.shuffle(clusters, new Random(System.nanoTime()));
+
+clusters:
for (Cluster cluster : clusters) {
if (cluster.getAllocationState() == AllocationState.Enabled) {
List hosts =
_hostDao.findByClusterId(cluster.getId());
if (hosts != null) {
+Collections.shuffle(hosts, new
Random(System.nanoTime()));
+
for (HostVO host : hosts) {
if (host.getResourceState() ==
ResourceState.Enabled) {
-return host;
+if (computeClusterMustSupportResign) {
+if
(_clusterDao.computeClusterSupportsResign(cluster.getId())) {
+return host;
+}
+else {
+// no other host in the cluster in
question should be able to satisfy our requirements here, so move on to the
next cluster
--- End diff --
a usefull comment! kudos, that rarely happens ;)
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1403#discussion_r62878912 --- Diff: engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java --- @@ -72,8 +77,10 @@ private static final Logger s_logger = Logger.getLogger(StorageSystemSnapshotStrategy.class); @Inject private AgentManager _agentMgr; +@Inject private ClusterDao _clusterDao; @Inject private DataStoreManager _dataStoreMgr; @Inject private HostDao _hostDao; +@Inject private HostDetailsDao _hostDetailsDao; --- End diff -- warning for not used --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r62876283
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -361,59 +603,221 @@ private String getProperty(long snapshotId, String
property) {
}
private Map getVolumeDetails(VolumeInfo volumeInfo) {
-Map sourceDetails = new HashMap();
+Map volumeDetails = new HashMap();
VolumeVO volumeVO = _volumeDao.findById(volumeInfo.getId());
long storagePoolId = volumeVO.getPoolId();
StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
-sourceDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
-sourceDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
-sourceDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
+volumeDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+volumeDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+volumeDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
ChapInfo chapInfo = _volumeService.getChapInfo(volumeInfo,
volumeInfo.getDataStore());
if (chapInfo != null) {
-sourceDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
-sourceDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
-sourceDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
-sourceDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
+volumeDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
+volumeDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
}
-return sourceDetails;
+return volumeDetails;
+}
+
+private Map getSnapshotDetails(SnapshotInfo
snapshotInfo) {
+Map snapshotDetails = new HashMap();
+
+long storagePoolId = snapshotInfo.getDataStore().getId();
+StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
+
+snapshotDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+snapshotDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+
+long snapshotId = snapshotInfo.getId();
+
+snapshotDetails.put(DiskTO.IQN, getProperty(snapshotId,
DiskTO.IQN));
+
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_SECRET));
+snapshotDetails.put(DiskTO.CHAP_TARGET_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_TARGET_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_TARGET_SECRET,
getProperty(snapshotId, DiskTO.CHAP_TARGET_SECRET));
+
+return snapshotDetails;
}
-public HostVO getHost(long dataStoreId) {
-StoragePoolVO storagePoolVO =
_storagePoolDao.findById(dataStoreId);
+private HostVO getHost(SnapshotInfo snapshotInfo) {
+HostVO hostVO = getHost(snapshotInfo.getDataCenterId(), true);
-List clusters =
_mgr.searchForClusters(storagePoolVO.getDataCenterId(), new Long(0),
Long.MAX_VALUE, HypervisorType.XenServer.toString());
+if (hostVO == null) {
+hostVO = getHost(snapshotInfo.getDataCenterId(), false);
-if (clusters == null) {
-throw new CloudRuntimeException("Unable to locate an
applicable cluster");
+if (hostVO == null) {
+throw new CloudRuntimeException("Unable to locate an
applicable host in data center with ID = " + snapshotInfo.getDataCenterId());
+}
}
-for (Cluster cluster : clusters) {
-if (cluster.getAllocationState() == AllocationState.Enabled) {
-List hosts =
_hostDao.findByClusterId(cluster.getId());
+return hostVO;
+}
-if (hosts != null) {
-for (HostVO host : hosts) {
-if (host.getResourceState() ==
ResourceState.Enabled) {
-return host;
-
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r62876182
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -361,59 +603,221 @@ private String getProperty(long snapshotId, String
property) {
}
private Map getVolumeDetails(VolumeInfo volumeInfo) {
-Map sourceDetails = new HashMap();
+Map volumeDetails = new HashMap();
VolumeVO volumeVO = _volumeDao.findById(volumeInfo.getId());
long storagePoolId = volumeVO.getPoolId();
StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
-sourceDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
-sourceDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
-sourceDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
+volumeDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+volumeDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+volumeDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
ChapInfo chapInfo = _volumeService.getChapInfo(volumeInfo,
volumeInfo.getDataStore());
if (chapInfo != null) {
-sourceDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
-sourceDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
-sourceDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
-sourceDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
+volumeDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
+volumeDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
}
-return sourceDetails;
+return volumeDetails;
+}
+
+private Map getSnapshotDetails(SnapshotInfo
snapshotInfo) {
+Map snapshotDetails = new HashMap();
+
+long storagePoolId = snapshotInfo.getDataStore().getId();
+StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
+
+snapshotDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+snapshotDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+
+long snapshotId = snapshotInfo.getId();
+
+snapshotDetails.put(DiskTO.IQN, getProperty(snapshotId,
DiskTO.IQN));
+
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_SECRET));
+snapshotDetails.put(DiskTO.CHAP_TARGET_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_TARGET_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_TARGET_SECRET,
getProperty(snapshotId, DiskTO.CHAP_TARGET_SECRET));
+
+return snapshotDetails;
}
-public HostVO getHost(long dataStoreId) {
-StoragePoolVO storagePoolVO =
_storagePoolDao.findById(dataStoreId);
+private HostVO getHost(SnapshotInfo snapshotInfo) {
+HostVO hostVO = getHost(snapshotInfo.getDataCenterId(), true);
-List clusters =
_mgr.searchForClusters(storagePoolVO.getDataCenterId(), new Long(0),
Long.MAX_VALUE, HypervisorType.XenServer.toString());
+if (hostVO == null) {
+hostVO = getHost(snapshotInfo.getDataCenterId(), false);
-if (clusters == null) {
-throw new CloudRuntimeException("Unable to locate an
applicable cluster");
+if (hostVO == null) {
+throw new CloudRuntimeException("Unable to locate an
applicable host in data center with ID = " + snapshotInfo.getDataCenterId());
+}
}
-for (Cluster cluster : clusters) {
-if (cluster.getAllocationState() == AllocationState.Enabled) {
-List hosts =
_hostDao.findByClusterId(cluster.getId());
+return hostVO;
+}
-if (hosts != null) {
-for (HostVO host : hosts) {
-if (host.getResourceState() ==
ResourceState.Enabled) {
-return host;
-
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r62876040
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -361,59 +608,225 @@ private String getProperty(long snapshotId, String
property) {
}
private Map getVolumeDetails(VolumeInfo volumeInfo) {
-Map sourceDetails = new HashMap();
+Map volumeDetails = new HashMap();
VolumeVO volumeVO = _volumeDao.findById(volumeInfo.getId());
long storagePoolId = volumeVO.getPoolId();
StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
-sourceDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
-sourceDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
-sourceDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
+volumeDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+volumeDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+volumeDetails.put(DiskTO.IQN, volumeVO.get_iScsiName());
ChapInfo chapInfo = _volumeService.getChapInfo(volumeInfo,
volumeInfo.getDataStore());
if (chapInfo != null) {
-sourceDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
-sourceDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
-sourceDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
-sourceDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
chapInfo.getInitiatorUsername());
+volumeDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
chapInfo.getInitiatorSecret());
+volumeDetails.put(DiskTO.CHAP_TARGET_USERNAME,
chapInfo.getTargetUsername());
+volumeDetails.put(DiskTO.CHAP_TARGET_SECRET,
chapInfo.getTargetSecret());
+}
+
+return volumeDetails;
+}
+
+private Map getSnapshotDetails(SnapshotInfo
snapshotInfo) {
+Map snapshotDetails = new HashMap();
+
+long storagePoolId = snapshotInfo.getDataStore().getId();
+StoragePoolVO storagePoolVO =
_storagePoolDao.findById(storagePoolId);
+
+snapshotDetails.put(DiskTO.STORAGE_HOST,
storagePoolVO.getHostAddress());
+snapshotDetails.put(DiskTO.STORAGE_PORT,
String.valueOf(storagePoolVO.getPort()));
+
+long snapshotId = snapshotInfo.getId();
+
+snapshotDetails.put(DiskTO.IQN, getProperty(snapshotId,
DiskTO.IQN));
+
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_INITIATOR_SECRET,
getProperty(snapshotId, DiskTO.CHAP_INITIATOR_SECRET));
+snapshotDetails.put(DiskTO.CHAP_TARGET_USERNAME,
getProperty(snapshotId, DiskTO.CHAP_TARGET_USERNAME));
+snapshotDetails.put(DiskTO.CHAP_TARGET_SECRET,
getProperty(snapshotId, DiskTO.CHAP_TARGET_SECRET));
+
+return snapshotDetails;
+}
+
+private HostVO getHost(SnapshotInfo snapshotInfo) {
+HostVO hostVO = getHost(snapshotInfo.getDataCenterId(), true);
+
+if (hostVO == null) {
+hostVO = getHost(snapshotInfo.getDataCenterId(), false);
+
+if (hostVO == null) {
+throw new CloudRuntimeException("Unable to locate an
applicable host");
+}
}
-return sourceDetails;
+return hostVO;
}
-public HostVO getHost(long dataStoreId) {
-StoragePoolVO storagePoolVO =
_storagePoolDao.findById(dataStoreId);
+private HostVO getHost(Long zoneId, boolean
computeClusterMustSupportResign) {
+if (zoneId == null) {
+throw new CloudRuntimeException("Zone ID cannot be null.");
+}
-List clusters =
_mgr.searchForClusters(storagePoolVO.getDataCenterId(), new Long(0),
Long.MAX_VALUE, HypervisorType.XenServer.toString());
+List clusters = _mgr.searchForClusters(zoneId,
new Long(0), Long.MAX_VALUE, HypervisorType.XenServer.toString());
if (clusters == null) {
-throw new CloudRuntimeException("Unable to locate an
applicable cluster");
+clusters = new ArrayList<>();
}
+Collections.shuffle(clusters, new Random(System.nanoTime()));
+
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r62875666
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -255,99 +473,123 @@ private Void
handleCreateVolumeFromSnapshotBothOnStorageSystem(SnapshotInfo snap
VolumeApiResult result = future.get();
+if (volumeDetail != null) {
+_volumeDetailsDao.remove(volumeDetail.getId());
+}
+
if (result.isFailed()) {
-s_logger.debug("Failed to create a volume: " +
result.getResult());
+s_logger.warn("Failed to create a volume: " +
result.getResult());
throw new CloudRuntimeException(result.getResult());
}
-}
-catch (Exception ex) {
-throw new CloudRuntimeException(ex.getMessage());
-}
-volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(),
volumeInfo.getDataStore());
+volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(),
volumeInfo.getDataStore());
-volumeInfo.processEvent(Event.MigrationRequested);
+volumeInfo.processEvent(Event.MigrationRequested);
-volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(),
volumeInfo.getDataStore());
+volumeInfo = _volumeDataFactory.getVolume(volumeInfo.getId(),
volumeInfo.getDataStore());
-HostVO hostVO = getHost(snapshotInfo.getDataStore().getId());
-
-String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
-int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
-CopyCommand copyCommand = new CopyCommand(snapshotInfo.getTO(),
volumeInfo.getTO(), primaryStorageDownloadWait,
VirtualMachineManager.ExecuteInSequence.value());
+if (useCloning) {
+copyCmdAnswer = performResignature(volumeInfo, hostVO);
+}
+else {
+// asking for a XenServer host here so we don't always
prefer to use XenServer hosts that support resigning
+// even when we don't need those hosts to do this kind of
copy work
+hostVO = getHost(snapshotInfo.getDataCenterId(), false);
-CopyCmdAnswer copyCmdAnswer = null;
+copyCmdAnswer = performCopyOfVdi(volumeInfo, snapshotInfo,
hostVO);
+}
-try {
-_volumeService.grantAccess(snapshotInfo, hostVO,
snapshotInfo.getDataStore());
-_volumeService.grantAccess(volumeInfo, hostVO,
volumeInfo.getDataStore());
+if (copyCmdAnswer == null || !copyCmdAnswer.getResult()) {
+if (copyCmdAnswer != null &&
!StringUtils.isEmpty(copyCmdAnswer.getDetails())) {
+errMsg = copyCmdAnswer.getDetails();
+}
+else {
+errMsg = "Unable to create volume from snapshot";
+}
+}
+}
+catch (Exception ex) {
+errMsg = ex.getMessage() != null ? ex.getMessage() : "Copy
operation failed";
+}
-Map srcDetails =
getSnapshotDetails(_storagePoolDao.findById(snapshotInfo.getDataStore().getId()),
snapshotInfo);
+CopyCommandResult result = new CopyCommandResult(null,
copyCmdAnswer);
-copyCommand.setOptions(srcDetails);
+result.setResult(errMsg);
-Map destDetails = getVolumeDetails(volumeInfo);
+callback.complete(result);
+}
-copyCommand.setOptions2(destDetails);
+/**
+ * If the underlying storage system is making use of read-only
snapshots, this gives the storage system the opportunity to
+ * create a volume from the snapshot so that we can copy the VHD file
that should be inside of the snapshot to secondary storage.
+ *
+ * The resultant volume must be writable because we need to resign the
SR and the VDI that should be inside of it before we copy
+ * the VHD file to secondary storage.
+ *
+ * If the storage system is using writable snapshots, then nothing
need be done by that storage system here because we can just
+ * resign the SR and the VDI that should be inside of the snapshot
before copying the VHD file to secondary storage.
+ */
+private void createVolumeFromSnapshot(HostVO hostVO, SnapshotInfo
snapshotInfo, boolean keepGrantedAccess) {
+SnapshotDetailsVO snapshotDetails =
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r62875028
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -172,78 +254,214 @@ private void validate(SnapshotInfo snapshotInfo) {
}
}
-private Void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
+private boolean usingBackendSnapshotFor(SnapshotInfo snapshotInfo) {
+String property = getProperty(snapshotInfo.getId(),
"takeSnapshot");
+
+return Boolean.parseBoolean(property);
+}
+
+private void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
try {
snapshotInfo.processEvent(Event.CopyingRequested);
}
catch (Exception ex) {
throw new CloudRuntimeException("This snapshot is not
currently in a state where it can be used to create a template.");
}
-HostVO hostVO = getHost(snapshotInfo.getDataStore().getId());
-DataStore srcDataStore = snapshotInfo.getDataStore();
+HostVO hostVO = getHost(snapshotInfo);
-String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
-int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
-CopyCommand copyCommand = new CopyCommand(snapshotInfo.getTO(),
templateInfo.getTO(), primaryStorageDownloadWait,
VirtualMachineManager.ExecuteInSequence.value());
-
-String errMsg = null;
+boolean usingBackendSnapshot =
usingBackendSnapshotFor(snapshotInfo);
+boolean computeClusterSupportsResign =
_clusterDao.computeClusterSupportsResign(hostVO.getClusterId());
-CopyCmdAnswer copyCmdAnswer = null;
+if (usingBackendSnapshot && !computeClusterSupportsResign) {
+throw new CloudRuntimeException("Unable to locate an
applicable host with which to perform a resignature operation");
+}
try {
-_volumeService.grantAccess(snapshotInfo, hostVO, srcDataStore);
+if (usingBackendSnapshot) {
+createVolumeFromSnapshot(hostVO, snapshotInfo, true);
+}
-Map srcDetails =
getSnapshotDetails(_storagePoolDao.findById(srcDataStore.getId()),
snapshotInfo);
+DataStore srcDataStore = snapshotInfo.getDataStore();
-copyCommand.setOptions(srcDetails);
+String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
+int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
+CopyCommand copyCommand = new
CopyCommand(snapshotInfo.getTO(), templateInfo.getTO(),
primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value());
+
+String errMsg = null;
+
+CopyCmdAnswer copyCmdAnswer = null;
-copyCmdAnswer = (CopyCmdAnswer)_agentMgr.send(hostVO.getId(),
copyCommand);
-}
-catch (Exception ex) {
-throw new CloudRuntimeException(ex.getMessage());
-}
-finally {
try {
-_volumeService.revokeAccess(snapshotInfo, hostVO,
srcDataStore);
+// If we are using a back-end snapshot, then we should
still have access to it from the hosts in the cluster that hostVO is in
+// (because we passed in true as the third parameter to
createVolumeFromSnapshot above).
+if (usingBackendSnapshot == false) {
+_volumeService.grantAccess(snapshotInfo, hostVO,
srcDataStore);
+}
+
+Map srcDetails =
getSnapshotDetails(snapshotInfo);
+
+copyCommand.setOptions(srcDetails);
+
+copyCmdAnswer =
(CopyCmdAnswer)_agentMgr.send(hostVO.getId(), copyCommand);
}
catch (Exception ex) {
-s_logger.debug(ex.getMessage(), ex);
+throw new CloudRuntimeException("Failed to create template
from snapshot : " + ex.getMessage());
}
+finally {
+try {
+_volumeService.revokeAccess(snapshotInfo, hostVO,
srcDataStore);
+}
+catch (Exception ex) {
+s_logger.warn(ex.getMessage(), ex);
+
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r62874090
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -172,78 +232,211 @@ private void validate(SnapshotInfo snapshotInfo) {
}
}
-private Void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
+private boolean usingBackendSnapshotFor(SnapshotInfo snapshotInfo) {
+String property = getProperty(snapshotInfo.getId(),
"takeSnapshot");
+
+return Boolean.parseBoolean(property);
+}
+
+private void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
try {
snapshotInfo.processEvent(Event.CopyingRequested);
}
catch (Exception ex) {
throw new CloudRuntimeException("This snapshot is not
currently in a state where it can be used to create a template.");
}
-HostVO hostVO = getHost(snapshotInfo.getDataStore().getId());
-DataStore srcDataStore = snapshotInfo.getDataStore();
-
-String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
-int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
-CopyCommand copyCommand = new CopyCommand(snapshotInfo.getTO(),
templateInfo.getTO(), primaryStorageDownloadWait,
VirtualMachineManager.ExecuteInSequence.value());
+HostVO hostVO = getHost(snapshotInfo);
-String errMsg = null;
+boolean usingBackendSnapshot =
usingBackendSnapshotFor(snapshotInfo);
+boolean computeClusterSupportsResign =
computeClusterSupportsResign(hostVO.getClusterId());
-CopyCmdAnswer copyCmdAnswer = null;
+if (usingBackendSnapshot && !computeClusterSupportsResign) {
+throw new CloudRuntimeException("Unable to locate an
applicable host with which to perform a resignature operation");
+}
try {
-_volumeService.grantAccess(snapshotInfo, hostVO, srcDataStore);
+if (usingBackendSnapshot) {
+createVolumeFromSnapshot(hostVO, snapshotInfo, true);
+}
-Map srcDetails =
getSnapshotDetails(_storagePoolDao.findById(srcDataStore.getId()),
snapshotInfo);
+DataStore srcDataStore = snapshotInfo.getDataStore();
-copyCommand.setOptions(srcDetails);
+String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
+int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
+CopyCommand copyCommand = new
CopyCommand(snapshotInfo.getTO(), templateInfo.getTO(),
primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value());
+
+String errMsg = null;
+
+CopyCmdAnswer copyCmdAnswer = null;
-copyCmdAnswer = (CopyCmdAnswer)_agentMgr.send(hostVO.getId(),
copyCommand);
-}
-catch (Exception ex) {
-throw new CloudRuntimeException(ex.getMessage());
-}
-finally {
try {
-_volumeService.revokeAccess(snapshotInfo, hostVO,
srcDataStore);
+// If we are using a back-end snapshot, then we should
still have access to it from the hosts in the cluster that hostVO is in
+// (because we passed in true as the third parameter to
createVolumeFromSnapshot above).
+if (usingBackendSnapshot == false) {
+_volumeService.grantAccess(snapshotInfo, hostVO,
srcDataStore);
+}
+
+Map srcDetails =
getSnapshotDetails(snapshotInfo);
+
+copyCommand.setOptions(srcDetails);
+
+copyCmdAnswer =
(CopyCmdAnswer)_agentMgr.send(hostVO.getId(), copyCommand);
}
catch (Exception ex) {
-s_logger.debug(ex.getMessage(), ex);
+throw new CloudRuntimeException(ex.getMessage());
}
+finally {
+try {
+_volumeService.revokeAccess(snapshotInfo, hostVO,
srcDataStore);
+}
+catch (Exception ex) {
+s_logger.debug(ex.getMessage(), ex);
+}
-if (copyCmdAnswer ==
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r62872442
--- Diff:
engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
---
@@ -172,78 +232,211 @@ private void validate(SnapshotInfo snapshotInfo) {
}
}
-private Void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
+private boolean usingBackendSnapshotFor(SnapshotInfo snapshotInfo) {
+String property = getProperty(snapshotInfo.getId(),
"takeSnapshot");
+
+return Boolean.parseBoolean(property);
+}
+
+private void handleCreateTemplateFromSnapshot(SnapshotInfo
snapshotInfo, TemplateInfo templateInfo,
AsyncCompletionCallback callback) {
try {
snapshotInfo.processEvent(Event.CopyingRequested);
}
catch (Exception ex) {
throw new CloudRuntimeException("This snapshot is not
currently in a state where it can be used to create a template.");
}
-HostVO hostVO = getHost(snapshotInfo.getDataStore().getId());
-DataStore srcDataStore = snapshotInfo.getDataStore();
-
-String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
-int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
-CopyCommand copyCommand = new CopyCommand(snapshotInfo.getTO(),
templateInfo.getTO(), primaryStorageDownloadWait,
VirtualMachineManager.ExecuteInSequence.value());
+HostVO hostVO = getHost(snapshotInfo);
-String errMsg = null;
+boolean usingBackendSnapshot =
usingBackendSnapshotFor(snapshotInfo);
+boolean computeClusterSupportsResign =
computeClusterSupportsResign(hostVO.getClusterId());
-CopyCmdAnswer copyCmdAnswer = null;
+if (usingBackendSnapshot && !computeClusterSupportsResign) {
+throw new CloudRuntimeException("Unable to locate an
applicable host with which to perform a resignature operation");
+}
try {
-_volumeService.grantAccess(snapshotInfo, hostVO, srcDataStore);
+if (usingBackendSnapshot) {
+createVolumeFromSnapshot(hostVO, snapshotInfo, true);
+}
-Map srcDetails =
getSnapshotDetails(_storagePoolDao.findById(srcDataStore.getId()),
snapshotInfo);
+DataStore srcDataStore = snapshotInfo.getDataStore();
-copyCommand.setOptions(srcDetails);
+String value =
_configDao.getValue(Config.PrimaryStorageDownloadWait.toString());
+int primaryStorageDownloadWait = NumbersUtil.parseInt(value,
Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue()));
+CopyCommand copyCommand = new
CopyCommand(snapshotInfo.getTO(), templateInfo.getTO(),
primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value());
+
+String errMsg = null;
+
+CopyCmdAnswer copyCmdAnswer = null;
-copyCmdAnswer = (CopyCmdAnswer)_agentMgr.send(hostVO.getId(),
copyCommand);
-}
-catch (Exception ex) {
-throw new CloudRuntimeException(ex.getMessage());
-}
-finally {
try {
--- End diff --
I can not see what Rafael means. I think the code has changed since he made
the comment. I do see this method as growing out of proportion so some
refactorring for readability and maintainability seems in order.
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1403#discussion_r62868269
--- Diff: engine/schema/src/com/cloud/dc/dao/ClusterDaoImpl.java ---
@@ -260,4 +268,41 @@ public boolean remove(Long id) {
sc.setParameters("dataCenterId", zoneId);
return customSearch(sc, null);
}
+
+@Override
+public boolean computeClusterSupportsResign(long clusterId) {
--- End diff --
sugested name change: computeWhetherClusterSupportsResign or even
computeWhetherClusterSupportsResigning
---
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.
---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1403#discussion_r62867309 --- Diff: engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreInfo.java --- @@ -36,6 +36,7 @@ static final String CHAP_INITIATOR_SECRET = "chapInitiatorSecret"; static final String CHAP_TARGET_USERNAME = "chapTargetUsername"; static final String CHAP_TARGET_SECRET = "chapTargetSecret"; +static final String REMOVE_AFTER_COPY = "removeAfterCopy"; --- End diff -- not lethal but these strings seem to warrant an enum as well, do they? --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-218365435 @jburwell can I get your final review on this one? I think that is the only thing we are missing on this one. Thx... --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1403#discussion_r62747336 --- Diff: core/src/com/cloud/agent/api/StartupRoutingCommand.java --- @@ -35,7 +35,7 @@ long memory; long dom0MinMemory; boolean poolSync; - +private boolean _supportsClonedVolumes; --- End diff -- I agree with Rafael on this. Let's not introduce more members starting with underscore but grow to cleaner code (slowly) --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-217481732 @jburwell can I get your final review on this? I think we are probably pretty close on this one. Thanks... --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-217228903 @jburwell Just wanted to point out that an earlier PR of mine, #816, has four Marvin integration tests (three out of four of those apply to this PR, too). I wanted to mention that so you know they are going into the CloudStack repo (even though they require SolidFire hardware to run successfully). There are a few methods in the tests that I'd like to extract to a utils module, but I plan to do in the near future as I've been spending a lot more time building out Marvin tests in general lately. Thanks! --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-217228739 @jburwell Just wanted to point out that an earlier PR of mine, #816, has four Marvin integration tests (three out of four of those apply to this PR, too). I wanted to mention that so you know they are going into the CloudStack repo (even though they require SolidFire hardware to run successfully). There are a few methods in the tests that I'd like to extract to a utils module, but I plan to do in the near future as I've been spending a lot more time building out Marvin tests in general lately. Thanks! --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-216354858 After rebasing today, I re-ran SolidFire-related automated integration tests (about 3.5 hours of tests) and all were successful. test_01_create_volume_snapshot_using_sf_snapshot (TestSnapshots.TestSnapshots) ... === TestName: test_01_create_volume_snapshot_using_sf_snapshot | Status : SUCCESS === ok test_02_create_volume_snapshot_using_sf_volume (TestSnapshots.TestSnapshots) ... === TestName: test_02_create_volume_snapshot_using_sf_volume | Status : SUCCESS === ok test_03_create_volume_snapshot_using_sf_volume_and_sf_snapshot (TestSnapshots.TestSnapshots) ... === TestName: test_03_create_volume_snapshot_using_sf_volume_and_sf_snapshot | Status : SUCCESS === ok -- Ran 3 tests in 7409.770s OK test_00_check_template_cache (TestVolumes.TestVolumes) ... === TestName: test_00_check_template_cache | Status : SUCCESS === ok Attach a volume to a stopped virtual machine, then start VM ... === TestName: test_01_attach_new_volume_to_stopped_VM | Status : SUCCESS === ok Attach, detach, and attach volume to a running VM ... === TestName: test_02_attach_detach_attach_volume | Status : SUCCESS === ok Attach volume to running VM, then reboot. ... === TestName: test_03_attached_volume_reboot_VM | Status : SUCCESS === ok Detach volume from a running VM, then reboot. ... === TestName: test_04_detach_volume_reboot | Status : SUCCESS === ok Detach volume from a stopped VM, then start. ... === TestName: test_05_detach_vol_stopped_VM_start | Status : SUCCESS === ok Attach a volume to a stopped virtual machine, then start VM ... === TestName: test_06_attach_volume_to_stopped_VM | Status : SUCCESS === ok Destroy and expunge VM with attached volume ... === TestName: test_07_destroy_expunge_VM_with_volume | Status : SUCCESS === ok Delete volume that was attached to a VM and is detached now ... === TestName: test_08_delete_volume_was_attached | Status : SUCCESS === ok Attach a data disk to a VM in one account and attach another data disk to a VM in another account ... === TestName: test_09_attach_volumes_multiple_accounts | Status : SUCCESS === ok Attach more than one disk to a VM ... === TestName: test_10_attach_more_than_one_disk_to_VM | Status : SUCCESS === ok -- Ran 11 tests in 1982.066s OK test_00_check_template_cache (TestVolumes.TestVolumes) ... === TestName: test_00_check_template_cache | Status : SUCCESS === ok Attach a volume to a stopped virtual machine, then start VM ... === TestName: test_01_attach_new_volume_to_stopped_VM | Status : SUCCESS === ok Attach, detach, and attach volume to a running VM ... === TestName: test_02_attach_detach_attach_volume | Status : SUCCESS === ok Attach volume to running VM, then reboot. ... === TestName: test_03_attached_volume_reboot_VM | Status : SUCCESS === ok Detach volume from a running VM, then reboot. ... === TestName: test_04_detach_volume_reboot | Status : SUCCESS === ok Detach volume from a stopped VM, then start. ... === TestName: test_05_detach_vol_stopped_VM_start | Status : SUCCESS === ok Attach a volume to a stopped virtual machine, then start VM ... === TestName: test_06_attach_volume_to_stopped_VM | Status : SUCCESS === ok Destroy and expunge VM with attached volume ... === TestName: test_07_destroy_expunge_VM_with_volume | Status : SUCCESS === ok Delete volume that was attached to a VM and is detached now ... === TestName: test_08_delete_volume_was_attached | Status : SUCCESS === ok Attach a data disk to a VM in one account and attach another data disk to a VM in another account ... === TestName: test_09_attach_volumes_multiple_accounts | Status : SUCCESS === ok Attach more than one disk to a VM ... === TestName: test_10_attach_more_than_one_disk_to_VM | Status : SUCCESS === ok -- Ran 11 tests in 2352.461s OK test_01_take_VM_snapshot (TestVMSnapshots.TestVMSnapshots) ... === TestName: test_01_take_VM_snapshot | Status : SUCCESS === ok test_02_take_VM_snapshot_with_data_disk (TestVMSnapshots.TestVMSnapshots) ... === TestName: test_02_take_VM_snapshot_with_data_disk | Status : SUCCESS === ok -- Ran 2 tests in 789.729s OK --- 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
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-216294699 I need some code reviews here. @jburwell can you let me know what that status of your review is? --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user rhtyd commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-216222647 @mike-tutkowski thanks, please rebase against master, squash the changes to a single commit tag:mergeready --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-213603509 @DaanHoogland no, that is the next iteration. I just started on a smaller scale to try to get some better coverage. I will be working continually to increase the test coverage in bubble (expect some pull requests soon to bubble with more complete testing), as well as starting to try to improve testing in general by fixing tests that are currently failing. It is going to be a slow process, but I am slowly working on getting us to a place where we can have more confidence in the tests we are running. --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user DaanHoogland commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-213591446 h @swill, 120/120 passed :+1: so how did you decide on them? Are these the ones you send the 'here goes nothing' mail about? --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-213479837 ## CI RESULTS ### 120/120 TESTS PASSED! View the results below... **Associated Uploads** **`/tmp/MarvinLogs/DeployDataCenter__Apr_22_2016_07_25_29_9Y5PQH`:** * [dc_entries.obj](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1403/tmp/MarvinLogs/DeployDataCenter__Apr_22_2016_07_25_29_9Y5PQH/dc_entries.obj) * [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1403/tmp/MarvinLogs/DeployDataCenter__Apr_22_2016_07_25_29_9Y5PQH/failed_plus_exceptions.txt) * [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1403/tmp/MarvinLogs/DeployDataCenter__Apr_22_2016_07_25_29_9Y5PQH/runinfo.txt) **`/tmp/MarvinLogs/test_projects_P6EJRK`:** * [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1403/tmp/MarvinLogs/test_projects_P6EJRK/failed_plus_exceptions.txt) * [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1403/tmp/MarvinLogs/test_projects_P6EJRK/results.txt) * [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1403/tmp/MarvinLogs/test_projects_P6EJRK/runinfo.txt) **`/tmp/MarvinLogs/test_templates_5NPAZM`:** * [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1403/tmp/MarvinLogs/test_templates_5NPAZM/failed_plus_exceptions.txt) * [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1403/tmp/MarvinLogs/test_templates_5NPAZM/results.txt) * [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1403/tmp/MarvinLogs/test_templates_5NPAZM/runinfo.txt) Uploads will be available until `2016-06-22 02:00:00 +0200 CEST` *Comment created by [`upr comment`](https://github.com/swill/upr).* --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-210087782 No problem. :) I had forgotten that I had renamed that file at one point. So, when I saw that you had a file by that name, I was really confused how you would have had access to a file that I had not yet checked into the repo (the current file with that name is for a project I'm still developing and I haven't opened a PR on it yet). --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-210085510 i will blow away my git repo and start from scratch and we will go from there. :) thx... --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-210084608 @swill I think I might know what happened. There used to be a ApiSolidFireServiceImpl.java, but I renamed it in 940d5b. It seems the file is likely still sitting on your filesystem and causing you this trouble. --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-210081474 @swill I just pushed a new commit to rename the SolidFire integration testing project (which you saw as failing). While that should not have anything to do with a failure, I'd be curious to have you re-run your build because ApiSolidFireServiceImpl (which was failing) should not even be a part of the codebase in that SHA. Thanks! --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-210072697 This might be a better link (as it includes the SHA): https://github.com/apache/cloudstack/tree/ee5370536ac3f87457d5f74adbbb8c6af88da449/plugins/api --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-210072035 @swill This is pretty strange. The code that is failing to compile for you is actually not part of the codebase for ee53705: https://github.com/mike-tutkowski/cloudstack/tree/xs-snapshots/plugins/api All API plug-ins are in the location above and the one failing for you is not in there. What do you think, Will? --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user mike-tutkowski commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-210066399 @swill Weird, I've compiled ee53705 with the following command on two separate machines and I ran regression tests against that SHA for a few hours yesterday: mvn -P developer,systemvm clean install -D noredist Let me look into this - thanks. --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-210012980 Looking closer at this code. It look like `primaryStoreDriver` could easily be `null` in this case, so that return statement could potentially cause a null pointer exception... --- 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. ---
[GitHub] cloudstack pull request: Taking fast and efficient volume snapshot...
Github user swill commented on the pull request: https://github.com/apache/cloudstack/pull/1403#issuecomment-210009483 This does not compile: ``` [ERROR] COMPILATION ERROR : [INFO] - [ERROR] /data/git/cs1/cloudstack/dist/rpmbuild/BUILD/cloudstack-4.9.0-SNAPSHOT/plugins/api/solidfire-intg-test/src/org/apache/cloudstack/solidfire/ApiSolidFireServiceImpl.java:[93,68] error: cannot find symbol [INFO] 1 error ``` It fails here: ``` [INFO] Apache CloudStack Plugin - Storage Volume default provider SUCCESS [2.494s] [INFO] Apache CloudStack Plugin - Storage Volume SolidFire Provider SUCCESS [6.477s] [INFO] Apache CloudStack Plugin - API SolidFire .. FAILURE [2.601s] [INFO] Apache CloudStack Plugin - API Discovery .. SKIPPED [INFO] Apache CloudStack Plugin - ACL Static Role Based .. SKIPPED ``` --- 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. ---
