On Mon, Nov 12, 2018 at 16:53:34 +0100, Michal Privoznik wrote:
> 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@xxxxxxxxxx>
> > ---
> > 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.
Not really. The virReportError is still there, just moved inside
virQEMUCapsInitQMP. No issue to see here I believe.
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
