Shahar Havivi has uploaded a new change for review. Change subject: engine: cannot set payload and cloud-init on same devices. ......................................................................
engine: cannot set payload and cloud-init on same devices. On initial run we automatic set cloud-init/sysprep as cdrom/floppy device - this cause conflict when using the same media. Change-Id: Ic5b701f058a2d87b2836d337f822d2f012d10bbe Bug-Url: https://bugzilla.redhat.com/1138314 Signed-off-by: Shahar Havivi <shah...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java M backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java M frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java M frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties M frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties 8 files changed, 110 insertions(+), 41 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/84/33884/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java index ed0c849..c863705 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java @@ -53,6 +53,7 @@ import org.ovirt.engine.core.common.businessentities.VmDevice; import org.ovirt.engine.core.common.businessentities.VmDeviceGeneralType; import org.ovirt.engine.core.common.businessentities.VmDeviceId; +import org.ovirt.engine.core.common.businessentities.VmPayload; import org.ovirt.engine.core.common.businessentities.VmPool; import org.ovirt.engine.core.common.businessentities.VmPoolType; import org.ovirt.engine.core.common.businessentities.VmRngDevice; @@ -127,6 +128,8 @@ super(runVmParams, commandContext); getParameters().setEntityInfo(new EntityInfo(VdcObjectType.VM, runVmParams.getVmId())); setStoragePoolId(getVm() != null ? getVm().getStoragePoolId() : null); + // Load payload from Database (only if none was sent via the parameters) + loadPayloadDevice(); } @@ -665,18 +668,34 @@ // if vm not initialized, use sysprep/cloud-init if (!getVm().isInitialized()) { VmHandler.updateVmInitFromDB(getVm().getStaticData(), false); + getVm().setInitializationType(InitializationType.None); if (osRepository.isWindows(getVm().getVmOsId())) { - getVm().setInitializationType(InitializationType.Sysprep); + if (!isPayloadExists(VmDeviceType.FLOPPY)) { + getVm().setInitializationType(InitializationType.Sysprep); + } } else if (getVm().getVmInit() != null) { - getVm().setInitializationType(InitializationType.CloudInit); - } - else { - getVm().setInitializationType(InitializationType.None); + if (!isPayloadExists(VmDeviceType.CDROM)) { + getVm().setInitializationType(InitializationType.CloudInit); + } } } } else { getVm().setInitializationType(getParameters().getInitializationType()); + // If the user asked for sysprep/cloud-init via run-once we eliminate + // the payload since we can only have one media (Floppy/CDROM) per payload. + if (getParameters().getInitializationType() == InitializationType.Sysprep && + isPayloadExists(VmDeviceType.FLOPPY)) { + getVm().setVmPayload(null); + } else if (getParameters().getInitializationType() == InitializationType.CloudInit && + isPayloadExists(VmDeviceType.CDROM)) { + getVm().setVmPayload(null); + } + } + // if we asked for floppy from Iso Domain we cannot + // have floppy payload since we are limited to only one floppy device + if (!StringUtils.isEmpty(getParameters().getFloppyPath()) && isPayloadExists(VmDeviceType.FLOPPY)) { + getVm().setVmPayload(null); } // get what cpu flags should be passed to vdsm according to cluster @@ -692,6 +711,37 @@ if (getFlow() != RunVmFlow.RESUME_HIBERNATE) { getVm().setHibernationVolHandle(getMemoryFromSnapshot()); } + } + + protected boolean isPayloadExists(VmDeviceType deviceType) { + if (getVm().getVmPayload() != null && getVm().getVmPayload().getDeviceType().equals(deviceType)) { + return true; + } + return false; + } + + protected void loadPayloadDevice() { + if (getParameters().getVmPayload() == null) { + VmPayload payload = getVmPayloadByDeviceType(VmDeviceType.CDROM); + if (payload != null) { + getVm().setVmPayload(payload); + } else { + getVm().setVmPayload(getVmPayloadByDeviceType(VmDeviceType.FLOPPY)); + } + } + } + + protected VmPayload getVmPayloadByDeviceType(VmDeviceType deviceType) { + List<VmDevice> vmDevices = getVmDeviceDao() + .getVmDeviceByVmIdTypeAndDevice(getVm().getId(), + VmDeviceGeneralType.DISK, + deviceType.getName()); + for (VmDevice vmDevice : vmDevices) { + if (VmPayload.isPayload(vmDevice.getSpecParams())) { + return new VmPayload(vmDevice); + } + } + return null; } protected void fetchVmDisksFromDb() { @@ -857,6 +907,20 @@ return validateSpaceRequirements(); } + // Note: that we are setting the payload from database in the ctor. + // + // Checking if the user sent Payload and Sysprep/Cloud-init at the same media - + // Currently we cannot use two payloads in the same media (cdrom/floppy) + if (getParameters().getInitializationType() != null) { + if (getParameters().getInitializationType() == InitializationType.Sysprep && getParameters().getVmPayload() != null && + getParameters().getVmPayload().getDeviceType() == VmDeviceType.FLOPPY) { + return failCanDoAction(VdcBllMessages.VMPAYLOAD_FLOPPY_WITH_SYSPREP); + } else if (getParameters().getInitializationType() == InitializationType.CloudInit && getParameters().getVmPayload() != null && + getParameters().getVmPayload().getDeviceType() == VmDeviceType.CDROM) { + return failCanDoAction(VdcBllMessages.VMPAYLOAD_CDROM_WITH_CLOUD_INIT); + } + } + return true; } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java index 58bd857..92715bc 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java @@ -19,7 +19,6 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; - import java.util.Set; import org.junit.Assert; import org.junit.Before; @@ -330,7 +329,11 @@ SimpleDependecyInjector.getInstance().bind(OsRepository.class, osRepository); RunVmParams param = new RunVmParams(Guid.newGuid()); - command = spy(new RunVmCommand<RunVmParams>(param)); + command = spy(new RunVmCommand<RunVmParams>(param) { + @Override + protected void loadPayloadDevice() { + } + }); mockIsoDomainListSyncronizer(); mockSuccessfulRunVmValidator(); doNothing().when(command).initParametersForExternalNetworks(); diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java index 6937646..92a1de8 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java @@ -654,6 +654,8 @@ VMPAYLOAD_INVALID_PAYLOAD_TYPE(ErrorType.CONFLICT), VMPAYLOAD_SIZE_EXCEEDED(ErrorType.CONSTRAINT_VIOLATION), VMPAYLOAD_FLOPPY_EXCEEDED(ErrorType.CONFLICT), + VMPAYLOAD_FLOPPY_WITH_SYSPREP(ErrorType.CONFLICT), + VMPAYLOAD_CDROM_WITH_CLOUD_INIT(ErrorType.CONFLICT), ACTION_TYPE_FAILED_BOOKMARK_NAME_ALREADY_EXISTS(ErrorType.CONFLICT), ACTION_TYPE_FAILED_BOOKMARK_INVALID_ID(ErrorType.BAD_PARAMETERS), diff --git a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties index 5bafc36..c20761a 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -1059,6 +1059,8 @@ VMPAYLOAD_INVALID_PAYLOAD_TYPE=VM Payload only supported in CDROM or Floppy devices VMPAYLOAD_SIZE_EXCEEDED=Payload is limited to ${size}K VMPAYLOAD_FLOPPY_EXCEEDED=Payload Floppy cannot be used when using an additional Floppy +VMPAYLOAD_FLOPPY_WITH_SYSPREP=Payload floppy deivce cannot be used with Sysprep via floppy device. +VMPAYLOAD_CDROM_WITH_CLOUD_INIT=Payload cdrom deivce cannot be used with Cloud-Init via cdrom device. ACTION_TYPE_FAILED_GLUSTER_VOLUME_IS_UP=Cannot ${action} ${type}. Gluster volume ${volumeName} is up. ACTION_TYPE_FAILED_GLUSTER_VOLUME_IS_DOWN=Cannot ${action} ${type}. Gluster Volume is down. ACTION_TYPE_FAILED_CAN_NOT_REMOVE_ALL_BRICKS_FROM_VOLUME=Cannot ${action} ${type}. Cannot remove all the bricks from a Volume. diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java index 121c472..c919eae 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java @@ -130,24 +130,12 @@ @Override protected void buildVmCD() { Map<String, Object> struct; + boolean hasPayload = vm.getVmPayload() != null && vm.getVmPayload().getDeviceType() == VmDeviceType.CDROM; // check if we have payload CD - if (vm.getVmPayload() != null && vm.getVmPayload().getDeviceType() == VmDeviceType.CDROM) { - VmDevice vmDevice = - new VmDevice(new VmDeviceId(Guid.newGuid(), vm.getId()), - VmDeviceGeneralType.DISK, - VmDeviceType.CDROM.getName(), - "", - 0, - vm.getVmPayload().getSpecParams(), - true, - true, - true, - "", - null, - null); + if (hasPayload) { struct = new HashMap<String, Object>(); - addCdDetails(vmDevice, struct, vm); - addDevice(struct, vmDevice, ""); + addCdDetails(vm.getVmPayload(), struct, vm); + addDevice(struct, vm.getVmPayload(), ""); } // check first if CD was given as a parameter if (vm.isRunOnce() && !StringUtils.isEmpty(vm.getCdPath())) { @@ -180,6 +168,12 @@ if (!vmDevice.getIsManaged()) { continue; } + // The Payload is loaded in via RunVmCommand to VM.Payload + // and its handled at the beginning of the method, so no + // need to add the device again + if (VmPayload.isPayload(vmDevice.getSpecParams())) { + continue; + } struct = new HashMap<String, Object>(); String cdPath = vm.getCdPath(); addCdDetails(vmDevice, struct, vm); @@ -191,24 +185,12 @@ @Override protected void buildVmFloppy() { - // check if we have payload CD - if (vm.getVmPayload() != null && vm.getVmPayload().getDeviceType() == VmDeviceType.FLOPPY) { - VmDevice vmDevice = - new VmDevice(new VmDeviceId(Guid.newGuid(), vm.getId()), - VmDeviceGeneralType.DISK, - VmDeviceType.FLOPPY.getName(), - "", - 0, - (vm.getVmPayload() == null) ? null : vm.getVmPayload().getSpecParams(), - true, - true, - true, - "", - null, - null); + // check if we have payload Floppy + boolean hasPayload = vm.getVmPayload() != null && vm.getVmPayload().getDeviceType() == VmDeviceType.FLOPPY; + if (hasPayload) { Map<String, Object> struct = new HashMap<String, Object>(); - addCdDetails(vmDevice, struct, vm); - addDevice(struct, vmDevice, ""); + addFloppyDetails(vm.getVmPayload(), struct); + addDevice(struct, vm.getVmPayload(), ""); } // check first if Floppy was given as a parameter else if (vm.isRunOnce() && !StringUtils.isEmpty(vm.getFloppyPath())) { @@ -218,7 +200,7 @@ VmDeviceType.FLOPPY.getName(), "", 0, - (vm.getVmPayload() == null) ? null : vm.getVmPayload().getSpecParams(), + null, true, true, true, @@ -246,6 +228,12 @@ if (!VmPayload.isPayload(vmDevice.getSpecParams()) && vmDevices.size() > 1) { continue; } + // The Payload is loaded in via RunVmCommand to VM.Payload + // and its handled at the beginning of the method, so no + // need to add the device again + if (VmPayload.isPayload(vmDevice.getSpecParams())) { + continue; + } Map<String, Object> struct = new HashMap<String, Object>(); String file = vm.getFloppyPath(); addFloppyDetails(vmDevice, struct); diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java index 9df22a7..376caa3 100644 --- a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java @@ -2771,6 +2771,12 @@ String VMPAYLOAD_FLOPPY_EXCEEDED(); + @DefaultStringValue("Payload floppy deivce cannot be used with Sysprep via floppy device.") + String VMPAYLOAD_FLOPPY_WITH_SYSPREP(); + + @DefaultStringValue("Payload cdrom deivce cannot be used with Cloud-Init via cdrom device.") + String VMPAYLOAD_CDROM_WITH_CLOUD_INIT(); + // Gluster Messages @DefaultStringValue("Cannot ${action} ${type}. Cluster ID is not valid.") String ACTION_TYPE_FAILED_CLUSTER_IS_NOT_VALID(); diff --git a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties index eefeaa0..b532903 100644 --- a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties +++ b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties @@ -966,6 +966,8 @@ VMPAYLOAD_INVALID_PAYLOAD_TYPE=VM Payload only supported in CDROM or Floppy devices VMPAYLOAD_SIZE_EXCEEDED=Payload is limited to ${size}K VMPAYLOAD_FLOPPY_EXCEEDED=Payload Floppy cannot be used when using an additional Floppy +VMPAYLOAD_FLOPPY_WITH_SYSPREP=Payload floppy deivce cannot be used with Sysprep via floppy device. +VMPAYLOAD_CDROM_WITH_CLOUD_INIT=Payload cdrom deivce cannot be used with Cloud-Init via cdrom device. ERROR_GET_STORAGE_DOMAIN_LIST=Cannot get Storage Domains list. ACTION_TYPE_FAILED_VDS_CLUSTER_DIFFERENT_ARCHITECTURES=Cannot ${action} ${type}. The host and the destination cluster architectures do not match. diff --git a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties index 2f97644..1013deb 100644 --- a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties +++ b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties @@ -1009,6 +1009,8 @@ VMPAYLOAD_INVALID_PAYLOAD_TYPE=VM Payload only supported in CDROM or Floppy devices VMPAYLOAD_SIZE_EXCEEDED=Payload is limited to ${size}K VMPAYLOAD_FLOPPY_EXCEEDED=Payload Floppy cannot be used when using an additional Floppy +VMPAYLOAD_FLOPPY_WITH_SYSPREP=Payload floppy deivce cannot be used with Sysprep via floppy device. +VMPAYLOAD_CDROM_WITH_CLOUD_INIT=Payload cdrom deivce cannot be used with Cloud-Init via cdrom device. ERROR_GET_STORAGE_DOMAIN_LIST=Cannot get Storage Domains list. ERROR_GET_IMAGE_LIST=Cannot get list of images from Storage Domain '${sdName}'. Please try again later. ACTION_TYPE_FAILED_VDS_CLUSTER_DIFFERENT_ARCHITECTURES=Cannot ${action} ${type}. The host and the destination cluster architectures do not match. -- To view, visit http://gerrit.ovirt.org/33884 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ic5b701f058a2d87b2836d337f822d2f012d10bbe Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.5 Gerrit-Owner: Shahar Havivi <shav...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
