Tal Nisan has posted comments on this change.

Change subject: core: Check all attached VMs when updating shared disk boot flag
......................................................................


Patch Set 3: (2 inline comments)

....................................................
File backend/manager/dbscripts/all_disks_sp.sql
Line 56: Create or replace FUNCTION GetVmBootDisk(v_vm_guid UUID) RETURNS SETOF 
all_disks AS $procedure$
Line 57: BEGIN
Line 58:       RETURN QUERY SELECT all_disks.*
Line 59:       FROM all_disks
Line 60:       LEFT JOIN vm_device ON vm_device.device_id = 
all_disks.image_group_id
1. Will do

2. Yes

3. The whole workflow ignore whether the boot disk is plugged or not so it's 
problematic to change it thus extending the GetDisksVmGuid procedure to include 
the is_boot will add to the normal operation of retrieving the boot disk also 
redundant checks on plugged status and filter, for a query this short though 
similiar I'd prefer to keep it as is
Line 61:       WHERE vm_device.vm_id = v_vm_guid and boot = true;
Line 62: END; $procedure$
Line 63: LANGUAGE plpgsql;
Line 64: 


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java
Line 275: 
Line 276:         for (VM vm : vms) {
Line 277:             Disk bootDisk = getDiskDao().getVmBootDisk(vm.getId());
Line 278:             if (bootDisk != null) {
Line 279:                 if (vmsContainBootDisk) { // In case this is not the 
first VM to fail, add delimiter
Thought about it buy wanted to avoid creating a new list for nothing
Line 280:                     vmsContainingBootDisk.append(", ");
Line 281:                 }
Line 282:                 vmsContainingBootDisk.append(vm.getName());
Line 283:                 vmsContainBootDisk = true;


--
To view, visit http://gerrit.ovirt.org/14709
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I82ee07e02e08d60f559017d9f8205ab7df41c5c3
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <tni...@redhat.com>
Gerrit-Reviewer: Alissa Bonas <abo...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@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: Vered Volansky <vvola...@redhat.com>
Gerrit-Reviewer: liron aravot <liron.ara...@gmail.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to