Liron Ar has posted comments on this change. Change subject: core: Support detach Storage Domain with disks. ......................................................................
Patch Set 1: (1 comment) reviewed only the stored procedure which should be changed IMO, will review the rest after this will be changed http://gerrit.ovirt.org/#/c/24286/1/packaging/dbscripts/vms_sp.sql File packaging/dbscripts/vms_sp.sql: Line 999: END; $procedure$ Line 1000: LANGUAGE plpgsql; Line 1001: Line 1002: Line 1003: Create or replace FUNCTION getAllVMsWithDisksOnOtherStorageDomain(v_storage_domain_id UUID) RETURNS SETOF GetNamesOfVmStatic_rs 1. this stored procedure performs total 3 queries, all from the same tables using the same joins, which should be changed. 2. this won't return vms that has lun disks attached which seems wrong, you should block those as well. 3. a better way of doing that imo is - *get disk count for vm on domain (very similar to what is done in *GetVmsByStorageDomainId only with also disk count) - return value should be vm with disk count on the domain [1] *get total disk count for vm this vm from vm_device (no joins required) [2] if the vm has more disks in [2] than in [1], this vm should be returned. Line 1004: AS $procedure$ Line 1005: BEGIN Line 1006: RETURN QUERY SELECT DISTINCT vm_static.vm_name Line 1007: FROM vm_static -- 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: 1 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: Liron Ar <[email protected]> Gerrit-Reviewer: Sergey Gotliv <[email protected]> Gerrit-Reviewer: Tal Nisan <[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
