From nobody Wed Nov 27 18:42:30 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1541966557774101.79106063464565; Sun, 11 Nov 2018 12:02:37 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id EB626C0A287C; Sun, 11 Nov 2018 20:02:35 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7E15D1054FD6; Sun, 11 Nov 2018 20:02:35 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id EDE70181A133; Sun, 11 Nov 2018 20:02:34 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id wABK023b023716 for ; Sun, 11 Nov 2018 15:00:02 -0500 Received: by smtp.corp.redhat.com (Postfix) id 0668B5C238; Sun, 11 Nov 2018 20:00:02 +0000 (UTC) Received: from cv1.redhat.com (ovpn-121-41.rdu2.redhat.com [10.10.121.41]) by smtp.corp.redhat.com (Postfix) with ESMTP id 679A75C1B4; Sun, 11 Nov 2018 19:59:54 +0000 (UTC) From: Chris Venteicher To: libvir-list@redhat.com Date: Sun, 11 Nov 2018 13:59:16 -0600 Message-Id: <20181111195930.17185-9-cventeic@redhat.com> In-Reply-To: <20181111195930.17185-1-cventeic@redhat.com> References: <20181111195930.17185-1-cventeic@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-loop: libvir-list@redhat.com Cc: walling@linux.ibm.com, Chris Venteicher , jdenemar@redhat.com, david@redhat.com Subject: [libvirt] [PATCH RFC 08/22] qemu_process: Persist stderr in qemuProcess struct X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Sun, 11 Nov 2018 20:02:36 +0000 (UTC) Content-Type: text/plain; charset="utf-8" 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 --- 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 @@ -4216,8 +4216,7 @@ static int virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps, const char *libDir, uid_t runUid, - gid_t runGid, - char **qmperr) + gid_t runGid) { qemuProcessPtr proc =3D NULL; qemuProcessPtr proc_kvm =3D NULL; @@ -4226,7 +4225,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps, bool forceTCG =3D false; =20 if (!(proc =3D qemuProcessNew(qemuCaps->binary, libDir, - runUid, runGid, qmperr, forceTCG))) + runUid, runGid, forceTCG))) goto cleanup; =20 if ((rc =3D qemuProcessRun(proc)) !=3D 0) { @@ -4242,7 +4241,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps, qemuProcessStopQmp(proc); =20 forceTCG =3D true; - proc_kvm =3D qemuProcessNew(qemuCaps->binary, libDir, runUid, runG= id, NULL, forceTCG); + proc_kvm =3D qemuProcessNew(qemuCaps->binary, libDir, runUid, runG= id, forceTCG); =20 if ((rc =3D qemuProcessRun(proc_kvm)) !=3D 0) { if (rc =3D=3D 1) @@ -4257,6 +4256,13 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps, ret =3D 0; =20 cleanup: + if (proc && !proc->mon) { + char *err =3D QEMU_PROCESS_ERROR(proc) ? QEMU_PROCESS_ERROR(proc) = : _("unknown error"); + + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to probe QEMU binary with QMP: %s"), err); + } + qemuProcessStopQmp(proc); qemuProcessStopQmp(proc_kvm); qemuProcessFree(proc); @@ -4296,7 +4302,6 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch, { virQEMUCapsPtr qemuCaps; struct stat sb; - char *qmperr =3D NULL; =20 if (!(qemuCaps =3D virQEMUCapsNew())) goto error; @@ -4323,15 +4328,8 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch, goto error; } =20 - 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); goto error; } @@ -4350,7 +4348,6 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch, } =20 cleanup: - VIR_FREE(qmperr); return qemuCaps; =20 error: diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index dda74d5b7a..a741d1cf91 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8091,6 +8091,7 @@ qemuProcessFree(qemuProcessPtr proc) VIR_FREE(proc->monpath); VIR_FREE(proc->monarg); VIR_FREE(proc->pidfile); + VIR_FREE(proc->qmperr); VIR_FREE(proc); } =20 @@ -8100,7 +8101,6 @@ qemuProcessNew(const char *binary, const char *libDir, uid_t runUid, gid_t runGid, - char **qmperr, bool forceTCG) { qemuProcessPtr proc =3D NULL; @@ -8115,8 +8115,6 @@ qemuProcessNew(const char *binary, =20 proc->runUid =3D runUid; proc->runGid =3D runGid; - proc->qmperr =3D qmperr; - =20 /* the ".sock" sufix is important to avoid a possible clash with a qemu * domain called "capabilities" @@ -8155,8 +8153,7 @@ qemuProcessNew(const char *binary, * 1 when probing QEMU failed */ int -qemuProcessRun(qemuProcessPtr proc) -{ +qemuProcessRun(qemuProcessPtr proc){ virDomainXMLOptionPtr xmlopt =3D NULL; const char *machine; int status =3D 0; @@ -8192,7 +8189,7 @@ qemuProcessRun(qemuProcessPtr proc) virCommandSetGID(proc->cmd, proc->runGid); virCommandSetUID(proc->cmd, proc->runUid); =20 - virCommandSetErrorBuffer(proc->cmd, proc->qmperr); + virCommandSetErrorBuffer(proc->cmd, &(proc->qmperr)); =20 /* Log, but otherwise ignore, non-zero status. */ if (virCommandRun(proc->cmd, &status) < 0) @@ -8200,7 +8197,7 @@ qemuProcessRun(qemuProcessPtr proc) =20 if (status !=3D 0) { VIR_DEBUG("QEMU %s exited with status %d: %s", - proc->binary, status, *proc->qmperr); + proc->binary, status, proc->qmperr); goto ignore; } =20 @@ -8262,7 +8259,6 @@ qemuProcessStopQmp(qemuProcessPtr proc) (long long)proc->pid, virStrerror(errno, ebuf, sizeof(ebuf))); =20 - VIR_FREE(*proc->qmperr); } if (proc->pidfile) unlink(proc->pidfile); diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index ab2640ce7c..abd80baf5c 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -220,7 +220,7 @@ struct _qemuProcess { char *binary; uid_t runUid; gid_t runGid; - char **qmperr; + char *qmperr; /* qemu process stderr */ char *monarg; char *monpath; char *pidfile; @@ -232,11 +232,13 @@ struct _qemuProcess { bool forceTCG; }; =20 +#define QEMU_PROCESS_ERROR(proc) \ + ((char *) (proc ? proc->qmperr: NULL)) + qemuProcessPtr qemuProcessNew(const char *binary, const char *libDir, uid_t runUid, gid_t runGid, - char **qmperr, bool forceTCG); =20 void qemuProcessFree(qemuProcessPtr proc); --=20 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list