Liron Ar has posted comments on this change.

Change subject: core, restapi: Introducing ImportVmFromConfigurationCommand
......................................................................


Patch Set 9: (11 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVmFromConfigurationQuery.java
Line 15:             try {
Line 16:                 
getQueryReturnValue().setReturnValue(ovfHelper.readVmFromOvf(getParameters().getVmConfiguration()));
Line 17:                 getQueryReturnValue().setSucceeded(true);
Line 18:             } catch (Exception e) {
Line 19:                 log.debug("failed to parse a given ovf configuration: 
\n" + getParameters().getVmConfiguration(), e);
RuntimeException could be thrown as well..as the OvfHelper is in utils, I 
"shouldn't" be aware here to the possibly thrown exceptions from it - therefore 
i prefer to leave it as "exception"
Line 20:                 getQueryReturnValue().setExceptionString("failed to 
parse a given ovf configuration " + e.getMessage());
Line 21:             }
Line 22:         }
Line 23:     }


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmFromConfigurationCommand.java
Line 42:             vmDisksToAttach = 
vmFromConfiguration.getDiskMap().values();
Line 43:             clearVmDisks(vmFromConfiguration);
Line 44:             getParameters().setCopyCollapse(true);
Line 45:             getParameters().setVm(vmFromConfiguration);
Line 46:             
getParameters().setContainerId(vmFromConfiguration.getId());
It's done here as it's bll logic related to the command, the caller shouldn't 
be aware to those changes as they aren't related to it...and to not repeat in 2 
differet places it if we will add gui support as well.
Line 47:         }
Line 48: 
Line 49:         setVdsGroupId(getParameters().getVdsGroupId());
Line 50:         
getParameters().setStoragePoolId(getVdsGroup().getStoragePoolId());


Line 83:             return 
AuditLogType.VM_IMPORT_FROM_CONFIGURATION_ATTACH_DISKS_FAILED;
Line 84:         }
Line 85: 
Line 86:         return 
AuditLogType.VM_IMPORT_FROM_CONFIGURATION_EXECUTED_SUCCESFULLY;
Line 87:     }
Done
Line 88: 


....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java
Line 376:     VM_IMPORT_INFO(144),
Line 377:     VM_PAUSED_EIO(145),
Line 378:     VM_PAUSED_EPERM(146),
Line 379:     VM_POWER_DOWN_FAILED(147),
Line 380:     VM_IMPORT_FROM_CONFIGURATION_EXECUTED_SUCCESFULLY(148),
Done
Line 381:     VM_IMPORT_FROM_CONFIGURATION_ATTACH_DISKS_FAILED(149),
Line 382: 
Line 383:     VM_MEMORY_UNDER_GUARANTEED_VALUE(148, 
AuditLogTimeInterval.MINUTE.getValue() * 15),
Line 384: 


....................................................
File 
backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties
Line 305: VM_PAUSED_EPERM=VM ${VmName} has paused due to storage permissions 
problem.
Line 306: VM_POWER_DOWN_FAILED=Shutdown of VM ${VmName} failed.
Line 307: VM_MEMORY_UNDER_GUARANTEED_VALUE=VM ${VmName} on host ${VdsName} was 
guaranteed ${MemGuaranteed} MB but currently has ${MemActual} MB
Line 308: VM_IMPORT_FROM_CONFIGURATION_EXECUTED_SUCCESFULLY=VM ${VmName} has 
been successfully imported from the given configuration.
Line 309: 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}.
That's the convention in most of the messages currently.. :-) so i kept it.
Line 310: VDS_INSTALL=Host ${VdsName} installed
Line 311: VDS_INSTALL_FAILED=Host ${VdsName} installation failed. 
${FailedInstallMessage}.
Line 312: VDS_INITIALIZING=Host ${VdsName} is initializing. Message: 
${ErrorMessage}
Line 313: VDS_INITIATED_RUN_VM=VM ${VmName} was restarted on Host ${VdsName}


....................................................
File 
backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd
Line 2219: 
Line 2220:   <xs:element name="initialization" type="Initialization"/>
Line 2221:   <xs:complexType name="Initialization">
Line 2222:         <xs:choice>
Line 2223:             <xs:element ref="configuration" minOccurs="0"/>
1. only one configuration.added maxoccurs

2. Done - as the tag name is "initialization" - it seemed to me like we should 
allow only one possible choice - but as it's used also for cloud init..i will 
remove the choice.
Line 2224:         </xs:choice>
Line 2225:    </xs:complexType>
Line 2226: 
Line 2227:   <xs:complexType name="VmPlacementPolicy">


Line 2270:           <xs:element name="custom_properties" 
type="CustomProperties" minOccurs="0"/>
Line 2271:           <xs:element name="payloads" type="Payloads" minOccurs="0"/>
Line 2272:           <xs:element name="statistics" type="Statistics" 
minOccurs="0" maxOccurs="1"/>
Line 2273:           <xs:element name="disks" type="Disks" minOccurs="0" 
maxOccurs="1"/>
Line 2274:           <xs:element ref="initialization" minOccurs="0"/>
Done
Line 2275:           <xs:element name="nics" type="Nics" minOccurs="0" 
maxOccurs="1"/>
Line 2276:           <xs:element name="tags" type="Tags" minOccurs="0" 
maxOccurs="1"/>
Line 2277:           <xs:element name="snapshots" type="Snapshots" 
minOccurs="0" maxOccurs="1"/>
Line 2278:           <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 178:           vm.tunnel_migration: xs:boolean
Line 179:           vm.payloads.payload--COLLECTION: {payload.type: 
'xs:string', payload.volume_id: 'xs:string', payload.file.name: 'xs:string', 
payload.file.content: 'xs:string'}
Line 180:           vm.cpu.cpu_tune.vcpu_pin--COLLECTION: {vcpu_pin.vcpu: 
'xs:int', vcpu_pin.cpu_set: 'xs:string'}
Line 181:       # the following signature is for adding VM from a configuration 
- requires the configuration type and the configuration data
Line 182:       - mandatoryArguments: {vm.initialization.configuration.type: 
'xs:string', vm.initialization.configuration.data: 'xs:string'}
Done
Line 183:     urlparams: {}
Line 184:     headers:
Line 185:       Content-Type: {value: application/xml|json, required: true}
Line 186:       Correlation-Id: {value: 'any string', required: false}


....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmsResource.java
Line 197:         if (namedCluster(vm)) {
Line 198:             clusterId = getClusterId(vm);
Line 199:         } else {
Line 200:             clusterId = 
Guid.createGuidFromString(vm.getCluster().getId());
Line 201:         }
I prefer to use "regular" if statements..but i don't really mind :)
Line 202: 
Line 203:         ImportVmParameters parameters = new ImportVmParameters();
Line 204:         parameters.setVm(vmConfiguration);
Line 205:         parameters.setVdsGroupId(clusterId);


....................................................
Commit Message
Line 13: 
Line 14: *The user can override the vm parameters set in the ovf by explicitly
Line 15: including overriding value in the REST call (take a look in the example
Line 16: below).
Line 17: 
Done
Line 18: Usage example:
Line 19: POST to:
Line 20: http://server:port/api/vms/
Line 21: (include Content-Type:application/xml header)


....................................................
File 
frontend/webadmin/modules/gwt-common/src/main/resources/org/ovirt/engine/core/Common.gwt.xml
Line 202:               <include name="common/businessentities/VdcRole.java" />
Line 203: 
Line 204:               <include 
name="common/businessentities/FenceActionType.java" />
Line 205:               <include 
name="common/businessentities/CopyVolumeType.java" />
Line 206:         <include 
name="common/businessentities/ConfigurationType.java" />
Done
Line 207:               <include 
name="common/businessentities/ImageOperation.java" />
Line 208:               <include 
name="common/businessentities/ImageDbOperationScope.java" />
Line 209:               <include name="common/businessentities/VdcOption.java" 
/>
Line 210:               <include name="common/errors/VdcFault.java" />


-- 
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: 9
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <lara...@redhat.com>
Gerrit-Reviewer: A Burden <abur...@redhat.com>
Gerrit-Reviewer: Alissa Bonas <abo...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Liron Ar <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to