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). Bug-Url: https://bugzilla.redhat.com/969767 Signed-off-by: Maor Lipchuk <[email protected]> Change-Id: I0bed22c005b9884902d409eb9b6c5bfe997c6c68 --- 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/36/17136/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..c8f9963 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,14 @@ 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; +import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogableBase; @LockIdNameAttribute public class AttachDiskToVmCommand<T extends AttachDettachVmDiskParameters> extends AbstractDiskVmCommand<T> { @@ -118,7 +122,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 +132,21 @@ } // 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()); + AuditLogableBase logable = new AuditLogableBase(); + this.addCustomValue("StorageDomain", storageDomain.getName()); + AuditLogDirector.log(logable, 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..d44e703 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 plug 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/17136 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0bed22c005b9884902d409eb9b6c5bfe997c6c68 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
