Omer Frenkel has uploaded a new change for review.
Change subject: Allow AddVmTemplate from non existing vm
......................................................................
Allow AddVmTemplate from non existing vm
Change-Id: Ica688607cca4a8f3480586b08f4b183d104df6ba
Signed-off-by: Omer Frenkel <ofren...@redhat.com>
---
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/action/AddVmTemplateParameters.java
2 files changed, 107 insertions(+), 74 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/07/14307/1
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 0cf670a..fe49b97 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
@@ -34,6 +34,7 @@
import org.ovirt.engine.core.common.businessentities.StorageDomain;
import org.ovirt.engine.core.common.businessentities.StorageDomainStatus;
import org.ovirt.engine.core.common.businessentities.StorageDomainType;
+import org.ovirt.engine.core.common.businessentities.VM;
import org.ovirt.engine.core.common.businessentities.VMStatus;
import org.ovirt.engine.core.common.businessentities.VmDynamic;
import org.ovirt.engine.core.common.businessentities.VmStatic;
@@ -60,6 +61,7 @@
private final List<DiskImage> mImages = new ArrayList<DiskImage>();
private List<PermissionSubject> permissionCheckSubject;
protected Map<Guid, DiskImage> diskInfoDestinationMap;
+ private boolean isVmInDb;
/**
* A list of the new disk images which were saved for the Template.
@@ -84,8 +86,11 @@
setVdsGroupId(parameterMasterVm.getVdsGroupId());
}
if (getVm() != null) {
+ isVmInDb = true;
VmHandler.updateDisksFromDb(getVm());
setStoragePoolId(getVm().getStoragePoolId());
+ } else {
+ setVm(new VM(parameterMasterVm,null,null));
}
diskInfoDestinationMap = parameters.getDiskInfoDestinationMap();
if (diskInfoDestinationMap == null) {
@@ -110,13 +115,16 @@
@Override
protected void executeCommand() {
- // get vm status from db to check its really down before locking
- VmDynamic vmDynamic =
DbFacade.getInstance().getVmDynamicDao().get(getVmId());
- if (vmDynamic.getStatus() != VMStatus.Down) {
- throw new VdcBLLException(VdcBllErrors.IRS_IMAGE_STATUS_ILLEGAL);
- }
+ // if template does not have disks, no need to lock
+ if (!mImages.isEmpty()) {
+ // get vm status from db to check its really down before locking
+ VmDynamic vmDynamic =
DbFacade.getInstance().getVmDynamicDao().get(getVmId());
+ if (vmDynamic.getStatus() != VMStatus.Down) {
+ throw new
VdcBLLException(VdcBllErrors.IRS_IMAGE_STATUS_ILLEGAL);
+ }
- VmHandler.LockVm(vmDynamic, getCompensationContext());
+ VmHandler.LockVm(vmDynamic, getCompensationContext());
+ }
setActionReturnValue(Guid.Empty);
setVmTemplateId(Guid.NewGuid());
getParameters().setVmTemplateId(getVmTemplateId());
@@ -138,7 +146,9 @@
addPermission();
AddVmTemplateImages();
List<VmNetworkInterface> vmInterfaces = addVmInterfaces();
- VmDeviceUtils.copyVmDevices(getVmId(), getVmTemplateId(),
newDiskImages, vmInterfaces);
+ if (isVmInDb) {
+ VmDeviceUtils.copyVmDevices(getVmId(), getVmTemplateId(),
newDiskImages, vmInterfaces);
+ }
setSucceeded(true);
return null;
}
@@ -148,21 +158,21 @@
// end the command synchronously
boolean pendingAsyncTasks =
!getReturnValue().getTaskIdList().isEmpty();
if (!pendingAsyncTasks) {
+ // TODO: temp hack, need to figure a better way to handle vm is
not in db
+ setVm(null);
endSuccessfullySynchronous();
}
}
@Override
protected boolean canDoAction() {
- if (getVdsGroup() == null ||
!getVm().getStoragePoolId().equals(getVdsGroup().getStoragePoolId())) {
+ if (getVdsGroup() == null) {
addCanDoActionMessage(VdcBllMessages.VDS_CLUSTER_IS_NOT_VALID);
return false;
}
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())) {
@@ -170,10 +180,6 @@
}
if
(!IsVmPriorityValueLegal(getParameters().getMasterVm().getPriority(),
getReturnValue()
.getCanDoActionMessages())) {
- return false;
- }
-
- if (!validateVmNotDuringSnapshot()) {
return false;
}
@@ -192,79 +198,93 @@
return false;
}
- Map<Guid, List<DiskImage>> sourceImageDomainsImageMap = new
HashMap<Guid, List<DiskImage>>();
- for (DiskImage image : mImages) {
- MultiValueMapUtils.addToMap(image.getStorageIds().get(0), image,
sourceImageDomainsImageMap);
- if (!diskInfoDestinationMap.containsKey(image.getId())) {
- Guid destStorageId =
- getParameters().getDestinationStorageDomainId() !=
null ? getParameters().getDestinationStorageDomainId()
- : image.getStorageIds().get(0);
- ArrayList<Guid> storageIds = new ArrayList<Guid>();
- storageIds.add(destStorageId);
- image.setStorageIds(storageIds);
- diskInfoDestinationMap.put(image.getId(), image);
+ // images related checks
+ if (!mImages.isEmpty()) {
+ if
(!getVm().getStoragePoolId().equals(getVdsGroup().getStoragePoolId())) {
+ addCanDoActionMessage(VdcBllMessages.VDS_CLUSTER_IS_NOT_VALID);
+ return false;
}
- }
- if (!validate(new StoragePoolValidator(getStoragePool()).isUp())) {
- return false;
- }
+ if (!validateVmNotDuringSnapshot()) {
+ return false;
+ }
- List<DiskImage> diskImagesToCheck =
ImagesHandler.filterImageDisks(mImages, true, false);
- DiskImagesValidator diskImagesValidator = new
DiskImagesValidator(diskImagesToCheck);
- if (!validate(diskImagesValidator.diskImagesNotIllegal()) ||
- !validate(diskImagesValidator.diskImagesNotLocked())) {
- return false;
- }
-
- MultipleStorageDomainsValidator storageDomainsValidator =
- new MultipleStorageDomainsValidator(getStoragePoolId(),
sourceImageDomainsImageMap.keySet());
- if (!validate(storageDomainsValidator.allDomainsExistAndActive())) {
- return false;
- }
-
- Map<Guid, StorageDomain> storageDomains = new HashMap<Guid,
StorageDomain>();
- Set<Guid> destImageDomains = getStorageGuidSet();
- destImageDomains.removeAll(sourceImageDomainsImageMap.keySet());
- for (Guid destImageDomain : destImageDomains) {
- StorageDomain storage =
DbFacade.getInstance().getStorageDomainDao().getForStoragePool(
- destImageDomain, getVm().getStoragePoolId());
- if (storage == null) {
- // if storage is null then we need to check if it doesn't
exist or
- // domain is not in the same storage pool as the vm
- if
(DbFacade.getInstance().getStorageDomainStaticDao().get(destImageDomain) ==
null) {
-
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_EXIST.toString());
- } else {
-
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_IN_STORAGE_POOL);
+ Map<Guid, List<DiskImage>> sourceImageDomainsImageMap = new
HashMap<Guid, List<DiskImage>>();
+ for (DiskImage image : mImages) {
+ MultiValueMapUtils.addToMap(image.getStorageIds().get(0),
image, sourceImageDomainsImageMap);
+ if (!diskInfoDestinationMap.containsKey(image.getId())) {
+ Guid destStorageId =
+ getParameters().getDestinationStorageDomainId() !=
null ? getParameters().getDestinationStorageDomainId()
+ : image.getStorageIds().get(0);
+ ArrayList<Guid> storageIds = new ArrayList<Guid>();
+ storageIds.add(destStorageId);
+ image.setStorageIds(storageIds);
+ diskInfoDestinationMap.put(image.getId(), image);
}
- return false;
}
- if (storage.getStatus() == null || storage.getStatus() !=
StorageDomainStatus.Active) {
-
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_STATUS_ILLEGAL.toString());
+
+ if (!validate(new StoragePoolValidator(getStoragePool()).isUp())) {
return false;
}
- if (storage.getStorageDomainType() ==
StorageDomainType.ImportExport
- || storage.getStorageDomainType() ==
StorageDomainType.ISO) {
-
-
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_TYPE_ILLEGAL);
+ List<DiskImage> diskImagesToCheck =
ImagesHandler.filterImageDisks(mImages, true, false);
+ DiskImagesValidator diskImagesValidator = new
DiskImagesValidator(diskImagesToCheck);
+ if (!validate(diskImagesValidator.diskImagesNotIllegal()) ||
+ !validate(diskImagesValidator.diskImagesNotLocked())) {
return false;
}
- storageDomains.put(destImageDomain, storage);
- }
- // update vm snapshots for storage free space check
- ImagesHandler.fillImagesBySnapshots(getVm());
- Map<StorageDomain, Integer> domainMap =
- StorageDomainValidator.getSpaceRequirementsForStorageDomains(
-
ImagesHandler.filterImageDisks(getVm().getDiskMap().values(), true, false),
- storageDomains,
- diskInfoDestinationMap);
- for (Map.Entry<StorageDomain, Integer> entry : domainMap.entrySet()) {
- if (!doesStorageDomainhaveSpaceForRequest(entry.getKey(),
entry.getValue())) {
+ MultipleStorageDomainsValidator storageDomainsValidator =
+ new MultipleStorageDomainsValidator(getStoragePoolId(),
sourceImageDomainsImageMap.keySet());
+ if (!validate(storageDomainsValidator.allDomainsExistAndActive()))
{
return false;
+ }
+
+ Map<Guid, StorageDomain> storageDomains = new HashMap<Guid,
StorageDomain>();
+ Set<Guid> destImageDomains = getStorageGuidSet();
+ destImageDomains.removeAll(sourceImageDomainsImageMap.keySet());
+ for (Guid destImageDomain : destImageDomains) {
+ StorageDomain storage =
DbFacade.getInstance().getStorageDomainDao().getForStoragePool(
+ destImageDomain, getVm().getStoragePoolId());
+ if (storage == null) {
+ // if storage is null then we need to check if it doesn't
exist or
+ // domain is not in the same storage pool as the vm
+ if
(DbFacade.getInstance().getStorageDomainStaticDao().get(destImageDomain) ==
null) {
+
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_EXIST.toString());
+ } else {
+
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_IN_STORAGE_POOL);
+ }
+ return false;
+ }
+ if (storage.getStatus() == null || storage.getStatus() !=
StorageDomainStatus.Active) {
+
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_STATUS_ILLEGAL.toString());
+ return false;
+ }
+
+ if (storage.getStorageDomainType() ==
StorageDomainType.ImportExport
+ || storage.getStorageDomainType() ==
StorageDomainType.ISO) {
+
+
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_TYPE_ILLEGAL);
+ return false;
+ }
+ storageDomains.put(destImageDomain, storage);
+ }
+ // update vm snapshots for storage free space check
+ ImagesHandler.fillImagesBySnapshots(getVm());
+
+ Map<StorageDomain, Integer> domainMap =
+
StorageDomainValidator.getSpaceRequirementsForStorageDomains(
+
ImagesHandler.filterImageDisks(getVm().getDiskMap().values(), true, false),
+ storageDomains,
+ diskInfoDestinationMap);
+
+ for (Map.Entry<StorageDomain, Integer> entry :
domainMap.entrySet()) {
+ if (!doesStorageDomainhaveSpaceForRequest(entry.getKey(),
entry.getValue())) {
+ return false;
+ }
}
}
+
return
AddVmCommand.CheckCpuSockets(getParameters().getMasterVm().getNumOfSockets(),
getParameters().getMasterVm().getCpuPerSocket(), getVdsGroup()
.getcompatibility_version().toString(),
getReturnValue().getCanDoActionMessages());
@@ -325,6 +345,7 @@
getVmTemplate().setQuotaId(getParameters().getMasterVm().getQuotaId());
getVmTemplate().setDedicatedVmForVds(getParameters().getMasterVm().getDedicatedVmForVds());
getVmTemplate().setMigrationSupport(getParameters().getMasterVm().getMigrationSupport());
+ getVmTemplate().setTemplateType(getParameters().getTemplateType());
DbFacade.getInstance().getVmTemplateDao().save(getVmTemplate());
getCompensationContext().snapshotNewEntity(getVmTemplate());
setActionReturnValue(getVmTemplate().getId());
@@ -413,7 +434,9 @@
}
private void endUnlockOps() {
- VmHandler.UnLockVm(getVm());
+ if (getVm() != null) {
+ VmHandler.UnLockVm(getVm());
+ }
VmTemplateHandler.UnLockVmTemplate(getVmTemplateId());
}
diff --git
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/AddVmTemplateParameters.java
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/AddVmTemplateParameters.java
index 8c7628e..c2afc12 100644
---
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/AddVmTemplateParameters.java
+++
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/AddVmTemplateParameters.java
@@ -6,6 +6,7 @@
import javax.validation.constraints.Size;
import org.ovirt.engine.core.common.businessentities.DiskImage;
+import org.ovirt.engine.core.common.businessentities.TemplateType;
import org.ovirt.engine.core.common.businessentities.VM;
import org.ovirt.engine.core.common.businessentities.VmStatic;
import org.ovirt.engine.core.common.utils.ValidationUtils;
@@ -25,6 +26,7 @@
private Guid privateVmTemplateID = Guid.Empty;
private Guid destinationStorageDomainId;
private HashMap<Guid, DiskImage> diskInfoDestinationMap;
+ private TemplateType templateType = TemplateType.TEMPLATE;
@Size(max = 40, message = "VALIDATION.VM_TEMPLATE.NAME.MAX", groups = {
CreateEntity.class, UpdateEntity.class })
@ValidI18NName(message =
"ACTION_TYPE_FAILED_NAME_MAY_NOT_CONTAIN_SPECIAL_CHARS")
@@ -102,4 +104,12 @@
public void setDiskInfoDestinationMap(HashMap<Guid, DiskImage>
diskInfoDestinationMap) {
this.diskInfoDestinationMap = diskInfoDestinationMap;
}
+
+ public TemplateType getTemplateType() {
+ return templateType;
+ }
+
+ public void setTemplateType(TemplateType templateType) {
+ this.templateType = templateType;
+ }
}
--
To view, visit http://gerrit.ovirt.org/14307
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ica688607cca4a8f3480586b08f4b183d104df6ba
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Omer Frenkel <ofren...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
