Adam Litke has posted comments on this change.
Change subject: netwiring: [2/4] Add API definitions.
......................................................................
Patch Set 5: (1 inline comment)
....................................................
File vdsm_api/vdsmapi-schema.json
Line 4885: # @portMirroring: #optional If present, indicates the traffic of
which networks
Line 4886: # should be mirrored to the interface identified by
alias.
Line 4887: #
Line 4888: # Since 4.10.3
Line 4889: ##
I am a bit concerned by the different semantics for optional parameters. I
think we need to decide what omitting an optional parameter should mean and
keep it the same across all fields. I think it's sensible for omit to mean
leave the current configuration unchanged.
For portMirroring, an empty array can mean disable mirroring on all networks
and if the param is completely omitted we don't change anything.
For network, I am not sure how to handle it. Using the empty string to
indicate disconnect is a pretty dirty hack.
Line 4890: {'type': 'vmUpdateInterfaceDeviceParams',
Line 4891: 'data': {'*network': 'str', '*linkActive': 'bool',
Line 4892: 'alias': 'str', '*portMirroring': '[str]'}}
Line 4893:
--
To view, visit http://gerrit.ovirt.org/9560
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d8352f237dbe0229dff368f7c1dfa4f5f8fc766
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Igor Lvovsky <[email protected]>
Gerrit-Reviewer: Livnat Peer <[email protected]>
Gerrit-Reviewer: ShaoHe Feng <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
