Skip to content

Commit

Permalink
core: create "MultipleStorageDomainsValidator" depending on the flow
Browse files Browse the repository at this point in the history
Changing "createMultipleStorageDomainsValidator()" to be protected
in the "MoveOrCopyDiskCommand" class and overriding it in its derived
"LiveMigrateDiskCommand" class to add also the source Storage Domain ID.
Thus no need to check in the parent "MoveOrCopyDiskCommand" class
whether it's an LSM flow or not, in order to add also the source Storage
Domain ID to be checked by the created "MultipleStorageDomainsValidator".

Also removing the redundant "isDomainWithinThresholds()" destination
Storage Domain validation from the "LiveMigrateDiskCommand" class, since
it is correctly performed by the above "MultipleStorageDomainsValidator"
that validates both source & destination Storage Domains from its parent's
"MoveOrCopyDiskCommand::validateSpaceRequirements()".

Signed-off-by: Pavel Bar <pbar@redhat.com>
Bug-Url: https://bugzilla.redhat.com/2116309
  • Loading branch information
barpavel committed Aug 31, 2022
1 parent 49bbf1b commit 2b95d46
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -270,15 +270,7 @@ protected boolean validateSpaceRequirements() {
return true;
}

List<Guid> sdsToValidate = new ArrayList<>();
sdsToValidate.add(getStorageDomainId());

if (getActionType() == ActionType.LiveMigrateDisk) {
sdsToValidate.add(getParameters().getSourceDomainId());
}

MultipleStorageDomainsValidator storageDomainsValidator = createMultipleStorageDomainsValidator(sdsToValidate);

MultipleStorageDomainsValidator storageDomainsValidator = createMultipleStorageDomainsValidator();
if (!validate(storageDomainsValidator.allDomainsWithinThresholds())) {
return false;
}
Expand Down Expand Up @@ -719,7 +711,10 @@ protected MultipleDiskVmElementValidator createMultipleDiskVmElementValidator()
diskVmElementDao.getAllDiskVmElementsByDiskId(getParameters().getImageGroupID()));
}

public MultipleStorageDomainsValidator createMultipleStorageDomainsValidator(List<Guid> sdsToValidate) {
protected MultipleStorageDomainsValidator createMultipleStorageDomainsValidator() {
List<Guid> sdsToValidate = new ArrayList<>();
sdsToValidate.add(getStorageDomainId());

return new MultipleStorageDomainsValidator(getStoragePoolId(), sdsToValidate);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.ovirt.engine.core.bll.storage.lsm;

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
Expand All @@ -25,6 +26,7 @@
import org.ovirt.engine.core.bll.tasks.CommandHelper;
import org.ovirt.engine.core.bll.tasks.interfaces.CommandCallback;
import org.ovirt.engine.core.bll.utils.PermissionSubject;
import org.ovirt.engine.core.bll.validator.storage.MultipleStorageDomainsValidator;
import org.ovirt.engine.core.bll.validator.storage.StorageDomainValidator;
import org.ovirt.engine.core.common.AuditLogType;
import org.ovirt.engine.core.common.FeatureSupported;
Expand Down Expand Up @@ -86,7 +88,8 @@
import org.ovirt.engine.core.vdsbroker.builder.vminfo.VmInfoBuildUtils;

@NonTransactiveCommandAttribute(forceCompensation = true)
public class LiveMigrateDiskCommand<T extends LiveMigrateDiskParameters> extends MoveOrCopyDiskCommand<T>implements SerialChildExecutingCommand {
public class LiveMigrateDiskCommand<T extends LiveMigrateDiskParameters> extends MoveOrCopyDiskCommand<T>
implements SerialChildExecutingCommand {

private Guid sourceQuotaId;
private Guid sourceDiskProfileId;
Expand Down Expand Up @@ -658,10 +661,6 @@ private StorageDomain getDstStorageDomain() {
}

protected boolean validateDestDomainsSpaceRequirements() {
if (!isStorageDomainWithinThresholds(getDstStorageDomain())) {
return false;
}

DiskImage diskImage = getDiskImageByImageId(getParameters().getImageId());
List<DiskImage> allImageSnapshots = diskImageDao.getAllSnapshotsForLeaf(diskImage.getImageId());
diskImage.getSnapshots().addAll(allImageSnapshots);
Expand All @@ -674,8 +673,14 @@ protected boolean validateDestDomainsSpaceRequirements() {
return true;
}

protected boolean isStorageDomainWithinThresholds(StorageDomain storageDomain) {
return validate(new StorageDomainValidator(storageDomain).isDomainWithinThresholds());
@Override
protected MultipleStorageDomainsValidator createMultipleStorageDomainsValidator() {
List<Guid> sdsToValidate = new ArrayList<>();

sdsToValidate.add(getParameters().getSourceStorageDomainId());
sdsToValidate.add(getParameters().getTargetStorageDomainId());

return new MultipleStorageDomainsValidator(getStoragePoolId(), sdsToValidate);
}

private DiskImage getDiskImageByImageId(Guid imageId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ protected void initializeCommand(DiskImage disk, VmEntityType vmEntityType) {
vm.setStatus(VMStatus.Down);
when(vmDao.get(any())).thenReturn(vm);

doReturn(multipleStorageDomainsValidator).when(command).createMultipleStorageDomainsValidator(any());
doReturn(multipleStorageDomainsValidator).when(command).createMultipleStorageDomainsValidator();
doReturn(multipleDiskVmElementValidator).when(command).createMultipleDiskVmElementValidator();
doReturn(diskValidator).when(command).createDiskValidator(disk);
doReturn(diskImagesValidator).when(command).createDiskImagesValidator(disk);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,6 @@ private void initStoragePool() {
private void mockValidators() {
doReturn(diskValidator).when(command).createDiskValidator(any());
doReturn(diskImagesValidator).when(command).createDiskImagesValidator(any());
doReturn(multipleStorageDomainsValidator).when(command).createMultipleStorageDomainsValidator(any());
doReturn(multipleStorageDomainsValidator).when(command).createMultipleStorageDomainsValidator();
}
}

0 comments on commit 2b95d46

Please sign in to comment.