Gilad Chaplik has posted comments on this change.

Change subject: restapi: NUMA feature restful API support
......................................................................


Patch Set 4: Code-Review-1

(7 comments)

please see inline.

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

Line 4131:   <xs:complexType name="NumaNode">
Line 4132:     <xs:complexContent>
Line 4133:       <xs:extension base="BaseResource">
Line 4134:         <xs:sequence>
Line 4135:           <xs:element ref="host" minOccurs="0" maxOccurs="1"/>
***
Line 4136:           <xs:element name="index" type="xs:int" minOccurs="0" 
maxOccurs="1"/>
Line 4137:           <xs:element name="memory" type="xs:long" minOccurs="0" 
maxOccurs="1"/>
Line 4138:           <xs:element name="cpu_list" type="xs:string" minOccurs="0" 
maxOccurs="1"/>
Line 4139:           <xs:element name="statistics" type="Statistics" 
minOccurs="0" maxOccurs="1"/>


Line 4134:         <xs:sequence>
Line 4135:           <xs:element ref="host" minOccurs="0" maxOccurs="1"/>
Line 4136:           <xs:element name="index" type="xs:int" minOccurs="0" 
maxOccurs="1"/>
Line 4137:           <xs:element name="memory" type="xs:long" minOccurs="0" 
maxOccurs="1"/>
Line 4138:           <xs:element name="cpu_list" type="xs:string" minOccurs="0" 
maxOccurs="1"/>
because REST is backward compatible, we need to make sure we can later 
integrate CPU pinning in this modeling of cpus.
Line 4139:           <xs:element name="statistics" type="Statistics" 
minOccurs="0" maxOccurs="1"/>
Line 4140:           <xs:element name="node_distance" type="xs:string" 
minOccurs="0" maxOccurs="1"/>
Line 4141:         </xs:sequence>
Line 4142:       </xs:extension>


Line 4135:           <xs:element ref="host" minOccurs="0" maxOccurs="1"/>
Line 4136:           <xs:element name="index" type="xs:int" minOccurs="0" 
maxOccurs="1"/>
Line 4137:           <xs:element name="memory" type="xs:long" minOccurs="0" 
maxOccurs="1"/>
Line 4138:           <xs:element name="cpu_list" type="xs:string" minOccurs="0" 
maxOccurs="1"/>
Line 4139:           <xs:element name="statistics" type="Statistics" 
minOccurs="0" maxOccurs="1"/>
can't understand what's statistics here? should be NumaNodeStat...
Line 4140:           <xs:element name="node_distance" type="xs:string" 
minOccurs="0" maxOccurs="1"/>
Line 4141:         </xs:sequence>
Line 4142:       </xs:extension>
Line 4143:     </xs:complexContent>


Line 4136:           <xs:element name="index" type="xs:int" minOccurs="0" 
maxOccurs="1"/>
Line 4137:           <xs:element name="memory" type="xs:long" minOccurs="0" 
maxOccurs="1"/>
Line 4138:           <xs:element name="cpu_list" type="xs:string" minOccurs="0" 
maxOccurs="1"/>
Line 4139:           <xs:element name="statistics" type="Statistics" 
minOccurs="0" maxOccurs="1"/>
Line 4140:           <xs:element name="node_distance" type="xs:string" 
minOccurs="0" maxOccurs="1"/>
@Juan, why we specify minOccurs="0" for type that will be used for view only 
(except ***)?
Line 4141:         </xs:sequence>
Line 4142:       </xs:extension>
Line 4143:     </xs:complexContent>
Line 4144:   </xs:complexType>


Line 4160:   </xs:complexType>
Line 4161: 
Line 4162:   <xs:complexType name="VirtualNumaNode">
Line 4163:     <xs:complexContent>
Line 4164:       <xs:extension base="BaseDevice">
sorry getting late, why extend base device? vNuma node should extend pNuma 
node. I see where you're going with that, vNuma should be a sub-collection in 
VM. needed for nested virtualization, because vNode is simply a Node and not a 
device.
Line 4165:         <xs:sequence>
Line 4166:           <xs:element name="index" type="xs:int" minOccurs="0" 
maxOccurs="1"/>
Line 4167:           <xs:element name="memory" type="xs:long" minOccurs="0" 
maxOccurs="1"/>
Line 4168:           <xs:element name="cpu_list" type="xs:string" minOccurs="0" 
maxOccurs="1"/>


Line 4165:         <xs:sequence>
Line 4166:           <xs:element name="index" type="xs:int" minOccurs="0" 
maxOccurs="1"/>
Line 4167:           <xs:element name="memory" type="xs:long" minOccurs="0" 
maxOccurs="1"/>
Line 4168:           <xs:element name="cpu_list" type="xs:string" minOccurs="0" 
maxOccurs="1"/>
Line 4169:           <xs:element name="pin_host_node_list" type="xs:string" 
minOccurs="0" maxOccurs="1"/>
All above should be removed.
should be as in the BE: list of pairs(ref to pnode, and boolean), the others 
will be taken from base and VM ref (in base we have host ref.
Line 4170:         </xs:sequence>
Line 4171:       </xs:extension>
Line 4172:     </xs:complexContent>
Line 4173:   </xs:complexType>


Line 4185:           </xs:element>
Line 4186:         </xs:sequence>
Line 4187:       </xs:extension>
Line 4188:     </xs:complexContent>
Line 4189:   </xs:complexType>
Am I wrong or we're missing Host cpu statistics?


-- 
To view, visit http://gerrit.ovirt.org/26943
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I72ed4b16c220decbf640f74c4aadffe423afc290
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Jason Liao <chuan.l...@hp.com>
Gerrit-Reviewer: Alexander Wels <aw...@redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Jason Liao <chuan.l...@hp.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
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