Conversation
|
Can one of the admins verify this patch? |
| LOG_WARNING_("can't create butterfly cgroup, fail cmd '%s'", | ||
| create_dir.c_str()); | ||
| std::string directory = std::string(cgroupPath) + "/butterfly" ; | ||
| if (mkdir((char*)directory.c_str(),S_IWGRP) < 0){ |
There was a problem hiding this comment.
mkdir take a const char * in parameter, so I don't think the cast is necessary.
Also, if you must cast, please use a C++ cast. (const_cast,static_cast,bit_cast,reinterpret_cast)
| create_dir.c_str()); | ||
| std::string directory = std::string(cgroupPath) + "/butterfly" ; | ||
| if (mkdir((char*)directory.c_str(),S_IWGRP) < 0){ | ||
| LOG_WARNING_("can't create directory already exists\n"); |
| std::ifstream ifs(file1); | ||
| std::ofstream ofs(file2); | ||
| if((ifs.is_open()) && (ofs.is_open())){ | ||
| ofs << ifs.rdbuf(); |
| std::ifstream in(fileToRead); | ||
| std::ofstream out(fileToWrite); | ||
| if ((in.is_open()) && (out.is_open())){ | ||
| std::string s; |
|
|
||
| int | ||
| main(int argc, char *argv[]) { | ||
| int main(int argc, char *argv[]) { |
There was a problem hiding this comment.
well, that might be right, bit that's unrelated to this commit, so if you want to push that fix, please do it in a separated commit.
outscale-hmi
force-pushed
the
fixCgroup
branch
from
e9f7258 to
35d15fd
Compare
| int setCgroups(std::string file1, std::string file2) { | ||
| std::ifstream ifs(file1); | ||
| std::ofstream ofs(file2); | ||
| if ((ifs.is_open()) && (ofs.is_open())) { |
There was a problem hiding this comment.
Could be simplify to:
if (!ifs.is_open() || !ofs.is_open())
return -1;
ofs << ifs.rdbuf();
return 0;
not a big deal, but economise 3 lines of code
| std::ofstream out(fileToWrite); | ||
| if ((in.is_open()) && (out.is_open())) { | ||
| std::string s; | ||
| while (getline(in, s)) { |
There was a problem hiding this comment.
this file should contain a number, I don't think the loop is necessary
| fileToRead = std::string(cgroupPath) + "/cpuset.mems"; | ||
| if (!access(fileToRead.c_str(), R_OK)) { | ||
| fileToWrite = std::string(cgroupPath) + "/butterfly/cpuset.mems"; | ||
| check = setCgroups(fileToRead, fileToWrite); |
There was a problem hiding this comment.
if (!setCgroups(fileToRead, fileToWrite)) {
LOG_WARNING_("can't set cgroup cpuset.mems");
}
| fileToRead = std::string(cgroupPath) + "/cpuset.cpus"; | ||
| fileToWrite = std::string(cgroupPath) + "/butterfly/cpuset.mems"; | ||
| check = setCgroups(fileToRead, fileToWrite); | ||
| if (check != 0) { |
There was a problem hiding this comment.
if (!setCgroups(fileToRead, fileToWrite)) {
LOG_WARNING_("can't set cgroup cpuset.cpus");
}
| if (oss.str().compare(word)) { | ||
| LOG_WARNING_("can't properly set cgroup pid"); | ||
| return; | ||
| } else { |
There was a problem hiding this comment.
as you return, you can remove the else
| BASH(setStr) { | ||
| int check = 0; | ||
| std::string fileToWrite = std::string(SrcCgroup()) + "/butterfly/tasks"; | ||
| check = setCgroups(oss.str(), fileToWrite); |
There was a problem hiding this comment.
if (!setCgroups(oss.str(), fileToWrite)) {
LOG_WARNING_("can't set cgroup pid");
}
| LOG_WARNING_("can't unset task from butterfly cgroup"); | ||
|
|
||
| std::string fileToRead = std::string(SrcCgroup()) + "/butterfly/tasks"; | ||
| std::string fileToWrite = std::string(SrcCgroup()) + "/bin/bash /tasks"; |
There was a problem hiding this comment.
and is wrong here too, sorry, I think it shoud be
std::string(SrcCgroup()) + "/task"
outscale-mgo
left a comment
outscale-mgo
left a comment
There was a problem hiding this comment.
It should be the last one,
Also I'd like to have more information on why, what and how you are doing this.
| create_dir.c_str()); | ||
| std::string directory = std::string(cgroupPath) + "/butterfly"; | ||
| if (mkdir(directory.c_str(), S_IWGRP) < 0) { | ||
| LOG_WARNING_("can't create directory already exists\n"); |
There was a problem hiding this comment.
I don't think the "already exists" is true,
I mean if mkdir fail, all you know is that you was unable to create the dir, assuming it's because the file alerady exist seems odd to me, you should either use errno or something to know why this fail, or just write "can't create directory: %s", directory.c_str()
| std::string fileToRead = std::string(cgroupPath) + "/cpu.shares"; | ||
| std::string fileToWrite = std::string(cgroupPath) + "/butterfly/cpu.shares"; | ||
| std::ifstream in(fileToRead); | ||
| std::ofstream out(fileToWrite); |
There was a problem hiding this comment.
I think a blank line is needed between declaration and code ?
| } | ||
| BASH("rmdir " + std::string(SrcCgroup()) + "/butterfly") { | ||
|
|
||
| int check = 0; |
|
|
||
| int check = 0; | ||
| std::string dirToRemove = std::string(SrcCgroup()) + "/butterfly"; | ||
| check = rmdir(dirToRemove.c_str()); |
There was a problem hiding this comment.
if (rmdir(dirToRemove.c_str()) < 0) {
LOG_WARNING_("can't destroy cgroup");
}
also rmdir man said that if rmdir fail, it return -1, so using !check is wrong here, and do the exact opposite of what you want
Signed-off-by: hanen mizouni <hanen.mizouni@outscale.com>
| } | ||
|
|
||
| fileToRead = std::string(cgroupPath) + "/cpuset.cpus"; | ||
| fileToWrite = std::string(cgroupPath) + "/butterfly/cpuset.mems"; |
There was a problem hiding this comment.
this code was useful for centos6, so we might need to test it there before merging
2 participants
Signed-off-by: hanen mizouni hanen.mizouni@outscale.com