Ori Liel has uploaded a new change for review.
Change subject: restapi: BZ861929 - Don't require template for clone-vm from
snapshot
......................................................................
restapi: BZ861929 - Don't require template for clone-vm from snapshot
Change-Id: I5e49482c2777c5fd6d0aa97ba2c4e1b37c2bec2e
Signed-off-by: Ori Liel <ol...@redhat.com>
---
M
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmsResource.java
M
backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmsResourceTest.java
2 files changed, 79 insertions(+), 63 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/33/8433/1
diff --git
a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmsResource.java
b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmsResource.java
index 06423a3..dd4d854 100644
---
a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmsResource.java
+++
b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmsResource.java
@@ -78,64 +78,74 @@
@Override
public Response add(VM vm) {
- validateParameters(vm, "name", "template.id|name", "cluster.id|name");
+ validateParameters(vm, "name", "cluster.id|name");
validateEnums(VM.class, vm);
- Guid templateId = getTemplateId(vm);
- VmStatic staticVm = getMapper(VM.class, VmStatic.class).map(vm,
-
getMapper(VmTemplate.class, VmStatic.class).map(lookupTemplate(templateId),
null));
-
- if (namedCluster(vm)) {
- staticVm.setvds_group_id(getClusterId(vm));
- }
-
- UsbPolicy usbPolicy = UsbResourceUtils.getUsbPolicy(vm.getUsb(),
lookupCluster(staticVm.getvds_group_id()));
- if (usbPolicy != null) {
- staticVm.setusb_policy(usbPolicy);
- }
-
- if (!isFiltered()) {
- //if the user set the host-name within placement-policy, rather
than the host-id (legal) -
- //resolve the host's ID, because it will be needed down the line
- if (vm.isSetPlacementPolicy() &&
vm.getPlacementPolicy().isSetHost()
- && vm.getPlacementPolicy().getHost().isSetName() &&
!vm.getPlacementPolicy().getHost().isSetId()) {
-
vm.getPlacementPolicy().getHost().setId(getHostId(vm.getPlacementPolicy().getHost().getName()));
- }
- } else {
- vm.setPlacementPolicy(null);
- }
-
Response response = null;
- Guid storageDomainId = ( vm.isSetStorageDomain() &&
vm.getStorageDomain().isSetId() ) ? asGuid(vm.getStorageDomain().getId()) :
Guid.Empty;
- if (vm.isSetSnapshots() && vm.getSnapshots().getSnapshots() != null
- && !vm.getSnapshots().getSnapshots().isEmpty()) {
- // If Vm has snapshots collection - this is a clone vm from
snapshot operation
- String snapshotId = getSnapshotId(vm.getSnapshots());
- org.ovirt.engine.core.common.businessentities.VM vmConfiguration =
getVmConfiguration(snapshotId);
- getMapper(VM.class, VmStatic.class).map(vm,
vmConfiguration.getStaticData());
- // If vm passed in the call has disks attached on them,
- // merge their data with the data of the disks on the configuration
- // The parameters to AddVmFromSnapshot hold an array list of Disks
- // and not List of Disks, as this is a GWT serialization
limitation,
- // and this parameter class serves GWT clients as well.
- HashMap<Guid, DiskImage> diskImagesByImageId =
getDiskImagesByIdMap(vmConfiguration.getDiskMap().values());
- if (vm.isSetDisks()) {
- prepareImagesForCloneFromSnapshotParams(vm.getDisks(),
diskImagesByImageId);
- }
- response =
- cloneVmFromSnapshot(vmConfiguration.getStaticData(),
- snapshotId,
- diskImagesByImageId);
- } else if (vm.isSetDisks() && vm.getDisks().isSetClone() &&
vm.getDisks().isClone()) {
- response = cloneVmFromTemplate(staticVm, vm, templateId);
- } else if (Guid.Empty.equals(templateId)) {
- response = addVmFromScratch(staticVm, vm, storageDomainId);
+ if (isCreateFromSnapshot(vm)) {
+ response = createVmFromSnapshot(vm);
} else {
- response = addVm(staticVm, vm, storageDomainId, templateId);
+ validateParameters(vm, "template.id|name");
+ Guid templateId = getTemplateId(vm);
+ VmStatic staticVm = getMapper(VM.class, VmStatic.class).map(vm,
+ getMapper(VmTemplate.class,
VmStatic.class).map(lookupTemplate(templateId), null));
+ if (namedCluster(vm)) {
+ staticVm.setvds_group_id(getClusterId(vm));
+ }
+ UsbPolicy usbPolicy = UsbResourceUtils.getUsbPolicy(vm.getUsb(),
lookupCluster(staticVm.getvds_group_id()));
+ if (usbPolicy != null) {
+ staticVm.setusb_policy(usbPolicy);
+ }
+ if (!isFiltered()) {
+ // if the user set the host-name within placement-policy,
rather than the host-id (legal) -
+ // resolve the host's ID, because it will be needed down the
line
+ if (vm.isSetPlacementPolicy() &&
vm.getPlacementPolicy().isSetHost()
+ && vm.getPlacementPolicy().getHost().isSetName()
+ && !vm.getPlacementPolicy().getHost().isSetId()) {
+
vm.getPlacementPolicy().getHost().setId(getHostId(vm.getPlacementPolicy().getHost().getName()));
+ }
+ } else {
+ vm.setPlacementPolicy(null);
+ }
+ Guid storageDomainId =
+ (vm.isSetStorageDomain() &&
vm.getStorageDomain().isSetId()) ? asGuid(vm.getStorageDomain().getId())
+ : Guid.Empty;
+ if (isCreateFromSnapshot(vm)) {
+ createVmFromSnapshot(vm);
+ } else if (vm.isSetDisks() && vm.getDisks().isSetClone() &&
vm.getDisks().isClone()) {
+ response = cloneVmFromTemplate(staticVm, vm, templateId);
+ } else if (Guid.Empty.equals(templateId)) {
+ response = addVmFromScratch(staticVm, vm, storageDomainId);
+ } else {
+ response = addVm(staticVm, vm, storageDomainId, templateId);
+ }
}
-
return removeRestrictedInfoFromResponse(response);
}
+ private boolean isCreateFromSnapshot(VM vm) {
+ return vm.isSetSnapshots() && vm.getSnapshots().getSnapshots() != null
+ && !vm.getSnapshots().getSnapshots().isEmpty();
+ }
+
+ private Response createVmFromSnapshot(VM vm) {
+ // If Vm has snapshots collection - this is a clone vm from snapshot
operation
+ String snapshotId = getSnapshotId(vm.getSnapshots());
+ org.ovirt.engine.core.common.businessentities.VM vmConfiguration =
getVmConfiguration(snapshotId);
+ getMapper(VM.class, VmStatic.class).map(vm,
vmConfiguration.getStaticData());
+ // If vm passed in the call has disks attached on them,
+ // merge their data with the data of the disks on the configuration
+ // The parameters to AddVmFromSnapshot hold an array list of Disks
+ // and not List of Disks, as this is a GWT serialization limitation,
+ // and this parameter class serves GWT clients as well.
+ HashMap<Guid, DiskImage> diskImagesByImageId =
getDiskImagesByIdMap(vmConfiguration.getDiskMap().values());
+ if (vm.isSetDisks()) {
+ prepareImagesForCloneFromSnapshotParams(vm.getDisks(),
diskImagesByImageId);
+ }
+ return cloneVmFromSnapshot(vmConfiguration.getStaticData(),
+ snapshotId,
+ diskImagesByImageId);
+ }
+
private Response removeRestrictedInfoFromResponse(Response response) {
if (isFiltered()) {
VM vm = (VM) response.getEntity();
diff --git
a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmsResourceTest.java
b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmsResourceTest.java
index b166b83..fa06868 100644
---
a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmsResourceTest.java
+++
b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmsResourceTest.java
@@ -460,11 +460,6 @@
@Test
public void testCloneVmFromSnapshot() throws Exception {
setUriInfo(setUpBasicUriExpectations());
- setUpEntityQueryExpectations(VdcQueryType.GetVmTemplate,
- GetVmTemplateParameters.class,
- new String[] { "Id" },
- new Object[] { GUIDS[1] },
- getTemplateEntity(0));
org.ovirt.engine.core.common.businessentities.VM vmConfiguration =
getEntity(0);
Map<Guid, org.ovirt.engine.core.common.businessentities.Disk>
diskImageMap = new HashMap<Guid,
org.ovirt.engine.core.common.businessentities.Disk>();
@@ -480,11 +475,6 @@
new String[] { "SnapshotId" },
new Object[] { GUIDS[1] },
vmConfiguration);
- setUpEntityQueryExpectations(VdcQueryType.GetVdsGroupByVdsGroupId,
- GetVdsGroupByVdsGroupIdParameters.class,
- new String[] { "VdsGroupId" },
- new Object[] { GUIDS[2] },
- getVdsGroupEntity());
setUpCreationExpectations(VdcActionType.AddVmFromSnapshot,
AddVmFromSnapshotParameters.class,
new String[] { "StorageDomainId" },
@@ -498,7 +488,9 @@
new Object[] { GUIDS[2] },
getEntity(2));
- Response response =
collection.add(createModel(createDisksCollection(),
createSnapshotsCollection(1)));
+ VM model = createModel(createDisksCollection(),
createSnapshotsCollection(1));
+ model.setTemplate(null);
+ Response response = collection.add(model);
assertEquals(201, response.getStatus());
assertTrue(response.getEntity() instanceof VM);
verifyModel((VM) response.getEntity(), 2);
@@ -756,7 +748,21 @@
collection.add(model);
fail("expected WebApplicationException on incomplete parameters");
} catch (WebApplicationException wae) {
- verifyIncompleteException(wae, "VM", "add", "template.id|name",
"cluster.id|name");
+ verifyIncompleteException(wae, "VM", "add", "cluster.id|name");
+ }
+ }
+
+ @Test
+ public void testAddIncompleteParameters2() throws Exception {
+ VM model = createModel(null);
+ model.setTemplate(null);
+ setUriInfo(setUpBasicUriExpectations());
+ control.replay();
+ try {
+ collection.add(model);
+ fail("expected WebApplicationException on incomplete parameters");
+ } catch (WebApplicationException wae) {
+ verifyIncompleteException(wae, "VM", "add", "template.id|name");
}
}
--
To view, visit http://gerrit.ovirt.org/8433
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5e49482c2777c5fd6d0aa97ba2c4e1b37c2bec2e
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ori Liel <ol...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
