Maor Lipchuk has uploaded a new change for review. Change subject: core: Fix NPE when scanning for non-existent domain. ......................................................................
core: Fix NPE when scanning for non-existent domain. Adding a validation in GetUnregisteredDiskQuery for existence of storage domain by the storage pool id and storage domain id, transferred in the parameters. Change-Id: I7c722ca3b003efd89ae5207bc941d6926b5f052c Bug-Url: https://bugzilla.redhat.com/954344 Signed-off-by: Maor Lipchuk <mlipc...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUnregisteredDiskQuery.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUnregisteredDisksQuery.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/GetUnregisteredDisksQueryTest.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java M backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties M frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java M frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties M frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties 8 files changed, 32 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/47/26247/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUnregisteredDiskQuery.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUnregisteredDiskQuery.java index dedc083..5ab0840 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUnregisteredDiskQuery.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUnregisteredDiskQuery.java @@ -4,6 +4,7 @@ import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.DiskInterface; +import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.common.queries.GetUnregisteredDiskQueryParameters; import org.ovirt.engine.core.common.vdscommands.GetImageInfoVDSCommandParameters; import org.ovirt.engine.core.common.vdscommands.StoragePoolDomainAndGroupIdBaseVDSCommandParameters; @@ -22,6 +23,12 @@ Guid storagePoolId = getParameters().getStoragePoolId(); Guid storageDomainId = getParameters().getStorageDomainId(); Guid diskId = getParameters().getDiskId(); + if (getDbFacade().getStorageDomainDao().getForStoragePool(storageDomainId, storagePoolId) == null) { + getQueryReturnValue().setExceptionString(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_EXIST_IN_DATA_CENTER.toString()); + getQueryReturnValue().setSucceeded(false); + return; + } + // Now get the list of volumes for each new image. StoragePoolDomainAndGroupIdBaseVDSCommandParameters getVolumesParameters = new StoragePoolDomainAndGroupIdBaseVDSCommandParameters( storagePoolId, storageDomainId, diskId); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUnregisteredDisksQuery.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUnregisteredDisksQuery.java index 9616ee6..196b47d 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUnregisteredDisksQuery.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetUnregisteredDisksQuery.java @@ -5,6 +5,7 @@ import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.DiskImage; +import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.common.interfaces.VDSBrokerFrontend; import org.ovirt.engine.core.common.queries.GetUnregisteredDiskQueryParameters; import org.ovirt.engine.core.common.queries.GetUnregisteredDisksQueryParameters; @@ -23,9 +24,15 @@ @Override protected void executeQueryCommand() { + VDSBrokerFrontend vdsBroker = getVdsBroker(); + if (getDbFacade().getStorageDomainDao().getForStoragePool(getStorageDomainId(), getStoragePoolId()) == null) { + getQueryReturnValue().setExceptionString(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_EXIST_IN_DATA_CENTER.toString()); + getQueryReturnValue().setSucceeded(false); + return; + } + // first, run getImagesList query into vdsm to get all of the images on the storage domain - then store in // imagesList - VDSBrokerFrontend vdsBroker = getVdsBroker(); VDSReturnValue imagesListResult = vdsBroker.RunVdsCommand(VDSCommandType.GetImagesList, new GetImagesListVDSCommandParameters(getStorageDomainId(), getStoragePoolId())); @SuppressWarnings("unchecked") diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/GetUnregisteredDisksQueryTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/GetUnregisteredDisksQueryTest.java index 4b27fac..bdadaa8 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/GetUnregisteredDisksQueryTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/GetUnregisteredDisksQueryTest.java @@ -20,6 +20,7 @@ import org.ovirt.engine.core.bll.interfaces.BackendInternal; import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.DiskImage; +import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.interfaces.VDSBrokerFrontend; import org.ovirt.engine.core.common.queries.GetUnregisteredDiskQueryParameters; import org.ovirt.engine.core.common.queries.GetUnregisteredDisksQueryParameters; @@ -31,6 +32,7 @@ import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.dao.DiskImageDAO; +import org.ovirt.engine.core.dao.StorageDomainDAO; @RunWith(MockitoJUnitRunner.class) public class GetUnregisteredDisksQueryTest @@ -60,6 +62,11 @@ @Test public void testGetUnregisteredDisks() { + DbFacade dbFacadeMock = getDbFacadeMockInstance(); + StorageDomainDAO storageDomainDAOMock = mock(StorageDomainDAO.class); + when(dbFacadeMock.getStorageDomainDao()).thenReturn(storageDomainDAOMock); + StorageDomain storageDomain = new StorageDomain(); + when(storageDomainDAOMock.getForStoragePool(storageDomainId, storagePoolId)).thenReturn(storageDomain); // Execute query getQuery().executeQueryCommand(); @@ -111,8 +118,11 @@ DbFacade dbFacadeMock = getDbFacadeMockInstance(); DiskImageDAO diskImageDAOMock = mock(DiskImageDAO.class); + StorageDomainDAO storageDomainDAOMock = mock(StorageDomainDAO.class); when(diskImageDAOMock.getAllSnapshotsForStorageDomain(eq(storageDomainId))).thenReturn(existingDiskImages); when(dbFacadeMock.getDiskImageDao()).thenReturn(diskImageDAOMock); + StorageDomain storageDomain = new StorageDomain(); + when(storageDomainDAOMock.getForStoragePool(storageDomainId, storagePoolId)).thenReturn(storageDomain); // Return the mocked backend when getBackend() is called on the query doReturn(backendMock).when(getQuery()).getBackend(); diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java index 607c170..7bf9d19 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java @@ -535,6 +535,7 @@ ACTION_TYPE_FAILED_LUNS_ALREADY_USED_BY_DISKS(ErrorType.CONFLICT), ACTION_TYPE_FAILED_STORAGE_POOL_NOT_EXIST(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_EXIST(ErrorType.BAD_PARAMETERS), + ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_EXIST_IN_DATA_CENTER(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_STORAGE_CONNECTION_NOT_EXIST(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_STORAGE_CONNECTION_FOR_DOMAIN_NOT_EXIST(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_STORAGE_CONNECTION_ALREADY_EXISTS(ErrorType.CONFLICT), diff --git a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties index de7341d..409d01d 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -445,6 +445,7 @@ ACTION_TYPE_FAILED_STORAGE_CONNECTION_WRONG_PARAMETERS_FOR_STORAGE_TYPE=Cannot ${action} ${type}. Connection parameters are invalid for this storage type. ACTION_TYPE_FAILED_STORAGE_CONNECTION_UNSUPPORTED_CHANGE_STORAGE_TYPE=Cannot ${action} ${type}. Storage type cannot be edited. ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_EXIST=Cannot ${action} ${type}. Storage Domain doesn't exist. +ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_EXIST_IN_DATA_CENTER=Storage Domain doesn't exist in Data Center. ACTION_TYPE_FAILED_CANNOT_CHANGE_STORAGE_DOMAIN_TYPE=Cannot ${action} ${type}. Cannot change Storage Domain type. ACTION_TYPE_FAILED_STORAGE_DOMAIN_STATUS_ILLEGAL=Cannot ${action} ${type}. The relevant Storage Domain is inaccessible.\n\ -Please handle Storage Domain issues and retry the operation. diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java index 9303a7a..1d66f7d 100644 --- a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java @@ -1234,6 +1234,9 @@ @DefaultStringValue("Cannot ${action} ${type}. Storage Domain doesn't exist.") String ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_EXIST(); + @DefaultStringValue("Storage Domain doesn't exist in Data Center.") + String ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_EXIST_IN_DATA_CENTER(); + @DefaultStringValue("Cannot ${action} ${type}. Cannot change Storage Domain type.") String ACTION_TYPE_FAILED_CANNOT_CHANGE_STORAGE_DOMAIN_TYPE(); diff --git a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties index 1420b70..8c4045b 100644 --- a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties +++ b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties @@ -418,6 +418,7 @@ ACTION_TYPE_FAILED_STORAGE_CONNECTION_WRONG_PARAMETERS_FOR_STORAGE_TYPE=Cannot ${action} ${type}. Connection parameters are invalid for this storage type. ACTION_TYPE_FAILED_STORAGE_CONNECTION_UNSUPPORTED_CHANGE_STORAGE_TYPE=Cannot ${action} ${type}. Storage type cannot be edited. ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_EXIST=Cannot ${action} ${type}. Storage Domain doesn't exist. +ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_EXIST_IN_DATA_CENTER=Storage Domain doesn't exist in Data Center. ACTION_TYPE_FAILED_CANNOT_CHANGE_STORAGE_DOMAIN_TYPE=Cannot ${action} ${type}. Cannot change Storage Domain type. ACTION_TYPE_FAILED_STORAGE_DOMAIN_STATUS_ILLEGAL=Cannot ${action} ${type}. The relevant Storage Domain is inaccessible.\n\ -Please handle Storage Domain issues and retry the operation. diff --git a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties index bdec5e5..920e71a 100644 --- a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties +++ b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties @@ -449,6 +449,7 @@ ACTION_TYPE_FAILED_STORAGE_CONNECTION_WRONG_PARAMETERS_FOR_STORAGE_TYPE=Cannot ${action} ${type}. Connection parameters are invalid for this storage type. ACTION_TYPE_FAILED_STORAGE_CONNECTION_UNSUPPORTED_CHANGE_STORAGE_TYPE=Cannot ${action} ${type}. Storage type cannot be edited. ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_EXIST=Cannot ${action} ${type}. Storage Domain doesn't exist. +ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_EXIST_IN_DATA_CENTER=Storage Domain doesn't exist in Data Center. ACTION_TYPE_FAILED_CANNOT_CHANGE_STORAGE_DOMAIN_TYPE=Cannot ${action} ${type}. Cannot change Storage Domain type. ACTION_TYPE_FAILED_STORAGE_DOMAIN_STATUS_ILLEGAL=Cannot ${action} ${type}. The relevant Storage Domain is inaccessible.\n\ -Please handle Storage Domain issues and retry the operation. -- To view, visit http://gerrit.ovirt.org/26247 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I7c722ca3b003efd89ae5207bc941d6926b5f052c Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.4 Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
