Sergey Gotliv has uploaded a new change for review. Change subject: engine: Ties more consitently between vm disk device and disk entities. ......................................................................
engine: Ties more consitently between vm disk device and disk entities. VmDevice is the model entity on the Engine that ties between VM and the disk attached to this VM. Disk is not the only entity which tied to VM by VmDevice, but the only to be affected by this patch. VmDevice keeps a few very important characteristics of the underlying disk, for example address which ties the disk to specific slot of the controller. Without that information or if this information is not consistent disk may become useless after being introduced to the VM. During creating VM from a template with Disks A and B. Disk A => Disk A' Disk B => Disk B' So, if VMDevice A ties between Disk A and Template then VmDevice A' should tie between newly created VM and Disk A'. This patch make this process more consistent as till now, Engine was up to the order of disks in the Map or List as they were retrieved from UI or Database. Change-Id: I8bf284ea4e05f03fc0a7e94a1ec901aacbdc1ab1 Bug-Url: https://bugzilla.redhat.com/993776 Signed-off-by: Sergey Gotliv <sgot...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java 6 files changed, 56 insertions(+), 29 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/54/17854/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java index b0c6ccb..47ad91a 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java @@ -69,7 +69,7 @@ diskImage.getId(), diskImage.getImageId(), parentCommandType); VdcReturnValueBase result = executeChildCopyingCommand(parameters); - handleCopyResult(newDiskImage, parameters, result); + handleCopyResult(diskImage, newDiskImage, parameters, result); } @Override @@ -160,23 +160,24 @@ /** * Handle the result of copying the image - * + * @param srcDiskImage + * disk image that represents the source image * @param copiedDiskImage - * disk image that represents the copied image at VDSM + * disk image that represents the copied image * @param parameters - * parameters for the child command that executes the copy at VDSM + * parameters for the child command that executes the copy * @param result * result of execution of child command */ - protected void handleCopyResult(DiskImage copiedDiskImage, - VdcActionParametersBase parameters, - VdcReturnValueBase result) { + private void handleCopyResult(DiskImage srcDiskImage, DiskImage copiedDiskImage, + VdcActionParametersBase parameters, VdcReturnValueBase result) { // If a copy cannot be made, abort if (!result.getSucceeded()) { throw new VdcBLLException(VdcBllErrors.VolumeCreationError); } else { ImagesHandler.addDiskImageWithNoVmDevice(copiedDiskImage); getTaskIdList().addAll(result.getInternalVdsmTaskIdList()); + srcDiskToTargetDiskMapping.put(srcDiskImage.getId(), copiedDiskImage.getId()); newDiskImages.add(copiedDiskImage); } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java index 6683dc7..1bebb0c 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java @@ -87,6 +87,8 @@ */ protected List<DiskImage> newDiskImages = new ArrayList<DiskImage>(); + protected Map<Guid, Guid> srcDiskToTargetDiskMapping = new HashMap<>(); + public AddVmCommand(T parameters) { super(parameters); // if we came from EndAction the VmId is not null @@ -609,7 +611,7 @@ protected void copyVmDevices() { VmDeviceUtils.copyVmDevices(getVmTemplateId(), getVmId(), - newDiskImages, + srcDiskToTargetDiskMapping, _vmInterfaces, getParameters().isSoundDeviceEnabled(), getParameters().isConsoleEnabled()); @@ -735,7 +737,9 @@ throw new VdcBLLException(result.getFault().getError()); } else { getTaskIdList().addAll(result.getInternalVdsmTaskIdList()); - newDiskImages.add((DiskImage) result.getActionReturnValue()); + DiskImage newImage = (DiskImage) result.getActionReturnValue(); + srcDiskToTargetDiskMapping.put(dit.getId(), newImage.getId()); + newDiskImages.add(newImage); } } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommand.java index b2c6ef8..4ae01e7 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommand.java @@ -245,7 +245,7 @@ getVm().getStaticData(), true, devices, - newDiskImages, + srcDiskToTargetDiskMapping, _vmInterfaces, getParameters().isSoundDeviceEnabled(), getParameters().isConsoleEnabled()); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java index 70dc2ba..25e1059 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java @@ -88,7 +88,9 @@ throw new VdcBLLException(VdcBllErrors.VolumeCreationError); } else { getTaskIdList().addAll(result.getInternalVdsmTaskIdList()); - newDiskImages.add((DiskImage) result.getActionReturnValue()); + DiskImage newImage = (DiskImage) result.getActionReturnValue(); + srcDiskToTargetDiskMapping.put(disk.getId(), newImage.getId()); + newDiskImages.add(newImage); } } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java index 537116c..36c6a21 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java @@ -76,6 +76,12 @@ private final List<DiskImage> newDiskImages = new ArrayList<DiskImage>(); /** + * A mapping between source disk id and target disk id. + * This mapping is used when creating new devices for this template. + */ + private Map<Guid, Guid> srcDiskToTargetDiskMapping = new HashMap<>(); + + /** * Constructor for command creation when compensation is applied on startup * * @param commandId @@ -214,7 +220,12 @@ AddVmTemplateImages(); List<VmNetworkInterface> vmInterfaces = addVmInterfaces(); if (isVmInDb) { - VmDeviceUtils.copyVmDevices(getVmId(), getVmTemplateId(), newDiskImages, vmInterfaces, getParameters().isSoundDeviceEnabled(), getParameters().isConsoleEnabled()); + VmDeviceUtils.copyVmDevices(getVmId(), + getVmTemplateId(), + srcDiskToTargetDiskMapping, + vmInterfaces, + getParameters().isSoundDeviceEnabled(), + getParameters().isConsoleEnabled()); } else { // sending true for isVm in order to create basic devices needed VmDeviceUtils.copyVmDevices(getVmId(), @@ -223,7 +234,7 @@ getVmTemplate(), true, Collections.<VmDevice> emptyList(), - newDiskImages, + srcDiskToTargetDiskMapping, vmInterfaces, getParameters().isSoundDeviceEnabled(), getParameters().isConsoleEnabled()); @@ -461,6 +472,8 @@ } getReturnValue().getVdsmTaskIdList().addAll(retValue.getInternalVdsmTaskIdList()); + DiskImage newImage = (DiskImage) retValue.getActionReturnValue(); + srcDiskToTargetDiskMapping.put(diskImage.getId(), newImage.getId()); newDiskImages.add((DiskImage) retValue.getActionReturnValue()); } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java index 16d7428..2524386 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java @@ -166,8 +166,8 @@ * * @param oldVm * @param newVmBase - * @param cluster compatibility version - * @param sound device enabled - if null, keep old state + * @param compatibilityVersion compatibility version + * @param isSoundDeviceEnabled device enabled - if null, keep old state */ public static void updateAudioDevice(VmBase oldVm, VmBase newVmBase, Version compatibilityVersion, Boolean isSoundDeviceEnabled) { boolean removeDevice = false; @@ -223,7 +223,16 @@ /** * Copy related data from the given VM/VmBase/VmDevice list to the destination VM/VmTemplate. */ - public static void copyVmDevices(Guid srcId, Guid dstId, VM vm, VmBase vmBase, boolean isVm, List<VmDevice> devicesDataToUse, List<DiskImage> disks, List<VmNetworkInterface> ifaces, boolean soundDeviceEnabled, boolean isConsoleEnabled) { + public static void copyVmDevices(Guid srcId, + Guid dstId, + VM vm, + VmBase vmBase, + boolean isVm, + List<VmDevice> devicesDataToUse, + Map<Guid, Guid> srcDiskToTargetDiskMapping, + List<VmNetworkInterface> ifaces, + boolean soundDeviceEnabled, + boolean isConsoleEnabled) { Guid id; int diskCount = 0; int ifaceCount = 0; @@ -254,9 +263,7 @@ switch(device.getType()) { case DISK: if (VmDeviceType.DISK.getName().equals(device.getDevice())) { - if (diskCount < disks.size()) { - id = (disks.get(diskCount++)).getId(); - } + id = srcDiskToTargetDiskMapping.get(device.getDeviceId()); } else if (VmDeviceType.CDROM.getName().equals(device.getDevice())) { // check here is source VM had CD (Vm from snapshot) String srcCdPath = (String) device.getSpecParams().get(VdsProperties.Path); @@ -373,23 +380,23 @@ null); } - /** - * Copies relevant entries on "Vm from Template" or "Template from VM" creation. - * - * @param srcId - * @param dstId - * @param disks - * The disks which were saved for the destination VM. - */ - public static void copyVmDevices(Guid srcId, Guid dstId, List<DiskImage> disks, List<VmNetworkInterface> ifaces, boolean soundDeviceEnabled, boolean isConsoleEnabled) { + public static void copyVmDevices(Guid srcId, + Guid dstId, + Map<Guid, Guid> srcDiskToTargetDiskMapping, + List<VmNetworkInterface> ifaces, + boolean soundDeviceEnabled, + boolean isConsoleEnabled) { VM vm = DbFacade.getInstance().getVmDao().get(dstId); VmBase vmBase = (vm != null) ? vm.getStaticData() : null; boolean isVm = (vmBase != null); + if (!isVm) { vmBase = DbFacade.getInstance().getVmTemplateDao().get(dstId); } + List<VmDevice> devices = dao.getVmDeviceByVmId(srcId); - copyVmDevices(srcId, dstId, vm, vmBase, isVm, devices, disks, ifaces, soundDeviceEnabled, isConsoleEnabled); + copyVmDevices(srcId, dstId, vm, vmBase, isVm, devices, srcDiskToTargetDiskMapping, + ifaces, soundDeviceEnabled, isConsoleEnabled); } private static void addVideoDevice(VmBase vm) { -- To view, visit http://gerrit.ovirt.org/17854 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I8bf284ea4e05f03fc0a7e94a1ec901aacbdc1ab1 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv <sgot...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
