From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Serge E. Hallyn" Subject: Re: [PATCH 1/1] Syslog are now containerized Date: Thu, 11 Feb 2010 11:48:43 -0600 Message-ID: <20100211174843.GF6884@us.ibm.com> References: <201002110552.o1B5qwbL024561@kernel.safe.ca> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <201002110552.o1B5qwbL024561-X4ZF2iejbABnc3BsFfMrZw@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Jean-Marc Pigeon Cc: Linux Containers List-Id: containers.vger.kernel.org Quoting Jean-Marc Pigeon (jmp-4qkeo2rQ0gg@public.gmane.org): > Added syslog.c such container /proc/kmsg and host /proc/kmsg > do not leak in each other. > Running rsyslog daemon within a container won't destroy > host kernel messages. Thanks Jean-Marc. But this really isn't doing most of what I'd recommended in my last emails (both public and private. In particular: > index cc4f453..3d0c73e 100644 > --- a/include/linux/user_namespace.h > +++ b/include/linux/user_namespace.h > @@ -14,6 +14,7 @@ struct user_namespace { > struct hlist_head uidhash_table[UIDHASH_SZ]; > struct user_struct *creator; > struct work_struct destroyer; > + struct syslog_ns *syslog; syslog_ns should be moved into nsproxy and unshared with a separate clone(CLONE_SYSLOG); > -static void emit_log_char(char c) > +static void emit_log_char(struct syslog_ns *syslog_ns, char c) > { > - LOG_BUF(log_end) = c; > - log_end++; > - if (log_end - log_start > log_buf_len) > - log_start = log_end - log_buf_len; > - if (log_end - con_start > log_buf_len) > - con_start = log_end - log_buf_len; > - if (logged_chars < log_buf_len) > - logged_chars++; > + LOG_BUF(syslog_ns, sys_log_end) = c; Taking syslog_ns from current in emit_log_char is not right. Emit_log_char() is called from printk which (the comment above printk warns us) can be called from any context. That was why I suggested: >! I think my patch is fundamentally wrong anyway: we should not >! use current's context at vprintk like i did. We should use an >! optional passed-in context from those callsites where we want to, >! and default to init otherwise. That means >! >! 1. put the core of vprintk() into vnsprintk() which takes a syslog >! namespace as argument >! >! 2. make vprintk() a wrapper around vnsprintk() which passes in >! init_syslog_ns to vnsprintk(). printk can remain unchanged. >! >! 4. make nsprintk() a wrapper around vnsprintk() which takes a syslog >! ns argument and pass it to vnsprintk() >! >! 3. do_syslog() can obviously be containerized the same way it >! is now. >! >! 4. take a printk call like the iptables ones you want and turn >! int into nsprintk syscall. >! >! The very comment above printk explains why I was an idiot to write >! let alone send that patch. thanks, -serge