Maor Lipchuk has uploaded a new change for review. Change subject: core: Add storage validation when plugging an image. ......................................................................
core: Add storage validation when plugging an image. When a plugging a disk to a VM, the engine should validate if the storage domain of the disk is active. There is no reason to call VDSM if it can't connect to the storage. The validation should be done on plug disk, and also on its derived operation attachDisk. On attach disk I added an audit log which will warn that the plug could not be performed (we still add an event of success since attach should work). Change-Id: Ieeb2522bc5aa280b9b98ef728737aeeaa82d1263 Bug-Url: https://bugzilla.redhat.com/969767 Signed-off-by: Maor Lipchuk <mlipc...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AttachDiskToVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommand.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java M backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties 5 files changed, 41 insertions(+), 5 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/70/17170/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AttachDiskToVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AttachDiskToVmCommand.java index 7a4db06..9addecb 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AttachDiskToVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AttachDiskToVmCommand.java @@ -8,6 +8,7 @@ import org.ovirt.engine.core.bll.utils.PermissionSubject; import org.ovirt.engine.core.bll.utils.VmDeviceUtils; import org.ovirt.engine.core.bll.validator.DiskValidator; +import org.ovirt.engine.core.bll.validator.StorageDomainValidator; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.VdcObjectType; import org.ovirt.engine.core.common.action.AttachDettachVmDiskParameters; @@ -17,6 +18,7 @@ import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.ImageStatus; import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotType; +import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.StoragePoolIsoMapId; import org.ovirt.engine.core.common.businessentities.VMStatus; import org.ovirt.engine.core.common.businessentities.VmDevice; @@ -24,12 +26,13 @@ import org.ovirt.engine.core.common.businessentities.VmDeviceId; import org.ovirt.engine.core.common.errors.VdcBLLException; import org.ovirt.engine.core.common.errors.VdcBllErrors; -import org.ovirt.engine.core.common.locks.LockingGroup; import org.ovirt.engine.core.common.errors.VdcBllMessages; +import org.ovirt.engine.core.common.locks.LockingGroup; import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.common.utils.VmDeviceType; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector; @LockIdNameAttribute public class AttachDiskToVmCommand<T extends AttachDettachVmDiskParameters> extends AbstractDiskVmCommand<T> { @@ -118,7 +121,7 @@ protected void executeVmCommand() { getVmStaticDAO().incrementDbGeneration(getVm().getId()); final VmDevice vmDevice = createVmDevice(); - getVmDeviceDao().save(vmDevice); + // update cached image List<Disk> imageList = new ArrayList<Disk>(); imageList.add(disk); @@ -128,7 +131,22 @@ } // update vm device boot order updateBootOrderInVmDevice(); - if (getParameters().isPlugUnPlug() && getVm().getStatus() != VMStatus.Down) { + if (disk.getDiskStorageType() == DiskStorageType.IMAGE) { + StorageDomain storageDomain = getStorageDomainDAO().getForStoragePool( + ((DiskImage) disk).getStorageIds().get(0), ((DiskImage) disk).getStoragePoolId()); + StorageDomainValidator storageDomainValidator = new StorageDomainValidator(storageDomain); + if (!validate(storageDomainValidator.isDomainExistAndActive())) { + log.errorFormat("The disk storage domain {0} is not active, Disk could not peform hot plug", + storageDomain.getId()); + this.addCustomValue("DiskAlias", disk.getDiskAlias()); + this.addCustomValue("VmName", getVm().getName()); + this.addCustomValue("StorageDomain", storageDomain.getName()); + AuditLogDirector.log(this, AuditLogType.USER_FAILED_HOTPLUG_DISK_STORAGE_INACTIVE); + } + vmDevice.setIsPlugged(Boolean.FALSE); + getVmDeviceDao().save(vmDevice); + } else if (getParameters().isPlugUnPlug() && getVm().getStatus() != VMStatus.Down) { + getVmDeviceDao().save(vmDevice); performPlugCommand(VDSCommandType.HotPlugDisk, disk, vmDevice); } setSucceeded(true); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommand.java index f2cd9a2..44d56eb 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommand.java @@ -5,13 +5,17 @@ import java.util.Map; import org.ovirt.engine.core.bll.utils.VmDeviceUtils; +import org.ovirt.engine.core.bll.validator.StorageDomainValidator; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.action.HotPlugDiskToVmParameters; import org.ovirt.engine.core.common.businessentities.Disk; +import org.ovirt.engine.core.common.businessentities.Disk.DiskStorageType; +import org.ovirt.engine.core.common.businessentities.DiskImage; +import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.VmDevice; import org.ovirt.engine.core.common.businessentities.VmDeviceId; -import org.ovirt.engine.core.common.locks.LockingGroup; import org.ovirt.engine.core.common.errors.VdcBllMessages; +import org.ovirt.engine.core.common.locks.LockingGroup; import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; import org.ovirt.engine.core.utils.transaction.TransactionMethod; @@ -43,7 +47,18 @@ isVmInUpPausedDownStatus() && isDiskExist(disk) && checkCanPerformPlugUnPlugDisk() && - isVmNotInPreviewSnapshot(); + isVmNotInPreviewSnapshot() && + imageValidation(); + } + + private boolean imageValidation() { + if (disk.getDiskStorageType() != DiskStorageType.IMAGE) { + return true; + } + StorageDomain storageDomain = getStorageDomainDAO().getForStoragePool( + ((DiskImage) disk).getStorageIds().get(0), ((DiskImage) disk).getStoragePoolId()); + StorageDomainValidator storageDomainValidator = new StorageDomainValidator(storageDomain); + return validate(storageDomainValidator.isDomainExistAndActive()); } private boolean checkCanPerformPlugUnPlugDisk() { diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java index 57c0366..c10fe08 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java @@ -184,6 +184,7 @@ USER_RUN_UNLOCK_ENTITY_SCRIPT(2024), USER_MOVE_IMAGE_GROUP_FAILED_TO_DELETE_SRC_IMAGE(2025), USER_MOVE_IMAGE_GROUP_FAILED_TO_DELETE_DST_IMAGE(2026), + USER_FAILED_HOTPLUG_DISK_STORAGE_INACTIVE(2027), // Quota audit logs USER_ADD_QUOTA(3000), diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java index c4ed541..36b92b6 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java @@ -417,6 +417,7 @@ severities.put(AuditLogType.USER_FAILED_HOTPLUG_DISK, AuditLogSeverity.ERROR); severities.put(AuditLogType.USER_HOTUNPLUG_DISK, AuditLogSeverity.NORMAL); severities.put(AuditLogType.USER_FAILED_HOTUNPLUG_DISK, AuditLogSeverity.ERROR); + severities.put(AuditLogType.USER_FAILED_HOTPLUG_DISK_STORAGE_INACTIVE, AuditLogSeverity.WARNING); severities.put(AuditLogType.USER_COPIED_TEMPLATE_DISK, AuditLogSeverity.NORMAL); severities.put(AuditLogType.USER_FAILED_COPY_TEMPLATE_DISK, AuditLogSeverity.ERROR); severities.put(AuditLogType.USER_COPIED_TEMPLATE_DISK_FINISHED_SUCCESS, AuditLogSeverity.NORMAL); diff --git a/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties b/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties index 84b55d0..ab3f95d 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties @@ -83,6 +83,7 @@ USER_FAILED_UPDATE_VM_DISK=Failed to update VM ${VmName} disk ${DiskAlias} (User: ${UserName}). USER_HOTPLUG_DISK=VM ${VmName} disk ${DiskAlias} was plugged by ${UserName}. USER_FAILED_HOTPLUG_DISK=Failed to plug disk ${DiskAlias} to VM ${VmName} (User: ${UserName}). +USER_FAILED_HOTPLUG_DISK_STORAGE_INACTIVE=Failed to activate disk ${DiskAlias} to VM ${VmName} since the storage domain ${StorageDomain} is inactive (User: ${UserName}). USER_HOTUNPLUG_DISK=VM ${VmName} disk ${DiskAlias} was unplugged by ${UserName}. USER_FAILED_HOTUNPLUG_DISK=Failed to unplug disk ${DiskAlias} from VM ${VmName} (User: ${UserName}). USER_COPIED_TEMPLATE_DISK=User ${UserName} is copying template disk ${DiskAlias} to domain ${StorageDomainName}. -- To view, visit http://gerrit.ovirt.org/17170 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ieeb2522bc5aa280b9b98ef728737aeeaa82d1263 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
