Maor Lipchuk has posted comments on this change. Change subject: core: OVFs that aren't stored on any domain should be stored on all ......................................................................
Patch Set 1: (6 comments) http://gerrit.ovirt.org/#/c/34292/1/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmStaticDAO.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmStaticDAO.java: Line 93: */ Line 94: public void remove(Guid id, boolean removePermissions); Line 95: Line 96: /** Line 97: * Retrieves all ids of vms and templates that have no attached disks matching the provided criteria. /s/all/all the /s/of/of the Line 98: * @param shareableDisks check for attached shareable disks Line 99: * @param snapshotDisks check for attached snapshotdisks Line 100: */ Line 101: public List<Guid> getVmAndTemplatesIdsWithoutAttachedImageDisks(boolean shareableDisks, boolean snapshotDisks); http://gerrit.ovirt.org/#/c/34292/1/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/FixturesTool.java File backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/FixturesTool.java: Line 252: * </ul> Line 253: */ Line 254: protected static final Guid VM_TEMPLATE_RHEL6_2 = new Guid("1b85420c-b84c-4f29-997e-0eb674b40b82"); Line 255: Line 256: protected static final Guid VM_TEMPLATE_WITHOUT_DISKS = new Guid("1b85420c-b84c-4f29-997e-0eb674b40b80"); Please remove this constant and add this information to the java doc of VM_TEMPLATE_RHEL5_2 Line 257: Line 258: protected static final Guid VM_WITHOUT_DISKS = new Guid("77296e00-0cad-4e5a-9299-008a7b6f4356"); Line 259: Line 260: /** Line 254: protected static final Guid VM_TEMPLATE_RHEL6_2 = new Guid("1b85420c-b84c-4f29-997e-0eb674b40b82"); Line 255: Line 256: protected static final Guid VM_TEMPLATE_WITHOUT_DISKS = new Guid("1b85420c-b84c-4f29-997e-0eb674b40b80"); Line 257: Line 258: protected static final Guid VM_WITHOUT_DISKS = new Guid("77296e00-0cad-4e5a-9299-008a7b6f4356"); No need to add an existing guid to the fixtures tool. We should add this information as a java doc to VM_RHEL5_POOL_51 Line 259: Line 260: /** Line 261: * Predefined template version for testing with the following properties : Line 262: * <ul> http://gerrit.ovirt.org/#/c/34292/1/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmStaticDAOTest.java File backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmStaticDAOTest.java: Line 205: } Line 206: Line 207: Line 208: @Test Line 209: public void getVmAndTemplatesIdsWithoutAttachedImageDisks() { Suggestion: I would also change the name to be something more shorter like GetEntitiesIdsWithoutImageDisks Line 210: List<Guid> ids = Line 211: dao.getVmAndTemplatesIdsWithoutAttachedImageDisks(false, false); Line 212: assertTrue(ids.contains(FixturesTool.VM_WITHOUT_DISKS)); Line 213: assertTrue(ids.contains(FixturesTool.VM_TEMPLATE_WITHOUT_DISKS)); http://gerrit.ovirt.org/#/c/34292/1/packaging/dbscripts/vms_sp.sql File packaging/dbscripts/vms_sp.sql: Line 592: Line 593: Line 594: Line 595: Line 596: Create or replace FUNCTION GetVmAndTemplatesIdsWithoutAttachedImageDisks(v_shareable BOOLEAN, v_active BOOLEAN) /s/Vm/VMs Suggestion: I would also change the name to be something more shorter like GetEntitiesIdWithoutImageDisks Line 597: RETURNS SETOF UUID STABLE Line 598: AS $procedure$ Line 599: BEGIN Line 600: RETURN QUERY SELECT vs.vm_guid Line 601: FROM vm_static vs Line 602: WHERE vs.vm_guid NOT IN (SELECT DISTINCT vd.vm_id Line 603: FROM vm_device vd INNER JOIN images_storage_domain_view i Line 604: ON i.image_group_id = vd.device_id Line 605: WHERE i.active = v_active AND i.shareable = v_shareable); in the future we will want that shareable disks will also be included in the OVF. perhaps we should change the condition to accept also shareable disks at the same query, as so: instead of : i.shareable = v_shareable it will be i.shareable IS NULL or i.shareable = v_shareable Line 606: END; $procedure$ Line 607: LANGUAGE plpgsql; Line 608: Line 609: -- To view, visit http://gerrit.ovirt.org/34292 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iaeecc6c4526800656315842384d83830fdf0b72b Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@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
