<div dir="ltr"><div>I've pushed the patch into the merge queue now. Any nits people find at this point we'll address post merge.<br><br></div>Awesome work QoS team!<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Aug 17, 2015 at 9:56 AM, Ihar Hrachyshka <span dir="ltr"><<a href="mailto:ihrachys@redhat.com" target="_blank">ihrachys@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">-----BEGIN PGP SIGNED MESSAGE-----<br>
Hash: SHA256<br>
<br>
</span>So the patch in question sit there for some time, and honestly, I<br>
haven't seen much interest from reviewers to take a look at it, apart<br>
from Assaf who played with the code and reported a bunch of minor issues<br>
.<br>
<br>
I think Kyle's plan was to wait until Fri and then merge.<br>
<br>
We had a git conflict on Fri though, so today I respin the patch<br>
again, hoping that it will either get more reviews or it's merged<br>
before we hit another conflict that can be inflicted by any new db<br>
migration.<br>
<br>
Ihar<br>
<span class=""><br>
On 08/12/2015 09:55 PM, Ihar Hrachyshka wrote:<br>
</span><div><div class="h5">> Hi all,<br>
><br>
> with great pleasure, I want to request a coordinated review for<br>
> merging feature/qos branch back to master:<br>
><br>
> <a href="https://review.openstack.org/#/c/212170/" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/212170/</a><br>
><br>
> Since it's a merge patch, gerrit fails to show the whole diff that<br>
> it introduces into master. To get over it, fetch the patch:<br>
><br>
> $ git review -d 212170<br>
><br>
> and then check the difference:<br>
><br>
> $ git fetch origin && git diff origin/master...<br>
><br>
> I think we should stick to review process originally suggested at<br>
> [1]. Specifically, since it's not reasonable to expect the whole<br>
> feature branch to be reviewed by a single person, I hope multiple<br>
> people will assign themselves to the job and split the pieces to<br>
> review based on devref document that describes the feature [2]<br>
> (Note that a new RPC push/pull mechanism is described in a separate<br>
> devref section [3]).<br>
><br>
> Note that we don't expect to tackle all review comments, however<br>
> tiny, in feature/qos. We are happy to handle major flaws there, but<br>
> for minor stuff, it's good to proceed in master. Nevertheless we<br>
> are happy to get minors too and collect them for post-merge.<br>
><br>
> Things we have in the tree:<br>
><br>
> - server: QoS API extension; QoS core resource extension; QoS ML2<br>
> extension driver; QoS versioned objects + base for new objects;<br>
> QoS supported rule types mechanism for ML2; QoS notification<br>
> drivers mechanism to update SDN controllers;<br>
><br>
> - RPC: new push/pull mechanisms for versioned objects to propagate<br>
> QoS objects into the agents;<br>
><br>
> - agent side: new L2 agent extensions mechanism, integrated into<br>
> OVS and SR-IOV agents; QoS l2 agent extension; OVS and SR-IOV QoS<br>
> drivers; ovs_lib and pci_lib changes.<br>
><br>
> I suggest to split review into following logical pieces:<br>
><br>
</div></div>> - API controller + service plugin + API tests; - Versioned objects:<br>
> neutron.objects.* - ML2: supported_qos_rule_types mechanism,<br>
> extension driver, update for get_device_details payload; - RPC<br>
<span class="">> mechanism (push/pull), resource manager, registries + notification<br>
</span>> drivers integration; - l2 extensions (manager, base interface) +<br>
> qos extension; - OVS integration with extension manager + OVS QoS<br>
> driver + ovs_lib changes; - SR-IOV agent integration with extension<br>
> manager + SR-IOV QoS driver + pci_lib changes; - functional tests.<br>
<span class="">><br>
> We will also need to update the spec:<br>
> <a href="https://review.openstack.org/#/c/199112/" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/199112/</a><br>
><br>
> Included test coverage:<br>
><br>
</span>> - unit tests; - API tests; - functional tests (more scenarios to<br>
> come in master); - fullstack tests [4] (not in the tree since we<br>
<span class="">> need to merge client and base fullstack patches first).<br>
><br>
> We have client patches up for review [5][6] and expect them to go<br>
> in after merge of server component.<br>
><br>
> We hope that we'll make fullstack in before closing the blueprint<br>
> in this cycle.<br>
><br>
> [1]:<br>
> <a href="http://lists.openstack.org/pipermail/openstack-dev/2015-July/069188.ht
ml" rel="noreferrer" target="_blank">http://lists.openstack.org/pipermail/openstack-dev/2015-July/069188.ht<br>
ml</a><br>
><br>
><br>
[2]:<br>
> <a href="http://git.openstack.org/cgit/openstack/neutron/tree/doc/source/devref" rel="noreferrer" target="_blank">http://git.openstack.org/cgit/openstack/neutron/tree/doc/source/devref</a><br>
/q<br>
><br>
><br>
uality_of_service.rst?h=feature/qos<br>
> [3]:<br>
> <a href="http://git.openstack.org/cgit/openstack/neutron/tree/doc/source/devref" rel="noreferrer" target="_blank">http://git.openstack.org/cgit/openstack/neutron/tree/doc/source/devref</a><br>
/r<br>
><br>
><br>
pc_callbacks.rst?h=feature/qos<br>
> [4]: <a href="https://review.openstack.org/202492" rel="noreferrer" target="_blank">https://review.openstack.org/202492</a> [5]:<br>
> <a href="https://review.openstack.org/189655" rel="noreferrer" target="_blank">https://review.openstack.org/189655</a> [6]:<br>
> <a href="https://review.openstack.org/198277" rel="noreferrer" target="_blank">https://review.openstack.org/198277</a> [7]:<br>
> <a href="https://review.openstack.org/202061" rel="noreferrer" target="_blank">https://review.openstack.org/202061</a><br>
><br>
</span>> ______________________________________________________________________<br>
<span class="">____<br>
><br>
><br>
OpenStack Development Mailing List (not for usage questions)<br>
> Unsubscribe:<br>
> <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
> <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
><br>
</span><span class="">-----BEGIN PGP SIGNATURE-----<br>
Version: GnuPG v2<br>
<br>
</span>iQEcBAEBCAAGBQJV0fYnAAoJEC5aWaUY1u57rtsH/iaQ5HCRuFDbhFsFAkGeW/hB<br>
gn/pR9lmx/hXUIkEWfGPIsgtEnuA8nIQ3knwLfvkrPxR60YHkCK5YeRDaTVd0IQb<br>
oV5njw3eMJablTtquPybSzUljfx+oCQ2pxwhXgWAcj5KucksXLcvC+lkfk5uQ1OT<br>
iFum1jCmZ+7Te8uPdjkgGxxxpLjnJJs0Na6i+GhRppRc/HK77jM31MggfA3dJw9y<br>
cdB0JN3w2tT4wbjtmtCsVgKVWeDuuKXlnZjmI0Do1Qm1YDC0NNPLNGcBTV70vyPB<br>
B8lGyk9kvtbzSQ701T3LEp8hRIL6Oto8cHRrt3jkfygrlXPQL8x1pwtjSD59bXU=<br>
=s4FB<br>
<div class="HOEnZb"><div class="h5">-----END PGP SIGNATURE-----<br>
<br>
__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</div></div></blockquote></div><br></div>