Omer Frenkel has uploaded a new change for review. Change subject: core: use instance type in addVm + permissions ......................................................................
core: use instance type in addVm + permissions Change-Id: If9058907d1021b09981671e905defb91ed645d65 Signed-off-by: Omer Frenkel <[email protected]> --- 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/AddVmTemplateCommand.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/ActionGroup.java 3 files changed, 133 insertions(+), 15 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/81/12281/1 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 8b90c6a..4760ae7 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 @@ -28,8 +28,11 @@ import org.ovirt.engine.core.common.businessentities.ActionGroup; import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.DiskImage; +import org.ovirt.engine.core.common.businessentities.ImageType; +import org.ovirt.engine.core.common.businessentities.InstanceType; import org.ovirt.engine.core.common.businessentities.MigrationSupport; import org.ovirt.engine.core.common.businessentities.OriginType; +import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.VDSGroup; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.VMStatus; @@ -38,8 +41,8 @@ import org.ovirt.engine.core.common.businessentities.VmPayload; import org.ovirt.engine.core.common.businessentities.VmStatic; import org.ovirt.engine.core.common.businessentities.VmStatistics; +import org.ovirt.engine.core.common.businessentities.VmTemplate; import org.ovirt.engine.core.common.businessentities.permissions; -import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.network.VmInterfaceType; import org.ovirt.engine.core.common.businessentities.network.VmNetworkInterface; import org.ovirt.engine.core.common.config.Config; @@ -52,6 +55,7 @@ import org.ovirt.engine.core.common.utils.VmDeviceType; import org.ovirt.engine.core.common.validation.group.CreateEntity; import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.compat.NGuid; import org.ovirt.engine.core.dal.VdcBllMessages; import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.dao.VmDynamicDAO; @@ -76,6 +80,12 @@ protected Map<Guid, StorageDomain> destStorages = new HashMap<Guid, StorageDomain>(); protected Map<Guid, List<DiskImage>> storageToDisksMap; protected String newMac = ""; + protected NGuid instanceTypeId; + protected InstanceType instanceType; + protected NGuid imageTypeId; + protected ImageType imageType; + private Guid vmInterfacesSourceId; + private VmTemplate vmDisksSource; /** * A list of the new disk images which were saved for the VM. @@ -91,19 +101,41 @@ setStorageDomainId(getParameters().getStorageDomainId()); if (parameters.getVmStaticData() != null) { setVmTemplateId(parameters.getVmStaticData().getVmtGuid()); + instanceTypeId = parameters.getVmStaticData().getInstanceTypeId(); + imageTypeId = parameters.getVmStaticData().getImageTypeId(); + vmInterfacesSourceId = parameters.getVmStaticData().getVmtGuid(); + vmDisksSource = getVmTemplate(); } - parameters.setEntityId(getVmId()); + + // override values here for canDoACtion to run with correct values, has to come before init-disks + updateVmObject(); + initTemplateDisks(); initStoragePoolId(); diskInfoDestinationMap = getParameters().getDiskInfoDestinationMap(); if (diskInfoDestinationMap == null) { diskInfoDestinationMap = new HashMap<Guid, DiskImage>(); } + } protected AddVmCommand(Guid commandId) { super(commandId); + } + + protected InstanceType getInstanceType() { + if (instanceType == null && instanceTypeId != null) { + instanceType = getVmTemplateDAO().getInstanceType(instanceTypeId.getValue()); + } + return instanceType; + } + + protected ImageType getImageType() { + if (imageType == null && imageTypeId != null) { + imageType = getVmTemplateDAO().getImageType(imageTypeId.getValue()); + } + return imageType; } protected void initStoragePoolId() { @@ -114,8 +146,8 @@ } protected void initTemplateDisks() { - if (getVmTemplate() != null) { - VmTemplateHandler.UpdateDisksFromDb(getVmTemplate()); + if (vmDisksSource != null) { + VmTemplateHandler.UpdateDisksFromDb(vmDisksSource); } } @@ -130,7 +162,7 @@ protected List<VmNetworkInterface> getVmInterfaces() { if (_vmInterfaces == null) { List<VmNetworkInterface> vmNetworkInterfaces = - DbFacade.getInstance().getVmNetworkInterfaceDao().getAllForTemplate(getVmTemplate().getId()); + DbFacade.getInstance().getVmNetworkInterfaceDao().getAllForTemplate(vmInterfacesSourceId); _vmInterfaces = (vmNetworkInterfaces != null) ? vmNetworkInterfaces : new ArrayList<VmNetworkInterface>(); @@ -145,7 +177,7 @@ _vmDisks = DbFacade.getInstance() .getDiskDao() - .getAllForVm(getVmTemplateId()); + .getAllForVm(vmDisksSource.getId()); } return _vmDisks; @@ -193,7 +225,7 @@ } protected Guid getStoragePoolIdFromSourceImageContainer() { - return getVmTemplate().getstorage_pool_id().getValue(); + return vmDisksSource.getstorage_pool_id().getValue(); } protected int getNeededDiskSize(Guid domainId) { @@ -337,6 +369,17 @@ return failCanDoAction(VdcBllMessages.VM_HOSTCPU_MUST_BE_PINNED_TO_HOST); } + //TODO: think of add vm in pool + if (returnValue && instanceTypeId != null && getInstanceType() == null) { + returnValue = false; + // invalid instance type + // addCanDoActionMessage(); + } + if (returnValue && imageTypeId != null && getImageType() == null) { + returnValue = false; + // invalid image type + // addCanDoActionMessage(); + } return returnValue && checkCpuSockets(); } @@ -354,8 +397,8 @@ retValue = validateProvidedDestinations(); } if (retValue && getImagesToCheckDestinationStorageDomains().size() != diskInfoDestinationMap.size()) { - log.errorFormat("Can not found any default active domain for one of the disks of template with id : {0}", - getVmTemplate().getId()); + log.errorFormat("Can not find any default active domain for one of the disks of template with id : {0}", + vmDisksSource.getId()); addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_MISSED_STORAGES_FOR_SOME_DISKS); retValue = false; } @@ -364,7 +407,7 @@ } protected Collection<DiskImage> getImagesToCheckDestinationStorageDomains() { - return getVmTemplate().getDiskMap().values(); + return vmDisksSource.getDiskMap().values(); } private boolean validateProvidedDestinations() { @@ -402,8 +445,8 @@ } protected void fillImagesMapBasedOnTemplate() { - ImagesHandler.fillImagesMapBasedOnTemplate(getVmTemplate(), - getStorageDomainDAO().getAllForStoragePool(getVmTemplate().getstorage_pool_id().getValue()), + ImagesHandler.fillImagesMapBasedOnTemplate(vmDisksSource, + getStorageDomainDAO().getAllForStoragePool(vmDisksSource.getstorage_pool_id().getValue()), diskInfoDestinationMap, destStorages, false); } @@ -453,7 +496,7 @@ if (returnValue && !getParameters().getDontCheckTemplateImages()) { for (StorageDomain storage : destStorages.values()) { - if (!VmTemplateCommand.isVmTemplateImagesReady(getVmTemplate(), storage.getId(), + if (!VmTemplateCommand.isVmTemplateImagesReady(vmDisksSource, storage.getId(), reasons, false, checkTemplateLock, true, true, storageToDisksMap.get(storage.getId()))) { return false; } @@ -627,7 +670,7 @@ } protected boolean addVmImages() { - if (getVmTemplate().getDiskMap().size() > 0) { + if (vmDisksSource.getDiskMap().size() > 0) { if (getVm().getStatus() != VMStatus.Down) { log.error("Cannot add images. VM is not Down"); throw new VdcBLLException(VdcBllErrors.IRS_IMAGE_STATUS_ILLEGAL); @@ -707,13 +750,48 @@ permissionList.add(new PermissionSubject(getVmTemplateId(), VdcObjectType.VmTemplate, getActionType().getActionGroup())); - if (getVmTemplate() != null && !getVmTemplate().getDiskList().isEmpty()) { + + // if using instance type, need create instance + if (getInstanceType() != null) { + permissionList.add(new PermissionSubject(instanceTypeId.getValue(), VdcObjectType.VmTemplate, ActionGroup.CREATE_INSTANCE)); + } + // if using image type, need create instance + if (getImageType() != null) { + permissionList.add(new PermissionSubject(imageTypeId.getValue(), VdcObjectType.VmTemplate, ActionGroup.CREATE_INSTANCE)); + } + if (getVmTemplate() != null && !vmDisksSource.getDiskList().isEmpty()) { addStoragePermissionByQuotaMode(permissionList, GuidUtils.getGuidValue(getStoragePoolId()), GuidUtils.getGuidValue(getStorageDomainId())); } addPermissionSubjectForCustomProperties(permissionList); return permissionList; + } + + /** + * user need permission on each object used: template, instance type, image type.. + * for cluster: if instance type is used, create_instance is enough + * but if user has create_template he can choose to use instance type or template + */ + @Override + protected boolean checkPermissions(final List<PermissionSubject> permSubjects) { + for (PermissionSubject permSubject : permSubjects) { + // if user is using instance type, then create_instance on the cluster is enough + if (permSubject.getObjectType() == VdcObjectType.VdsGroups && instanceTypeId != null) { + permSubject.setActionGroup(ActionGroup.CREATE_INSTANCE); + if (checkSinglePermission(permSubject, getReturnValue().getCanDoActionMessages())) { + continue; + } + + // create_vm is overriding in case no create_instance, try again with it + permSubject.setActionGroup(getActionType().getActionGroup()); + } + + if (!checkSinglePermission(permSubject, getReturnValue().getCanDoActionMessages())) { + return false; + } + } + return true; } protected void addPermissionSubjectForCustomProperties(List<PermissionSubject> permissionList) { @@ -796,4 +874,40 @@ } return list; } + + /** + * This method override vm values with the instance type values + * in case instance type is selected for this vm + */ + private void updateVmObject() { + InstanceType instanceType = getInstanceType(); + VmStatic vmStatic = getParameters().getVmStaticData(); + if (instanceType != null) { + vmStatic.setMemSizeMb(instanceType.getMemSizeMb()); + vmStatic.setNumOfSockets(instanceType.getNumOfSockets()); + vmStatic.setCpuPerSocket(instanceType.getCpuPerSocket()); + vmStatic.setDefaultBootSequence(instanceType.getDefaultBootSequence()); + vmStatic.setDefaultDisplayType(instanceType.getDefaultDisplayType()); + vmStatic.setPriority(instanceType.getPriority()); + //vmStatic.setMinAllocatedMem(instanceType.getmin()); + //override smart-card device + //override payload device + //override sound-card device + //override balloon device + + // set vm interface source id to be the instance type, vm interface are taken from it + vmInterfacesSourceId = instanceType.getId(); + } + + ImageType imageType = getImageType(); + if (imageType != null) { + vmStatic.setOs(imageType.getOs()); + vmStatic.setIsoPath(imageType.getIsoPath()); + vmStatic.setInitrdUrl(imageType.getInitrdUrl()); + vmStatic.setKernelUrl(imageType.getKernelUrl()); + vmStatic.setKernelParams(imageType.getKernelParams()); + // set vm disks source to be the image type, vm disks are taken from it + vmDisksSource = (VmTemplate)imageType; + } + } } 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 c77f4e5..dcc512f 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 @@ -155,6 +155,8 @@ for (DiskImage diskImage : getVm().getDiskList()) { mImages.add(diskImage); } + + // TODO: can ImageType be without images? if (!VmHandler.isMemorySizeLegal(getParameters().getMasterVm().getOs(), getParameters().getMasterVm().getMemSizeMb(), getReturnValue().getCanDoActionMessages(), getVdsGroup().getcompatibility_version().toString())) { diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/ActionGroup.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/ActionGroup.java index 43eceaf..bbd1814 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/ActionGroup.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/ActionGroup.java @@ -36,6 +36,8 @@ CHANGE_VM_CUSTOM_PROPERTIES(14, RoleType.ADMIN, VdcObjectType.VM, true, ApplicationMode.VirtOnly), + CREATE_INSTANCE(15, RoleType.USER, VdcObjectType.VM, false, ApplicationMode.VirtOnly), + // host (vds) actions groups CREATE_HOST(100, RoleType.ADMIN, VdcObjectType.VDS, true), EDIT_HOST_CONFIGURATION(101, RoleType.ADMIN, VdcObjectType.VDS, true), -- To view, visit http://gerrit.ovirt.org/12281 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: If9058907d1021b09981671e905defb91ed645d65 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Omer Frenkel <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
