From mboxrd@z Thu Jan 1 00:00:00 1970 From: Zhigang Wang Subject: Re: [PATCH] pygrub: add syslog support to pygrub Date: Thu, 19 Jul 2012 15:39:23 -0400 Message-ID: <5008626B.3030402@oracle.com> References: <1342724875.18848.70.camel@dagon.hellion.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1342724875.18848.70.camel@dagon.hellion.org.uk> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: xen-devel List-Id: xen-devel@lists.xenproject.org On 07/19/2012 03:07 PM, Ian Campbell wrote: > On Thu, 2012-07-19 at 19:19 +0100, Zhigang Wang wrote: >> # HG changeset patch >> # User Zhigang Wang >> # Date 1342720736 14400 >> # Node ID ec9655b30a5fa5b5abb3e05505f681f9be559613 >> # Parent 43e21ce7f22151524b800a6cf0ac4ba1233b34a7 >> pygrub: add syslog support to pygrub >> >> Currently, when pygrub failed, we don't know the reason because xend/xl will >> not capture the stderr message. > Actual xl in unstable will log to /var/log/xen/bootloader..log > (or something like that) and print a message on failure directing the > user to it. I see. But it seems the design is wrong: you want all logs in stderr. > >> This patch will log the error message to syslog. > I suppose there is no harm in also (optionally?) logging to syslog as > well, but I think we want to keep the messages to stdout also. I think we should: logs to syslog, errors to stderr. > >> Also in this patch: >> >> 1). Fix indentation for some lines. >> 2). Removed some trailing spaces. > These two make up the vast bulk of the changes and really obscure the > actual functional changes. > > Please can you do this cleanup in a separate patch. > >> 3). Mark 'isconfig' a duplicate option of 'debug' and remove the currently broken code: >> >> if isconfig: >> chosencfg = run_grub(file, entry, fs, incfg["args"]) >> >> 'fs' is not defined yet here, so it will raise an exception. > This should also be a separate patch. > > I think all of this is 4.3 material and will therefore have to wait > until 4.2 branches. Please can you resend at that point. > > Ian. I can do it on 4.3. Thanks, Zhigang