You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Archana kumari <ar...@gmail.com> on 2014/03/13 12:42:12 UTC

Review Request 19184: Fixed a TODO(bmahler): Add namespacing to cgroups to enforce the expected structure, e.g., cgroups::cpuacct::stat.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19184/
-----------------------------------------------------------

Review request for mesos.


Repository: mesos-git


Description
-------

Fixed a issue related to Jira 1076


Diffs
-----

  src/linux/cgroups.hpp 65f0958 
  src/linux/cgroups.cpp 6f95376 
  src/slave/containerizer/isolators/cgroups/cpushare.cpp f74e80e 
  src/slave/containerizer/isolators/cgroups/mem.cpp 9e9c55e 

Diff: https://reviews.apache.org/r/19184/diff/


Testing
-------


Thanks,

Archana kumari


Re: Review Request 19184: Fixed a TODO(bmahler): Add namespacing to cgroups to enforce the expected structure, e.g., cgroups::cpuacct::stat.

Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19184/#review37060
-----------------------------------------------------------



src/linux/cgroups.cpp
<https://reviews.apache.org/r/19184/#comment68315>

    this definition doesn't match the declaration. Did you build this patch?
    
    1. there's no return value.
    2. you have strings passed by value rather than const reference (as they are in the declaration.
    



src/slave/containerizer/isolators/cgroups/cpushare.cpp
<https://reviews.apache.org/r/19184/#comment68316>

    This doesn't really add anything as you still have the repetition in the hierarchies lookup. Perhaps the methods should take the entire hierarchy as a parameter instead?
    
    Then this would be:
    
    stat = cgroups::cpuacct::stat(heirarchies, info->cgroup);
    


- Dominic Hamon


On March 13, 2014, 4:42 a.m., Archana kumari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19184/
> -----------------------------------------------------------
> 
> (Updated March 13, 2014, 4:42 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Fixed a issue related to Jira 1076
> 
> 
> Diffs
> -----
> 
>   src/linux/cgroups.hpp 65f0958 
>   src/linux/cgroups.cpp 6f95376 
>   src/slave/containerizer/isolators/cgroups/cpushare.cpp f74e80e 
>   src/slave/containerizer/isolators/cgroups/mem.cpp 9e9c55e 
> 
> Diff: https://reviews.apache.org/r/19184/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Archana kumari
> 
>


Re: Review Request 19184: Fixed a TODO(bmahler): Add namespacing to cgroups to enforce the expected structure, e.g., cgroups::cpuacct::stat.

Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19184/#review37337
-----------------------------------------------------------


Bad patch!

Reviews applied: [19184]

Failed command: make -j3 check GTEST_FILTER='' >/dev/null

Error:
 ev.c:1531:31: warning: 'ev_default_loop_ptr' initialized and declared 'extern' [enabled by default]
ev.c: In function 'evpipe_write':
ev.c:2160:17: warning: ignoring return value of 'write', declared with attribute warn_unused_result [-Wunused-result]
ev.c:2172:17: warning: ignoring return value of 'write', declared with attribute warn_unused_result [-Wunused-result]
ev.c: In function 'pipecb':
ev.c:2193:16: warning: ignoring return value of 'read', declared with attribute warn_unused_result [-Wunused-result]
ev.c:2207:16: warning: ignoring return value of 'read', declared with attribute warn_unused_result [-Wunused-result]
In file included from /usr/include/c++/4.6/ext/hash_set:61:0,
                 from src/glog/stl_logging.h:54,
                 from src/stl_logging_unittest.cc:34:
/usr/include/c++/4.6/backward/backward_warning.h:33:2: warning: #warning This file includes at least one deprecated or antiquated header which may be removed without further notice at a future date. Please use a non-deprecated interface with equivalent functionality instead. For a listing of replacement headers and interfaces, consult the file backward_warning.h. To disable this warning use -Wno-deprecated. [-Wcpp]
In file included from src/utilities.h:73:0,
                 from src/googletest.h:38,
                 from src/stl_logging_unittest.cc:48:
src/base/mutex.h:137:0: warning: "_XOPEN_SOURCE" redefined [enabled by default]
/usr/include/features.h:166:0: note: this is the location of the previous definition
warning: no files found matching 'Makefile' under directory 'docs'
warning: no files found matching 'indexsidebar.html' under directory 'docs'
zip_safe flag not set; analyzing archive contents...
linux/cgroups.cpp:75:79: error: ISO C++ forbids declaration of 'stat' with no type [-fpermissive]
linux/cgroups.cpp:75:79: error: declaration of 'int stat(const string&, const string&)' not in a namespace surrounding 'cgroups::cpuacct'
linux/cgroups.cpp:75:79: error: 'int cgroups::cpuacct::stat(const string&, const string&)' should have been declared inside 'cgroups::cpuacct'
linux/cgroups.cpp: In function 'int cgroups::cpuacct::stat(const string&, const string&)':
linux/cgroups.cpp:75:79: error: new declaration 'int cgroups::cpuacct::stat(const string&, const string&)'
./linux/cgroups.hpp:482:38: error: ambiguates old declaration 'Try<hashmap<std::basic_string<char>, long unsigned int> > cgroups::cpuacct::stat(const string&, const string&)'
linux/cgroups.cpp:76:7: error: 'heirarchy' was not declared in this scope
linux/cgroups.cpp:77:1: error: no return statement in function returning non-void [-Werror=return-type]
linux/cgroups.cpp: At global scope:
linux/cgroups.cpp:78:75: error: ISO C++ forbids declaration of 'stat' with no type [-fpermissive]
linux/cgroups.cpp:78:75: error: declaration of 'int stat(const string&, const string&)' not in a namespace surrounding 'cgroups::cpu'
linux/cgroups.cpp:78:75: error: 'int cgroups::cpu::stat(const string&, const string&)' should have been declared inside 'cgroups::cpu'
linux/cgroups.cpp: In function 'int cgroups::cpu::stat(const string&, const string&)':
linux/cgroups.cpp:78:75: error: new declaration 'int cgroups::cpu::stat(const string&, const string&)'
./linux/cgroups.hpp:399:38: error: ambiguates old declaration 'Try<hashmap<std::basic_string<char>, long unsigned int> > cgroups::cpu::stat(const string&, const string&)'
linux/cgroups.cpp:80:7: error: 'heirarchy' was not declared in this scope
linux/cgroups.cpp:81:1: error: no return statement in function returning non-void [-Werror=return-type]
linux/cgroups.cpp: At global scope:
linux/cgroups.cpp:82:78: error: ISO C++ forbids declaration of 'stat' with no type [-fpermissive]
linux/cgroups.cpp:82:78: error: declaration of 'int stat(const string&, const string&)' not in a namespace surrounding 'cgroups::memory'
linux/cgroups.cpp:82:78: error: 'int cgroups::memory::stat(const string&, const string&)' should have been declared inside 'cgroups::memory'
linux/cgroups.cpp: In function 'int cgroups::memory::stat(const string&, const string&)':
linux/cgroups.cpp:82:78: error: new declaration 'int cgroups::memory::stat(const string&, const string&)'
./linux/cgroups.hpp:436:38: error: ambiguates old declaration 'Try<hashmap<std::basic_string<char>, long unsigned int> > cgroups::memory::stat(const string&, const string&)'
linux/cgroups.cpp:84:7: error: 'heirarchy' was not declared in this scope
linux/cgroups.cpp:85:1: error: no return statement in function returning non-void [-Werror=return-type]
cc1plus: all warnings being treated as errors
make[2]: *** [linux/libmesos_no_3rdparty_la-cgroups.lo] Error 1
make[2]: *** Waiting for unfinished jobs....
slave/containerizer/isolators/cgroups/cpushare.cpp: In member function 'virtual process::Future<mesos::ResourceStatistics> mesos::internal::slave::CgroupsCpushareIsolatorProcess::usage(const mesos::ContainerID&)':
slave/containerizer/isolators/cgroups/cpushare.cpp:378:29: error: type/value mismatch at argument 1 in template parameter list for 'template<class T> class Try'
slave/containerizer/isolators/cgroups/cpushare.cpp:378:29: error:   expected a type, got 'cgroups::cpuacct::stat'
slave/containerizer/isolators/cgroups/cpushare.cpp:378:36: error: invalid type in declaration before '=' token
slave/containerizer/isolators/cgroups/cpushare.cpp:379:53: error: invalid initialization of reference of type 'const string& {aka const std::basic_string<char>&}' from expression of type 'hashmap<std::basic_string<char>, std::basic_string<char> >'
./linux/cgroups.hpp:482:38: error: in passing argument 1 of 'Try<hashmap<std::basic_string<char>, long unsigned int> > cgroups::cpuacct::stat(const string&, const string&)'
slave/containerizer/isolators/cgroups/cpushare.cpp:381:12: error: request for member 'isError' in 'stat', which is of non-class type 'int'
slave/containerizer/isolators/cgroups/cpushare.cpp:382:59: error: request for member 'error' in 'stat', which is of non-class type 'int'
slave/containerizer/isolators/cgroups/cpushare.cpp:385:32: error: request for member 'get' in 'stat', which is of non-class type 'int'
slave/containerizer/isolators/cgroups/cpushare.cpp:386:34: error: request for member 'get' in 'stat', which is of non-class type 'int'
slave/containerizer/isolators/cgroups/cpushare.cpp:394:61: error: cannot convert 'Try<hashmap<std::basic_string<char>, long unsigned int> >' to 'int' in assignment
slave/containerizer/isolators/cgroups/cpushare.cpp:396:12: error: request for member 'isError' in 'stat', which is of non-class type 'int'
slave/containerizer/isolators/cgroups/cpushare.cpp:397:55: error: request for member 'error' in 'stat', which is of non-class type 'int'
slave/containerizer/isolators/cgroups/cpushare.cpp:400:38: error: request for member 'get' in 'stat', which is of non-class type 'int'
slave/containerizer/isolators/cgroups/cpushare.cpp:405:40: error: request for member 'get' in 'stat', which is of non-class type 'int'
slave/containerizer/isolators/cgroups/cpushare.cpp:410:42: error: request for member 'get' in 'stat', which is of non-class type 'int'
make[2]: *** [slave/containerizer/isolators/cgroups/libmesos_no_3rdparty_la-cpushare.lo] Error 1
make[1]: *** [check] Error 2
make: *** [check-recursive] Error 1


- Mesos ReviewBot


On March 16, 2014, 11:28 a.m., Archana kumari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19184/
> -----------------------------------------------------------
> 
> (Updated March 16, 2014, 11:28 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Fixed a issue related to Jira 1076
> 
> 
> Diffs
> -----
> 
>   src/linux/cgroups.hpp 65f0958 
>   src/linux/cgroups.cpp 6f95376 
>   src/slave/containerizer/isolators/cgroups/cpushare.cpp f74e80e 
>   src/slave/containerizer/isolators/cgroups/mem.cpp 9e9c55e 
> 
> Diff: https://reviews.apache.org/r/19184/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Archana kumari
> 
>


Re: Review Request 19184: Fixed a TODO(bmahler): Add namespacing to cgroups to enforce the expected structure, e.g., cgroups::cpuacct::stat.

Posted by Archana kumari <ar...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19184/
-----------------------------------------------------------

(Updated March 16, 2014, 11:28 a.m.)


Review request for mesos, Benjamin Hindman and Ben Mahler.


Repository: mesos-git


Description
-------

Fixed a issue related to Jira 1076


Diffs
-----

  src/linux/cgroups.hpp 65f0958 
  src/linux/cgroups.cpp 6f95376 
  src/slave/containerizer/isolators/cgroups/cpushare.cpp f74e80e 
  src/slave/containerizer/isolators/cgroups/mem.cpp 9e9c55e 

Diff: https://reviews.apache.org/r/19184/diff/


Testing
-------


Thanks,

Archana kumari


Re: Review Request 19184: Fixed a TODO(bmahler): Add namespacing to cgroups to enforce the expected structure, e.g., cgroups::cpuacct::stat.

Posted by Archana kumari <ar...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19184/
-----------------------------------------------------------

(Updated March 15, 2014, 2:30 a.m.)


Review request for mesos and Ben Mahler.


Repository: mesos-git


Description
-------

Fixed a issue related to Jira 1076


Diffs (updated)
-----

  src/linux/cgroups.hpp 65f0958 
  src/linux/cgroups.cpp 6f95376 
  src/slave/containerizer/isolators/cgroups/cpushare.cpp f74e80e 
  src/slave/containerizer/isolators/cgroups/mem.cpp 9e9c55e 

Diff: https://reviews.apache.org/r/19184/diff/


Testing
-------


Thanks,

Archana kumari


Re: Review Request 19184: Fixed a TODO(bmahler): Add namespacing to cgroups to enforce the expected structure, e.g., cgroups::cpuacct::stat.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19184/#review37231
-----------------------------------------------------------


Hey Archana, remember that the point of the TODO was to return a 'struct' of the expected information so that callers don't have to guess which keys are present in the hashmap. Looking forward to your update!


src/slave/containerizer/isolators/cgroups/cpushare.cpp
<https://reviews.apache.org/r/19184/#comment68650>

    This is still returning a hashmap, it would be nice if this returned a cgroups::cpuacct::Stat struct that contains the expected information.
    
    Ditto for memory.


- Ben Mahler


On March 13, 2014, 9:41 p.m., Archana kumari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19184/
> -----------------------------------------------------------
> 
> (Updated March 13, 2014, 9:41 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Fixed a issue related to Jira 1076
> 
> 
> Diffs
> -----
> 
>   src/linux/cgroups.hpp 65f0958 
>   src/linux/cgroups.cpp 6f95376 
>   src/slave/containerizer/isolators/cgroups/cpushare.cpp f74e80e 
>   src/slave/containerizer/isolators/cgroups/mem.cpp 9e9c55e 
> 
> Diff: https://reviews.apache.org/r/19184/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Archana kumari
> 
>


Re: Review Request 19184: Fixed a TODO(bmahler): Add namespacing to cgroups to enforce the expected structure, e.g., cgroups::cpuacct::stat.

Posted by Archana kumari <ar...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19184/
-----------------------------------------------------------

(Updated March 13, 2014, 9:41 p.m.)


Review request for mesos and Ben Mahler.


Repository: mesos-git


Description
-------

Fixed a issue related to Jira 1076


Diffs
-----

  src/linux/cgroups.hpp 65f0958 
  src/linux/cgroups.cpp 6f95376 
  src/slave/containerizer/isolators/cgroups/cpushare.cpp f74e80e 
  src/slave/containerizer/isolators/cgroups/mem.cpp 9e9c55e 

Diff: https://reviews.apache.org/r/19184/diff/


Testing
-------


Thanks,

Archana kumari


Re: Review Request 19184: Fixed a TODO(bmahler): Add namespacing to cgroups to enforce the expected structure, e.g., cgroups::cpuacct::stat.

Posted by Archana kumari <ar...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19184/
-----------------------------------------------------------

(Updated March 13, 2014, 8:42 p.m.)


Review request for mesos.


Repository: mesos-git


Description
-------

Fixed a issue related to Jira 1076


Diffs (updated)
-----

  src/linux/cgroups.hpp 65f0958 
  src/linux/cgroups.cpp 6f95376 
  src/slave/containerizer/isolators/cgroups/cpushare.cpp f74e80e 
  src/slave/containerizer/isolators/cgroups/mem.cpp 9e9c55e 

Diff: https://reviews.apache.org/r/19184/diff/


Testing
-------


Thanks,

Archana kumari


Re: Review Request 19184: Fixed a TODO(bmahler): Add namespacing to cgroups to enforce the expected structure, e.g., cgroups::cpuacct::stat.

Posted by Archana kumari <ar...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19184/
-----------------------------------------------------------

(Updated March 13, 2014, 8:08 p.m.)


Review request for mesos.


Repository: mesos-git


Description
-------

Fixed a issue related to Jira 1076


Diffs (updated)
-----

  src/linux/cgroups.hpp 65f0958 
  src/linux/cgroups.cpp 6f95376 
  src/slave/containerizer/isolators/cgroups/cpushare.cpp f74e80e 
  src/slave/containerizer/isolators/cgroups/mem.cpp 9e9c55e 

Diff: https://reviews.apache.org/r/19184/diff/


Testing
-------


Thanks,

Archana kumari


Re: Review Request 19184: Fixed a TODO(bmahler): Add namespacing to cgroups to enforce the expected structure, e.g., cgroups::cpuacct::stat.

Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19184/#review37063
-----------------------------------------------------------



src/linux/cgroups.hpp
<https://reviews.apache.org/r/19184/#comment68321>

    this shouldn't be inside the memory namespace



src/linux/cgroups.cpp
<https://reviews.apache.org/r/19184/#comment68319>

    { on new line, and elsewhere



src/linux/cgroups.cpp
<https://reviews.apache.org/r/19184/#comment68320>

    kill leading whitespace, and elsewhere



src/linux/cgroups.cpp
<https://reviews.apache.org/r/19184/#comment68322>

    two new lines here


- Ian Downes


On March 13, 2014, 11:42 a.m., Archana kumari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19184/
> -----------------------------------------------------------
> 
> (Updated March 13, 2014, 11:42 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Fixed a issue related to Jira 1076
> 
> 
> Diffs
> -----
> 
>   src/linux/cgroups.hpp 65f0958 
>   src/linux/cgroups.cpp 6f95376 
>   src/slave/containerizer/isolators/cgroups/cpushare.cpp f74e80e 
>   src/slave/containerizer/isolators/cgroups/mem.cpp 9e9c55e 
> 
> Diff: https://reviews.apache.org/r/19184/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Archana kumari
> 
>