Dhaval Giani - 13:22 15.12.10 wrote:
> On Wed, Dec 15, 2010 at 10:12 AM, Michal Hrusecky <mic...@hrusecky.net> wrote:
> > Rules can be written using wildcards, but destinations had to be static.
> > This patch adds support for following strings in destination:
> >
> > %u - uid
> > %U - username, uid in case of error
> > %g - gid
> > %G - group name, gid in case of error
> > %p - pid
> > %P - proccess name, pid in case of error
> >
>
> So do we need this distinction? Let's always try to put it in the
> username/groupname/processname and the uid/gid/pid in case of an
> error? Makes it easier, doesn't it? (Also do you see a usecase where
> people would prefer pid/gid/uid over the name itself?)
I can imagine the case, when I've got not very reliable network and I'm
still running LDAP/NIS/... Then resolving uid/gid to login or group name
can be slow and might even fail. Then as a sysadmin I would prefer to
use direct uid/gid. But on local computer/reliable network, I would use
names as they are easier for humans.
> > So more general rules can be specified using wildcards. Example rule can
> > be:
> >
> > *...@users * %G/%U
> >
> > This will put all users in their own cgroups named by their login and
> > group.
> >
> > Signed-off-by: Michal Hrusecky <mic...@hrusecky.net>
> > ---
> > src/api.c | 82
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 files changed, 81 insertions(+), 1 deletions(-)
> >
> > diff --git a/src/api.c b/src/api.c
> > index 859190a..29b8627 100644
> > --- a/src/api.c
> > +++ b/src/api.c
> > @@ -2366,6 +2366,12 @@ int cgroup_change_cgroup_flags(uid_t uid, gid_t gid,
> > /* Temporary pointer to a rule */
> > struct cgroup_rule *tmp = NULL;
> >
> > + /* Temporary variables for destination substitution */
> > + char newdest[FILENAME_MAX];
>
> char *newdest and use asprintf.
>
> hmm. having said that, just using the array might be easier :-). I
> will let you figure it out :P
With asprintf I would have to deal with reallocation and I think that it
would make code even uglier :-) And we have limit to the size of
variable anyway.
> > + int i, j;
> > + struct passwd * user_info;
> > + struct group * group_info;
> > +
>
> Coding style
> struct passwd *user_info;
> struct group *group_info;
>
> > /* Return codes */
> > int ret = 0;
> >
> > @@ -2418,7 +2424,81 @@ int cgroup_change_cgroup_flags(uid_t uid, gid_t gid,
> > do {
> > cgroup_dbg("Executing rule %s for PID %d... ", tmp->username,
> > pid);
> > - ret = cgroup_change_cgroup_path(tmp->destination,
> > + // Destination substitutions
>
> /* */ as opposed to //
>
> > + for(j = i = 0; i < strlen(tmp->destination); i++, j++) {
> > + if(tmp->destination[i] == '%') {
> > + switch(tmp->destination[++i]) {
> > + case 'u':
> > + j += snprintf(newdest+j,
> > + FILENAME_MAX-j,
> > + "%d", uid);
> > + i++;
> > + break;
> > + case 'U':
> > + user_info = getpwuid(uid);
> > + if(user_info) {
> > + j +=
> > snprintf(newdest+j,
> > +
> > FILENAME_MAX-j,
> > + "%s",
> > + user_info ->
> > + pw_name);
> > + } else {
> > + j +=
> > snprintf(newdest+j,
> > +
> > FILENAME_MAX-j,
> > + "%d", uid);
> > + }
> > + i++;
> > + break;
> > + case 'g':
> > + j += snprintf(newdest+j,
> > + FILENAME_MAX-j,
> > + "%d", gid);
> > + i++;
> > + break;
> > + case 'G':
> > + group_info = getgrgid(gid);
> > + if(group_info) {
> > + j +=
> > snprintf(newdest+j,
> > +
> > FILENAME_MAX-j,
> > + "%s",
> > + group_info
> > ->
> > + gr_name);
> > + } else {
> > + j +=
> > snprintf(newdest+j,
> > +
> > FILENAME_MAX-j,
> > + "%d", gid);
> > + }
> > + i++;
> > + break;
> > + case 'p':
> > + j += snprintf(newdest+j,
> > + FILENAME_MAX-j,
> > + "%d", pid);
> > + i++;
> > + break;
> > + case 'P':
> > + if(procname) {
> > + j +=
> > snprintf(newdest+j,
> > +
> > FILENAME_MAX-j,
> > + "%s",
> > + procname);
> > + } else {
> > + j +=
> > snprintf(newdest+j,
> > +
> > FILENAME_MAX-j,
> > + "%d", pid);
> > + }
> > + i++;
> > + break;
> > + default:
> > + newdest[j++] = '%';
> > + }
> > + }
> > + if(tmp->destination[i] == '\\')
> > + i++;
> > + newdest[j] = tmp->destination[i];
> > + }
> > + newdest[j] = 0;
> > + ret = cgroup_change_cgroup_path(newdest,
> > pid, (const char * const *)tmp->controllers);
> > if (ret) {
> > cgroup_dbg("FAILED! (Error Code: %d)\n", ret);
>
> At first glance it seems fine. I will need to take a closer look
> though. In the meantime, some testing results also help a long way in
> merging stuff ;-) I will try to give it a run in the next two days!
>
> Thanks for this effort!
> Dhaval
Thanks for the review, will submit new version soon.
--
Michal Hrusecky <mic...@hrusecky.net>
------------------------------------------------------------------------------
Lotusphere 2011
Register now for Lotusphere 2011 and learn how
to connect the dots, take your collaborative environment
to the next level, and enter the era of Social Business.
http://p.sf.net/sfu/lotusphere-d2d
_______________________________________________
Libcg-devel mailing list
Libcg-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libcg-devel
