Liron Ar has posted comments on this change. Change subject: core: block different operations for OVF disks ......................................................................
Patch Set 4: (4 comments) http://gerrit.ovirt.org/#/c/24179/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractDiskVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractDiskVmCommand.java: Line 197: addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_OVF_DISK_NOT_SUPPORTED); Line 198: return true; Line 199: } Line 200: return false; Line 201: } > This should be in DiskImageValidator, IMHO Done, moved it to DiskValidator (it's property of disk and not of diskimage so we'll be able to add support for lun disks as ovf stores later on). Line 202: Line 203: private boolean isDiskExistInVm(Disk disk) { Line 204: List<VM> listVms = getVmDAO().getVmsListForDisk(disk.getId(), true); Line 205: for (VM vm : listVms) { http://gerrit.ovirt.org/#/c/24179/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java: Line 117: } Line 118: return true; Line 119: } Line 120: Line 121: protected boolean isDiskUsedAsOvfStore() { > should be added to LiveMigrateVmDisks as well? at the moment it's not relevant as the ovf disk can't be attached to vms. Line 122: if (getImage().isOvfStore()) { Line 123: addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_OVF_DISK_NOT_SUPPORTED); Line 124: return true; Line 125: } http://gerrit.ovirt.org/#/c/24179/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java: Line 112: private boolean canRemoveDiskBasedOnImageStorageCheck() { Line 113: boolean retValue = true; Line 114: DiskImage diskImage = getDiskImage(); Line 115: Line 116: if (diskImage.isOvfStore() && diskImage.getImageStatus() != ImageStatus.ILLEGAL) { > what's the use-case of removing an ovf disk? isn't it dangerous to allow it it's allowed to be removed only when it's illegal (on that case we don't update the ovfs to it), when the ovf disk isn't created succesfully there's an audit log saying that it's illegal and might be removed by the user (and that till it's done, ovf update for that disk won't occur). Line 117: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_OVF_DISK_NOT_IN_APPLICABLE_STATUS); Line 118: } Line 119: Line 120: if (diskImage.getVmEntityType() != null && diskImage.getVmEntityType().isTemplateType()) { http://gerrit.ovirt.org/#/c/24179/4/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties: Line 80: The following VMs cannot be migrated because they have activated Disk Snapshot attached: \n \n\ Line 81: ${disksInfo} \n \n\ Line 82: please deactivate/detach the Disk snapshots or turn off those VMs and try again. Line 83: ACTION_TYPE_FAILED_OVF_DISK_NOT_SUPPORTED=Cannot ${action} ${type}. The operation is currently not supported for disks used as OVF store. Line 84: ACTION_TYPE_FAILED_OVF_DISK_NOT_IN_APPLICABLE_STATUS=Cannot ${action} ${type}. The operation cannot be performed for disks used as OVF store that are in the given status. > consider stating explicitly the status of the disk Done Line 85: VDS_CANNOT_REMOVE_DEFAULT_VDS_GROUP=Cannot remove default Host Cluster. Line 86: VDS_CANNOT_REMOVE_VDS_DETECTED_RUNNING_VM=Cannot ${action} ${type}. One or more VMs are still running on this Host. Line 87: VDS_CANNOT_REMOVE_VDS_GROUP_VDS_DETECTED=Cannot ${action} ${type}. Host Cluster contains one or more Hosts. Line 88: VDS_CANNOT_REMOVE_VDS_STATUS_ILLEGAL=Cannot ${action} ${type}. Host is operational. Please switch Host to Maintenance mode first. -- To view, visit http://gerrit.ovirt.org/24179 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic2b7b9f39dcf5da76be75c1106e062ff41ca8068 Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liron Ar <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Liron Ar <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
