You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jie Yu <yu...@gmail.com> on 2015/06/03 00:05:20 UTC
Review Request 34957: Fixed a bug in qdisc search function.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34957/
-----------------------------------------------------------
Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
Repository: mesos
Description
-------
Fixed a bug in qdisc search function.
Diffs
-----
src/linux/routing/queueing/internal.hpp d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70
Diff: https://reviews.apache.org/r/34957/diff/
Testing
-------
sudo make check
Thanks,
Jie Yu
Re: Review Request 34957: Fixed a bug in qdisc search function.
Posted by Jie Yu <yu...@gmail.com>.
> On June 2, 2015, 11:09 p.m., Paul Brett wrote:
> > src/linux/routing/queueing/internal.hpp, line 143
> > <https://reviews.apache.org/r/34957/diff/1/?file=976870#file976870line143>
> >
> > Not your problem, but we really should either explain why we have to increment the reference counter here but nowhere else or just fix Netlink to hide this (it does hide most of the cleanup logic)
>
> Jie Yu wrote:
> Yeah, we should definitely add some comments. Can you sugguest how to fix Netlink to hide this?
>
> Paul Brett wrote:
> Chi and I were talking about adding a copy constructor to Netlink that would increment the reference counter using helper functions like the destructors, but that needs a separate ticket and some more thought.
OK, I added a NOTE to explain why nl_object_get(o) is needed here.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34957/#review86303
-----------------------------------------------------------
On June 2, 2015, 10:05 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34957/
> -----------------------------------------------------------
>
> (Updated June 2, 2015, 10:05 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed a bug in qdisc search function.
>
>
> Diffs
> -----
>
> src/linux/routing/queueing/internal.hpp d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70
>
> Diff: https://reviews.apache.org/r/34957/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 34957: Fixed a bug in qdisc search function.
Posted by Paul Brett <pa...@twopensource.com>.
> On June 2, 2015, 11:09 p.m., Paul Brett wrote:
> > src/linux/routing/queueing/internal.hpp, line 143
> > <https://reviews.apache.org/r/34957/diff/1/?file=976870#file976870line143>
> >
> > Not your problem, but we really should either explain why we have to increment the reference counter here but nowhere else or just fix Netlink to hide this (it does hide most of the cleanup logic)
>
> Jie Yu wrote:
> Yeah, we should definitely add some comments. Can you sugguest how to fix Netlink to hide this?
Chi and I were talking about adding a copy constructor to Netlink that would increment the reference counter using helper functions like the destructors, but that needs a separate ticket and some more thought.
- Paul
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34957/#review86303
-----------------------------------------------------------
On June 2, 2015, 10:05 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34957/
> -----------------------------------------------------------
>
> (Updated June 2, 2015, 10:05 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed a bug in qdisc search function.
>
>
> Diffs
> -----
>
> src/linux/routing/queueing/internal.hpp d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70
>
> Diff: https://reviews.apache.org/r/34957/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 34957: Fixed a bug in qdisc search function.
Posted by Jie Yu <yu...@gmail.com>.
> On June 2, 2015, 11:09 p.m., Paul Brett wrote:
> > src/linux/routing/queueing/internal.hpp, line 143
> > <https://reviews.apache.org/r/34957/diff/1/?file=976870#file976870line143>
> >
> > Not your problem, but we really should either explain why we have to increment the reference counter here but nowhere else or just fix Netlink to hide this (it does hide most of the cleanup logic)
Yeah, we should definitely add some comments. Can you sugguest how to fix Netlink to hide this?
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34957/#review86303
-----------------------------------------------------------
On June 2, 2015, 10:05 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34957/
> -----------------------------------------------------------
>
> (Updated June 2, 2015, 10:05 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed a bug in qdisc search function.
>
>
> Diffs
> -----
>
> src/linux/routing/queueing/internal.hpp d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70
>
> Diff: https://reviews.apache.org/r/34957/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 34957: Fixed a bug in qdisc search function.
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34957/#review86303
-----------------------------------------------------------
src/linux/routing/queueing/internal.hpp
<https://reviews.apache.org/r/34957/#comment138281>
Not your problem, but we really should either explain why we have to increment the reference counter here but nowhere else or just fix Netlink to hide this (it does hide most of the cleanup logic)
- Paul Brett
On June 2, 2015, 10:05 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34957/
> -----------------------------------------------------------
>
> (Updated June 2, 2015, 10:05 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed a bug in qdisc search function.
>
>
> Diffs
> -----
>
> src/linux/routing/queueing/internal.hpp d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70
>
> Diff: https://reviews.apache.org/r/34957/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 34957: Fixed a bug in qdisc search function.
Posted by Cong Wang <cw...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34957/#review86301
-----------------------------------------------------------
Ship it!
This bug was found by Paul, just FYI.
- Cong Wang
On June 2, 2015, 10:05 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34957/
> -----------------------------------------------------------
>
> (Updated June 2, 2015, 10:05 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed a bug in qdisc search function.
>
>
> Diffs
> -----
>
> src/linux/routing/queueing/internal.hpp d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70
>
> Diff: https://reviews.apache.org/r/34957/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 34957: Fixed a bug in qdisc search function.
Posted by Paul Brett <pa...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34957/#review86305
-----------------------------------------------------------
Ship it!
Ship It!
- Paul Brett
On June 2, 2015, 10:05 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34957/
> -----------------------------------------------------------
>
> (Updated June 2, 2015, 10:05 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, Paul Brett, and Cong Wang.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed a bug in qdisc search function.
>
>
> Diffs
> -----
>
> src/linux/routing/queueing/internal.hpp d43a9fd405af2e59dc57cfc7ba9b5e77cb9f6b70
>
> Diff: https://reviews.apache.org/r/34957/diff/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>