You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Gilbert Song <so...@gmail.com> on 2017/08/09 17:38:22 UTC
Review Request 61531: [WIP] Fixed the device number proto 'major' and
'minor' to avoid MACROs.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61531/
-----------------------------------------------------------
Review request for mesos, Ilya Pronin, Jie Yu, James Peach, Qian Zhang, and Vinod Kone.
Repository: mesos
Description
-------
Fixed the device number proto 'major' and 'minor' to avoid MACROs.
Diffs
-----
include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461
include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa
src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 96014b560377b5c716e30781de18d34c2ea8e17b
Diff: https://reviews.apache.org/r/61531/diff/1/
Testing
-------
Thanks,
Gilbert Song
Re: Review Request 61531: Fixed the device number proto 'major' and
'minor' to avoid MACROs.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61531/#review182516
-----------------------------------------------------------
src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp
Line 354 (original), 354 (patched)
<https://reviews.apache.org/r/61531/#comment258426>
You also need to include `<sys/sysmacros.h>` for this usage.
- James Peach
On Aug. 9, 2017, 7:05 p.m., Gilbert Song wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61531/
> -----------------------------------------------------------
>
> (Updated Aug. 9, 2017, 7:05 p.m.)
>
>
> Review request for mesos, Ilya Pronin, Jie Yu, James Peach, Qian Zhang, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed the device number proto 'major' and 'minor' to avoid MACROs.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461
> include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa
> src/linux/cgroups.hpp e708747f16fa4579bbd7d0e9e4bf969c05625568
> src/linux/cgroups.cpp 21f9983cb34a7b37d65b802dc56bc4804077664b
> src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 96014b560377b5c716e30781de18d34c2ea8e17b
>
>
> Diff: https://reviews.apache.org/r/61531/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Gilbert Song
>
>
Re: Review Request 61531: Fixed the device number proto 'major' and
'minor' to avoid MACROs.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61531/#review182697
-----------------------------------------------------------
Patch looks great!
Reviews applied: [61531]
Logs available here: http://104.210.40.105/logs/master/61531
- Mesos Reviewbot Windows
On Aug. 9, 2017, 6:24 p.m., Gilbert Song wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61531/
> -----------------------------------------------------------
>
> (Updated Aug. 9, 2017, 6:24 p.m.)
>
>
> Review request for mesos, Ilya Pronin, Jie Yu, James Peach, Qian Zhang, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed the device number proto 'major' and 'minor' to avoid MACROs.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461
> include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa
> src/linux/cgroups.hpp e708747f16fa4579bbd7d0e9e4bf969c05625568
> src/linux/cgroups.cpp 21f9983cb34a7b37d65b802dc56bc4804077664b
> src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 96014b560377b5c716e30781de18d34c2ea8e17b
>
>
> Diff: https://reviews.apache.org/r/61531/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Gilbert Song
>
>
Re: Review Request 61531: Fixed the device number proto 'major' and
'minor' to avoid MACROs.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61531/#review182632
-----------------------------------------------------------
Ship it!
Ship It!
- James Peach
On Aug. 9, 2017, 10:24 p.m., Gilbert Song wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61531/
> -----------------------------------------------------------
>
> (Updated Aug. 9, 2017, 10:24 p.m.)
>
>
> Review request for mesos, Ilya Pronin, Jie Yu, James Peach, Qian Zhang, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed the device number proto 'major' and 'minor' to avoid MACROs.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461
> include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa
> src/linux/cgroups.hpp e708747f16fa4579bbd7d0e9e4bf969c05625568
> src/linux/cgroups.cpp 21f9983cb34a7b37d65b802dc56bc4804077664b
> src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 96014b560377b5c716e30781de18d34c2ea8e17b
>
>
> Diff: https://reviews.apache.org/r/61531/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Gilbert Song
>
>
Re: Review Request 61531: Fixed the device number proto 'major' and
'minor' to avoid MACROs.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61531/
-----------------------------------------------------------
(Updated Aug. 9, 2017, 3:24 p.m.)
Review request for mesos, Ilya Pronin, Jie Yu, James Peach, Qian Zhang, and Vinod Kone.
Repository: mesos
Description
-------
Fixed the device number proto 'major' and 'minor' to avoid MACROs.
Diffs (updated)
-----
include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461
include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa
src/linux/cgroups.hpp e708747f16fa4579bbd7d0e9e4bf969c05625568
src/linux/cgroups.cpp 21f9983cb34a7b37d65b802dc56bc4804077664b
src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 96014b560377b5c716e30781de18d34c2ea8e17b
Diff: https://reviews.apache.org/r/61531/diff/3/
Changes: https://reviews.apache.org/r/61531/diff/2-3/
Testing
-------
make check
Thanks,
Gilbert Song
Re: Review Request 61531: Fixed the device number proto 'major' and
'minor' to avoid MACROs.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61531/
-----------------------------------------------------------
(Updated Aug. 9, 2017, 12:05 p.m.)
Review request for mesos, Ilya Pronin, Jie Yu, James Peach, Qian Zhang, and Vinod Kone.
Changes
-------
Addressed James' comment.
Summary (updated)
-----------------
Fixed the device number proto 'major' and 'minor' to avoid MACROs.
Repository: mesos
Description
-------
Fixed the device number proto 'major' and 'minor' to avoid MACROs.
Diffs (updated)
-----
include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461
include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa
src/linux/cgroups.hpp e708747f16fa4579bbd7d0e9e4bf969c05625568
src/linux/cgroups.cpp 21f9983cb34a7b37d65b802dc56bc4804077664b
src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 96014b560377b5c716e30781de18d34c2ea8e17b
Diff: https://reviews.apache.org/r/61531/diff/2/
Changes: https://reviews.apache.org/r/61531/diff/1-2/
Testing (updated)
-------
make check
Thanks,
Gilbert Song
Re: Review Request 61531: Fixed the device number proto 'major' and
'minor' to avoid MACROs.
Posted by Gilbert Song <so...@gmail.com>.
> On Aug. 9, 2017, 11:07 a.m., James Peach wrote:
> > We still need to switch to using the `major` and `minor` macros from `sys/sysmacros.h` so avoid the deprecation warning. [Here's](https://paste.fedoraproject.org/paste/QvCDIi9ZUeLG2MhVIDwhOA) what I would recommend.
> >
> > ```
> > In file included from ../../src/slave/containerizer/mesos/linux_launcher.cpp:34:0:
> > ../../src/linux/cgroups.hpp:418:13: error: In the GNU C Library, "major" is defined
> > by <sys/sysmacros.h>. For historical compatibility, it is
> > currently defined by <sys/types.h> as well, but we plan to
> > remove this soon. To use "major", include <sys/sysmacros.h>
> > directly. If you did not intend to use a system-defined macro
> > "major", you should undefine it after including <sys/types.h>. [-Werror]
> > inline unsigned int getMajor() const { return major(value); }
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > ```
>
> James Peach wrote:
> We also need `sys/sysmacros.h` in `src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp`.
Thanks for the great comment, James!
- Gilbert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61531/#review182503
-----------------------------------------------------------
On Aug. 9, 2017, 12:05 p.m., Gilbert Song wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61531/
> -----------------------------------------------------------
>
> (Updated Aug. 9, 2017, 12:05 p.m.)
>
>
> Review request for mesos, Ilya Pronin, Jie Yu, James Peach, Qian Zhang, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed the device number proto 'major' and 'minor' to avoid MACROs.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461
> include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa
> src/linux/cgroups.hpp e708747f16fa4579bbd7d0e9e4bf969c05625568
> src/linux/cgroups.cpp 21f9983cb34a7b37d65b802dc56bc4804077664b
> src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 96014b560377b5c716e30781de18d34c2ea8e17b
>
>
> Diff: https://reviews.apache.org/r/61531/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Gilbert Song
>
>
Re: Review Request 61531: [WIP] Fixed the device number proto 'major'
and 'minor' to avoid MACROs.
Posted by James Peach <jp...@apache.org>.
> On Aug. 9, 2017, 6:07 p.m., James Peach wrote:
> > We still need to switch to using the `major` and `minor` macros from `sys/sysmacros.h` so avoid the deprecation warning. [Here's](https://paste.fedoraproject.org/paste/QvCDIi9ZUeLG2MhVIDwhOA) what I would recommend.
> >
> > ```
> > In file included from ../../src/slave/containerizer/mesos/linux_launcher.cpp:34:0:
> > ../../src/linux/cgroups.hpp:418:13: error: In the GNU C Library, "major" is defined
> > by <sys/sysmacros.h>. For historical compatibility, it is
> > currently defined by <sys/types.h> as well, but we plan to
> > remove this soon. To use "major", include <sys/sysmacros.h>
> > directly. If you did not intend to use a system-defined macro
> > "major", you should undefine it after including <sys/types.h>. [-Werror]
> > inline unsigned int getMajor() const { return major(value); }
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > ```
We also need `sys/sysmacros.h` in `src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp`.
- James
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61531/#review182503
-----------------------------------------------------------
On Aug. 9, 2017, 5:38 p.m., Gilbert Song wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61531/
> -----------------------------------------------------------
>
> (Updated Aug. 9, 2017, 5:38 p.m.)
>
>
> Review request for mesos, Ilya Pronin, Jie Yu, James Peach, Qian Zhang, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed the device number proto 'major' and 'minor' to avoid MACROs.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461
> include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa
> src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 96014b560377b5c716e30781de18d34c2ea8e17b
>
>
> Diff: https://reviews.apache.org/r/61531/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gilbert Song
>
>
Re: Review Request 61531: [WIP] Fixed the device number proto 'major'
and 'minor' to avoid MACROs.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61531/#review182503
-----------------------------------------------------------
We still need to switch to using the `major` and `minor` macros from `sys/sysmacros.h` so avoid the deprecation warning. [Here's](https://paste.fedoraproject.org/paste/QvCDIi9ZUeLG2MhVIDwhOA) what I would recommend.
```
In file included from ../../src/slave/containerizer/mesos/linux_launcher.cpp:34:0:
../../src/linux/cgroups.hpp:418:13: error: In the GNU C Library, "major" is defined
by <sys/sysmacros.h>. For historical compatibility, it is
currently defined by <sys/types.h> as well, but we plan to
remove this soon. To use "major", include <sys/sysmacros.h>
directly. If you did not intend to use a system-defined macro
"major", you should undefine it after including <sys/types.h>. [-Werror]
inline unsigned int getMajor() const { return major(value); }
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
- James Peach
On Aug. 9, 2017, 5:38 p.m., Gilbert Song wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61531/
> -----------------------------------------------------------
>
> (Updated Aug. 9, 2017, 5:38 p.m.)
>
>
> Review request for mesos, Ilya Pronin, Jie Yu, James Peach, Qian Zhang, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed the device number proto 'major' and 'minor' to avoid MACROs.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461
> include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa
> src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 96014b560377b5c716e30781de18d34c2ea8e17b
>
>
> Diff: https://reviews.apache.org/r/61531/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gilbert Song
>
>