Shahar Havivi has posted comments on this change. Change subject: API: Vm Init - new Feature ......................................................................
Patch Set 14: (10 comments) http://gerrit.ovirt.org/#/c/23021/14/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd File backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd: Line 594: <xs:element ref="os_types" minOccurs="0"/> Line 595: <xs:element ref="disk_formats" minOccurs="0"/> Line 596: <xs:element ref="disk_interfaces" minOccurs="0"/> Line 597: <xs:element ref="vm_affinities" minOccurs="0"/> Line 598: <xs:element ref="custom_properties" minOccurs="0"/> > Please reuse this "boot_protocols" element. Done Line 599: <xs:element ref="boot_protocols" minOccurs="0"/> Line 600: <xs:element ref="error_handling" minOccurs="0"/> Line 601: <xs:element ref="storage_formats" minOccurs="0"/> Line 602: <xs:element ref="creation_states" minOccurs="0"/> Line 2559: <xs:element ref="domain" minOccurs="0" maxOccurs="1"/> Line 2560: <xs:element name="custom_properties" type="CustomProperties" minOccurs="0"/> Line 2561: <xs:element name="payloads" type="Payloads" minOccurs="0"/> Line 2562: <xs:element name="statistics" type="Statistics" minOccurs="0" maxOccurs="1"/> Line 2563: <xs:element name="disks" type="Disks" minOccurs="0" maxOccurs="1"/> > Why not put the new properties inside this element instead of defining a ne Done Line 2564: <xs:element ref="initialization" minOccurs="0" maxOccurs="1"/> Line 2565: <xs:element name="nics" type="Nics" minOccurs="0" maxOccurs="1"/> Line 2566: <xs:element name="tags" type="Tags" minOccurs="0" maxOccurs="1"/> Line 2567: <xs:element name="snapshots" type="Snapshots" minOccurs="0" maxOccurs="1"/> Line 2572: <xs:element ref="usb" minOccurs="0" maxOccurs="1"/> Line 2573: <xs:element name="tunnel_migration" type="xs:boolean" minOccurs="0" maxOccurs="1"/> Line 2574: <xs:element ref="virtio_scsi" minOccurs="0" maxOccurs="1"/> Line 2575: <xs:element ref="permissions" minOccurs="0" maxOccurs="1"/> Line 2576: <xs:element ref="vm_init" minOccurs="0" maxOccurs="1"/> > We don't use this kind of "vm_" prefixes in general (there are exceptions). I will move the fields to the initialization element Line 2577: <xs:element ref="vmpool" minOccurs="0" maxOccurs="1"> Line 2578: <xs:annotation> Line 2579: <xs:appinfo> Line 2580: <jaxb:property name="VmPool"/> Line 2710: <xs:enumeration value="NONE"/> Line 2711: <xs:enumeration value="DHCP"/> Line 2712: <xs:enumeration value="STATIC_IP"/> Line 2713: </xs:restriction> Line 2714: </xs:simpleType> > We don't use enumerated types in the RESTAPI. Please use "xs:string" instea Done Line 2715: <xs:element name="vm_init_network" type="VmInitNetwork"/> Line 2716: Line 2717: <xs:complexType name="VmInitNetwork"> Line 2718: <xs:sequence> Line 2731: <xs:complexType name="VmInit"> Line 2732: <xs:sequence> Line 2733: <xs:element name="hostname" type="xs:string" minOccurs="0" maxOccurs="1"/> Line 2734: <xs:element name="domain" type="xs:string" minOccurs="0" maxOccurs="1"/> Line 2735: <xs:element name="timeZone" type="xs:string" minOccurs="0" maxOccurs="1"/> > Don't use camel case, use "timezone". Done Line 2736: <xs:element name="authorized_keys" type="xs:string" minOccurs="0" maxOccurs="1"/> Line 2737: <xs:element name="regenerate_keys" type="xs:boolean" minOccurs="0" maxOccurs="1"/> Line 2738: <xs:element name="dns_servers" type="xs:string" minOccurs="0" maxOccurs="1"/> Line 2739: <xs:element name="dns_search" type="xs:string" minOccurs="0" maxOccurs="1"/> http://gerrit.ovirt.org/#/c/23021/14/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/TemplateMapper.java File backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/TemplateMapper.java: Line 137 Line 138 Line 139 Line 140 Line 141 > Doens't this break backwards compatibility? What will happen after the chan Done Line 252 Line 253 Line 254 Line 255 Line 256 > Same here, what about backwards compatibility? Done Line 332 Line 333 Line 334 Line 335 Line 336 > Same here, what about backwards compatibility? Done http://gerrit.ovirt.org/#/c/23021/14/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/VmMapper.java File backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/VmMapper.java: Line 1050 Line 1051 Line 1052 Line 1053 Line 1054 > What happens with CloudInit files? Are we removing support for them? Done Line 33: import org.ovirt.engine.api.model.IP; Line 34: import org.ovirt.engine.api.model.IPs; Line 35: import org.ovirt.engine.api.model.MemoryPolicy; Line 36: import org.ovirt.engine.api.model.NIC; Line 37: import org.ovirt.engine.api.model.NetBootProtocol; > Use the existing BootProtocol. Done Line 38: import org.ovirt.engine.api.model.OperatingSystem; Line 39: import org.ovirt.engine.api.model.OsType; Line 40: import org.ovirt.engine.api.model.Payload; Line 41: import org.ovirt.engine.api.model.Quota; -- To view, visit http://gerrit.ovirt.org/23021 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I817de4fd7c7efcc3740583ede7a96bd522015660 Gerrit-PatchSet: 14 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Shahar Havivi <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Juan Hernandez <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Ori Liel <[email protected]> Gerrit-Reviewer: Roy Golan <[email protected]> Gerrit-Reviewer: Shahar Havivi <[email protected]> Gerrit-Reviewer: Tomas Jelinek <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
