Daniel Erez has posted comments on this change. Change subject: core: support for partial snapshot creation ......................................................................
Patch Set 4: (12 comments) .................................................... Commit Message Line 5: CommitDate: 2013-12-29 09:44:05 +0200 Line 6: Line 7: core: support for partial snapshot creation Line 8: Line 9: Introducing a support for creating a snapshot using Done Line 10: a specified disks list, i.e. taking a snapshot only Line 11: one the selected disks. Line 12: Line 13: * CreateAllSnapshotsFromVmParameters: Line 7: core: support for partial snapshot creation Line 8: Line 9: Introducing a support for creating a snapshot using Line 10: a specified disks list, i.e. taking a snapshot only Line 11: one the selected disks. Done Line 12: Line 13: * CreateAllSnapshotsFromVmParameters: Line 14: ** Added an optional disks list member. Line 15: Line 16: * CreateAllSnapshotsFromVmCommand: Line 17: ** Updated accordingly to handle the specified disks list. Line 18: Line 19: * SnapshotManager: Line 20: ** added addActiveSnapshot method signature that accepts Done Line 21: disks list argument. Line 22: ** modified addSnaphot methods accordingly. Line 23: Line 24: Feature: http://www.ovirt.org/Features/Single_Disk_Snapshot Line 18: Line 19: * SnapshotManager: Line 20: ** added addActiveSnapshot method signature that accepts Line 21: disks list argument. Line 22: ** modified addSnaphot methods accordingly. Done Line 23: Line 24: Feature: http://www.ovirt.org/Features/Single_Disk_Snapshot Line 25: Line 26: Change-Id: Ieb7cb389229219bbb0212e3ee58cc45c39727c75 .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java Line 91: } Line 92: Line 93: Line 94: private List<DiskImage> getDiskImagesForVm() { Line 95: return ImagesHandler.filterImageDisks(DbFacade.getInstance().getDiskDao().getAllForVm(getVmId()), true, true, true); Yeah, it's just an extraction of existing code. I'' fix that separately. Line 96: } Line 97: Line 98: /** Line 99: * Filter all allowed snapshot disks. Line 101: */ Line 102: protected List<DiskImage> getDisksList() { Line 103: if (cachedSelectedActiveDisks == null) { Line 104: // Get disks from the specified parameters or according to the VM Line 105: if (getParameters().getDisks() == null) { No need, we should support taking a snapshot without disks. Line 106: cachedSelectedActiveDisks = getDiskImagesForVm(); Line 107: } Line 108: else { Line 109: List<DiskImage> imagesForVm = getDiskImagesForVm(); .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java Line 706: public static void updateAllDiskImageSnapshotsStatus(Guid diskId, ImageStatus status) { Line 707: DbFacade.getInstance().getImageDao().updateStatusOfImagesByImageGroupId(diskId, status); Line 708: } Line 709: Line 710: public static DiskImage getDiskImageById(Guid id, List<DiskImage> diskImages) { Hmm, not sure we need to change the convention of using List, it's quite common in most of our APIs. Line 711: for (DiskImage diskImage : diskImages) { Line 712: if (diskImage.getId().equals(id)) { Line 713: return diskImage; Line 714: } Line 721: * @param images1 1st list Line 722: * @param images2 2nd list Line 723: * @return Line 724: */ Line 725: public static List<DiskImage> imagesSubtract(List<DiskImage> images1, List<DiskImage> images2) { OK :) Line 726: List<DiskImage> subtract = new ArrayList<>(); Line 727: for (DiskImage image : images1) { Line 728: if (ImagesHandler.getDiskImageById(image.getId(), images2) == null) { Line 729: subtract.add(image); Line 729: subtract.add(image); Line 730: } Line 731: } Line 732: return subtract; Line 733: } Since I need it based on ID rather than on equals(). .................................................... File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImagesHandlerTest.java Line 100: List<DiskImage> list2 = new ArrayList<>(Arrays.asList(disk2, disk3)); Line 101: Line 102: List<DiskImage> intersection = ImagesHandler.imagesSubtract(list1, list2); Line 103: Line 104: assertTrue("Intersection should contain only disk1", intersection.size() == 1 && intersection.contains(disk1)); Done Line 105: } .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/CreateAllSnapshotsFromVmParameters.java Line 12: import org.ovirt.engine.core.common.validation.annotation.ValidDescription; Line 13: import org.ovirt.engine.core.common.validation.group.CreateEntity; Line 14: import org.ovirt.engine.core.compat.Guid; Line 15: Line 16: import java.util.List; Done Line 17: Line 18: public class CreateAllSnapshotsFromVmParameters extends VmOperationParameterBase implements java.io.Serializable { Line 19: private static final long serialVersionUID = 847791941815264795L; Line 20: Line 35: Line 36: @JsonIgnore Line 37: private Set<Guid> diskIdsToIgnoreInChecks; Line 38: Line 39: private List<DiskImage> disks; GWT 2.5 seems to handle it correctly.. Line 40: Line 41: public CreateAllSnapshotsFromVmParameters() { Line 42: needsLocking = true; Line 43: diskIdsToIgnoreInChecks = Collections.emptySet(); -- To view, visit http://gerrit.ovirt.org/22775 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieb7cb389229219bbb0212e3ee58cc45c39727c75 Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> 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
