You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2022/11/08 11:42:22 UTC
[GitHub] [cloudstack] GutoVeronezi commented on a diff in pull request #6661: Enable live volume migration for StorPool and small fixes
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org