Allon Mureinik has uploaded a new change for review.
Change subject: engine: Use isBlockDomain() where possible
......................................................................
engine: Use isBlockDomain() where possible
Replaced all the places that checked if a StorageType was ISCSI or FCP
with isBlockDomain().
This achieves two purposes:
1. The code is cleaner, and easier to read.
2. It protects against possible future bugs if a third block storage
type is added.
Change-Id: I60fc4cd333e954cc5075f3b3040054a7213a2f71
Signed-off-by: Allon Mureinik <amure...@redhat.com>
---
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmTemplateCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageDomainCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetVgListQuery.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommand.java
M
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageDomainResource.java
M
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/StorageDomainMapper.java
M
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java
M
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageModel.java
M
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewDiskModel.java
11 files changed, 32 insertions(+), 52 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/89/14189/1
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java
index 4aed69b..c335c65 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java
@@ -231,11 +231,11 @@
}
private boolean isExceedMaxBlockDiskSize() {
- StorageType storageType = getStorageDomain().getStorageType();
- boolean isBlockStorageDomain = storageType == StorageType.ISCSI ||
storageType == StorageType.FCP;
- boolean isRequestedLargerThanMaxSize = getRequestDiskSpace() >
Config.<Integer> GetValue(ConfigValues.MaxBlockDiskSize);
+ if (getStorageDomain().getStorageType().isBlockDomain()) {
+ return getRequestDiskSpace() > Config.<Integer>
GetValue(ConfigValues.MaxBlockDiskSize);
+ }
- return isBlockStorageDomain && isRequestedLargerThanMaxSize;
+ return true;
}
/**
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java
index 2bf55da..dea9807 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java
@@ -421,7 +421,7 @@
DiskImageBase diskInfo, List<String> messages) {
boolean result = true;
if ((diskInfo.getVolumeType() == VolumeType.Preallocated &&
diskInfo.getVolumeFormat() == VolumeFormat.COW)
- || ((storageDomain.getStorageType() == StorageType.FCP ||
storageDomain.getStorageType() == StorageType.ISCSI) && (diskInfo
+ || ((storageDomain.getStorageType().isBlockDomain()) &&
(diskInfo
.getVolumeType() == VolumeType.Sparse &&
diskInfo.getVolumeFormat() == VolumeFormat.RAW))
|| (diskInfo.getVolumeFormat() == VolumeFormat.Unassigned ||
diskInfo.getVolumeType() == VolumeType.Unassigned)) {
// not supported
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmTemplateCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmTemplateCommand.java
index 01baca7..8186694 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmTemplateCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmTemplateCommand.java
@@ -23,6 +23,7 @@
import org.ovirt.engine.core.common.businessentities.DiskImage;
import org.ovirt.engine.core.common.businessentities.DiskImageDynamic;
import org.ovirt.engine.core.common.businessentities.Entities;
+import org.ovirt.engine.core.common.businessentities.StorageDomain;
import org.ovirt.engine.core.common.businessentities.StorageDomainStatic;
import org.ovirt.engine.core.common.businessentities.StorageDomainType;
import org.ovirt.engine.core.common.businessentities.StorageType;
@@ -32,7 +33,6 @@
import org.ovirt.engine.core.common.businessentities.VolumeFormat;
import org.ovirt.engine.core.common.businessentities.VolumeType;
import org.ovirt.engine.core.common.businessentities.image_storage_domain_map;
-import org.ovirt.engine.core.common.businessentities.StorageDomain;
import org.ovirt.engine.core.common.businessentities.network.Network;
import
org.ovirt.engine.core.common.businessentities.network.VmNetworkInterface;
import
org.ovirt.engine.core.common.businessentities.network.VmNetworkStatistics;
@@ -226,8 +226,7 @@
* The image to check and change if needed.
*/
private void changeRawToCowIfSparseOnBlockDevice(StorageType storageType,
DiskImage image) {
- if ((storageType == StorageType.FCP
- || storageType == StorageType.ISCSI)
+ if (storageType.isBlockDomain()
&& image.getVolumeFormat() == VolumeFormat.RAW
&& image.getVolumeType() == VolumeType.Sparse) {
image.setvolumeFormat(VolumeFormat.COW);
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageDomainCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageDomainCommand.java
index 84a5473..72c93cb 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageDomainCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageDomainCommand.java
@@ -7,17 +7,17 @@
import java.util.Set;
import org.ovirt.engine.core.bll.Backend;
-import org.ovirt.engine.core.common.AuditLogType;
import org.ovirt.engine.core.bll.utils.PermissionSubject;
+import org.ovirt.engine.core.common.AuditLogType;
import org.ovirt.engine.core.common.VdcObjectType;
import org.ovirt.engine.core.common.action.StorageDomainManagementParameter;
import org.ovirt.engine.core.common.businessentities.SANState;
+import org.ovirt.engine.core.common.businessentities.StorageDomain;
+import org.ovirt.engine.core.common.businessentities.StorageDomainDynamic;
+import org.ovirt.engine.core.common.businessentities.StorageDomainStatic;
import org.ovirt.engine.core.common.businessentities.StorageDomainType;
import org.ovirt.engine.core.common.businessentities.StorageFormatType;
import org.ovirt.engine.core.common.businessentities.StorageType;
-import org.ovirt.engine.core.common.businessentities.StorageDomainDynamic;
-import org.ovirt.engine.core.common.businessentities.StorageDomainStatic;
-import org.ovirt.engine.core.common.businessentities.StorageDomain;
import org.ovirt.engine.core.common.businessentities.storage_pool;
import org.ovirt.engine.core.common.utils.Pair;
import org.ovirt.engine.core.common.validation.group.CreateEntity;
@@ -174,7 +174,7 @@
StorageType storageType = getStorageDomain().getStorageType();
StorageDomainType storageDomainFunction =
getStorageDomain().getStorageDomainType();
- boolean isBlockStorage = storageType == StorageType.ISCSI ||
storageType == StorageType.FCP;
+ boolean isBlockStorage = storageType.isBlockDomain();
boolean isDataStorageDomain = storageDomainFunction ==
StorageDomainType.Data;
// V2 is applicable only for block data storage domains
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetVgListQuery.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetVgListQuery.java
index 715732d..483e06a 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetVgListQuery.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetVgListQuery.java
@@ -4,7 +4,6 @@
import org.ovirt.engine.core.bll.Backend;
import org.ovirt.engine.core.bll.QueriesCommandBase;
-import org.ovirt.engine.core.common.businessentities.StorageType;
import org.ovirt.engine.core.common.businessentities.StorageDomain;
import org.ovirt.engine.core.common.queries.VdsIdParametersBase;
import org.ovirt.engine.core.common.vdscommands.VDSCommandType;
@@ -30,8 +29,7 @@
new Predicate<StorageDomain>() {
@Override
public boolean eval(StorageDomain storageDomain) {
- return storageDomain.getStorageType() ==
StorageType.ISCSI
- || storageDomain.getStorageType() ==
StorageType.FCP;
+ return storageDomain.getStorageType().isBlockDomain();
}
});
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommand.java
index 504dd3b..7c14054 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommand.java
@@ -106,8 +106,7 @@
}
StorageType spType = storagePool.getstorage_pool_type();
- if (targetFormat == StorageFormatType.V2
- && spType != StorageType.ISCSI && spType != StorageType.FCP) {
+ if (targetFormat == StorageFormatType.V2 && !spType.isBlockDomain()) {
// There is no format V2 for domains that aren't ISCSI/FCP
return;
}
diff --git
a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageDomainResource.java
b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageDomainResource.java
index 1ab745d..596b0c7 100644
---
a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageDomainResource.java
+++
b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageDomainResource.java
@@ -1,5 +1,7 @@
package org.ovirt.engine.api.restapi.resource;
+import static
org.ovirt.engine.api.restapi.resource.BackendStorageDomainsResource.SUB_COLLECTIONS;
+
import java.util.ArrayList;
import java.util.LinkedList;
import java.util.List;
@@ -16,9 +18,10 @@
import org.ovirt.engine.api.model.VMs;
import org.ovirt.engine.api.resource.AssignedPermissionsResource;
import org.ovirt.engine.api.resource.DisksResource;
-import org.ovirt.engine.api.resource.RemovableStorageDomainContentsResource;
import org.ovirt.engine.api.resource.FilesResource;
+import org.ovirt.engine.api.resource.RemovableStorageDomainContentsResource;
import org.ovirt.engine.api.resource.StorageDomainResource;
+import org.ovirt.engine.api.restapi.util.StorageDomainHelper;
import org.ovirt.engine.core.common.VdcObjectType;
import org.ovirt.engine.core.common.action.ExtendSANStorageDomainParameters;
import org.ovirt.engine.core.common.action.StorageDomainManagementParameter;
@@ -26,20 +29,16 @@
import org.ovirt.engine.core.common.action.VdcActionParametersBase;
import org.ovirt.engine.core.common.action.VdcActionType;
import org.ovirt.engine.core.common.businessentities.StorageDomainSharedStatus;
-import org.ovirt.engine.core.common.businessentities.VDS;
-
import org.ovirt.engine.core.common.businessentities.StorageDomainStatic;
import org.ovirt.engine.core.common.businessentities.StorageServerConnections;
+import org.ovirt.engine.core.common.businessentities.StorageType;
+import org.ovirt.engine.core.common.businessentities.VDS;
import org.ovirt.engine.core.common.interfaces.SearchType;
import org.ovirt.engine.core.common.queries.GetDeviceListQueryParameters;
import org.ovirt.engine.core.common.queries.GetPermissionsForObjectParameters;
import org.ovirt.engine.core.common.queries.StorageDomainQueryParametersBase;
import org.ovirt.engine.core.common.queries.VdcQueryType;
import org.ovirt.engine.core.compat.Guid;
-import org.ovirt.engine.core.common.businessentities.StorageType;
-import org.ovirt.engine.api.restapi.util.StorageDomainHelper;
-
-import static
org.ovirt.engine.api.restapi.resource.BackendStorageDomainsResource.SUB_COLLECTIONS;
public class BackendStorageDomainResource extends
AbstractBackendSubResource<StorageDomain,
org.ovirt.engine.core.common.businessentities.StorageDomain> implements
StorageDomainResource {
@@ -68,15 +67,8 @@
org.ovirt.engine.core.common.businessentities.StorageDomain entity =
getEntity(storageDomainResolver, true);
StorageDomain model = map(entity, new StorageDomain());
StorageType storageType = entity.getStorageType();
- if (storageType != null) {
- switch (storageType) {
- case ISCSI:
- case FCP:
+ if (storageType != null && storageType.isBlockDomain()) {
extendStorageDomain(incoming, model, storageType);
- break;
- default:
- break;
- }
}
return addLinks(performUpdate(incoming,
diff --git
a/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/StorageDomainMapper.java
b/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/StorageDomainMapper.java
index 30e0005..95d55b5 100644
---
a/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/StorageDomainMapper.java
+++
b/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/StorageDomainMapper.java
@@ -10,9 +10,9 @@
import org.ovirt.engine.api.model.StorageType;
import org.ovirt.engine.api.model.VolumeGroup;
import org.ovirt.engine.api.restapi.model.StorageFormat;
+import org.ovirt.engine.api.restapi.utils.GuidUtils;
import org.ovirt.engine.core.common.businessentities.StorageDomainStatic;
import org.ovirt.engine.core.common.businessentities.StorageServerConnections;
-import org.ovirt.engine.api.restapi.utils.GuidUtils;
public class StorageDomainMapper {
@@ -120,8 +120,7 @@
}
model.setStorage(new Storage());
model.getStorage().setType(map(entity.getStorageType(), null));
- if (entity.getStorageType() ==
org.ovirt.engine.core.common.businessentities.StorageType.ISCSI ||
- entity.getStorageType() ==
org.ovirt.engine.core.common.businessentities.StorageType.FCP) {
+ if (entity.getStorageType().isBlockDomain()) {
model.getStorage().setVolumeGroup(new VolumeGroup());
model.getStorage().getVolumeGroup().setId(entity.getStorage());
}
diff --git
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java
index 2dc547f..5a937f7 100644
---
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java
+++
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java
@@ -968,15 +968,9 @@
}
public static VolumeFormat GetDiskVolumeFormat(VolumeType volumeType,
StorageType storageType) {
- switch (storageType) {
- case NFS:
- case LOCALFS:
- case POSIXFS:
- case GLUSTERFS:
+ if (storageType.isFileDomain()) {
return VolumeFormat.RAW;
-
- case ISCSI:
- case FCP:
+ } else if (storageType.isBlockDomain()) {
switch (volumeType) {
case Sparse:
return VolumeFormat.COW;
@@ -987,8 +981,7 @@
default:
return VolumeFormat.Unassigned;
}
-
- default:
+ } else {
return VolumeFormat.Unassigned;
}
}
diff --git
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageModel.java
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageModel.java
index a552cde..d1b361d 100644
---
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageModel.java
+++
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageModel.java
@@ -681,12 +681,12 @@
{
formats.add(StorageFormatType.V1);
}
- else if ((getSelectedItem().getType() == StorageType.ISCSI ||
getSelectedItem().getType() == StorageType.FCP)
+ else if (getSelectedItem().getType().isBlockDomain()
&&
dataCenter.getcompatibility_version().compareTo(Version.v3_0) < 0)
{
formats.add(StorageFormatType.V1);
}
- else if ((getSelectedItem().getType() == StorageType.ISCSI ||
getSelectedItem().getType() == StorageType.FCP)
+ else if (getSelectedItem().getType().isBlockDomain()
&&
dataCenter.getcompatibility_version().compareTo(Version.v3_0) == 0)
{
formats.add(StorageFormatType.V2);
diff --git
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewDiskModel.java
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewDiskModel.java
index 2350b56..e2bceec 100644
---
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewDiskModel.java
+++
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewDiskModel.java
@@ -10,9 +10,9 @@
import org.ovirt.engine.core.common.businessentities.DiskInterface;
import org.ovirt.engine.core.common.businessentities.LUNs;
import org.ovirt.engine.core.common.businessentities.LunDisk;
+import org.ovirt.engine.core.common.businessentities.StorageDomain;
import org.ovirt.engine.core.common.businessentities.StoragePoolStatus;
import org.ovirt.engine.core.common.businessentities.StorageType;
-import org.ovirt.engine.core.common.businessentities.StorageDomain;
import org.ovirt.engine.core.common.businessentities.VolumeType;
import org.ovirt.engine.core.common.businessentities.storage_pool;
import org.ovirt.engine.core.common.queries.ConfigurationValues;
@@ -127,7 +127,7 @@
}
else {
getIsWipeAfterDelete().setIsChangable(true);
- getIsWipeAfterDelete().setEntity((Boolean)
AsyncDataProvider.GetConfigValuePreConverted(ConfigurationValues.SANWipeAfterDelete));
+
getIsWipeAfterDelete().setEntity(AsyncDataProvider.GetConfigValuePreConverted(ConfigurationValues.SANWipeAfterDelete));
}
}
@@ -179,7 +179,7 @@
AddDiskParameters parameters = new AddDiskParameters(getVmId(),
getDisk());
if ((Boolean) getIsInternal().getEntity()) {
StorageDomain storageDomain = (StorageDomain)
getStorageDomain().getSelectedItem();
- ((AddDiskParameters)
parameters).setStorageDomainId(storageDomain.getId());
+ parameters.setStorageDomainId(storageDomain.getId());
}
Frontend.RunAction(VdcActionType.AddDisk, parameters, new
IFrontendActionAsyncCallback() {
@@ -223,7 +223,7 @@
: ((StorageDomain)
getStorageDomain().getSelectedItem()).getStorageType();
IntegerValidation sizeValidation = new IntegerValidation();
sizeValidation.setMinimum(1);
- if (storageType == StorageType.ISCSI || storageType ==
StorageType.FCP) {
+ if (storageType.isBlockDomain()) {
sizeValidation.setMaximum((Integer)
AsyncDataProvider.GetConfigValuePreConverted(ConfigurationValues.MaxBlockDiskSize));
}
getSize().validateEntity(new IValidation[] { new NotEmptyValidation(),
sizeValidation });
--
To view, visit http://gerrit.ovirt.org/14189
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I60fc4cd333e954cc5075f3b3040054a7213a2f71
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <amure...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
