From: Serge E. Hallyn <serue@xxxxxxxxxx> Matt Helsley originally suggested this to avoid having two format strings. This is not bisect-safe and therefore not even compile-tested. Every call to ckpt_write_err must be updated to use a single format. Changelog: Oct 29: merge with the next patch, moving ckpt_generate_fmt() into checkpoing/sys.c and making it non-static so that we can use it in ckpt_error(). Oct 28: also fix comment above ckpt_generate_fmt() Oct 27: ensure %(T) has a closing paren Signed-off-by: Serge E. Hallyn <serue@xxxxxxxxxx> --- checkpoint/checkpoint.c | 105 +++----------------------------------------- checkpoint/sys.c | 95 +++++++++++++++++++++++++++++++++++++++ include/linux/checkpoint.h | 13 +++-- 3 files changed, 110 insertions(+), 103 deletions(-) diff --git a/checkpoint/checkpoint.c b/checkpoint/checkpoint.c index 6eb8f3b..c6be4f9 100644 --- a/checkpoint/checkpoint.c +++ b/checkpoint/checkpoint.c @@ -96,104 +96,15 @@ int ckpt_write_string(struct ckpt_ctx *ctx, char *str, int len) return ckpt_write_obj_type(ctx, str, len, CKPT_HDR_STRING); } -/* - * __ckpt_generate_fmt - generate standard checkpoint error message - * @ctx: checkpoint context - * @fmt0: c/r-format string - * @fmt: message format - * - * This generates a unified format of checkpoint error messages, to - * ease (after the failure) inspection by userspace tools. It converts - * the (printf) message @fmt into a new format: "[PREFMT]: fmt". - * - * PREFMT is constructed from @fmt0 by subtituting format snippets - * according to the contents of @fmt0. The format characters in - * @fmt0 can be E (error), O (objref), P (pointer), S (string) and - * V (variable/symbol). For example, E will generate a "err %d" in - * PREFMT (see prefmt_array below). - * - * If @fmt0 begins with T, PREFMT will begin with "pid %d tsk %s" - * with the pid and the tsk->comm of the currently checkpointed task. - * The latter is taken from ctx->tsk, and is it the responsbilility of - * the caller to have a valid pointer there (in particular, functions - * that iterate on the processes: collect_objects, checkpoint_task, - * and tree_count_tasks). - * - * The caller of ckpt_write_err() and _ckpt_write_err() must provide - * the additional variabes, in order, to match the @fmt0 (except for - * the T key), e.g.: - * - * ckpt_writ_err(ctx, "TEO", "FILE flags %d", err, objref, flags); - * - * Here, T is simply passed, E expects an integer (err), O expects an - * integer (objref), and the last argument matches the format string. - */ -static char *__ckpt_generate_fmt(struct ckpt_ctx *ctx, char *fmt0, char *fmt) -{ - static int warn_notask = 0; - static int warn_prefmt = 0; - char *format; - int i, j, len = 0; - - static struct { - char key; - char *fmt; - } prefmt_array[] = { - { 'E', "err %d" }, - { 'O', "obj %d" }, - { 'P', "ptr %p" }, - { 'V', "sym %pS" }, - { 'S', "str %s" }, - { 0, "??? %pS" }, - }; - - /* - * 17 for "pid %d" (plus space) - * 21 for "tsk %s" (tsk->comm) - * up to 8 per varfmt entry - */ - format = kzalloc(37 + 8 * strlen(fmt0) + strlen(fmt), GFP_KERNEL); - if (!format) - return NULL; - - format[len++] = '['; - - if (fmt0[0] == 'T') { - if (ctx->tsk) - len = sprintf(format, "pid %d tsk %s ", - task_pid_vnr(ctx->tsk), ctx->tsk->comm); - else if (warn_notask++ < 5) - printk(KERN_ERR "c/r: no target task set\n"); - fmt0++; - } - - for (i = 0; i < strlen(fmt0); i++) { - for (j = 0; prefmt_array[j].key; j++) - if (prefmt_array[j].key == fmt0[i]) - break; - if (!prefmt_array[j].key && warn_prefmt++ < 5) - printk(KERN_ERR "c/r: unknown prefmt %c\n", fmt0[i]); - len += sprintf(&format[len], "%s ", prefmt_array[j].fmt); - } - - if (len > 1) - sprintf(&format[len-1], "]: %s", fmt); /* erase last space */ - else - sprintf(format, "%s", fmt); - - return format; -} - -/* see _ckpt_generate_fmt for information on @fmt0 */ -static void __ckpt_generate_err(struct ckpt_ctx *ctx, char *fmt0, - char *fmt, va_list ap) +/* see ckpt_generate_fmt for information on @fmt extensions */ +static void __ckpt_generate_err(struct ckpt_ctx *ctx, char *fmt, va_list ap) { va_list aq; char *format; char *str; int len; - format = __ckpt_generate_fmt(ctx, fmt0, fmt); + format = ckpt_generate_fmt(ctx, fmt); va_copy(aq, ap); /* @@ -223,15 +134,14 @@ static void __ckpt_generate_err(struct ckpt_ctx *ctx, char *fmt0, * @fmt: message format * @...: arguments * - * See _ckpt_generate_fmt for information on @fmt0. * Use this during checkpoint to report while holding a spinlock */ -void __ckpt_write_err(struct ckpt_ctx *ctx, char *fmt0, char *fmt, ...) +void __ckpt_write_err(struct ckpt_ctx *ctx, char *fmt, ...) { va_list ap; va_start(ap, fmt); - __ckpt_generate_err(ctx, fmt0, fmt, ap); + __ckpt_generate_err(ctx, fmt, ap); va_end(ap); } @@ -242,10 +152,9 @@ void __ckpt_write_err(struct ckpt_ctx *ctx, char *fmt0, char *fmt, ...) * @fmt: error string format * @...: error string arguments * - * See _ckpt_generate_fmt for information on @fmt0. * If @fmt is null, the string in the ctx->err_string will be used (and freed) */ -int ckpt_write_err(struct ckpt_ctx *ctx, char *fmt0, char *fmt, ...) +int ckpt_write_err(struct ckpt_ctx *ctx, char *fmt, ...) { va_list ap; char *str; @@ -253,7 +162,7 @@ int ckpt_write_err(struct ckpt_ctx *ctx, char *fmt0, char *fmt, ...) if (fmt) { va_start(ap, fmt); - __ckpt_generate_err(ctx, fmt0, fmt, ap); + __ckpt_generate_err(ctx, fmt, ap); va_end(ap); } diff --git a/checkpoint/sys.c b/checkpoint/sys.c index 260a1ee..9b2de18 100644 --- a/checkpoint/sys.c +++ b/checkpoint/sys.c @@ -339,6 +339,101 @@ int walk_task_subtree(struct task_struct *root, return (ret < 0 ? ret : total); } +/* + * ckpt_generate_fmt - generate standard checkpoint error message + * @ctx: checkpoint context + * @fmt: message format + * + * This generates a unified format of checkpoint error messages, to + * ease (after the failure) inspection by userspace tools. It converts + * the (printf) message @fmt into a new format: "[PREFMT]: fmt". + * + * PREFMT is constructed from @fmt by subtituting format snippets + * according to the contents of @fmt. The format characters in + * @fmt can be %(E) (error), %(O) (objref), %(P) (pointer), %(S) (string), + * %(C) (bytes read/written out of checkpoint image so far), * and %(V) + * (variable/symbol). For example, %(E) will generate a "err %d" + * in PREFMT. + * + * If @fmt begins with %(T), PREFMT will begin with "pid %d tsk %s" + * with the pid and the tsk->comm of the currently checkpointed task. + * The latter is taken from ctx->tsk, and is it the responsbilility of + * the caller to have a valid pointer there (in particular, functions + * that iterate on the processes: collect_objects, checkpoint_task, + * and tree_count_tasks). + * + * The caller of ckpt_write_err() and _ckpt_write_err() must provide + * the additional variabes, in order, to match the @fmt0 (except for + * the T key), e.g.: + * + * ckpt_writ_err(ctx, "TEO", "FILE flags %d", err, objref, flags); + * + * Here, T is simply passed, E expects an integer (err), O expects an + * integer (objref), and the last argument matches the format string. + * + * XXX Do we want 'T' and 'C' to simply always be prepended? + */ +char *ckpt_generate_fmt(struct ckpt_ctx *ctx, char *fmt) +{ + char *format; + int alloclen, len = 0; + int first = 1; + + /* + * 17 for "pid %d" (plus space) + * 21 for "tsk %s" (tsk->comm) + * up to 8 per varfmt entry + */ + alloclen = 37 + 8 * strlen(fmt); + format = kzalloc(alloclen, GFP_KERNEL); + if (!format) + return NULL; + + for (; *fmt; fmt++) { + BUG_ON(len > alloclen); + if (*fmt != '%' || fmt[1] != '(' || fmt[3] != ')') { + format[len++] = *fmt; + continue; + } + if (!first) + format[len++] = ' '; + else + first = 0; + switch (fmt[2]) { + case 'E': + len += sprintf(format+len, "[%s]", "err %d"); + break; + case 'O': + len += sprintf(format+len, "[%s]", "obj %d"); + break; + case 'P': + len += sprintf(format+len, "[%s]", "ptr %p"); + break; + case 'V': + len += sprintf(format+len, "[%s]", "sym %pS"); + break; + case 'S': + len += sprintf(format+len, "[%s]", "str %s"); + break; + case 'T': + if (ctx->tsk) + len += sprintf(format+len, "[pid %d tsk %s]", + task_pid_vnr(ctx->tsk), ctx->tsk->comm); + else + len += sprintf(format+len, "[pid -1 tsk NULL]"); + break; + default: + printk(KERN_ERR "c/r: bad format specifier %c\n", + fmt[2]); + BUG(); + } + + fmt += 3; + } + format[len] = '\0'; + + return format; +} /** * sys_checkpoint - checkpoint a container diff --git a/include/linux/checkpoint.h b/include/linux/checkpoint.h index dfcb59b..8a1eaa7 100644 --- a/include/linux/checkpoint.h +++ b/include/linux/checkpoint.h @@ -70,12 +70,13 @@ extern int ckpt_write_buffer(struct ckpt_ctx *ctx, void *ptr, int len); extern int ckpt_write_string(struct ckpt_ctx *ctx, char *str, int len); /* - * Generate a checkpoint error message with unified format, of the - * form: "[PREFMT]: @fmt", where PREFMT is constructed from @fmt0. See - * checkpoint/checkpoint.c:__ckpt_generate_fmt() for details. + * Generate a checkpoint error message with unified format. Format + * can include tokens like %(T) for checkpoint-specific arguments, + * which must come before non-checkpoint-specific ones. + * See checkpoint/checkpoint.c:__ckpt_generate_fmt() for details. */ -extern void __ckpt_write_err(struct ckpt_ctx *ctx, char *fmt0, char *fmt, ...); -extern int ckpt_write_err(struct ckpt_ctx *ctx, char *fmt0, char *fmt, ...); +extern void __ckpt_write_err(struct ckpt_ctx *ctx, char *fmt, ...); +extern int ckpt_write_err(struct ckpt_ctx *ctx, char *fmt, ...); extern int _ckpt_read_obj_type(struct ckpt_ctx *ctx, void *ptr, int len, int type); @@ -370,6 +371,8 @@ static inline void restore_debug_free(struct ckpt_ctx *ctx) {} #endif /* CONFIG_CHECKPOINT_DEBUG */ +extern char *ckpt_generate_fmt(struct ckpt_ctx *ctx, char *fmt); + #endif /* CONFIG_CHECKPOINT */ #endif /* __KERNEL__ */ -- 1.6.1 _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/containers