Re: [libvirt] [PATCH RFC 08/22] qemu_process: Persist stderr in qemuProcess struct

On 11/11/2018 08:59 PM, Chris Venteicher wrote: > A qemuProcess struct tracks the entire lifespan of a single QEMU Process > including storing error output when the process terminates or activation > fails. > > Error output remains available until qemuProcessFree is called. > > The qmperr buffer no longer needs to be maintained outside of the > qemuProcess structure because the structure is used for a single QEMU > process and the structures can be maintained as long as required > to retrieve the process error info. > > Capabilities init code is refactored but continues to report QEMU > process error data only when the initial (non KVM) QEMU process activation > fails to result in a usable monitor connection for retrieving > capabilities. > > The VIR_ERR_INTERNAL_ERROR "Failed to probe QEMU binary" reporting is > moved into virQEMUCapsInitQMP function and the stderr string is > extracted from the qemuProcess struct using a macro to retrieve the > string. > > The same error and log message should be generated, in the same > conditions, after this patch as before. > > Signed-off-by: Chris Venteicher <cventeic(a)redhat.com&gt; > --- > src/qemu/qemu_capabilities.c | 27 ++++++++++++--------------- > src/qemu/qemu_process.c | 12 ++++-------- > src/qemu/qemu_process.h | 6 ++++-- > 3 files changed, 20 insertions(+), 25 deletions(-) > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > index a957c3bdbd..f5e327097e 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c

> @@ -4323,15 +4328,8 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch, > goto error; > } > > - if (virQEMUCapsInitQMP(qemuCaps, libDir, runUid, runGid, &qmperr) < 0) { > - virQEMUCapsLogProbeFailure(binary); > - goto error; > - } > - > - if (!qemuCaps->usedQMP) { > - virReportError(VIR_ERR_INTERNAL_ERROR, > - _("Failed to probe QEMU binary with QMP: %s"), > - qmperr ? qmperr : _("unknown error")); > + if (virQEMUCapsInitQMP(qemuCaps, libDir, runUid, runGid) < 0 || > + !qemuCaps->usedQMP) { > virQEMUCapsLogProbeFailure(binary); Oh, this won't fly. So virReportError() sets the error object and virQEMUCapsLogProbeFailure() uses it (by calling virGetLastErrorMessage()). But since you're removing the virReportError() call then there's no error object to get the error message from. IOW this will probably log: "Failed to probe capabilities for /usr/bin/qemu: no error" even though later the actual qemu error message is logged.