cgroup: convert bash to Cpp by outscale-hmi · Pull Request #401 · outscale/butterfly

outscale-hmi · 2020-02-24T17:31:34Z

Signed-off-by: hanen mizouni hanen.mizouni@outscale.com

outscale-mgo · 2020-02-24T17:34:03Z

Can one of the admins verify this patch?

outscale-mgo

small detail, and indentations

outscale-mgo · 2020-02-25T10:19:30Z

api/server/app.cc

-        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){


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)

outscale-mgo · 2020-02-25T10:19:40Z

api/server/app.cc

-                     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");


2 space instead of 4

outscale-mgo · 2020-02-25T10:19:50Z

api/server/app.cc

+    std::ifstream ifs(file1);
+    std::ofstream ofs(file2);
+    if((ifs.is_open()) && (ofs.is_open())){
+	ofs << ifs.rdbuf();


bad indentation

outscale-mgo · 2020-02-25T10:20:05Z

api/server/app.cc

+    std::ifstream in(fileToRead);
+    std::ofstream out(fileToWrite);
+    if ((in.is_open()) && (out.is_open())){
+	std::string s;


bad indentation

outscale-mgo · 2020-02-25T10:29:27Z

api/server/app.cc

-
-int
-main(int argc, char *argv[]) {
+int main(int argc, char *argv[]) {


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-mgo · 2020-03-16T12:16:03Z

api/server/app.cc

+int setCgroups(std::string file1, std::string file2) {
+    std::ifstream ifs(file1);
+    std::ofstream ofs(file2);
+    if ((ifs.is_open()) && (ofs.is_open())) {


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

outscale-mgo · 2020-03-16T12:18:24Z

api/server/app.cc

+    std::ofstream out(fileToWrite);
+    if ((in.is_open()) && (out.is_open())) {
+        std::string s;
+        while (getline(in, s)) {


this file should contain a number, I don't think the loop is necessary

outscale-mgo · 2020-03-16T12:21:30Z

api/server/app.cc

+    fileToRead = std::string(cgroupPath) + "/cpuset.mems";
+    if (!access(fileToRead.c_str(), R_OK)) {
+        fileToWrite = std::string(cgroupPath) + "/butterfly/cpuset.mems";
+        check = setCgroups(fileToRead, fileToWrite);


if (!setCgroups(fileToRead, fileToWrite)) { LOG_WARNING_("can't set cgroup cpuset.mems"); }

outscale-mgo · 2020-03-16T12:22:04Z

api/server/app.cc

+        fileToRead = std::string(cgroupPath) + "/cpuset.cpus";
+        fileToWrite = std::string(cgroupPath) + "/butterfly/cpuset.mems";
+        check = setCgroups(fileToRead, fileToWrite);
+        if (check != 0) {


if (!setCgroups(fileToRead, fileToWrite)) {
LOG_WARNING_("can't set cgroup cpuset.cpus");
}

outscale-mgo · 2020-03-16T12:22:34Z

api/server/app.cc

+        if (oss.str().compare(word)) {
+            LOG_WARNING_("can't properly set cgroup pid");
+            return;
+        } else {


as you return, you can remove the else

outscale-mgo · 2020-03-16T12:23:22Z

api/server/app.cc

-    BASH(setStr) {
+    int check = 0;
+    std::string fileToWrite = std::string(SrcCgroup()) + "/butterfly/tasks";
+    check = setCgroups(oss.str(), fileToWrite);


if (!setCgroups(oss.str(), fileToWrite)) { LOG_WARNING_("can't set cgroup pid"); }

outscale-mgo · 2020-03-16T12:29:09Z

api/server/app.cc

-        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";


and is wrong here too, sorry, I think it shoud be
std::string(SrcCgroup()) + "/task"

outscale-mgo

It should be the last one,
Also I'd like to have more information on why, what and how you are doing this.

outscale-mgo · 2020-03-17T12:46:58Z

api/server/app.cc

-                     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");


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()

outscale-mgo · 2020-03-17T12:48:39Z

api/server/app.cc

+    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);


I think a blank line is needed between declaration and code ?

outscale-mgo · 2020-03-17T12:57:05Z

api/server/app.cc

    }
-    BASH("rmdir " + std::string(SrcCgroup()) + "/butterfly") {
+
+    int check = 0;


well, you can remove this

outscale-mgo · 2020-03-17T12:59:29Z

api/server/app.cc

+
+    int check = 0;
+    std::string dirToRemove = std::string(SrcCgroup()) + "/butterfly";
+    check = rmdir(dirToRemove.c_str());


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>

outscale-mgo · 2020-05-12T16:16:13Z

api/server/app.cc

+        }
+
+        fileToRead = std::string(cgroupPath) + "/cpuset.cpus";
+        fileToWrite = std::string(cgroupPath) + "/butterfly/cpuset.mems";


s/cpuset.mems/cpuset.cpus/

this code was useful for centos6, so we might need to test it there before merging

outscale-mgo suggested changes Feb 25, 2020

View reviewed changes

outscale-hmi force-pushed the fixCgroup branch from e9f7258 to 35d15fd Compare February 25, 2020 10:52

outscale-mgo suggested changes Mar 16, 2020

View reviewed changes

outscale-hmi force-pushed the fixCgroup branch from 35d15fd to 09a0ac5 Compare March 16, 2020 17:30

outscale-mgo reviewed Mar 17, 2020

View reviewed changes

outscale-hmi force-pushed the fixCgroup branch from 09a0ac5 to 9aa736b Compare March 17, 2020 15:56

outscale-hmi changed the title ~~cgroup: convert bash to Cpp~~ cgroup: convert bash to Cpp Mar 18, 2020

outscale-hmi force-pushed the fixCgroup branch from 9aa736b to 815a462 Compare March 31, 2020 10:05

outscale-mgo approved these changes Apr 3, 2020

View reviewed changes

cgroup: convert bash to Cpp

c5694cc

Signed-off-by: hanen mizouni <hanen.mizouni@outscale.com>

outscale-hmi force-pushed the fixCgroup branch from 815a462 to c5694cc Compare April 6, 2020 10:26

outscale-mgo suggested changes May 12, 2020

View reviewed changes

Conversation

outscale-hmi commented Feb 24, 2020

Uh oh!

outscale-mgo commented Feb 24, 2020

Uh oh!

outscale-mgo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

outscale-mgo Mar 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

outscale-mgo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

outscale-mgo Mar 16, 2020 •

edited

Loading