On 08/11/2011 06:18 AM, Peter Krempa wrote:
Early errors during start of libvirtd didn't have
an error reporting mechanism and caused libvirtd
to exit silently (only the return value indicated
an error). This patch adds error messages printed
to stderr if verbose parameter is specified to the
daemon.
fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=728654
---
daemon/libvirtd.c | 40 ++++++++++++++++++++++++++++++++--------
1 files changed, 32 insertions(+), 8 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 53f1002..c3867af 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -1328,14 +1328,20 @@ int main(int argc, char **argv) {
case 'p':
VIR_FREE(pid_file);
- if (!(pid_file = strdup(optarg)))
+ if (!(pid_file = strdup(optarg))) {
+ if (verbose)
+ fprintf(stderr, _("ERROR: Can't allocate memory\n"));
exit(EXIT_FAILURE);
+ }
break;
As written, if you specify 'libvirtd --pid-file xyz --verbose', you
still get no message; you have to specify 'libvirtd --verbose --pid-file
xyz' for this to be useful.
But I think that in general with daemon programs, it is permissible to
write to stderr without needing --verbose in the case of an early exit.
That is, if the daemon can't even start, then it should output why; a
silent early exit is never nice. I think these messages should be
unconditional, and that --verbose should be reserved for conditionally
documenting that we are about to attempt steps that normally succeed,
rather than for conditionally diagnosing steps that have already failed.
- if (daemonSetupLogging(config, privileged, verbose,
godaemon)< 0)
+ if (daemonSetupLogging(config, privileged, verbose, godaemon)< 0) {
+ if (verbose)
+ fprintf(stderr, _("ERROR: Can't initialise logging\n"));
s/initialise/initialize/ - translated messages should be US spelling,
and leave en_UK.po as the source of UK spellings visible to the user.
exit(EXIT_FAILURE);
+ }
+
+ /* error logging is up, use libvirt's error logging from now */
if (!pid_file&& privileged&&
daemonPidFilePath(privileged,
-&pid_file)< 0)
+&pid_file)< 0) {
+ VIR_ERROR(_("Can't determine pid file path."));
exit(EXIT_FAILURE);
+ }
This hunk and below are appropriate - once we are far enough along to
use logging, then it is better to use logging than stderr before calling
an early exit().
I think fixing the first half of the patch to not depend on --verbose
probably warrants a v2.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library
http://libvirt.org