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/04/22 22:46:44 UTC
[GitHub] [cloudstack] GutoVeronezi commented on a diff in pull request #6012: Add volume group handling
GutoVeronezi commented on code in PR #6012:
URL: https://github.com/apache/cloudstack/pull/6012#discussion_r856618564
##########
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java:
##########
@@ -3566,12 +3568,31 @@ private static DiskTO[] sortVolumesByDeviceId(DiskTO[] volumes) {
@Override
public int compare(DiskTO arg0, DiskTO arg1) {
+ if (arg0.getType().equals(Volume.Type.ROOT)){
+ return -1;
+ }
+ if(arg1.getType().equals(Volume.Type.ROOT)){
Review Comment:
```suggestion
if (arg1.getType().equals(Volume.Type.ROOT)) {
```
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java:
##########
@@ -259,6 +265,20 @@ public Map<String, Long> getDataDiskToDiskOfferingList() {
return dataDiskToDiskOfferingMap;
}
+ public Map<String, Integer> getVolumeGroups() {
+ Map<String, Integer> dataDiskVolumeGroups = new HashMap<>();
+ if (MapUtils.isNotEmpty(dataDiskToDiskOfferingList)) {
Review Comment:
We can revert the if statement and a return to reduce code indentation.
##########
server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java:
##########
@@ -692,8 +692,13 @@ private Pair<DiskProfile, StoragePool> importDisk(UnmanagedInstanceTO.Disk disk,
chainInfo = gson.toJson(diskInfo);
}
StoragePool storagePool = getStoragePool(disk, zone, cluster);
+
+ if ( useControllerConfiguration ){
Review Comment:
```suggestion
if (useControllerConfiguration) {
```
##########
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java:
##########
@@ -3566,12 +3568,31 @@ private static DiskTO[] sortVolumesByDeviceId(DiskTO[] volumes) {
@Override
public int compare(DiskTO arg0, DiskTO arg1) {
+ if (arg0.getType().equals(Volume.Type.ROOT)){
Review Comment:
```suggestion
if (arg0.getType().equals(Volume.Type.ROOT)) {
```
##########
engine/schema/src/main/java/com/cloud/storage/VolumeGroupVO.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * Copyright 2022 The Apache Software Foundation.
Review Comment:
@DK101010, based in the documentation regarding Apache's source code, we should not add the copyright line to the header (see https://github.com/apache/cloudstack/pull/4954#discussion_r625084415). Could you take a look on all the files you added, please?
##########
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java:
##########
@@ -3566,12 +3568,31 @@ private static DiskTO[] sortVolumesByDeviceId(DiskTO[] volumes) {
@Override
public int compare(DiskTO arg0, DiskTO arg1) {
+ if (arg0.getType().equals(Volume.Type.ROOT)){
+ return -1;
+ }
+ if(arg1.getType().equals(Volume.Type.ROOT)){
+ return 1;
+ }
+ if(arg0.getGroupNumber() > -1 && arg1.getGroupNumber() > -1){
+ return checkDiskSeq(arg0, arg1);
+ }
+ if(arg0.getGroupNumber() > -1){
Review Comment:
```suggestion
if (arg0.getGroupNumber() > -1) {
```
##########
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java:
##########
@@ -3566,12 +3568,31 @@ private static DiskTO[] sortVolumesByDeviceId(DiskTO[] volumes) {
@Override
public int compare(DiskTO arg0, DiskTO arg1) {
+ if (arg0.getType().equals(Volume.Type.ROOT)){
+ return -1;
+ }
+ if(arg1.getType().equals(Volume.Type.ROOT)){
+ return 1;
+ }
+ if(arg0.getGroupNumber() > -1 && arg1.getGroupNumber() > -1){
Review Comment:
```suggestion
if (arg0.getGroupNumber() > -1 && arg1.getGroupNumber() > -1) {
```
##########
server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java:
##########
@@ -281,6 +286,16 @@ protected VirtualMachineTO toVirtualMachineTO(VirtualMachineProfile vmProfile) {
return to;
}
+ private List<DiskTO> addGroupsToDisks(long vmId, List<DiskTO> disks){
+ disks.forEach((DiskTO disk) -> {
+ VolumeGroupVO volumeGroup = _volumeGroupDao.findByVmAndVolume(vmId, disk.getData().getId());
+ if(volumeGroup != null){
Review Comment:
```suggestion
if (volumeGroup != null) {
```
##########
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java:
##########
@@ -3566,12 +3568,31 @@ private static DiskTO[] sortVolumesByDeviceId(DiskTO[] volumes) {
@Override
public int compare(DiskTO arg0, DiskTO arg1) {
+ if (arg0.getType().equals(Volume.Type.ROOT)){
+ return -1;
+ }
+ if(arg1.getType().equals(Volume.Type.ROOT)){
+ return 1;
+ }
+ if(arg0.getGroupNumber() > -1 && arg1.getGroupNumber() > -1){
+ return checkDiskSeq(arg0, arg1);
+ }
+ if(arg0.getGroupNumber() > -1){
+ return -1;
+ }
+ if(arg1.getGroupNumber() > -1){
Review Comment:
```suggestion
if (arg1.getGroupNumber() > -1) {
```
--
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