Arik Hadas has uploaded a new change for review.
Change subject: core: change the visibility of validation method in
RunVmValidator
......................................................................
core: change the visibility of validation method in RunVmValidator
The visibility of validation methods in RunVmValidator that should not
be invoked from outside of this class is changed to 'protected'. Those
are the validation methods that gets all the data they need for their
checks as parameters (inversion of control principle, to make them more
testable).
In addition, hasSpaceForSnapshots method is moved to the utility methods
section as it is just a utility method used by the validateStatelessVm
validation.
Change-Id: Iee1ef7ed2f812cf929328c8219ed95e98d2959ae
Signed-off-by: Arik Hadas <aha...@redhat.com>
---
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
1 file changed, 58 insertions(+), 53 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/44/18244/1
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
index 981dc00..8d7d5a7 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
@@ -79,6 +79,11 @@
protected RunVmValidator() {
}
+
+ //////////////////////////////
+ /// API validation methods ///
+ //////////////////////////////
+
/**
* A general method for run vm validations. used in runVmCommand and in
VmPoolCommandBase
* @param messages
@@ -116,11 +121,33 @@
return true;
}
- //////////////////////////
- /// Validation methods ///
- //////////////////////////
+ /**
+ * @return true if all VM network interfaces are valid
+ */
+ public ValidationResult validateNetworkInterfaces() {
+ ValidationResult validationResult = validateInterfacesConfigured(vm);
+ if (!validationResult.isValid()) {
+ return validationResult;
+ }
- public boolean validateVmProperties(VM vm, List<String> messages) {
+ validationResult = validateInterfacesAttachedToClusterNetworks(vm,
getClusterNetworksNames(), getInterfaceNetworkNames());
+ if (!validationResult.isValid()) {
+ return validationResult;
+ }
+
+ validationResult =
validateInterfacesAttachedToVmNetworks(getClusterNetworks(),
getInterfaceNetworkNames());
+ if (!validationResult.isValid()) {
+ return validationResult;
+ }
+
+ return ValidationResult.VALID;
+ }
+
+ ///////////////////////////////////
+ /// Internal validation methods ///
+ ///////////////////////////////////
+
+ protected boolean validateVmProperties(VM vm, List<String> messages) {
List<ValidationError> validationErrors =
getVmPropertiesUtils().validateVMProperties(
vm.getVdsGroupCompatibilityVersion(),
@@ -134,7 +161,7 @@
return true;
}
- public ValidationResult validateBootSequence(VM vm, BootSequence
bootSequence, List<Disk> vmDisks) {
+ protected ValidationResult validateBootSequence(VM vm, BootSequence
bootSequence, List<Disk> vmDisks) {
BootSequence boot_sequence = (bootSequence != null) ?
bootSequence : vm.getDefaultBootSequence();
Guid storagePoolId = vm.getStoragePoolId();
@@ -174,7 +201,7 @@
* The VM's image disks
* @return <code>true</code> if the VM can be run, <code>false</code> if
not
*/
- public ValidationResult validateStorageDomains(VM vm, boolean
isInternalExecution,
+ protected ValidationResult validateStorageDomains(VM vm, boolean
isInternalExecution,
List<DiskImage> vmImages) {
if (vmImages.isEmpty()) {
return ValidationResult.VALID;
@@ -203,7 +230,7 @@
/**
* Check isValid only if VM is not HA VM
*/
- public ValidationResult validateImagesForRunVm(VM vm, List<DiskImage>
vmDisks) {
+ protected ValidationResult validateImagesForRunVm(VM vm, List<DiskImage>
vmDisks) {
if (vmDisks.isEmpty()) {
return ValidationResult.VALID;
}
@@ -212,7 +239,7 @@
new DiskImagesValidator(vmDisks).diskImagesNotLocked() :
ValidationResult.VALID;
}
- public ValidationResult validateIsoPath(VM vm, String diskPath, String
floppyPath) {
+ protected ValidationResult validateIsoPath(VM vm, String diskPath, String
floppyPath) {
if (vm.isAutoStartup() || (StringUtils.isEmpty(diskPath) &&
StringUtils.isEmpty(floppyPath))) {
return ValidationResult.VALID;
}
@@ -233,7 +260,7 @@
return ValidationResult.VALID;
}
- public ValidationResult vmDuringInitialization(VM vm) {
+ protected ValidationResult vmDuringInitialization(VM vm) {
if (vm.isRunning() || vm.getStatus() == VMStatus.NotResponding ||
isVmDuringInitiating(vm)) {
return new
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_RUNNING);
@@ -242,7 +269,7 @@
return ValidationResult.VALID;
}
- public ValidationResult validateVdsStatus(VM vm) {
+ protected ValidationResult validateVdsStatus(VM vm) {
if (vm.getStatus() == VMStatus.Paused && vm.getRunOnVds() != null) {
VDS vds = getVdsDao().get(new Guid(vm.getRunOnVds().toString()));
if (vds.getStatus() != VDSStatus.Up) {
@@ -254,7 +281,7 @@
return ValidationResult.VALID;
}
- public ValidationResult validateStatelessVm(VM vm, List<Disk> plugDisks,
Boolean stateless) {
+ protected ValidationResult validateStatelessVm(VM vm, List<Disk>
plugDisks, Boolean stateless) {
// if the VM is not stateless, there is nothing to check
if (stateless != null ? !stateless : !vm.isStateless()) {
return ValidationResult.VALID;
@@ -278,29 +305,10 @@
return ValidationResult.VALID;
}
- public ValidationResult validateVmStatusUsingMatrix(VM vm) {
+ protected ValidationResult validateVmStatusUsingMatrix(VM vm) {
if (!VdcActionUtils.CanExecute(Arrays.asList(vm), VM.class,
VdcActionType.RunVm)) {
return new
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_VM_STATUS_ILLEGAL);
- }
-
- return ValidationResult.VALID;
- }
-
- /**
- * check that we can create snapshots for all disks
- * @param vm
- * @return true if all storage domains have enough space to create
snapshots for this VM plugged disks
- */
- public ValidationResult hasSpaceForSnapshots(VM vm, List<Disk> plugDisks) {
- Integer minSnapshotSize = Config.<Integer>
GetValue(ConfigValues.InitStorageSparseSizeInGB);
- Map<StorageDomain, Integer> mapStorageDomainsToNumOfDisks =
mapStorageDomainsToNumOfDisks(vm, plugDisks);
- for (Entry<StorageDomain, Integer> e :
mapStorageDomainsToNumOfDisks.entrySet()) {
- ValidationResult validationResult =
- new
StorageDomainValidator(e.getKey()).isDomainHasSpaceForRequest(minSnapshotSize *
e.getValue());
- if (!validationResult.isValid()) {
- return validationResult;
- }
}
return ValidationResult.VALID;
@@ -312,28 +320,6 @@
}
return new StoragePoolValidator(storagePool).isUp();
- }
-
- /**
- * @return true if all VM network interfaces are valid
- */
- public ValidationResult validateNetworkInterfaces() {
- ValidationResult validationResult = validateInterfacesConfigured(vm);
- if (!validationResult.isValid()) {
- return validationResult;
- }
-
- validationResult = validateInterfacesAttachedToClusterNetworks(vm,
getClusterNetworksNames(), getInterfaceNetworkNames());
- if (!validationResult.isValid()) {
- return validationResult;
- }
-
- validationResult =
validateInterfacesAttachedToVmNetworks(getClusterNetworks(),
getInterfaceNetworkNames());
- if (!validationResult.isValid()) {
- return validationResult;
- }
-
- return ValidationResult.VALID;
}
/**
@@ -527,4 +513,23 @@
}
return cachedClusterNetworksNames;
}
+
+ /**
+ * check that we can create snapshots for all disks
+ * @param vm
+ * @return true if all storage domains have enough space to create
snapshots for this VM plugged disks
+ */
+ public ValidationResult hasSpaceForSnapshots(VM vm, List<Disk> plugDisks) {
+ Integer minSnapshotSize = Config.<Integer>
GetValue(ConfigValues.InitStorageSparseSizeInGB);
+ Map<StorageDomain, Integer> mapStorageDomainsToNumOfDisks =
mapStorageDomainsToNumOfDisks(vm, plugDisks);
+ for (Entry<StorageDomain, Integer> e :
mapStorageDomainsToNumOfDisks.entrySet()) {
+ ValidationResult validationResult =
+ new
StorageDomainValidator(e.getKey()).isDomainHasSpaceForRequest(minSnapshotSize *
e.getValue());
+ if (!validationResult.isValid()) {
+ return validationResult;
+ }
+ }
+
+ return ValidationResult.VALID;
+ }
}
--
To view, visit http://gerrit.ovirt.org/18244
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iee1ef7ed2f812cf929328c8219ed95e98d2959ae
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <aha...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
