Liron Aravot has posted comments on this change. Change subject: core, db: disk profile for disk image ......................................................................
Patch Set 7: (2 comments) http://gerrit.ovirt.org/#/c/29036/7/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java: Line 127: params.setParentParameters(getParameters()); Line 128: params.setEntityInfo(getParameters().getEntityInfo()); Line 129: params.setQuotaId(diskInfoDestinationMap.get(disk.getId()).getQuotaId() != null ? Line 130: diskInfoDestinationMap.get(disk.getId()).getQuotaId() : null); Line 131: params.setDiskProfileId(diskInfoDestinationMap.get(disk.getId()).getDiskProfileId()); > that's exactly what I'm doing. nop? the problem is the order of the patches..the patch that supposed to the that is down on the series..so when reviewing this one it's impossible to know that it was added. later on. If its not much of a work I'd squash those patches together, otherwise it's very hard to track and review. Line 132: return params; Line 133: } Line 134: Line 135: @Override http://gerrit.ovirt.org/#/c/29036/7/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/ImageStorageDomainMapDaoTest.java File backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/ImageStorageDomainMapDaoTest.java: Line 115: image_storage_domain_map.getDiskProfileId(), not(equalTo(Guid.newGuid()))); Line 116: // change to newDiskProfileId Line 117: dao.updateDiskProfileByImageGroupId(EXISTING_DISK_ID, EXISTING_DOMAIN_ID, FixturesTool.DISK_PROFILE_2); Line 118: // fetch the image again Line 119: image_storage_domain_map = dao.getAllByImageId(EXISTING_IMAGE_ID).get(0); > how is it possible? image can have records on two domains, which means two records on the image_storage_domain_map table with the same image id. as you take always the first one received you don't have guarantee which one you'll get, Line 120: // check that the new disk profile is the inserted one Line 121: assertEquals("disk profile wasn't changed", Line 122: image_storage_domain_map.getDiskProfileId(), Line 123: FixturesTool.DISK_PROFILE_2); -- To view, visit http://gerrit.ovirt.org/29036 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9488a1e31e85013a01ee088fdf71854f765059c9 Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com> Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Liron Aravot <lara...@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
