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@redhat.com>
---
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 = NULL;
qemuProcessPtr proc_kvm = NULL;
@@ -4226,7 +4225,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
bool forceTCG = false;
if (!(proc = qemuProcessNew(qemuCaps->binary, libDir,
- runUid, runGid, qmperr, forceTCG)))
+ runUid, runGid, forceTCG)))
goto cleanup;
if ((rc = qemuProcessRun(proc)) != 0) {
@@ -4242,7 +4241,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
qemuProcessStopQmp(proc);
forceTCG = true;
- proc_kvm = qemuProcessNew(qemuCaps->binary, libDir, runUid, runGid, NULL, forceTCG);
+ proc_kvm = qemuProcessNew(qemuCaps->binary, libDir, runUid, runGid, forceTCG);
if ((rc = qemuProcessRun(proc_kvm)) != 0) {
if (rc == 1)
@@ -4257,6 +4256,13 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
ret = 0;
cleanup:
+ if (proc && !proc->mon) {
+ char *err = 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 = NULL;
if (!(qemuCaps = virQEMUCapsNew()))
goto error;
@@ -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);
goto error;
}
@@ -4350,7 +4348,6 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
}
cleanup:
- VIR_FREE(qmperr);
return qemuCaps;
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);
}
@@ -8100,7 +8101,6 @@ qemuProcessNew(const char *binary,
const char *libDir,
uid_t runUid,
gid_t runGid,
- char **qmperr,
bool forceTCG)
{
qemuProcessPtr proc = NULL;
@@ -8115,8 +8115,6 @@ qemuProcessNew(const char *binary,
proc->runUid = runUid;
proc->runGid = runGid;
- proc->qmperr = qmperr;
-
/* 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 = NULL;
const char *machine;
int status = 0;
@@ -8192,7 +8189,7 @@ qemuProcessRun(qemuProcessPtr proc)
virCommandSetGID(proc->cmd, proc->runGid);
virCommandSetUID(proc->cmd, proc->runUid);
- virCommandSetErrorBuffer(proc->cmd, proc->qmperr);
+ virCommandSetErrorBuffer(proc->cmd, &(proc->qmperr));
/* Log, but otherwise ignore, non-zero status. */
if (virCommandRun(proc->cmd, &status) < 0)
@@ -8200,7 +8197,7 @@ qemuProcessRun(qemuProcessPtr proc)
if (status != 0) {
VIR_DEBUG("QEMU %s exited with status %d: %s",
- proc->binary, status, *proc->qmperr);
+ proc->binary, status, proc->qmperr);
goto ignore;
}
@@ -8262,7 +8259,6 @@ qemuProcessStopQmp(qemuProcessPtr proc)
(long long)proc->pid,
virStrerror(errno, ebuf, sizeof(ebuf)));
- 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;
};
+#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);
void qemuProcessFree(qemuProcessPtr proc);
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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@redhat.com>
> ---
> 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 = NULL;
> qemuProcessPtr proc_kvm = NULL;
> @@ -4226,7 +4225,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
> bool forceTCG = false;
>
> if (!(proc = qemuProcessNew(qemuCaps->binary, libDir,
> - runUid, runGid, qmperr, forceTCG)))
> + runUid, runGid, forceTCG)))
> goto cleanup;
>
> if ((rc = qemuProcessRun(proc)) != 0) {
> @@ -4242,7 +4241,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
> qemuProcessStopQmp(proc);
>
> forceTCG = true;
> - proc_kvm = qemuProcessNew(qemuCaps->binary, libDir, runUid, runGid, NULL, forceTCG);
> + proc_kvm = qemuProcessNew(qemuCaps->binary, libDir, runUid, runGid, forceTCG);
>
> if ((rc = qemuProcessRun(proc_kvm)) != 0) {
> if (rc == 1)
> @@ -4257,6 +4256,13 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
> ret = 0;
>
> cleanup:
> + if (proc && !proc->mon) {
> + char *err = QEMU_PROCESS_ERROR(proc) ? QEMU_PROCESS_ERROR(proc) : _("unknown error");
or just:
char *err = proc->qmperr;
if (!err)
err = _("uknown 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 = NULL;
>
> if (!(qemuCaps = virQEMUCapsNew()))
> goto error;
> @@ -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.
Up until now, patches 01-07 look reasonable.
> goto error;
> }
> @@ -4350,7 +4348,6 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
> }
>
> cleanup:
> - VIR_FREE(qmperr);
> return qemuCaps;
>
> 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);
> }
>
> @@ -8100,7 +8101,6 @@ qemuProcessNew(const char *binary,
> const char *libDir,
> uid_t runUid,
> gid_t runGid,
> - char **qmperr,
> bool forceTCG)
> {
> qemuProcessPtr proc = NULL;
> @@ -8115,8 +8115,6 @@ qemuProcessNew(const char *binary,
>
> proc->runUid = runUid;
> proc->runGid = runGid;
> - proc->qmperr = qmperr;
> -
>
> /* 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){
Ooops, this is not right ;-)
> virDomainXMLOptionPtr xmlopt = NULL;
> const char *machine;
> int status = 0;
> @@ -8192,7 +8189,7 @@ qemuProcessRun(qemuProcessPtr proc)
> virCommandSetGID(proc->cmd, proc->runGid);
> virCommandSetUID(proc->cmd, proc->runUid);
>
> - virCommandSetErrorBuffer(proc->cmd, proc->qmperr);
> + virCommandSetErrorBuffer(proc->cmd, &(proc->qmperr));
>
> /* Log, but otherwise ignore, non-zero status. */
> if (virCommandRun(proc->cmd, &status) < 0)
> @@ -8200,7 +8197,7 @@ qemuProcessRun(qemuProcessPtr proc)
>
> if (status != 0) {
> VIR_DEBUG("QEMU %s exited with status %d: %s",
> - proc->binary, status, *proc->qmperr);
> + proc->binary, status, proc->qmperr);
> goto ignore;
> }
>
> @@ -8262,7 +8259,6 @@ qemuProcessStopQmp(qemuProcessPtr proc)
> (long long)proc->pid,
> virStrerror(errno, ebuf, sizeof(ebuf)));
>
> - 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;
> };
>
> +#define QEMU_PROCESS_ERROR(proc) \
> + ((char *) (proc ? proc->qmperr: NULL))
> +
I don't think we need this macro. BTW, if you install 'cppi' you'll
notice that there needs to be a space afer # and before define.
> qemuProcessPtr qemuProcessNew(const char *binary,
> const char *libDir,
> uid_t runUid,
> gid_t runGid,
> - char **qmperr,
> bool forceTCG);
>
> void qemuProcessFree(qemuProcessPtr proc);
>
Michal
P.S. this is where I stop my review for today. Have some errands to run,
will resume review tomorrow.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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@redhat.com>
> > ---
> > 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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Sun, Nov 11, 2018 at 13:59:16 -0600, 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
As explained for the previous patch the initial QEMU process actually
checks KVM if available.
> 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@redhat.com>
> ---
> 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)
Yes, hiding qmperror inside virQEMUCapsInitQMP (actually inside the new
function which will be introduced once virQEMUCapsInitQMP is refactored
as I suggested in my review for the previous patch) is definitely a good
idea.
> {
> qemuProcessPtr proc = NULL;
> qemuProcessPtr proc_kvm = NULL;
> @@ -4226,7 +4225,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
> bool forceTCG = false;
>
> if (!(proc = qemuProcessNew(qemuCaps->binary, libDir,
> - runUid, runGid, qmperr, forceTCG)))
> + runUid, runGid, forceTCG)))
> goto cleanup;
The new function can just return directly from here...
>
> if ((rc = qemuProcessRun(proc)) != 0) {
> @@ -4242,7 +4241,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
> qemuProcessStopQmp(proc);
>
> forceTCG = true;
> - proc_kvm = qemuProcessNew(qemuCaps->binary, libDir, runUid, runGid, NULL, forceTCG);
> + proc_kvm = qemuProcessNew(qemuCaps->binary, libDir, runUid, runGid, forceTCG);
>
> if ((rc = qemuProcessRun(proc_kvm)) != 0) {
> if (rc == 1)
> @@ -4257,6 +4256,13 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
> ret = 0;
>
> cleanup:
> + if (proc && !proc->mon) {
... which means you wouldn't need to check for proc at this point.
> + char *err = QEMU_PROCESS_ERROR(proc) ? QEMU_PROCESS_ERROR(proc) : _("unknown error");
But even now you don't need to check for proc inside the macro. Which
makes the main (though still very tiny) reason for introducing the
macro. In other words, just drop the macro and use proc->qmperr
directly.
> +
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to probe QEMU binary with QMP: %s"), err);
> + }
And this would report an error even if ret == 0.
> +
> qemuProcessStopQmp(proc);
> qemuProcessStopQmp(proc_kvm);
> qemuProcessFree(proc);
> @@ -4296,7 +4302,6 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
> {
> virQEMUCapsPtr qemuCaps;
> struct stat sb;
> - char *qmperr = NULL;
>
> if (!(qemuCaps = virQEMUCapsNew()))
> goto error;
> @@ -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) {
There should be no need to keep the !qemuCaps->usedQMP check after this
refactoring. See my notes to the next patch for more details.
> virQEMUCapsLogProbeFailure(binary);
> goto error;
> }
> @@ -4350,7 +4348,6 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
> }
>
> cleanup:
> - VIR_FREE(qmperr);
> return qemuCaps;
>
> error:
...
> 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 */
You could even make the vairable name self-explanatory:
char *stderr;
> char *monarg;
> char *monpath;
> char *pidfile;
> @@ -232,11 +232,13 @@ struct _qemuProcess {
> bool forceTCG;
> };
>
> +#define QEMU_PROCESS_ERROR(proc) \
> + ((char *) (proc ? proc->qmperr: NULL))
> +
Just drop this.
> qemuProcessPtr qemuProcessNew(const char *binary,
> const char *libDir,
> uid_t runUid,
> gid_t runGid,
> - char **qmperr,
> bool forceTCG);
>
> void qemuProcessFree(qemuProcessPtr proc);
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.