Michael Pasternak has posted comments on this change.
Change subject: rest-api: exposing setupnetworks action under
/api/hosts/{id}/nics
......................................................................
Patch Set 6: (7 inline comments)
....................................................
File
backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/model/BootProtocol.java
Line 23:
in HostNicMapper you should create mappings for NONE type,
also please add test for this enum member
....................................................
File
backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata_v-3.1.yaml
Line 1120: action.host_nics.host_nic.bonding.slaves.host_nic--LIST:
{name|id: 'xs:string'}
this structure is not correct, host_nics is collection as well, its should be
expressed
by LIST as slaves, e.g:
action.host_nics.host_nic--LIST: {bonding.slaves.host_nic--LIST: {name|id:
'xs:string'}}
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendHostNicsResource.java
Line 184: }
why this is needed? we set network only on top level resources such as bond
for instance, AFAIU this will cause slaves to have network as well - when this
is not the case.
Line 348: private List<VdsNetworkInterface> nicsToInterfaces(List<HostNIC>
hostNics) {
the standard name of such mapper is 'mapCollection()'
Line 357: ifaces.add(slaveIface);
slaves always returned in collection of /hosts/xx/nics, so if someone will do:
1. coll = GET /hosts/xx/nics
2. changes on coll
3. POST /hosts/xx/nics/setupnetworks with coll,
you will add slaves to your 'VdsNetworkInterface iface' list twice
....................................................
File
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/HostNicMapper.java
Line 54: if (model.isSetBonding()) {
you need to check that it actually have the slaves, otherwise it's not bond
....................................................
Commit Message
Line 11:
good job!,
1. you also need to write tests, we not committing any method without
appropriate test
2. you also need to regenerate SDK to support this
--
To view, visit http://gerrit.ovirt.org/1112
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9464f344605b99a21806170cf340e8fd8063dff6
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Roy Golan <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: [email protected]
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
