Serge E. Hallyn wrote: > Quoting Oren Laadan (orenl@xxxxxxxxxxx): >> >> Serge E. Hallyn wrote: >>> Quoting Oren Laadan (orenl@xxxxxxxxxxx): >>>> Serge E. Hallyn wrote: >>>>> Quoting Matt Helsley (matthltc@xxxxxxxxxx): >>>>>>> @@ -401,6 +409,9 @@ char *ckpt_generate_fmt(struct ckpt_ctx *ctx, char *fmt) >>>>>>> case 'E': >>>>>>> len += sprintf(format+len, "[%s]", "err %d"); >>>>>>> break; >>>>>>> + case 'C': /* count of bytes read/written to checkpoint image */ >>>>>>> + len += sprintf(format+len, "[%s]", "pos %d"); >>>>>>> + break; >>>>>> Instead we could always output ckpt->total and then we wouldn't need %(C). I >>>>>> suspect it's such a useful piece of information that it'll be repeated >>>>>> in many/all format strings eventually. >>>>> Yes, likewise %(T). If that's what we want to do. >>>> I agree. For the cases when there is not task, can put "none" >>>> >>>>> Should we discuss here what we want an entry to look like? For both >>>>> ckpt_write_err (to the checkpoint image) and ckpt_error()? >>>>> >>>> Yes please ! >>> Actually %T isn't the current task, right, so it shouldn't always be prepended? >>> It actually is only meaningful during checkpoint_task(), collect_objs(), and >>> __tree_count_tasks? >>> >>> Ok, so how about: >>> >>> 1. ckpt_write_err() always also calls ckpt_error() (which in turn calls >>> ckpt_debug). Avoid duplication which exists in several places >>> right now. >>> 2. We always prepend: >>> >>> [current->pid]:[ctx->root_pid]:[ctx->active_pid]:[ctx->errno][ctx->total] >>> >>> The %(X) expansions if specified come whereever they are in the fmt >>> string (which is what's happening now with my patchset). >> So somewhere should set ctx->errno during a checkpoint. >> >> I suppose active_pid is for restart, but it's redundant isn't it ? >> (it's always active_pid) - is it the different between top-level pid-ns >> and "current" pid-ns ? > > No, I figured it would be meaningful for instance in places like > wait_task_active(). Perhaps then leave it out of the default printing, and have the specific debug messages there write it explicitly. > >> Instead of writing root_pid repeatedly, why not write sometime at the >> beginning some "global" info about the checkpoint/restart ? (e.g. >> the root_pid ...) > > Well it is written out (for restart) at the end, so I suppose I should > switch restore_debug_free() to using ckpt_error() instead of ckpt_debug(). Yes, that will be helpful to reduce the noise :) Oren. _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/containers
