On Fri, Feb 13, 2009 at 09:46:16AM +0100, Ivana Varekova wrote:
> Balbir Singh wrote:
>> * Ivana Varekova <[email protected]> [2009-02-03 09:12:38]:
>>
>>   
>>> Balbir Singh wrote:
>>>     
>>>> * [email protected] <[email protected]> [2009-02-02 13:22:47]:
>>>>
>>>>         
>>>>> This patch standardize error messages form of the second part of 
>>>>> cgclassify.
>>>>>
>>>>> Signed-off-by: Ivana Varekova <[email protected]>
>>>>>
>>>>>             
>>>> The patch looks good, but the approach we've thought of for the longer
>>>> run is different. We want to save errno to a special thread specific
>>>> variable. In the configuration errno does not have to be thread
>>>> specific, since we don't allow threads to parse a configuration file
>>>> in parallel. Instead of the adding new error codes, could you please
>>>> add the capability to save errno and print it out on error?
>>>>  
>>>>
>>>>         
>>> OK I will change it.
>>> Ivana Varekova
>>>
>>>
>>>     
>>
>> Thanks, Ivana!
>>
>>   
> The patch is attached, I hope it's OK this way.
> Signed-off-by: Ivana Varekova <varek...@re...>
> Revewed-by: Jan Safranek <jsafr...@re...>
>


Need the full email address. So it shoud be

Signed-off-by: Ivana Varekova <[email protected]>

Also missing a changelog. :(. I am missing the context completely.


> diff -up ./api.c.orig ./api.c
> --- ./api.c.orig      2009-02-06 13:51:40.000000000 +0100
> +++ ./api.c   2009-02-13 09:38:40.000000000 +0100
> @@ -48,6 +48,15 @@
> 
>  #define VERSION(ver) #ver
> 
> +/* the errno which happend the last time
> + * (have to be thread specific) */
> +__thread int last_errno;
> +
> +#define MAXLEN 256
> +
> +/* the value have to be thread specific */
> +__thread char errtext[MAXLEN];
> +
>  /*
>   * Remember to bump this up for major API changes.
>   */
> @@ -94,6 +103,7 @@ char *cgroup_strerror_codes[] = {
>       "Cgroup parsing failed",
>       "Cgroup, rules file does not exist",
>       "Cgroup mounting failed",
> +     "The config file can not be opend",
>  };
> 
>  static int cg_chown_file(FTS *fts, FTSENT *ent, uid_t owner, gid_t group)
> @@ -294,14 +304,16 @@ static int cgroup_parse_rules(bool cache
>               dbg("Failed to open configuration file %s with"
>                               " error: %s\n", CGRULES_CONF_FILE,
>                               strerror(errno));
> -             ret = errno;
> +             last_errno = errno;
> +             ret = ECGCANTOPENCF;
>               goto finish;
>       }
> 
>       buff = calloc(CGROUP_RULE_MAXLINE, sizeof(char));
>       if (!buff) {
>               dbg("Out of memory?  Error: %s\n", strerror(errno));
> -             ret = errno;
> +             last_errno = errno;
> +             ret = ECGOTHER;
>               goto close_unlock;
>       }
> 
> @@ -442,7 +454,8 @@ static int cgroup_parse_rules(bool cache
>               newrule = calloc(1, sizeof(struct cgroup_rule));
>               if (!newrule) {
>                       dbg("Out of memory?  Error: %s\n", strerror(errno));
> -                     ret = errno;
> +                     last_errno = errno;
> +                     ret = ECGOTHER;
>                       goto cleanup;
>               }
> 
> @@ -565,6 +578,7 @@ int cgroup_init()
>        */
>       buf = malloc(FILENAME_MAX);
>       if (!buf) {
> +             last_errno = errno;
>               ret = ECGOTHER;
>               goto unlock_exit;
>       }
> @@ -777,11 +791,13 @@ int cgroup_attach_task_pid(struct cgroup
>                               dbg("Error writing tid %d to %s:%s\n",
>                                               tid, path, strerror(errno));
>                               fclose(tasks);
> +                             last_errno = errno;
>                               return ECGOTHER;
>                       }
> 
>                       ret = fflush(tasks);
>                       if (ret) {
> +                             last_errno = errno;
>                               dbg("Error writing tid  %d to %s:%s\n",
>                                               tid, path, strerror(errno));
>                               fclose(tasks);
> @@ -822,6 +838,7 @@ int cgroup_attach_task_pid(struct cgroup
>                       }
>                       ret = fprintf(tasks, "%d", tid);
>                       if (ret < 0) {
> +                             last_errno = errno;
>                               dbg("Error writing tid %d to %s:%s\n",
>                                               tid, path, strerror(errno));
>                               fclose(tasks);
> @@ -829,6 +846,7 @@ int cgroup_attach_task_pid(struct cgroup
>                       }
>                       ret = fflush(tasks);
>                       if (ret) {
> +                             last_errno = errno;
>                               dbg("Error writing tid  %d to %s:%s\n",
>                                               tid, path, strerror(errno));
>                               fclose(tasks);
> @@ -872,12 +890,16 @@ static int cg_mkdir_p(const char *path)
> 
>       buf = getcwd(cwd, FILENAME_MAX);
> 
> -     if (!buf)
> +     if (!buf) {
> +             last_errno = errno;
>               return ECGOTHER;
> +     }
> 
>       real_path = strdup(path);
> -     if (!real_path)
> +     if (!real_path) {
> +             last_errno = errno;
>               return ECGOTHER;
> +     }
> 
>       do {
>               while (real_path[j] != '\0' && real_path[j] != '/')
> @@ -892,6 +914,7 @@ static int cg_mkdir_p(const char *path)
>               ret = mkdir(str, S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH);
>               wd = strdup(str);
>               if (!wd) {
> +                     last_errno = errno;
>                       ret = ECGOTHER;
>                       break;
>               }
> @@ -1030,6 +1053,7 @@ int cgroup_modify_cgroup(struct cgroup *
>                       ret = asprintf(&path, "%s%s", base,
>                               cgroup->controller[i]->values[j]->name);
>                       if (ret < 0) {
> +                             last_errno = errno;
>                               error = ECGOTHER;
>                               goto err;
>                       }
> @@ -1175,6 +1199,7 @@ int cgroup_create_cgroup(struct cgroup *
>               base = strdup(path);
> 
>               if (!base) {
> +                     last_errno = errno;
>                       error = ECGOTHER;
>                       goto err;
>               }
> @@ -1191,6 +1216,7 @@ int cgroup_create_cgroup(struct cgroup *
>                       ret = asprintf(&path, "%s%s", base,
>                                       cgroup->controller[k]->values[j]->name);
>                       if (ret < 0) {
> +                             last_errno = errno;
>                               error = ECGOTHER;
>                               goto err;
>                       }
> @@ -1215,12 +1241,14 @@ int cgroup_create_cgroup(struct cgroup *
>                       free(path);
>                       ret = asprintf(&path, "%s/tasks", base);
>                       if (ret < 0) {
> +                             last_errno = errno;
>                               error = ECGOTHER;
>                               goto err;
>                       }
>                       error = chown(path, cgroup->tasks_uid,
>                                                       cgroup->tasks_gid);
>                       if (error) {
> +                             last_errno = errno;
>                               error = ECGOTHER;
>                               goto err;
>                       }
> @@ -1403,6 +1431,7 @@ int cgroup_delete_cgroup(struct cgroup *
>                                       cgroup->controller[i]->name))
>                       continue;
>               error = rmdir(path);
> +             last_errno = errno;
>       }
>  open_err:
>       if (ignore_migration) {
> @@ -1411,8 +1440,10 @@ open_err:
>                                               cgroup->controller[i]->name))
>                               continue;
>                       error = rmdir(path);
> -                     if (error < 0 && errno == ENOENT)
> +                     if (error < 0 && errno == ENOENT) {
> +                             last_errno = errno;
>                               error = 0;
> +                     }
>               }
>       }
>       if (error)
> @@ -1441,8 +1472,10 @@ static int cg_rd_ctrl_file(char *subsys,
>               return ECGROUPVALUENOTEXIST;
> 
>       *value = malloc(CG_VALUE_MAX);
> -     if (!*value)
> +     if (!*value) {
> +             last_errno = errno;
>               return ECGOTHER;
> +     }
> 
>       /*
>        * using %as crashes when we try to read from files like
> @@ -1596,11 +1629,13 @@ int cgroup_get_cgroup(struct cgroup *cgr
>               ret = asprintf(&control_path, "%s/tasks", path);
> 
>               if (ret < 0) {
> +                     last_errno = errno;
>                       error = ECGOTHER;
>                       goto unlock_error;
>               }
> 
>               if (stat(control_path, &stat_buffer)) {
> +                     last_errno = errno;
>                       free(control_path);
>                       error = ECGOTHER;
>                       goto unlock_error;
> @@ -1620,6 +1655,7 @@ int cgroup_get_cgroup(struct cgroup *cgr
> 
>               dir = opendir(path);
>               if (!dir) {
> +                     last_errno = errno;
>                       error = ECGOTHER;
>                       goto unlock_error;
>               }
> @@ -1739,8 +1775,10 @@ static int cg_prepare_controller_array(c
> 
>               if (temp) {
>                       controllers[j] = strdup(temp);
> -                     if (!controllers[j])
> +                     if (!controllers[j]) {
> +                             last_errno = errno;
>                               return ECGOTHER;
> +                     }
>               }
>               j++;
>       } while (temp);
> @@ -2137,6 +2175,7 @@ int cgroup_get_current_controller_path(p
>                */
>               if (ret != 3 || ret == EOF) {
>                       dbg("read failed for pid_cgroup_fd ret %d\n", ret);
> +                     last_errno = errno;
>                       ret = ECGOTHER;
>                       goto done;
>               }
> @@ -2147,6 +2186,7 @@ int cgroup_get_current_controller_path(p
>                                                               == 0) {
>                               *current_path = strdup(cgroup_path);
>                               if (!*current_path) {
> +                                     last_errno = errno;
>                                       ret = ECGOTHER;
>                                       goto done;
>                               }
> @@ -2168,5 +2208,19 @@ cleanup_path:
>  char *cgroup_strerror(int code)
>  {
>       assert((code >= ECGROUPNOTCOMPILED) && (code < ECGSENTINEL));
> +     if ((code == ECGOTHER) || (code == ECGCANTOPENCF)) {
> +             snprintf(errtext, MAXLEN, "%s: %s",
> +                     cgroup_strerror_codes[code % ECGROUPNOTCOMPILED],
> +                     strerror(cgroup_get_last_errno()));
> +             return errtext;
> +     }
>       return cgroup_strerror_codes[code % ECGROUPNOTCOMPILED];
>  }
> +
> +/**
> + * Return last errno, which caused ECGOTHER/ECGCANTOPENCF error.
> + */
> +int cgroup_get_last_errno()
> +{
> +    return last_errno;
> +}
> diff -up ./cgclassify.c.orig ./cgclassify.c
> --- ./cgclassify.c.orig       2009-02-06 13:51:40.000000000 +0100
> +++ ./cgclassify.c    2009-02-13 09:29:49.000000000 +0100
> @@ -140,7 +140,8 @@ int main(int argc, char *argv[])
>               ret = cgroup_change_cgroup_uid_gid(euid, egid, pid);
>               if (ret) {
>                       fprintf(stderr, "Error: change of cgroup failed for"
> -                                     " pid %d\n", pid);
> +                                     " pid %d: %s\n",
> +                                     pid, cgroup_strerror(ret));
>                       return ret;
>               }
>       }
> diff -up ./libcgroup.h.orig ./libcgroup.h
> --- ./libcgroup.h.orig        2009-02-06 13:51:40.000000000 +0100
> +++ ./libcgroup.h     2009-02-13 09:30:08.000000000 +0100
> @@ -88,12 +88,13 @@ enum cgroup_errors {
>       /* Represents error coming from other libraries like glibc. libcgroup
>        * users need to check errno upon encoutering ECGOTHER.
>        */
> -     ECGOTHER,
> +     ECGOTHER,       /* OS error, see errno */
>       ECGROUPNOTEQUAL,
>       ECGCONTROLLERNOTEQUAL,
>       ECGROUPPARSEFAIL, /* Failed to parse rules configuration file. */
>       ECGROUPNORULES, /* Rules list does not exist. */
>       ECGMOUNTFAIL,
> +     ECGCANTOPENCF,  /* OS error, see errno */
>       ECGSENTINEL,    /* Please insert further error codes above this */
>  };
> 
> @@ -195,6 +196,12 @@ int cgroup_get_current_controller_path(p
>   */
>  char *cgroup_strerror(int code);
> 
> +/**
> + * Return last errno, which caused ECGOTHER error.
> + */
> +int cgroup_get_last_errno();
> +
> +
>  /* The wrappers for filling libcg structures */
> 
>  struct cgroup *cgroup_new_cgroup(const char *name);
> diff -up ./libcgroup.map.orig ./libcgroup.map
> --- ./libcgroup.map.orig      2009-02-06 13:51:40.000000000 +0100
> +++ ./libcgroup.map   2009-02-10 11:19:27.000000000 +0100
> @@ -44,4 +44,5 @@ local:
>  CGROUP_0.32.1 {
>  global:
>       cgroup_strerror;
> +     cgroup_get_last_errno;
>  } CGROUP_0.32;

You need to put this is a new version.


-- 
regards,
Dhaval

------------------------------------------------------------------------------
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H
_______________________________________________
Libcg-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/libcg-devel

Reply via email to