Skip to content

New column for LXC containers name using the CGroups #217

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

New column for LXC containers name using the CGroups #217

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions linux/LinuxProcess.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ ProcessFieldData Process_fields[] = {
#endif
#ifdef HAVE_CGROUP
[CGROUP] = { .name = "CGROUP", .title = " CGROUP ", .description = "Which cgroup the process is in", .flags = PROCESS_FLAG_LINUX_CGROUP, },
[LXC] = { .name = "LXC", .title = " LXC ", .description = "Which LXC constainer the process is in", .flags = PROCESS_FLAG_LINUX_CGROUP, },
#endif
[OOM] = { .name = "OOM", .title = " OOM ", .description = "OOM (Out-of-Memory) killer score", .flags = PROCESS_FLAG_LINUX_OOM, },
[IO_PRIORITY] = { .name = "IO_PRIORITY", .title = "IO ", .description = "I/O priority", .flags = PROCESS_FLAG_LINUX_IOPRIO, },
Expand Down Expand Up @@ -148,6 +149,7 @@ void Process_delete(Object* cast) {
Process_done((Process*)cast);
#ifdef HAVE_CGROUP
free(this->cgroup);
free(this->lxc);
#endif
free(this->secattr);
free(this->ttyDevice);
Expand Down Expand Up @@ -261,6 +263,7 @@ void LinuxProcess_writeField(Process* this, RichString* str, ProcessField field)
#endif
#ifdef HAVE_CGROUP
case CGROUP: xSnprintf(buffer, n, "%-10s ", lp->cgroup ? lp->cgroup : ""); break;
case LXC: xSnprintf(buffer, n, "%-10s ", lp->lxc ? lp->lxc : ""); break;
#endif
case OOM: xSnprintf(buffer, n, "%4u ", lp->oom); break;
case IO_PRIORITY: {
Expand Down Expand Up @@ -362,6 +365,8 @@ long LinuxProcess_compare(const void* v1, const void* v2) {
#ifdef HAVE_CGROUP
case CGROUP:
return strcmp(p1->cgroup ? p1->cgroup : "", p2->cgroup ? p2->cgroup : "");
case LXC:
return strcmp(p1->lxc ? p1->lxc : "", p2->lxc ? p2->lxc : "");
#endif
case OOM:
return ((int)p2->oom - (int)p1->oom);
Expand Down
6 changes: 5 additions & 1 deletion linux/LinuxProcess.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,10 @@ typedef enum LinuxProcessFields {
M_PSSWP = 121,
CTXT = 122,
SECATTR = 123,
LAST_PROCESSFIELD = 124,
#ifdef HAVE_CGROUP
LXC = 124,
#endif
LAST_PROCESSFIELD = 125,
} LinuxProcessField;

#include "IOPriority.h"
Expand Down Expand Up @@ -130,6 +133,7 @@ typedef struct LinuxProcess_ {
#endif
#ifdef HAVE_CGROUP
char* cgroup;
char* lxc;
#endif
unsigned int oom;
char* ttyDevice;
Expand Down
33 changes: 33 additions & 0 deletions linux/LinuxProcessList.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,33 @@ static void LinuxProcessList_readCGroupFile(LinuxProcess* process, const char* d
if (!ok) break;
char* group = strchr(buffer, ':');
if (!group) break;

if (!process->lxc) {
Copy link
Member

@cgzones cgzones Oct 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the LXC cgroup never change for a running process? (Cause it's only computed once)

Copy link
Member

@BenBE BenBE Oct 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically it could (changed process namespace), but practically it doesn't.

// Each line have 3 columns separated by a ':', the 3rd column contains the pathname
char* groupPathname = strchr(group + 1, ':') + 1;
if (groupPathname) {
if ( String_startsWith(groupPathname, "/lxc.payload/") || String_startsWith(groupPathname, "/lxc.payload.") ) {
// The process is inside a LXC container using CGroup V2 (/lxc.payload.) or a modern CGroup V1 terminology (/lxc.payload/), for a better readability, only the container name is kept
char *slashpostion = strchr((groupPathname + 13), '/');
if (slashpostion) {
// There is a '/' in the CGroup string (after the initial "/lxc.payload"), a '\0' will truncate the string at this position
groupPathname[(slashpostion - groupPathname)] = '\0';
Copy link
Member

@cgzones cgzones Oct 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we are modifying the string behind groupPathname, which points into the string group, which get written later in line 568 (int wrote = snprintf(at, left, "%s", group);).
Do we really want to modify group?
Or maybe move the LXC block to the end of the while loop, so any changes to group are discarded.

Copy link
Author

@jb-boin jb-boin Oct 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In all fairness, the current implementation of the CGROUP column is fairly useless on modern systems as it will show lengty SystemD CGroups for services started by it that breaks the alignment of columns as the values are too long compared to the column size.

I initially did put my code inside the LinuxProcessList_readCGroupFile() function in the middle of the CGROUP column code for simplicity but maybe it could be separated in another function as there probably wont be anybody using both columns at the same time or create two sub function called inside LinuxProcessList_readCGroupFile() for each of the columns.

}
free(process->lxc);
process->lxc = String_trim(groupPathname + 13);
} else if (String_startsWith(groupPathname, "/lxc/")) {
// The process is inside a LXC container using a legacy CGroup V1 (/lxc/), for a better readability, only the container name is kept
char *slashpostion = strchr((groupPathname + 5), '/');
if (slashpostion) {
// There is a '/' in the CGroup string (after the initial "/lxc/"), a '\0' will truncate the string at this position
groupPathname[(slashpostion - groupPathname)] = '\0';
Copy link
Member

@cgzones cgzones Oct 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dito

}
free(process->lxc);
process->lxc = String_trim(groupPathname + 5);
}
}
}

if (at != output) {
*at = ';';
at++;
Expand All @@ -542,6 +569,12 @@ static void LinuxProcessList_readCGroupFile(LinuxProcess* process, const char* d
left -= wrote;
}
fclose(file);
if (!process->lxc) {
Copy link
Member

@cgzones cgzones Oct 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole block is now a no-op (it is only entered if process->lxc is NULL)

// The process is not in a LXC container
free(process->lxc);
// To show an empty value instead of (null)
process->lxc = NULL;
}
free(process->cgroup);
process->cgroup = xStrdup(output);
}
Expand Down