GutoVeronezi commented on code in PR #6661:
URL: https://github.com/apache/cloudstack/pull/6661#discussion_r967271594
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java:
##########
@@ -709,24 +709,34 @@ public void copyAsync(DataObject srcData, DataObject
dstData, AsyncCompletionCal
} else {
// download volume - first copies to secondary
VolumeObjectTO srcTO = (VolumeObjectTO)srcData.getTO();
-
StorPoolUtil.spLog("StorpoolPrimaryDataStoreDriverImpl.copyAsnc SRC path=%s ",
srcTO.getPath());
-
StorPoolUtil.spLog("StorpoolPrimaryDataStoreDriverImpl.copyAsnc DST
canonicalName=%s ", dstData.getDataStore().getClass().getCanonicalName());
+
StorPoolUtil.spLog("StorpoolPrimaryDataStoreDriverImpl.copyAsnc SRC path=%s DST
canonicalName=%s ", srcTO.getPath(),
dstData.getDataStore().getClass().getCanonicalName());
PrimaryDataStoreTO checkStoragePool =
dstData.getTO().getDataStore() instanceof PrimaryDataStoreTO ?
(PrimaryDataStoreTO)dstData.getTO().getDataStore() : null;
final String name =
StorPoolStorageAdaptor.getVolumeNameFromPath(srcTO.getPath(), true);
-
StorPoolUtil.spLog("StorpoolPrimaryDataStoreDriverImpl.copyAsnc DST
tmpSnapName=%s ,srcUUID=%s", name, srcTO.getUuid());
if (checkStoragePool != null &&
checkStoragePool.getPoolType().equals(StoragePoolType.StorPool)) {
SpConnectionDesc conn =
StorPoolUtil.getSpConnection(dstData.getDataStore().getUuid(),
dstData.getDataStore().getId(), storagePoolDetailsDao, primaryStoreDao);
String baseOn =
StorPoolStorageAdaptor.getVolumeNameFromPath(srcTO.getPath(), true);
- //uuid tag will be the same as srcData.uuid
- String volumeName = srcData.getUuid();
-
StorPoolUtil.spLog("StorpoolPrimaryDataStoreDriverImpl.copyAsnc volumeName=%s,
baseOn=%s", volumeName, baseOn);
- final SpApiResponse response =
StorPoolUtil.volumeCopy(volumeName, baseOn, "volume", srcInfo.getMaxIops(),
conn);
- srcTO.setSize(srcData.getSize());
-
srcTO.setPath(StorPoolUtil.devPath(StorPoolUtil.getNameFromResponse(response,
false)));
-
StorPoolUtil.spLog("StorpoolPrimaryDataStoreDriverImpl.copyAsnc DST to=%s",
srcTO);
-
- answer = new CopyCmdAnswer(srcTO);
+
+ String vmUuid = null;
+ String vcPolicyTag = null;
+
+ VMInstanceVO vm = null;
+ if (srcInfo.getInstanceId() != null) {
+ vm =
vmInstanceDao.findById(srcInfo.getInstanceId());
+ }
+
+ if (vm != null) {
+ vmUuid = vm.getUuid();
+ vcPolicyTag = getVcPolicyTag(vm.getId());
+ }
+
+ if (vm != null && vm.getState().equals(State.Running))
{
+ // migrate volume to another StorPool template
Review Comment:
We could turn this comment into the method's JavaDoc.
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java:
##########
@@ -792,6 +775,71 @@ public void copyAsync(DataObject srcData, DataObject
dstData, AsyncCompletionCal
callback.complete(res);
}
+ private Answer migrateVolumeToStorPool(DataObject srcData, DataObject
dstData, VolumeInfo srcInfo,
Review Comment:
We could use some unit tests for this method.
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java:
##########
@@ -709,24 +709,34 @@ public void copyAsync(DataObject srcData, DataObject
dstData, AsyncCompletionCal
} else {
// download volume - first copies to secondary
VolumeObjectTO srcTO = (VolumeObjectTO)srcData.getTO();
-
StorPoolUtil.spLog("StorpoolPrimaryDataStoreDriverImpl.copyAsnc SRC path=%s ",
srcTO.getPath());
-
StorPoolUtil.spLog("StorpoolPrimaryDataStoreDriverImpl.copyAsnc DST
canonicalName=%s ", dstData.getDataStore().getClass().getCanonicalName());
+
StorPoolUtil.spLog("StorpoolPrimaryDataStoreDriverImpl.copyAsnc SRC path=%s DST
canonicalName=%s ", srcTO.getPath(),
dstData.getDataStore().getClass().getCanonicalName());
PrimaryDataStoreTO checkStoragePool =
dstData.getTO().getDataStore() instanceof PrimaryDataStoreTO ?
(PrimaryDataStoreTO)dstData.getTO().getDataStore() : null;
final String name =
StorPoolStorageAdaptor.getVolumeNameFromPath(srcTO.getPath(), true);
-
StorPoolUtil.spLog("StorpoolPrimaryDataStoreDriverImpl.copyAsnc DST
tmpSnapName=%s ,srcUUID=%s", name, srcTO.getUuid());
if (checkStoragePool != null &&
checkStoragePool.getPoolType().equals(StoragePoolType.StorPool)) {
SpConnectionDesc conn =
StorPoolUtil.getSpConnection(dstData.getDataStore().getUuid(),
dstData.getDataStore().getId(), storagePoolDetailsDao, primaryStoreDao);
String baseOn =
StorPoolStorageAdaptor.getVolumeNameFromPath(srcTO.getPath(), true);
- //uuid tag will be the same as srcData.uuid
- String volumeName = srcData.getUuid();
-
StorPoolUtil.spLog("StorpoolPrimaryDataStoreDriverImpl.copyAsnc volumeName=%s,
baseOn=%s", volumeName, baseOn);
- final SpApiResponse response =
StorPoolUtil.volumeCopy(volumeName, baseOn, "volume", srcInfo.getMaxIops(),
conn);
- srcTO.setSize(srcData.getSize());
-
srcTO.setPath(StorPoolUtil.devPath(StorPoolUtil.getNameFromResponse(response,
false)));
-
StorPoolUtil.spLog("StorpoolPrimaryDataStoreDriverImpl.copyAsnc DST to=%s",
srcTO);
-
- answer = new CopyCmdAnswer(srcTO);
+
+ String vmUuid = null;
+ String vcPolicyTag = null;
+
+ VMInstanceVO vm = null;
+ if (srcInfo.getInstanceId() != null) {
+ vm =
vmInstanceDao.findById(srcInfo.getInstanceId());
+ }
+
+ if (vm != null) {
+ vmUuid = vm.getUuid();
+ vcPolicyTag = getVcPolicyTag(vm.getId());
+ }
+
+ if (vm != null && vm.getState().equals(State.Running))
{
+ // migrate volume to another StorPool template
+ answer = migrateVolume(srcData, dstData, name,
conn);
+ } else {
+ //copy volume to another pool
Review Comment:
We could turn this comment into the method's JavaDoc.
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java:
##########
@@ -792,6 +775,71 @@ public void copyAsync(DataObject srcData, DataObject
dstData, AsyncCompletionCal
callback.complete(res);
}
+ private Answer migrateVolumeToStorPool(DataObject srcData, DataObject
dstData, VolumeInfo srcInfo,
+ VolumeObjectTO srcTO, final String volumeName) {
+ Answer answer;
+ SpConnectionDesc conn =
StorPoolUtil.getSpConnection(dstData.getDataStore().getUuid(),
dstData.getDataStore().getId(), storagePoolDetailsDao, primaryStoreDao);
+ String baseOn =
StorPoolStorageAdaptor.getVolumeNameFromPath(srcTO.getPath(), true);
+
+ String vmUuid = null;
+ String vcPolicyTag = null;
+
+ VMInstanceVO vm = null;
+ if (srcInfo.getInstanceId() != null) {
+ vm = vmInstanceDao.findById(srcInfo.getInstanceId());
+ }
+
+ if (vm != null) {
+ vmUuid = vm.getUuid();
+ vcPolicyTag = getVcPolicyTag(vm.getId());
+ }
+
+ if (vm != null && vm.getState().equals(State.Running)) {
+ // migrate volume to another StorPool template
Review Comment:
This comment could be turned to a javadoc for the method.
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java:
##########
@@ -792,6 +775,71 @@ public void copyAsync(DataObject srcData, DataObject
dstData, AsyncCompletionCal
callback.complete(res);
}
+ private Answer migrateVolumeToStorPool(DataObject srcData, DataObject
dstData, VolumeInfo srcInfo,
+ VolumeObjectTO srcTO, final String volumeName) {
+ Answer answer;
+ SpConnectionDesc conn =
StorPoolUtil.getSpConnection(dstData.getDataStore().getUuid(),
dstData.getDataStore().getId(), storagePoolDetailsDao, primaryStoreDao);
+ String baseOn =
StorPoolStorageAdaptor.getVolumeNameFromPath(srcTO.getPath(), true);
+
+ String vmUuid = null;
+ String vcPolicyTag = null;
+
+ VMInstanceVO vm = null;
+ if (srcInfo.getInstanceId() != null) {
+ vm = vmInstanceDao.findById(srcInfo.getInstanceId());
+ }
+
+ if (vm != null) {
+ vmUuid = vm.getUuid();
+ vcPolicyTag = getVcPolicyTag(vm.getId());
+ }
+
+ if (vm != null && vm.getState().equals(State.Running)) {
+ // migrate volume to another StorPool template
+ answer = migrateVolume(srcData, dstData, volumeName, conn);
+ } else {
+ //copy volume to another pool
Review Comment:
This comment could be turned to a javadoc for the method.
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java:
##########
@@ -792,6 +775,71 @@ public void copyAsync(DataObject srcData, DataObject
dstData, AsyncCompletionCal
callback.complete(res);
}
+ private Answer migrateVolumeToStorPool(DataObject srcData, DataObject
dstData, VolumeInfo srcInfo,
+ VolumeObjectTO srcTO, final String volumeName) {
+ Answer answer;
+ SpConnectionDesc conn =
StorPoolUtil.getSpConnection(dstData.getDataStore().getUuid(),
dstData.getDataStore().getId(), storagePoolDetailsDao, primaryStoreDao);
+ String baseOn =
StorPoolStorageAdaptor.getVolumeNameFromPath(srcTO.getPath(), true);
+
+ String vmUuid = null;
+ String vcPolicyTag = null;
+
+ VMInstanceVO vm = null;
+ if (srcInfo.getInstanceId() != null) {
+ vm = vmInstanceDao.findById(srcInfo.getInstanceId());
+ }
+
+ if (vm != null) {
+ vmUuid = vm.getUuid();
+ vcPolicyTag = getVcPolicyTag(vm.getId());
+ }
+
+ if (vm != null && vm.getState().equals(State.Running)) {
+ // migrate volume to another StorPool template
+ answer = migrateVolume(srcData, dstData, volumeName, conn);
+ } else {
+ //copy volume to another pool
+ answer = copyVolume(srcInfo, srcTO, conn, baseOn, vmUuid,
vcPolicyTag);
+ }
+ return answer;
+ }
+
+ private Answer copyVolume(VolumeInfo srcInfo, VolumeObjectTO srcTO,
SpConnectionDesc conn, String baseOn, String vmUuid, String vcPolicyTag) {
+ //uuid tag will be the same as srcData.uuid
+ String volumeName = srcInfo.getUuid();
+ Long iops = (srcInfo.getMaxIops() != null &&
srcInfo.getMaxIops().longValue() > 0) ? srcInfo.getMaxIops() : null;
+ SpApiResponse response = StorPoolUtil.volumeCopy(volumeName, baseOn,
"volume", iops, vmUuid, vcPolicyTag, conn);
+ if (response.getError() != null) {
+ return new CopyCmdAnswer(String.format("Could not copy volume [%s]
due to %s", baseOn, response.getError()));
+ }
+ String newVolume = StorPoolUtil.getNameFromResponse(response, false);
+
+ StorPoolUtil.spLog("StorpoolPrimaryDataStoreDriverImpl.copyAsnc copy
volume[%s] from pool[%s] with a new name [%s]",
+ baseOn, srcInfo.getDataStore().getName(), newVolume);
+
+ srcTO.setSize(srcInfo.getSize());
+ srcTO.setPath(StorPoolUtil.devPath(newVolume));
+
+ return new CopyCmdAnswer(srcTO);
+ }
+
+ private Answer migrateVolume(DataObject srcData, DataObject dstData,
String name, SpConnectionDesc conn) {
Review Comment:
We could use some unit tests for this method.
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java:
##########
@@ -709,27 +702,17 @@ public void copyAsync(DataObject srcData, DataObject
dstData, AsyncCompletionCal
} else {
// download volume - first copies to secondary
VolumeObjectTO srcTO = (VolumeObjectTO)srcData.getTO();
-
StorPoolUtil.spLog("StorpoolPrimaryDataStoreDriverImpl.copyAsnc SRC path=%s ",
srcTO.getPath());
-
StorPoolUtil.spLog("StorpoolPrimaryDataStoreDriverImpl.copyAsnc DST
canonicalName=%s ", dstData.getDataStore().getClass().getCanonicalName());
+
StorPoolUtil.spLog("StorpoolPrimaryDataStoreDriverImpl.copyAsnc SRC path=%s DST
canonicalName=%s ", srcTO.getPath(),
dstData.getDataStore().getClass().getCanonicalName());
PrimaryDataStoreTO checkStoragePool =
dstData.getTO().getDataStore() instanceof PrimaryDataStoreTO ?
(PrimaryDataStoreTO)dstData.getTO().getDataStore() : null;
- final String name =
StorPoolStorageAdaptor.getVolumeNameFromPath(srcTO.getPath(), true);
-
StorPoolUtil.spLog("StorpoolPrimaryDataStoreDriverImpl.copyAsnc DST
tmpSnapName=%s ,srcUUID=%s", name, srcTO.getUuid());
+ final String volumeName =
StorPoolStorageAdaptor.getVolumeNameFromPath(srcTO.getPath(), true);
if (checkStoragePool != null &&
checkStoragePool.getPoolType().equals(StoragePoolType.StorPool)) {
- SpConnectionDesc conn =
StorPoolUtil.getSpConnection(dstData.getDataStore().getUuid(),
dstData.getDataStore().getId(), storagePoolDetailsDao, primaryStoreDao);
- String baseOn =
StorPoolStorageAdaptor.getVolumeNameFromPath(srcTO.getPath(), true);
- //uuid tag will be the same as srcData.uuid
- String volumeName = srcData.getUuid();
-
StorPoolUtil.spLog("StorpoolPrimaryDataStoreDriverImpl.copyAsnc volumeName=%s,
baseOn=%s", volumeName, baseOn);
- final SpApiResponse response =
StorPoolUtil.volumeCopy(volumeName, baseOn, "volume", srcInfo.getMaxIops(),
conn);
- srcTO.setSize(srcData.getSize());
-
srcTO.setPath(StorPoolUtil.devPath(StorPoolUtil.getNameFromResponse(response,
false)));
-
StorPoolUtil.spLog("StorpoolPrimaryDataStoreDriverImpl.copyAsnc DST to=%s",
srcTO);
-
- answer = new CopyCmdAnswer(srcTO);
+ //live migrate from one StorPool storage to another
+ //migrate volume to StorPool storage
Review Comment:
This comment can be turned to a javadoc for the method.
##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java:
##########
@@ -792,6 +775,71 @@ public void copyAsync(DataObject srcData, DataObject
dstData, AsyncCompletionCal
callback.complete(res);
}
+ private Answer migrateVolumeToStorPool(DataObject srcData, DataObject
dstData, VolumeInfo srcInfo,
+ VolumeObjectTO srcTO, final String volumeName) {
+ Answer answer;
+ SpConnectionDesc conn =
StorPoolUtil.getSpConnection(dstData.getDataStore().getUuid(),
dstData.getDataStore().getId(), storagePoolDetailsDao, primaryStoreDao);
+ String baseOn =
StorPoolStorageAdaptor.getVolumeNameFromPath(srcTO.getPath(), true);
+
+ String vmUuid = null;
+ String vcPolicyTag = null;
+
+ VMInstanceVO vm = null;
+ if (srcInfo.getInstanceId() != null) {
+ vm = vmInstanceDao.findById(srcInfo.getInstanceId());
+ }
+
+ if (vm != null) {
+ vmUuid = vm.getUuid();
+ vcPolicyTag = getVcPolicyTag(vm.getId());
+ }
+
+ if (vm != null && vm.getState().equals(State.Running)) {
+ // migrate volume to another StorPool template
+ answer = migrateVolume(srcData, dstData, volumeName, conn);
+ } else {
+ //copy volume to another pool
+ answer = copyVolume(srcInfo, srcTO, conn, baseOn, vmUuid,
vcPolicyTag);
+ }
+ return answer;
+ }
+
+ private Answer copyVolume(VolumeInfo srcInfo, VolumeObjectTO srcTO,
SpConnectionDesc conn, String baseOn, String vmUuid, String vcPolicyTag) {
Review Comment:
We could use some unit tests for this method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
