On Thu, 16 Feb 2023 14:32:50 +0100
Michal Privoznik <mprivozn(a)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(a)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.