[PATCH v2 3/5] qemu_passt: Make passt report errors to stderr whenever possible
Laine Stump
laine at redhat.com
Thu Feb 16 16:37:39 UTC 2023
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 at 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 at 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.
More information about the libvir-list
mailing list