Sergey Gotliv has posted comments on this change.
Change subject: core: update managed device map when import cloned VM.
......................................................................
Patch Set 2: Code-Review-1
(2 comments)
1. Although this bug is talking about Import VM scenario, I think the same
thing will happen in Import Vm Template,
Therefore, please, fix ImportVmTemplateCommand either - maybe the code can be
reused or open another bug for Template.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java
Line 847: * - All the disk volumes
Line 848: * @param disk
Line 849: * - The disk which is about to be cloned
Line 850: */
Line 851: private void generateNewDiskId(List<DiskImage> diskList,
DiskImage disk, Map<Guid, VmDevice> managedDeviceMap) {
You added new input parameter, but didn't change javadoc(description and
@params)
Line 852: Guid newGuidForDisk = Guid.newGuid();
Line 853:
Line 854: // Copy the disk so it will preserve the old disk id and
image id.
Line 855: newDiskIdForDisk.put(newGuidForDisk, DiskImage.copyOf(disk));
Line 856:
Line 857: Guid oldDiskId = disk.getId();
Line 858: managedDeviceMap.put(newGuidForDisk,
managedDeviceMap.get(disk.getId()));
Line 859: managedDeviceMap.remove(oldDiskId);
Line 860: disk.setId(newGuidForDisk);
1. Please, move this logic to another method, it has nothing to do with
"generateNewDiskId".
2. Replace disk.getId() with "oldDiskId" or remove this local variable(I would
remove...)
3. Is it possible that you insert null value into the map?
I mean what if this is old VM and there is no device:
managedDeviceMap.get(disk.getId()) == null?
Line 861: disk.setImageId(Guid.newGuid());
Line 862: for (DiskImage diskImage : diskList) {
Line 863: diskImage.setId(disk.getId());
Line 864: }
--
To view, visit http://gerrit.ovirt.org/19954
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I65a99cc0783f670b9288cc9a066cda4d253492a6
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
