On Thu, 16 Feb 2023 14:32:50 +0100
Michal Privoznik <mprivozn@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
> 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
