From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTP id 97B4A5A0082 for ; Thu, 16 Feb 2023 17:27:19 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1676564838; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=opU58wyjh+ibYJvVM1bvNpA/8yHVvxC3PTkMyfUW3/Q=; b=G7GerfjEdCYGkp//kF/XDLIxkAzRfUVTiqNJD8k0J6kbr9wK/ZVSirD/BWUXzTapBWaRAR /g6ndYjU/N0A4aTsOgU6e+q3xZi5djZ0anq1uhnL55R008OGwKnUxMH78tUOMtvywQhNrl bQKKOZNjwgR3UNS1RZ1a5orqHuQQFK4= Received: from mail-ed1-f69.google.com (mail-ed1-f69.google.com [209.85.208.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-452-PZZeJVepPyuUME7mODM9hw-1; Thu, 16 Feb 2023 11:27:14 -0500 X-MC-Unique: PZZeJVepPyuUME7mODM9hw-1 Received: by mail-ed1-f69.google.com with SMTP id ev18-20020a056402541200b004a621e993a8so2262042edb.13 for ; Thu, 16 Feb 2023 08:27:14 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=opU58wyjh+ibYJvVM1bvNpA/8yHVvxC3PTkMyfUW3/Q=; b=sICwjVBDacK74JnV/BfLSXx47NUB6TDHGK+yk25EnoX3PprX6K9YwpGvX2i6W8UkU5 Sfjr8bWLJwqw8vCNuCo937RvffO0Z/8ddyx9yscjuAGwcyRfA+dm3Rv0XV8Ek3YMS0af ouZazg0yb4OPjctJun+oO5N5bWy+7UOFPyRgSztHkLllFGO7i778w5KKU54cF1USHTpp JZHtmMACrVIYOQK9teKhORIe/0gkvfTf56AKI4RIv4t//SqpBA4KigIar4jj6ntu3FIS /peGYf0eCve/Q+VBLfPY8Ebf1++Rln/XLXOLsIaETUSw7QFHomQn3x/wAP0SxvgWrOTn i+qQ== X-Gm-Message-State: AO0yUKVewAIT7QxylH31dTgbiNvDEaILTKweyqiw04asjiXfK6OJG+u8 pc9TmwAFPPLITMXQeZMUjaY8ddkB62VDoDCyB+nwlgCDxuA9Y5tI1eF1KB+ZPYSP0OFeePKIs39 y6c61p0mhEK5otlfZzA== X-Received: by 2002:a50:ee19:0:b0:4ac:b6db:3ed0 with SMTP id g25-20020a50ee19000000b004acb6db3ed0mr5680076eds.39.1676564832809; Thu, 16 Feb 2023 08:27:12 -0800 (PST) X-Google-Smtp-Source: AK7set8NWwJMG6uqsj0WhW40CmG9JKvuppPC77DLlMM4RwqSH2MvADlLHU9eeEPuSun/AWSEAVpRZg== X-Received: by 2002:a50:ee19:0:b0:4ac:b6db:3ed0 with SMTP id g25-20020a50ee19000000b004acb6db3ed0mr5680063eds.39.1676564832553; Thu, 16 Feb 2023 08:27:12 -0800 (PST) Received: from [10.43.2.39] (nat-pool-brq-t.redhat.com. [213.175.37.10]) by smtp.gmail.com with ESMTPSA id s18-20020a170906355200b008b14cfef83asm984940eja.199.2023.02.16.08.27.12 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 16 Feb 2023 08:27:12 -0800 (PST) Message-ID: Date: Thu, 16 Feb 2023 17:27:11 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 Subject: Re: [PATCH v2 3/5] qemu_passt: Make passt report errors to stderr whenever possible To: Stefano Brivio References: <20230216170702.3ad76aeb@elisabeth> From: =?UTF-8?B?TWljaGFsIFByw612b3puw61r?= In-Reply-To: <20230216170702.3ad76aeb@elisabeth> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-MailFrom: mprivozn@redhat.com X-Mailman-Rule-Hits: nonmember-moderation X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation Message-ID-Hash: G6TP6FPHDMPV7ITVBVWRCTQJTM43L2PS X-Message-ID-Hash: G6TP6FPHDMPV7ITVBVWRCTQJTM43L2PS X-Mailman-Approved-At: Thu, 16 Feb 2023 18:44:41 +0100 CC: libvir-list@redhat.com, passt-dev@passt.top X-Mailman-Version: 3.3.3 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 2/16/23 17:07, Stefano Brivio wrote: > On Thu, 16 Feb 2023 14:32:50 +0100 > Michal Privoznik 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 >> --- >> 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. 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. Michal