Michael Pasternak has posted comments on this change.
Change subject: restapi: cloud-init - rest api for start vm
......................................................................
Patch Set 15: Code-Review-1
(8 comments)
....................................................
File
backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd
Line 2393: <xs:element name="dns" type="DNS"/>
Line 2394:
Line 2395: <xs:complexType name="DNS">
Line 2396: <xs:sequence>
Line 2397: <xs:element name="dns_servers">
1. "servers" - you already in context of dns
2. please extract "servers" to standalone " collection type that extends
BaseResources
(as we do for all other collections, see "VMs" for details)
Line 2398: <xs:complexType>
Line 2399: <xs:sequence>
Line 2400: <xs:element name="dns_server" type="xs:string"
minOccurs="0" maxOccurs="unbounded"/>
Line 2401: </xs:sequence>
Line 2400: <xs:element name="dns_server" type="xs:string"
minOccurs="0" maxOccurs="unbounded"/>
Line 2401: </xs:sequence>
Line 2402: </xs:complexType>
Line 2403: </xs:element>
Line 2404: <xs:element name="search_domains">
same
Line 2405: <xs:complexType>
Line 2406: <xs:sequence>
Line 2407: <xs:element name="search_domain" type="xs:string"
minOccurs="0" maxOccurs="unbounded"/>
Line 2408: </xs:sequence>
Line 2421: </xs:complexType>
Line 2422:
Line 2423: <xs:element name="authorized_keys" type="AuthorizedKeys"/>
Line 2424:
Line 2425: <xs:complexType name="AuthorizedKeys">
please follow collection pattern we're using (see "VMs" for details)
Line 2426: <xs:sequence>
Line 2427: <xs:element ref="authorized_key" minOccurs="0"
maxOccurs="unbounded"/>
Line 2428: </xs:sequence>
Line 2429: </xs:complexType>
Line 2431: <xs:element name="cloud_init" type="CloudInit"/>
Line 2432:
Line 2433: <xs:complexType name="CloudInit">
Line 2434: <xs:sequence>
Line 2435: <xs:element name="host_name" type="xs:string" minOccurs="0"/>
please use ref to host, to hold host details: i.e
<host>
<name>...
</host>
Line 2436: <xs:element name="network" minOccurs="0">
Line 2437: <xs:complexType>
Line 2438: <xs:sequence>
Line 2439: <xs:element ref="nics" minOccurs="0"/>
Line 2436: <xs:element name="network" minOccurs="0">
Line 2437: <xs:complexType>
Line 2438: <xs:sequence>
Line 2439: <xs:element ref="nics" minOccurs="0"/>
Line 2440: <xs:element ref="dns" minOccurs="0"/>
can you please give some background on the modelling of network/nics/dns?
Line 2441: </xs:sequence>
Line 2442: </xs:complexType>
Line 2443: </xs:element> <!-- </network> -->
Line 2444: <xs:element ref="authorized_keys" minOccurs="0"/>
Line 2444: <xs:element ref="authorized_keys" minOccurs="0"/>
Line 2445: <xs:element name="regenerate_ssh_keys" type="xs:boolean"
minOccurs="0"/>
Line 2446: <xs:element name="timezone" type="xs:string" minOccurs="0"/>
Line 2447: <xs:element ref="users" minOccurs="0"/>
Line 2448: <xs:element ref="payload_file" minOccurs="0"
maxOccurs="unbounded"/>
please use ref to payload_files collection
Line 2449: </xs:sequence>
Line 2450: </xs:complexType>
Line 2451:
Line 2452: <xs:complexType name="VmPlacementPolicy">
....................................................
File
backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml
Line 292: cloud-init.regenerate_ssh_keys: 'xs:boolean',
Line 293: cloud-init.timezone: 'xs:string',
Line 294: cloud-init.users--COLLECTION: {users.password:
'xs:string',users.name: 'xs:string'},
Line 295: cloud-init.files--COLLECTION: {files.path: 'xs:string',
files.content: 'xs:string',
Line 296: files.encoding: 'xs:string'}}
all collection base descriptors are wrong, see [1] for more details.
[1] vm.payloads.payload--COLLECTION
Line 297: description: start a virtual machine in the system identified
by the given id with the options specified in the request body
Line 298: urlparams: {}
Line 299: headers:
Line 300: Content-Type: {value: application/xml|json, required: true}
....................................................
File
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/VmMapper.java
Line 911: }
Line 912: }
Line 913:
Line 914: @Mapping(from = CloudInit.class, to = CloudInitParameters.class)
Line 915: public static CloudInitParameters map(CloudInit model,
CloudInitParameters template) {
where do you using this mapping?, it should be called from map(vm, vmstatic),
otherwise it won't get in to the vmstatic upon vm creation/run
Line 916: CloudInitParameters entity = template != null ? template :
new CloudInitParameters();
Line 917:
Line 918: if (model.isSetHostName()) {
Line 919: entity.setHostname(model.getHostName());
--
To view, visit http://gerrit.ovirt.org/15537
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ad0bfeca23cf8d4b2887010081d63c258032611
Gerrit-PatchSet: 15
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett <gpadg...@redhat.com>
Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Ori Liel <ol...@redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
