Re: [libvirt] [PATCH 2/3] Improve LXC startup error reporting

On 03/07/13 16:58, Daniel P. Berrange wrote: >On Thu, Mar 07, 2013 at 04:40:05PM +0100, Peter Krempa wrote: >>On 03/06/13 17:16, Daniel P. Berrange wrote: >>>From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; >>> >>>Currently we rely on a VIR_ERROR message being logged by the >>>virRaiseError function to report LXC startup errors. This gives >>>the right message, but is rather ugly and can be truncated >>>if lots of log messages are written. Change the LXC controller >>>to explicitly print any virErrorPtr message to stderr. Then >>>change the driver to skip over anything that looks like a log >>>message. >>> >>>The result is that this >>> >>>error: Failed to start domain busy >>>error: internal error guest failed to start: 2013-03-04 19:46:42.846+0000: 1734: info : libvirt version: 1.0.2 >>>2013-03-04 19:46:42.846+0000: 1734: error : virFileLoopDeviceAssociate:600 : Unable to open /root/disk.raw: No such file or directory >>> >>>changes to >>> >>>error: Failed to start domain busy >>>error: internal error guest failed to start: Unable to open /root/disk.raw: No such file or directory >>>Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; >>>--- >>> src/lxc/lxc_controller.c | 7 ++- >>> src/lxc/lxc_process.c | 130 ++++++++++++++++++++++++++++++++--------------- >>> 2 files changed, 94 insertions(+), 43 deletions(-) >>> >>>diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c >>>index 15aa334..78e8a70 100644 >>>--- a/src/lxc/lxc_controller.c >>>+++ b/src/lxc/lxc_controller.c >>>@@ -1706,7 +1706,6 @@ int main(int argc, char *argv[]) >>> rc = virLXCControllerRun(ctrl); >>> >>> cleanup: >>>- virPidFileDelete(LXC_STATE_DIR, name); >> >> From the code later on, it doesn't seem you want retain the pid >>file. If you do so, errors won't be reported later on ... [1] > > >>>+ >>> /** >>> * virLXCProcessStart: >>> * @conn: pointer to connection >>>@@ -1124,9 +1164,15 @@ int virLXCProcessStart(virConnectPtr conn, >>> >>> /* And get its pid */ >>> if ((r = virPidFileRead(driver->stateDir, vm->def->name, &vm->pid)) < 0) { >> >>[1] ... here. > >This handles the case where libvirt_lxc failed to startup at all, >so never wrote the pid file. There is aready code later in this >method which handles the case where libvirt_lxc started, but then >quit early, which still reads the log output. > Ah, okay then. So ACK to the patch if you move the output of the error message in lxc_controller.c right after the cleanup label.