Maor Lipchuk has uploaded a new change for review.

Change subject: core: Sync VM disks with new guid when importing a VM.
......................................................................

core: Sync VM disks with new guid when importing a VM.

Currently ImportVmCommand use three different kinds of containers when
importing a VM: image id, disk id, and the new disk which is about to be
added to the DB.

The use of those containers has dependency with each other,
and should be reflected in only one container so it will remain in sync.

the following fix, put those parameters into one map, and use this map
when copying a new disk into the Data Center.

Change-Id: If47f87b480d01dfd426a1aa24ddf48d75d3ff334
Bug-Url: https://bugzilla.redhat.com/970485
Signed-off-by: Maor Lipchuk <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java
1 file changed, 29 insertions(+), 20 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/42/17042/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java
index a365120..d7eec14 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java
@@ -99,8 +99,11 @@
 
     private static VmStatic vmStaticForDefaultValues = new VmStatic();
     private List<DiskImage> imageList;
-    private final List<Guid> diskGuidList = new ArrayList<Guid>();
-    private final List<Guid> imageGuidList = new ArrayList<Guid>();
+
+    /*
+     * Map which contains the disk guid (new if cloned) and the disk 
parameters from the export domain.
+     */
+    private final Map<Guid, DiskImage> newDiskGuidForDisk = new HashMap<>();
     private final List<String> macsAdded = new ArrayList<String>();
     private final SnapshotsManager snapshotsManager = new SnapshotsManager();
 
@@ -704,11 +707,10 @@
 
     @Override
     protected void moveOrCopyAllImageGroups(Guid containerID, 
Iterable<DiskImage> disks) {
-        int i = 0;
         for (DiskImage disk : disks) {
             VdcReturnValueBase vdcRetValue = 
Backend.getInstance().runInternalAction(
                     VdcActionType.CopyImageGroup,
-                    buildMoveOrCopyImageGroupParametersForDisk(disk, 
containerID, i++),
+                    buildMoveOrCopyImageGroupParametersForDisk(disk, 
containerID),
                     
ExecutionHandler.createDefaultContexForTasks(getExecutionContext()));
             if (!vdcRetValue.getSucceeded()) {
                 throw new VdcBLLException(vdcRetValue.getFault().getError(),
@@ -719,11 +721,12 @@
         }
     }
 
-    private MoveOrCopyImageGroupParameters 
buildMoveOrCopyImageGroupParametersForDisk(DiskImage disk, Guid containerID, 
int i) {
-        Guid destinationDomain = 
imageToDestinationDomainMap.get(diskGuidList.get(i));
+    private MoveOrCopyImageGroupParameters 
buildMoveOrCopyImageGroupParametersForDisk(DiskImage disk, Guid containerID) {
+        Guid originalDiskId = newDiskGuidForDisk.get(disk.getId()).getId();
+        Guid destinationDomain = 
imageToDestinationDomainMap.get(originalDiskId);
         MoveOrCopyImageGroupParameters params = new 
MoveOrCopyImageGroupParameters(containerID,
-                diskGuidList.get(i),
-                imageGuidList.get(i),
+                originalDiskId,
+                newDiskGuidForDisk.get(disk.getId()).getImageId(),
                 disk.getId(),
                 disk.getImageId(),
                 destinationDomain, getMoveOrCopyImageOperation());
@@ -737,9 +740,9 @@
         params.setEntityInfo(new EntityInfo(VdcObjectType.VM, 
getVm().getId()));
         params.setQuotaId(disk.getQuotaId() != null ? disk.getQuotaId() : 
getParameters().getQuotaId());
         if (getParameters().getVm().getDiskMap() != null
-                && 
getParameters().getVm().getDiskMap().containsKey(diskGuidList.get(i))) {
+                && 
getParameters().getVm().getDiskMap().containsKey(originalDiskId)) {
             DiskImageBase diskImageBase =
-                    (DiskImageBase) 
getParameters().getVm().getDiskMap().get(diskGuidList.get(i));
+                    (DiskImageBase) 
getParameters().getVm().getDiskMap().get(originalDiskId);
             params.setVolumeType(diskImageBase.getVolumeType());
             params.setVolumeFormat(diskImageBase.getVolumeFormat());
         }
@@ -769,11 +772,9 @@
                 setDiskStorageDomainInfo(disk);
 
                 if (getParameters().isImportAsNewEntity()) {
-                    disk.setId(Guid.newGuid());
-                    disk.setImageId(Guid.newGuid());
-                    for (int i = 0; i < diskList.size() - 1; i++) {
-                         diskList.get(i).setId(disk.getId());
-                    }
+                    resetNewIdForDisk(diskList, disk);
+                } else {
+                    newDiskGuidForDisk.put(disk.getId(), disk);
                 }
                 disk.setCreationDate(new Date());
                 saveImage(disk);
@@ -789,9 +790,6 @@
             for (DiskImage disk : getVm().getImages()) {
                 disk.setActive(false);
                 setDiskStorageDomainInfo(disk);
-
-                diskGuidList.add(disk.getId());
-                imageGuidList.add(disk.getImageId());
                 saveImage(disk);
                 snapshotId = disk.getVmSnapshotId();
                 saveSnapshotIfNotExists(snapshotId, disk);
@@ -801,8 +799,7 @@
             int aliasCounter = 0;
             for (List<DiskImage> diskList : images.values()) {
                 DiskImage disk = getActiveVolumeDisk(diskList);
-                diskGuidList.add(disk.getId());
-                imageGuidList.add(disk.getImageId());
+                newDiskGuidForDisk.put(disk.getId(), disk);
                 snapshotId = disk.getVmSnapshotId();
                 disk.setActive(true);
                 ImagesHandler.setDiskAlias(disk, getVm(), ++aliasCounter);
@@ -815,6 +812,18 @@
         }
     }
 
+    private void resetNewIdForDisk(List<DiskImage> diskList, DiskImage disk) {
+        Guid newGuidForDisk = Guid.newGuid();
+
+        // Copy the disk so it will preserve the old disk id and image id.
+        newDiskGuidForDisk.put(newGuidForDisk, DiskImage.copyOf(disk));
+        disk.setId(newGuidForDisk);
+        disk.setImageId(Guid.newGuid());
+        for (int i = 0; i < diskList.size() - 1; i++) {
+             diskList.get(i).setId(disk.getId());
+        }
+    }
+
     private static DiskImage getActiveVolumeDisk(List<DiskImage> diskList) {
         return diskList.get(diskList.size() - 1);
     }


-- 
To view, visit http://gerrit.ovirt.org/17042
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: If47f87b480d01dfd426a1aa24ddf48d75d3ff334
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to