Liron Ar has posted comments on this change. Change subject: core: Support detach Storage Domain containing entities. ......................................................................
Patch Set 27: (10 comments) http://gerrit.ovirt.org/#/c/24286/27/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 211: succeeded = false; Line 212: } Line 213: if (!vmsInPreview.isEmpty()) { Line 214: addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DELETE_VMS_IN_PREVIEW); Line 215: addCanDoActionMessage(String.format("$vms %1$s", StringUtils.join(vmsInPool, ","))); wrong list passed to StringUtils.join...should be vmsInPreview Line 216: succeeded = false; Line 217: } Line 218: return succeeded; Line 219: } http://gerrit.ovirt.org/#/c/24286/27/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/StorageDomainValidator.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/StorageDomainValidator.java: Line 102: } Line 103: Line 104: // TODO: Validation should be removed once we support detach of storage domain with VMs containing multiple disks Line 105: // resides on different storage domains. Line 106: public ValidationResult hasVmsOrTemplatesWithDisksOnOtherStorageDomains() { this should be separated into different methods with different messages, otherwise the users will need to guess what is what. Line 107: // If there are VMs with disks on other storage domain we should block the operation. Line 108: List<VM> vms = getDbFacade().getVmDao().getAllVMsWithDisksOnOtherStorageDomain(storageDomain.getId()); Line 109: List<VmTemplate> vmTemplates = Line 110: getDbFacade().getVmTemplateDao().getAllTemplatesWithDisksOnOtherStorageDomain(storageDomain.getId()); Line 108: List<VM> vms = getDbFacade().getVmDao().getAllVMsWithDisksOnOtherStorageDomain(storageDomain.getId()); Line 109: List<VmTemplate> vmTemplates = Line 110: getDbFacade().getVmTemplateDao().getAllTemplatesWithDisksOnOtherStorageDomain(storageDomain.getId()); Line 111: Line 112: if (!vms.isEmpty()) { the if is wrong, should check also if tehere are templates. Line 113: List<String> entityNames = new ArrayList<>(); Line 114: for (VM vm : vms) { Line 115: entityNames.add(vm.getName()); Line 116: } http://gerrit.ovirt.org/#/c/24286/27/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskImageDAO.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskImageDAO.java: Line 99: */ Line 100: List<DiskImage> getImagesWithNoDisk(Guid vmId); Line 101: Line 102: /** Line 103: * Return all images which related to the Storage Domain /which related/ that reside on the given storage domain Line 104: * Line 105: * @param storageDomainId Line 106: * The Storage Domain to be fetched entities from. Line 107: * @return List of DiskImages related to the Storage Domain. http://gerrit.ovirt.org/#/c/24286/27/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmTemplateDAO.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmTemplateDAO.java: Line 186: */ Line 187: VmTemplate getTemplateWithLatestVersionInChain(Guid id); Line 188: Line 189: /** Line 190: * Retrieves all Template names which contains disks on other Storage Domain other then the storageDomain GUID. /s/Template names/Templates /s/contains/has Line 191: * Line 192: * @param storageDomainGuid Line 193: * the storage domain GUID Line 194: * @return List of Templates http://gerrit.ovirt.org/#/c/24286/27/packaging/dbscripts/vms_sp.sql File packaging/dbscripts/vms_sp.sql: Line 1038: LANGUAGE plpgsql; Line 1039: Line 1040: Line 1041: Line 1042: ? Line 1043: Create or replace FUNCTION GetVmsByAdGroupNames(v_ad_group_names VARCHAR(250)) RETURNS SETOF vms STABLE Line 1044: AS $procedure$ Line 1045: BEGIN Line 1046: RETURN QUERY select distinct vms.* from vms Line 1079: AS $procedure$ Line 1080: BEGIN Line 1081: RETURN QUERY SELECT DISTINCT vms.* Line 1082: FROM vms Line 1083: INNER JOIN (SELECT vm_static.vm_guid we can add the vm_entity_type to the inner query to reduce the returned rows. Line 1084: FROM vm_static Line 1085: INNER JOIN vm_device vd ON vd.vm_id = vm_static.vm_guid Line 1086: INNER JOIN images i ON i.image_group_id = vd.device_id Line 1087: INNER JOIN (SELECT image_id Line 1374: END; $procedure$ Line 1375: LANGUAGE plpgsql; Line 1376: Line 1377: Line 1378: Create or replace FUNCTION GetAllVmTemplatesWithDisksOnOtherStorageDomain(v_storage_domain_id UUID) RETURNS SETOF vms STABLE SETOF templates Line 1379: AS $procedure$ Line 1380: BEGIN Line 1381: RETURN QUERY SELECT DISTINCT templates.* Line 1382: FROM vm_templates_view templates Line 1379: AS $procedure$ Line 1380: BEGIN Line 1381: RETURN QUERY SELECT DISTINCT templates.* Line 1382: FROM vm_templates_view templates Line 1383: INNER JOIN (SELECT vm_static.vm_guid we can add the vm_entity_type to the inner query to reduce the returned rows. Line 1384: FROM vm_static Line 1385: INNER JOIN vm_device vd ON vd.vm_id = vm_static.vm_guid Line 1386: INNER JOIN images i ON i.image_group_id = vd.device_id Line 1387: INNER JOIN (SELECT image_id Line 1386: INNER JOIN images i ON i.image_group_id = vd.device_id Line 1387: INNER JOIN (SELECT image_id Line 1388: FROM image_storage_domain_map Line 1389: WHERE image_storage_domain_map.storage_domain_id = v_storage_domain_id) isd_map Line 1390: ON i.image_guid = isd_map.image_id) vms_with_disks_on_storage_domain ON templates.vmt_guid = vms_with_disks_on_storage_domain.vm_guid names in that stored procedure are "vms" all over Line 1391: INNER JOIN vm_device vd ON vd.vm_id = templates.vmt_guid Line 1392: INNER JOIN images i ON i.image_group_id = vd.device_id Line 1393: INNER JOIN image_storage_domain_map on i.image_guid = image_storage_domain_map.image_id Line 1394: WHERE image_storage_domain_map.storage_domain_id != v_storage_domain_id; -- 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: 27 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Liran Zelkha <lzel...@redhat.com> Gerrit-Reviewer: Liron Ar <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
