public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: Laine Stump <laine@redhat.com>
To: Stefano Brivio <sbrivio@redhat.com>
Cc: libvir-list@redhat.com, passt-dev@passt.top,
	"Michal Prívozník" <mprivozn@redhat.com>
Subject: Re: [PATCH v2 3/5] qemu_passt: Make passt report errors to stderr whenever possible
Date: Thu, 16 Feb 2023 11:37:39 -0500	[thread overview]
Message-ID: <d8761b70-7cbe-9e57-aa76-fd89455778c5@redhat.com> (raw)
In-Reply-To: <f738a1fd-c9d8-6436-e5a3-c05b0ccd2802@redhat.com>

On 2/16/23 11:27 AM, Michal Prívozník wrote:
> On 2/16/23 17:07, Stefano Brivio wrote:
>> 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.
>>
> 
> Except those patches are not merged yet. And as we are getting close to
> the release I'd like to make this work with what we have now. We've been
> burned plenty of times with QEMU to learn our lesson. We've merged
> patches that were based not on QEMU's git, but some patches on top
> thinking - yeah, the API won't change. And then it did.

The patches are in passt, not QEMU, and Stefano will be cutting a new 
passt release "within a day or two".

> 
> Now we don't require a release (which would be ideal), but at least for
> patches to be merged. When they get merged then yeah, this can be reworked.



  parent reply	other threads:[~2023-02-16 16:37 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-16 13:32 [PATCH v2 0/5] qemu_passt: Fix issues with PID file Michal Privoznik
2023-02-16 13:32 ` [PATCH v2 1/5] qemu_passt: Avoid double daemonizing passt Michal Privoznik
2023-02-16 16:06   ` Stefano Brivio
2023-02-16 16:34   ` Laine Stump
2023-02-16 13:32 ` [PATCH v2 2/5] qemu_passt: Report passt's error on failed start Michal Privoznik
2023-02-16 16:06   ` Stefano Brivio
2023-02-16 16:38   ` Laine Stump
2023-02-16 13:32 ` [PATCH v2 3/5] qemu_passt: Make passt report errors to stderr whenever possible Michal Privoznik
2023-02-16 15:42   ` Jonathon Jongsma
2023-02-16 16:21     ` Michal Prívozník
2023-02-16 22:48       ` David Gibson
2023-02-16 16:07   ` Stefano Brivio
2023-02-16 16:27     ` Michal Prívozník
2023-02-16 16:35       ` Stefano Brivio
2023-02-16 16:37       ` Laine Stump [this message]
2023-02-16 16:33   ` Laine Stump
2023-02-16 13:32 ` [PATCH v2 4/5] qemu_passt: Deduplicate passt killing code Michal Privoznik
2023-02-16 16:07   ` Stefano Brivio
2023-02-16 16:38     ` Michal Prívozník
2023-02-16 17:05       ` Stefano Brivio
2023-02-16 16:42   ` Laine Stump
2023-02-16 13:32 ` [PATCH v2 5/5] qemu_passt: Let passt write the PID file Michal Privoznik
2023-02-16 16:47   ` Laine Stump
2023-02-16 16:35 ` [PATCH v2 0/5] qemu_passt: Fix issues with " Laine Stump
2023-02-17 12:51   ` Michal Prívozník

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d8761b70-7cbe-9e57-aa76-fd89455778c5@redhat.com \
    --to=laine@redhat.com \
    --cc=libvir-list@redhat.com \
    --cc=mprivozn@redhat.com \
    --cc=passt-dev@passt.top \
    --cc=sbrivio@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).