Re: [PATCH v2 3/5] qemu_passt: Make passt report errors to stderr whenever possible

[Stefano Brivio <sbrivio@redhat.com>]

On Thu, 16 Feb 2023 14:32:50 +0100
Michal Privoznik <mprivozn@redhat.com> wrote:

> Passt has '--stderr' argument which makes it report error onto
> stderr rather to system log. Unfortunately, it's currently
> impossible to use both '--log-file' and '--stderr', so pass the
> latter only if the former isn't passed. Then, use the stderr to
> produce more user friendly error message on failed start.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/qemu/qemu_passt.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c
> index c082c149cd..881205449b 100644
> --- a/src/qemu/qemu_passt.c
> +++ b/src/qemu/qemu_passt.c
> @@ -171,8 +171,13 @@ qemuPasstStart(virDomainObj *vm,
>      if (net->sourceDev)
>          virCommandAddArgList(cmd, "--interface", net->sourceDev, NULL);
>  
> -    if (net->backend.logFile)
> +    if (net->backend.logFile) {
>          virCommandAddArgList(cmd, "--log-file", net->backend.logFile, NULL);
> +    } else {
> +        /* By default, passt logs into system logger. But we are interested
> +         * into errors too. Make it print errors onto stderr. */
> +        virCommandAddArg(cmd, "--stderr");
> +    }

There's no need for this, see my previous email, archived at:
  https://listman.redhat.com/archives/libvir-list/2023-February/237880.html

>  
>      /* Add IP address info */
>      for (i = 0; i < net->guestIP.nips; i++) {
> @@ -265,8 +270,19 @@ qemuPasstStart(virDomainObj *vm,
>          goto error;
>  
>      if (cmdret < 0 || exitstatus != 0) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       _("Could not start 'passt': %s"), NULLSTR(errbuf));
> +        if (STRNEQ_NULLABLE(errbuf, "")) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Could not start 'passt': %s"), errbuf);
> +        } else if (net->backend.logFile) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Could not start 'passt': look into %s for error"),
> +                           net->backend.logFile);

...and this won't be needed either, with Laine's series for passt. It
might actually be a bit misleading.

> +        } else {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Could not start 'passt': exit status = '%d'"),
> +                           exitstatus);
> +        }
> +
>          goto error;
>      }
>  

So all in all I think this is unnecessary, but also kind of harmless.

-- 
Stefano