Juan Hernandez has posted comments on this change.

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


Patch Set 17:

(4 comments)

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

Line 2485:   <xs:complexType name="Initialization">
Line 2486:       <xs:sequence>
Line 2487:         <xs:element name="configuration" type="Configuration" 
minOccurs="0" maxOccurs="1"/>
Line 2488:         <xs:element name="cloud_init" type="CloudInit" minOccurs="0" 
maxOccurs="1"/>
Line 2489:         <xs:element name="hostname" type="xs:string" minOccurs="0" 
maxOccurs="1"/>
There are other places where we use "host_name", can we use it here as well?
Line 2490:         <xs:element name="domain" type="xs:string" minOccurs="0" 
maxOccurs="1"/>
Line 2491:         <xs:element name="timezone" type="xs:string" minOccurs="0" 
maxOccurs="1"/>
Line 2492:         <xs:element name="authorized_keys" type="xs:string" 
minOccurs="0" maxOccurs="1"/>
Line 2493:         <xs:element name="regenerate_keys" type="xs:boolean" 
minOccurs="0" maxOccurs="1"/>


Line 2489:         <xs:element name="hostname" type="xs:string" minOccurs="0" 
maxOccurs="1"/>
Line 2490:         <xs:element name="domain" type="xs:string" minOccurs="0" 
maxOccurs="1"/>
Line 2491:         <xs:element name="timezone" type="xs:string" minOccurs="0" 
maxOccurs="1"/>
Line 2492:         <xs:element name="authorized_keys" type="xs:string" 
minOccurs="0" maxOccurs="1"/>
Line 2493:         <xs:element name="regenerate_keys" type="xs:boolean" 
minOccurs="0" maxOccurs="1"/>
These are SSH keys, right? Why not "authorized_ssh_keys" and 
"regenerate_ssh_keys"?
Line 2494:         <xs:element name="dns_servers" type="xs:string" 
minOccurs="0" maxOccurs="1"/>
Line 2495:         <xs:element name="dns_search" type="xs:string" minOccurs="0" 
maxOccurs="1"/>
Line 2496:         <xs:element ref="host_nics" minOccurs="0" maxOccurs="1"/>
Line 2497:         <xs:element name="winkey" type="xs:string" minOccurs="0" 
maxOccurs="1"/>


Line 2492:         <xs:element name="authorized_keys" type="xs:string" 
minOccurs="0" maxOccurs="1"/>
Line 2493:         <xs:element name="regenerate_keys" type="xs:boolean" 
minOccurs="0" maxOccurs="1"/>
Line 2494:         <xs:element name="dns_servers" type="xs:string" 
minOccurs="0" maxOccurs="1"/>
Line 2495:         <xs:element name="dns_search" type="xs:string" minOccurs="0" 
maxOccurs="1"/>
Line 2496:         <xs:element ref="host_nics" minOccurs="0" maxOccurs="1"/>
This means that the representation of this element will be like this:

  <initialization>
    <host_nics>
      <host_nic>...</host_nic>
    </host_nics>
  </initialization>

Using the name "host_nics" here is confussing, in my opinion, as it makes me 
think that it refers to the hypervisor nics. The natural name for this element 
is just "nics". Please create new "GuestNic" and "GuestNics" types containing 
only the required fields, copying from "HostNIC" and "HostNics".

Then here do the following:

  <xs:element name="nics" type="GuestNics" minOccurs="0" maxOccurs="1"/>

The net result should be the following:

  <initialization>
    <nics>
     <nic>..</nic>
    </nics>
  </initialization>

Ideally we should have a "BaseNIC" type containing all the properties shared by 
all the NIC entities, and then both "HostNIC" and "GuestNIC" should extend 
that. We will do that later.
Line 2497:         <xs:element name="winkey" type="xs:string" minOccurs="0" 
maxOccurs="1"/>
Line 2498:         <xs:element name="root_password" type="xs:string" 
minOccurs="0" maxOccurs="1"/>
Line 2499:         <xs:element name="custom_script" type="xs:string" 
minOccurs="0" maxOccurs="1"/>
Line 2500:       </xs:sequence>


Line 2493:         <xs:element name="regenerate_keys" type="xs:boolean" 
minOccurs="0" maxOccurs="1"/>
Line 2494:         <xs:element name="dns_servers" type="xs:string" 
minOccurs="0" maxOccurs="1"/>
Line 2495:         <xs:element name="dns_search" type="xs:string" minOccurs="0" 
maxOccurs="1"/>
Line 2496:         <xs:element ref="host_nics" minOccurs="0" maxOccurs="1"/>
Line 2497:         <xs:element name="winkey" type="xs:string" minOccurs="0" 
maxOccurs="1"/>
It this the Windows license key? If so I would suggest to name it 
"windows_license_key".
Line 2498:         <xs:element name="root_password" type="xs:string" 
minOccurs="0" maxOccurs="1"/>
Line 2499:         <xs:element name="custom_script" type="xs:string" 
minOccurs="0" maxOccurs="1"/>
Line 2500:       </xs:sequence>
Line 2501:   </xs:complexType>


-- 
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: 17
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shav...@redhat.com>
Gerrit-Reviewer: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@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: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: Shahar Havivi <shav...@redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjeli...@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

Reply via email to