Shahar Havivi has posted comments on this change.

Change subject: restapi: Vm Init - new Feature
......................................................................


Patch Set 20:

(8 comments)

http://gerrit.ovirt.org/#/c/23021/20//COMMIT_MSG
Commit Message:

Line 3: AuthorDate: 2013-12-29 14:39:37 +0200
Line 4: Commit:     Shahar Havivi <[email protected]>
Line 5: CommitDate: 2014-01-22 14:46:38 +0200
Line 6: 
Line 7: API: Vm Init - new Feature
> Please change the subject prefix to "restapi:".
Done
Line 8: 
Line 9: Summary:
Line 10: This Feature will allow persistent of Windows Sysprep and Cloud-Init
Line 11: data to the Database. By persisting the data Admin can create a 
template


http://gerrit.ovirt.org/#/c/23021/20/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd
File 
backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd:

Line 3077: 
Line 3078: 
Line 3079:   <!-- Host NICs -->
Line 3080:   <!-- Host NIC is use for configuration of host hypervisor NIC as 
well as
Line 3081:        Initialization NIC which represent initialization - via cloud 
init and windows Sysprep -->
> I think that you don't need this now.
Done
Line 3082: 
Line 3083:   <xs:element name="host_nic" type="HostNIC"/>
Line 3084: 
Line 3085:   <xs:element name="host_nics" type="HostNics"/>


Line 3102:           <xs:element name="mtu" type="xs:int" minOccurs="0"/>
Line 3103:           <xs:element name="bridged" type="xs:boolean" minOccurs="0" 
maxOccurs="1"/>
Line 3104:           <xs:element name="custom_configuration" type="xs:boolean" 
minOccurs="0" maxOccurs="1"/>
Line 3105:           <xs:element name="override_configuration" 
type="xs:boolean" minOccurs="0" maxOccurs="1"/>
Line 3106:           <xs:element name="on_boot" type="xs:boolean" 
minOccurs="0"/>
> Don't need this either.
Done
Line 3107:           <!-- Also a rel="master" link for bonded nics -->
Line 3108:         </xs:sequence>
Line 3109:       </xs:extension>
Line 3110:     </xs:complexContent>


Line 3130:       <xs:sequence>
Line 3131:           <xs:element name="name" type="xs:string" minOccurs="0" 
maxOccurs="1"/>
Line 3132:           <xs:element ref="ip" minOccurs="0"/>
Line 3133:           <xs:element name="boot_protocol" type="xs:string" 
minOccurs="0" maxOccurs="1"/>
Line 3134:           <xs:element name="on_boot" type="xs:boolean" 
minOccurs="0"/>
> The default value of maxOccurs is 1, but you are explicitly putting it in s
Done
Line 3135:       </xs:sequence>
Line 3136:   </xs:complexType>
Line 3137: 
Line 3138:   <xs:complexType name="GuestNicsConfiguration">


Line 3138:   <xs:complexType name="GuestNicsConfiguration">
Line 3139:       <xs:sequence>
Line 3140:           <xs:element name="nics" type="GuestNicConfiguration" 
minOccurs="0" maxOccurs="unbounded" />
Line 3141:       </xs:sequence>
Line 3142:   </xs:complexType>
> Please use the same indentation than the rest of the files: 2 spaces.
Done
Line 3143: 
Line 3144:   <xs:element name="host_nic_states" type="HostNICStates"/>
Line 3145: 
Line 3146:   <xs:complexType name="HostNICStates">


http://gerrit.ovirt.org/#/c/23021/20/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/HostNicMapper.java
File 
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/HostNicMapper.java:

Line 74:                 entity.setBondOptions(buf.toString().substring(0, 
buf.length() - 1));
Line 75:             }
Line 76:         }
Line 77:         if(model.isSetBootProtocol()){
Line 78:             NetworkBootProtocol networkBootProtocol = 
map(BootProtocol.fromValue(model.getBootProtocol()), NetworkBootProtocol.NONE);
> Why are you changing this? This patch shouldn't touch the HostNic mapper.
Done
Line 79:             if(networkBootProtocol != null){
Line 80:                 entity.setBootProtocol(networkBootProtocol);
Line 81:             }
Line 82:         }


http://gerrit.ovirt.org/#/c/23021/20/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 1079:         return entity;
Line 1080:     }
Line 1081: 
Line 1082:     @Mapping(from = VmInitNetwork.class, to = 
GuestNicConfiguration.class)
Line 1083:     public static GuestNicConfiguration map(VmInitNetwork model, 
GuestNicConfiguration template) {
> The name of the VmInitNework parameter should be "entity", and the name of 
I assume you meant GuestNicConfiguration is the model
done
Line 1084:         GuestNicConfiguration entity = template != null ? template : 
new GuestNicConfiguration();
Line 1085: 
Line 1086:         entity.setName(model.getName());
Line 1087:         entity.setOnBoot(model.getStartOnBoot());


Line 1110:                 return null;
Line 1111:             }
Line 1112:         }
Line 1113:         return null;
Line 1114:     }
> This method is identical to the one in HostNicMapper. Can you create a new 
Done
Line 1115: 
Line 1116: 
Line 1117:     @Mapping(from = CloudInit.class, to = VmInit.class)
Line 1118:     public static VmInit map(CloudInit model, VmInit template) {


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

Reply via email to