Michael Pasternak has posted comments on this change.

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


Patch Set 9: I would prefer that you didn't submit this

(4 inline comments)

....................................................
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. could it be that there is more than a single configuration in the 
<"initialization">?, if so, you should be adding new type <configurations> and 
referring it from here (just looking ahead cause we won't be able to change 
this modelling in the future, at least it won't be that easy)

2. using <xs:choice> means that user will have to use /or "configuration" /or 
something else, but not the both, i'm not sure we want this.

(i'd just make it a element in the type)
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"/>
should have maxOccurs="1"
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'}
you should be duplicating all optional args from the previous signature (i 
guess they will be valuable to override "configuration" like ovf)
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}


....................................................
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: 
please describe this feature in the FeatureHelper

thanks
Line 18: Usage example:
Line 19: POST to:
Line 20: http://server:port/api/vms/
Line 21: (include Content-Type:application/xml header)


-- 
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