Serge E. Hallyn wrote: > Until now, debug data was sent to syslog. That's not really > right. > > This patch sends ckpt_debug output to a logfile provided by > the caller. If no logfile is provided, then no debug output > is needed. So the user can pass in -1 for the logfd to say > so. I suggest to keep an option to write to the console/syslog too. Otherwise we may lose important debug info if a nasty crash occurs ... > > Note that this does not address the potential (inevitable?) > lockup of writing out a debug msg while checkpointing the > debug file. In practice so far it seems to work rather well. > (with quite a bit of testing) I believe the only concern here is that our debug code should not write any debug info while the respective inode is locked. > > This also means that we have to be more careful than we have > been about not writing out sensitive data. > > This is pure RFC, not meant to be pretty. > > The split into ckpt_debug and ckpt_err (see changelog) suggests > that we should rearrange (and be more consistent about) how and > when we print out debug info. Left as an exercise for later. I hate homework... Besides, I think we need to address it now. This patch is pretty intrusive and will be painful when I fold to ckpt-v19. I do not want to go through the process twice. It's definitely time to come up with guidelines to when/where/how to add debugging output, and when/where/how to add error output. In the 'how' section, besides avoiding leaking sensitive data, I'd like to come up with a canonical format that will be easy to read and to parse automatically (improve over ckpt_write_err). Hmm... ckpt_write_err() should change too -- be written to the log instead. > > Changelog: > Oct 21: split ckpt_debug into ckpt_debug and ckpt_err. > Git rid of the split by memory debug info etc. The split is useful to control the amount of log. > Since userspace actively asks for debug info, I > also made it not depend on CONFIG_CHECKPOINT_DEBUG. > We may want to put it back again to limit kernel > size, but for now it's a distraction, and I'm not > convinced it makes sense Then add CONFIG_CHECKPOINT_LOGGING ? (automatically implied by CONFIG_CHECKPOINT_DEBUG) > > Signed-off-by: Serge E. Hallyn <serue@xxxxxxxxxx> > --- > arch/s390/kernel/compat_wrapper.S | 2 + > arch/x86/mm/checkpoint.c | 11 ++--- > checkpoint/checkpoint.c | 22 ++++---- > checkpoint/files.c | 29 +++++------ > checkpoint/memory.c | 43 ++++++++------- > checkpoint/namespace.c | 3 - > checkpoint/objhash.c | 33 +++++++------ > checkpoint/process.c | 85 +++++++++++++++---------------- > checkpoint/restart.c | 101 +++++++++++++++++-------------------- > checkpoint/signal.c | 5 +-- > checkpoint/sys.c | 94 ++++++++++++++++++++++------------ > drivers/char/tty_io.c | 22 ++++---- > include/linux/checkpoint.h | 85 ++++++++++++++++--------------- > include/linux/checkpoint_types.h | 5 +- > include/linux/syscalls.h | 5 +- > ipc/checkpoint.c | 13 ++--- > ipc/checkpoint_msg.c | 13 ++--- > ipc/checkpoint_sem.c | 13 ++--- > ipc/checkpoint_shm.c | 15 ++--- > kernel/cred.c | 2 +- > lib/Kconfig.debug | 4 +- > mm/filemap.c | 2 +- > mm/shmem.c | 2 +- > net/checkpoint.c | 46 ++++++++-------- > net/ipv4/checkpoint.c | 26 ++++----- > net/unix/checkpoint.c | 65 +++++++++++++----------- > 26 files changed, 380 insertions(+), 366 deletions(-) Uhhh... when this matures from RFC to patch-for-inclusion, make sure to split it nicely for me ... please. Oren. _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/containers
