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. 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.