Maor Lipchuk has posted comments on this change.
Change subject: core: Sync VM disks with new guid when importing a VM.
......................................................................
Patch Set 1: (3 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java
Line 101: private List<DiskImage> imageList;
Line 102:
Line 103: /*
Line 104: * Map which contains the disk guid (new if cloned) and the disk
parameters from the export domain.
Line 105: */
Regarding to the java doc comment, will change to id.
I think diskIdToOrignalDisk will still be a bit hazy, since it is a new guid
generated for the disk.
any other suggestions?
Line 106: private final Map<Guid, DiskImage> newDiskGuidForDisk = new
HashMap<>();
Line 107: private final List<String> macsAdded = new ArrayList<String>();
Line 108: private final SnapshotsManager snapshotsManager = new
SnapshotsManager();
Line 109:
Line 727: MoveOrCopyImageGroupParameters params = new
MoveOrCopyImageGroupParameters(containerID,
Line 728: originalDiskId,
Line 729: newDiskGuidForDisk.get(disk.getId()).getImageId(),
Line 730: disk.getId(),
Line 731: disk.getImageId(),
I didn't follow.
If you referring to the parameter constructor then it reflects :
disk id to copy from
image id of the disk to copy from
new disk id that we will copy to
new disk image id that we will copy to
Line 732: destinationDomain, getMoveOrCopyImageOperation());
Line 733: params.setParentCommand(getActionType());
Line 734: params.setUseCopyCollapse(getParameters().getCopyCollapse());
Line 735: params.setCopyVolumeType(CopyVolumeType.LeafVol);
Line 814:
Line 815: private void resetNewIdForDisk(List<DiskImage> diskList,
DiskImage disk) {
Line 816: Guid newGuidForDisk = Guid.newGuid();
Line 817:
Line 818: // Copy the disk so it will preserve the old disk id and
image id.
How about I will rename the method to generateNewDiskId,I think that the
implementation part which put this disk in the map could be considered asa an
implementation detail which does not need to be reflected in the API.
I will appreciate other method name suggestions ofcourse
Line 819: newDiskGuidForDisk.put(newGuidForDisk,
DiskImage.copyOf(disk));
Line 820: disk.setId(newGuidForDisk);
Line 821: disk.setImageId(Guid.newGuid());
Line 822: for (int i = 0; i < diskList.size() - 1; i++) {
--
To view, visit http://gerrit.ovirt.org/17042
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If47f87b480d01dfd426a1aa24ddf48d75d3ff334
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
