Liron Ar has posted comments on this change.
Change subject: core: restapi: Introducing ImportVmFromConfigurationCommand
......................................................................
Patch Set 7: (14 inline comments)
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/ImportVmFromConfigurationParameters.java
Line 9: @NotNull
Line 10: private String vmConfiguration;
Line 11: @NotNull
Line 12: private ConfigurationType configurationType;
Line 13:
no longer relevant as this file has been removed in the last patchset
Line 14: public ImportVmFromConfigurationParameters(ConfigurationType
configurationType, String vmConfiguration, Guid vdsGroupId) {
Line 15: super();
Line 16: this.configurationType = configurationType;
Line 17: this.vmConfiguration = vmConfiguration;
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/ConfigurationType.java
Line 1: package org.ovirt.engine.core.common.businessentities;
Line 2:
Line 3: public enum ConfigurationType {
I prefer to not tie it to the import vm flow as we could also use that enum
regardless..but i can't think of a better name now. any other suggestions? I
have nothing against changing it :)
Line 4: OVF;
....................................................
File
backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties
Line 301: VM_PAUSED_EIO=VM ${VmName} has paused due to storage I/O problem.
Line 302: VM_PAUSED_EPERM=VM ${VmName} has paused due to storage permissions
problem.
Line 303: VM_POWER_DOWN_FAILED=Shutdown of VM ${VmName} failed.
Line 304: VM_IMPORT_FROM_CONFIGURATION_EXECUTED_SUCCESFULLY=VM ${VmName} has
been successfully imported from the given configuration.
Line 305: VM_IMPORT_FROM_CONFIGURATION_ATTACH_DISKS_FAILED=VM ${VmName} has
been imported from the given configuration, failed to attach to it following
disk(s): ${DiskAliases}.
as the disks are attached to the vm i think that the message is a bit
misleading. can you please suggest a message that will note the users that we
failed to attach the disks to the vm?
thanks!
Line 306: VDS_INSTALL=Host ${VdsName} installed
Line 307: VDS_INSTALL_FAILED=Host ${VdsName} installation failed.
${FailedInstallMessage}.
Line 308: VDS_INITIALIZING=Host ${VdsName} is initializing. Message:
${ErrorMessage}
Line 309: VDS_INITIATED_RUN_VM=VM ${VmName} was restarted on Host ${VdsName}
....................................................
File
backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/model/ConfigurationType.java
Line 1: package org.ovirt.engine.api.model;
Line 2:
Line 3: public enum ConfigurationType {
Line 4: OVF;
Line 5:
Done
Line 6: public static ConfigurationType fromValue(String value) {
Line 7: try {
Line 8: return valueOf(value.toUpperCase());
Line 9: } catch (IllegalArgumentException e) {
....................................................
File
backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd
Line 2091: </xs:annotation>
Line 2092: </xs:element>
Line 2093: </xs:sequence>
Line 2094: </xs:complexType>
Line 2095:
Done
Line 2096: <xs:complexType name="Configuration">
Line 2097: <xs:choice>
Line 2098: <xs:element name="type" type="xs:string" minOccurs="1"/>
Line 2099: <xs:element name="data" type="xs:string" minOccurs="1"/>
Line 2098: <xs:element name="type" type="xs:string" minOccurs="1"/>
Line 2099: <xs:element name="data" type="xs:string" minOccurs="1"/>
Line 2100: </xs:choice>
Line 2101: </xs:complexType>
Line 2102:
Done
Line 2103: <xs:complexType name="Initialization">
Line 2104: <xs:choice>
Line 2105: <xs:element name="configuration" type="Configuration"
minOccurs="1"/>
Line 2106: </xs:choice>
Line 2152: <xs:element name="custom_properties"
type="CustomProperties" minOccurs="0"/>
Line 2153: <xs:element name="payloads" type="Payloads" minOccurs="0"/>
Line 2154: <xs:element name="statistics" type="Statistics"
minOccurs="0" maxOccurs="1"/>
Line 2155: <xs:element name="disks" type="Disks" minOccurs="0"
maxOccurs="1"/>
Line 2156: <xs:element name="initialization" type="Initialization"
minOccurs="0"/>
Done
Line 2157: <xs:element name="nics" type="Nics" minOccurs="0"
maxOccurs="1"/>
Line 2158: <xs:element name="tags" type="Tags" minOccurs="0"
maxOccurs="1"/>
Line 2159: <xs:element name="snapshots" type="Snapshots"
minOccurs="0" maxOccurs="1"/>
Line 2160: <xs:element name="placement_policy"
type="VmPlacementPolicy" minOccurs="0" maxOccurs="1"/>
....................................................
File
backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml
Line 134: vm.os.kernel: xs:string
Line 135: vm.disks.clone: xs:boolean
Line 136: vm.tunnel_migration: xs:boolean
Line 137: vm.initialization.configuration.type: 'xs:string'
Line 138: vm.initialization.configuration.data: 'xs:string'
Done
Line 139: vm.payloads.payload--COLLECTION: {payload.type:
'xs:string', payload.file.name: 'xs:string', payload.file.content: 'xs:string'}
Line 140: vm.cpu.cpu_tune.vcpu_pin--COLLECTION: {vcpu_pin.vcpu:
'xs:int', vcpu_pin.cpu_set: 'xs:string'}
Line 141: # the following signature is for clone VM from a Snapshot -
requires the Snapshot ID
Line 142: - mandatoryArguments: {vm.name: 'xs:string',
vm.template.id|name: 'xs:string', vm.cluster.id|name: 'xs:string',
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmsResource.java
Line 82: public Response add(VM vm) {
Line 83: validateParameters(vm, "cluster.id|name");
Line 84: validateEnums(VM.class, vm);
Line 85: Response response = null;
Line 86: if (vm.getInitialization() != null) {
1. Done
2. Changed per your suggestion, the implementation in this patchset had the
"constraint" for configuration being set in the api.xsd file (minOccurs = 1),
changed per your suggestion with minor change, if initialization is set, i
execute the validateparameters on the configuration type and data, to not
automatically go to the else clause in that case.
3. This is checked in the validateEnums method, am i missing something? the
command is written intentionally that we could add support for other
configuration types and it would be transparent to the implementation here.
Line 87: response = importVmFromConfiguration(vm);
Line 88: } else {
Line 89: validateParameters(vm, "name");
Line 90: if (isCreateFromSnapshot(vm)) {
Line 181: return payload;
Line 182: }
Line 183:
Line 184: public Response importVmFromConfiguration(VM vm) {
Line 185: Configuration config =
vm.getInitialization().getConfiguration();
in this patchset it can't be null because of the "constraint" in api.xsd.
after the changes in the last comment, it's checked also before so unless you
want me to, i didn't add it here as well.
Line 186: ImportVmFromConfigurationParameters params =
Line 187: new
ImportVmFromConfigurationParameters(VmMapper.map(ConfigurationType.fromValue(config.getType()),
null), config.getData().trim() , getClusterId(vm));
Line 188: return performCreate(VdcActionType.ImportVmFromConfiguration,
Line 189: params,
Line 183:
Line 184: public Response importVmFromConfiguration(VM vm) {
Line 185: Configuration config =
vm.getInitialization().getConfiguration();
Line 186: ImportVmFromConfigurationParameters params =
Line 187: new
ImportVmFromConfigurationParameters(VmMapper.map(ConfigurationType.fromValue(config.getType()),
null), config.getData().trim() , getClusterId(vm));
in this patchset it can't be null because of the "constraint" in api.xsd.
after the changes in the last comment, it's checked also before so unless you
want me to, i didn't add it here as well.
Line 188: return performCreate(VdcActionType.ImportVmFromConfiguration,
Line 189: params,
Line 190: new QueryIdResolver<Guid>(VdcQueryType.GetVmByVmId,
IdQueryParameters.class));
Line 191: }
....................................................
File
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/VmMapper.java
Line 607: switch (configurationType) {
Line 608: case OVF: return
org.ovirt.engine.core.common.businessentities.ConfigurationType.OVF;
Line 609: default: return null;
Line 610: }
Line 611: }
1. we don't have use currently for the opposite direction, so i didn't add it
before..added now :)
2. Done
Line 612:
Line 613: @Mapping(from = org.ovirt.engine.api.model.VmDeviceType.class, to
= VmDeviceType.class)
Line 614: public static VmDeviceType
map(org.ovirt.engine.api.model.VmDeviceType deviceType, VmDeviceType template) {
Line 615: switch (deviceType) {
....................................................
Commit Message
Line 3: AuthorDate: 2013-07-04 16:50:55 +0300
Line 4: Commit: Liron Aravot <[email protected]>
Line 5: CommitDate: 2013-07-07 16:36:04 +0300
Line 6:
Line 7: core: restapi: Introducing ImportVmFromConfigurationCommand
Done
Line 8:
Line 9: ImportVmFromConfigurationCommand is used for adding a vm to the engine
Line 10: with a configuration specified in an ovf file.
Line 11: The command adds the vm and related devices as specified in the given
Line 4: Commit: Liron Aravot <[email protected]>
Line 5: CommitDate: 2013-07-07 16:36:04 +0300
Line 6:
Line 7: core: restapi: Introducing ImportVmFromConfigurationCommand
Line 8:
Done
Line 9: ImportVmFromConfigurationCommand is used for adding a vm to the engine
Line 10: with a configuration specified in an ovf file.
Line 11: The command adds the vm and related devices as specified in the given
Line 12: configuration and then attempts to attach to the vm the related disks.
--
To view, visit http://gerrit.ovirt.org/15894
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9390bcc11e5f3cb8c8c89ce791aabe63fe0ca341
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <[email protected]>
Gerrit-Reviewer: A Burden <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
