Hi, After discussing generic concerns about the current proposed patch series[1] for the bandwidth-resource-provider bp [2] with Matt, I decided to write a summary that might help other reviewers. Grouping of the pathes ---------------------- The whole nova implementation is one long patch series that can be splitted into smaller logical steps * Already merged patches added the two new standard resource classes and added a field to RequestSpec to store the resource request of each neutron port. * The open patch series start at [3] now and the patches between [3] [4] introduce the ability to boot a server with neutron ports that has resource request. These patches implement handling the extra resources during boot, re-schedule and delete. These patches only work if a server has maximum one neutron port that has resource request. * The patches after [4] until [5] implement the calculation and communiction of the mapping between ports and resource providers that are fulfilling the given port resource request. (The reason why this is done in nova described below.) So these patches allow having more than one neutron port per server having resource request. * the next two patches [6][7] implement rejecting use cases that was decided to out of scope for the feature (interface attach with resource requst, booting with network having resource request) * The next patch [8] add support for removing resource allocation during interface detach if the detached port has resource request * The rest of the series up until [9] implement support for selecting VFs form the same PF that the bandwidth was allocated from. This allows handling bandwidth and VF resources properly even if there are more than one PFs on the same compute connected to the same physnet. What is still missing from the patch series: * Support for any kind of server move operations (resize, migrate, evacuate, live-migrate, unshelve) * Upgrade support: Bound SRIOV ports might already have QoS policy that would need healing resource allocation in placement. * Supporting separation of QoS aware and non aware ports on the same host. This can be done with host aggregates today, but the bp described possible ways to support a better separation. * Supporting multisegment network that has more than one segment having physnet name. For this we would need any-trait support from placement. The spec for it is approved but the implementation is on hold due to placement extraction. Order of the patches -------------------- One comment I got from Matt is that the patches are in wrong order. The current order allows booting server with bandwidth request _before_ every other server operation is supported (or rejected). This could lead to the violation of the resource view of the system. For example booting a server with a QoS aware port will result in bandwidth allocation on the compute host. But then later on migrating that server to another host does not properly handle moving the allocation to the target host. In my view reordering the patches to only allow booting the first server with bandwdith after every other operation is supported is not feasible. The current chain is already 17 patches long and it does not touch server moving operations at all. Also if I cannot boot a server with bandwidth then I cannot test that such server can be moved properly. So all the functional test would end up in the last patch of the series. However I think we can use the feature flag approach. We can implement and verify everything in the current order without the end user seeing or causing any kind of inconsistencies. The nova implementation is only activated if a neutron port has a resource_request field. This field is added to the neutron API as an API extension. We can mark this extension experimental while we are working through the missing pieces of the feature. But we can also turn on that extension in our test envirnoment to verify each and every step during the implementation process. Which RP fulfills the request of a neutron port ----------------------------------------------- Neutron expects a single RP uuid sent by nova in the port binding that tells neutron about which RP providers the resources for that port. This is the famous problem to find which request group is fulfilled by which resource provider in an allocation candidate. There are 3 possible solutions: * Neutron does the mapping. This would require Neutron to see each port binding for a given server as a single request. This is not the case today and not feasible to hack around in neutron. * Placement does the mapping. This is technically feasible and in my oppinion this is the good solution. But placement is in feature freeze so the spec [10] describing this change is put on hold. * Nova does the mapping. This is what pathes after [4] until [5] implement. It is a temporary solution until the placement based solution can be provided. The current patches are structured in a way that all ovo and neutronv2/api impacts in nova are reusable for the placement based solution later without change. There is only one patch [11] that will be thrown away when the placement based solution is available. I hope this mail helps reviewing the series. Cheers, gibi [1] https://review.openstack.org/#/q/topic:bp/bandwidth-resource-provider+(statu...) [2] https://blueprints.launchpad.net/nova/+spec/bandwidth-resource-provider [3] https://review.openstack.org/#/c/625941/ [4] https://review.openstack.org/#/c/567268/ [5] https://review.openstack.org/#/c/573317/ [6] https://review.openstack.org/#/c/570078 [7] https://review.openstack.org/#/c/570079 [8] https://review.openstack.org/#/c/622421 [9] https://review.openstack.org/#/c/623543 [10] https://review.openstack.org/#/c/597601 [11] https://review.openstack.org/#/c/616239