[libvirt] [PATCH] daemon: Add early libvirtd start verbose errors.

[Eric Blake]

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 at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org