Daniel Erez has posted comments on this change. Change subject: core: Support detach Storage Domain with disks. ......................................................................
Patch Set 11: (8 comments) http://gerrit.ovirt.org/#/c/24286/11/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBase.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBase.java: Line 144: } Line 145: Line 146: protected boolean canDetachStorageDomainWithVmsAndDisks(StorageDomain storageDomain) { Line 147: // If there are VMs with disks on other storage domain we should block the operation. Line 148: List<VM> vms = getVmDAO().getAllVMsWithDisksOnOtherStorageDomain(storageDomain.getId()); can be extracted to a validator? Line 149: if (!vms.isEmpty()) { Line 150: List<String> vmNames = new ArrayList<>(); Line 151: for (VM vm : vms) { Line 152: vmNames.add(vm.getName()); Line 150: List<String> vmNames = new ArrayList<>(); Line 151: for (VM vm : vms) { Line 152: vmNames.add(vm.getName()); Line 153: } Line 154: log.errorFormat("Failed to remove Storage Domain {0} since there are vms which contain disks related to other storage domains {1}.", s/vms/VMs Line 155: storageDomain.getId(), Line 156: StringUtils.join(vmNames, ",")); Line 157: addCanDoActionMessage(String.format("$vms %1$s", StringUtils.join(vmNames, ","))); Line 158: addCanDoActionMessage(VdcBllMessages.VDS_GROUP_CANNOT_DETACH_DATA_DOMAIN_FROM_LOCAL_STORAGE); Line 205: } Line 206: return succeeded; Line 207: } Line 208: Line 209: protected boolean detachStorageDomainWithEntities(StorageDomain storageDomain) { consider adding a comment about the necessity and usage of the return value Line 210: // Check if we have entities related to the Storage Domain. Line 211: List<VM> vmsForStorageDomain = getVmDAO().getAllForStorageDomain(storageDomain.getId()); Line 212: List<VmTemplate> vmTemplatesForStorageDomain = getVmTemplateDAO().getAllForStorageDomain(storageDomain.getId()); Line 213: List<DiskImage> disksForStorageDomain = getDiskImageDao().getAllForStorageDomain(storageDomain.getId()); Line 240: }); Line 241: } Line 242: Line 243: protected static UnregisteredOVFDataDAO getUnregisteredOVFDataDao() { Line 244: return DbFacade.getInstance().getUnregisteredOVFDataDao(); use 'getDbFacade' Line 245: } Line 246: Line 247: protected boolean checkStoragePoolStatus(StoragePoolStatus status) { Line 248: boolean returnValue = false; http://gerrit.ovirt.org/#/c/24286/11/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java: Line 806: AuditLogTimeInterval.MINUTE.getValue() * 15), Line 807: STORAGE_ALERT_SMALL_VG_METADATA(1000, AuditLogSeverity.WARNING, Line 808: AuditLogTimeInterval.HOUR.getValue() * 12), Line 809: USER_ATTACH_STORAGE_DOMAINS_TO_POOL(1002), Line 810: USER_ATTACH_STORAGE_DOMAINS_TO_POOL_FAILED(1003), why removing the severity? Line 811: STORAGE_DOMAIN_TASKS_ERROR(1004), Line 812: UPDATE_OVF_FOR_STORAGE_POOL_FAILED(1005), Line 813: UPGRADE_STORAGE_POOL_ENCOUNTERED_PROBLEMS(1006), Line 814: REFRESH_REPOSITORY_IMAGE_LIST_INCOMPLETE(1007), http://gerrit.ovirt.org/#/c/24286/11/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties: Line 249: ACTION_TYPE_FAILED_STORAGE_DOMAIN_ALREADY_CONTAINS_DISK=Cannot ${action} ${type}. The Storage Domain already contains the target disk(s). Line 250: ACTION_TYPE_FAILED_STORAGE_DOMAIN_CONTAINS_DISK_ON_OTHER_STORAGE_DOMAIN=Cannot ${action} ${type}. The following VMs/Templates contain disks related to other storage domains: ${vms}. Line 251: ACTION_TYPE_FAILED_STORAGE_DELETE_PROTECTED=Cannot ${action} ${type}. The following VMs are delete protected: ${vms}. Line 252: ACTION_TYPE_FAILED_STORAGE_VMS_NOT_DOWN=Cannot ${action} ${type}. The following VMs are not down: ${vms}. Line 253: ACTION_TYPE_FAILED_STORAGE_VMS_IN_POOL=Cannot ${action} ${type}. The following VMs are attached to pool: ${vms}. consider adding pool's name as well Line 254: ACTION_TYPE_FAILED_STORAGE_DOMAIN_NAME_ALREADY_EXIST=Cannot ${action} ${type}. The Storage Domain name is already in use. Line 255: ACTION_TYPE_FAILED_STORAGE_DOMAIN_ALREADY_EXIST=Cannot ${action} ${type}. The Storage Domain already exists. Line 256: ACTION_TYPE_FAILED_STORAGE_POOL_NAME_ALREADY_EXIST=Cannot ${action} ${type}. The Data Center name is already in use. Line 257: ACTION_TYPE_FAILED_TEMPLATE_NOT_FOUND_ON_DESTINATION_DOMAIN=Cannot ${action} ${type}. The selected Storage Domain does not contain the VM Template. http://gerrit.ovirt.org/#/c/24286/11/packaging/dbscripts/disk_images_sp.sql File packaging/dbscripts/disk_images_sp.sql: Line 157: Create or replace FUNCTION GetAllForStorageDomain(v_storage_domain_id UUID) Line 158: RETURNS SETOF images_storage_domain_view STABLE Line 159: AS $procedure$ Line 160: BEGIN Line 161: RETURN QUERY SELECT i.* rename 'i' Line 162: FROM images_storage_domain_view i Line 163: INNER JOIN storage_for_image_view ON i.image_guid = storage_for_image_view.image_id Line 164: WHERE active Line 165: AND i.storage_id = v_storage_domain_id; Line 161: RETURN QUERY SELECT i.* Line 162: FROM images_storage_domain_view i Line 163: INNER JOIN storage_for_image_view ON i.image_guid = storage_for_image_view.image_id Line 164: WHERE active Line 165: AND i.storage_id = v_storage_domain_id; keep indention conventions... Line 166: END; $procedure$ Line 167: LANGUAGE plpgsql; Line 168: Line 169: -- To view, visit http://gerrit.ovirt.org/24286 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I971fe6acd4a2667a09487c5e1108cf7c759587f1 Gerrit-PatchSet: 11 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Liran Zelkha <[email protected]> Gerrit-Reviewer: Liron Ar <[email protected]> Gerrit-Reviewer: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Sergey Gotliv <[email protected]> Gerrit-Reviewer: Tal Nisan <[email protected]> Gerrit-Reviewer: [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
