Re: [libvirt] [PATCH 2/6] vz: get additional error information from job correctly

On 10.08.2016 20:44, Maxim Nestratov wrote: > First, make function logPrlEventErrorHelper be void and only > print information (if any) from an event. > Second, don't rewrite original error with any errors we get > during parsing event info. > Third, ignore PRL_ERR_NO_DATA at all. > > Signed-off-by: Maxim Nestratov <mnestratov(a)virtuozzo.com&gt; > --- > src/vz/vz_sdk.c | 32 ++++++++++++++------------------ > 1 file changed, 14 insertions(+), 18 deletions(-) > > diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c > index f81b320..45cac65 100644 > --- a/src/vz/vz_sdk.c > +++ b/src/vz/vz_sdk.c > @@ -105,19 +105,12 @@ logPrlErrorHelper(PRL_RESULT err, const char *filename, > } \ > } while (0) > -static PRL_RESULT > +static void > logPrlEventErrorHelper(PRL_HANDLE event, const char *filename, > const char *funcname, size_t linenr) > { > - PRL_RESULT ret, retCode; > char *msg1 = NULL, *msg2 = NULL; > PRL_UINT32 len = 0; > - int err = -1; > - > - if ((ret = PrlEvent_GetErrCode(event, &retCode))) { > - logPrlError(ret); > - return ret; > - } > PrlEvent_GetErrString(event, PRL_TRUE, PRL_FALSE, NULL, &len); > @@ -136,13 +129,9 @@ logPrlEventErrorHelper(PRL_HANDLE event, const char *filename, > virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INTERNAL_ERROR, > filename, funcname, linenr, > _("%s %s"), msg1, msg2); > - err = 0; > - > cleanup: > VIR_FREE(msg1); > VIR_FREE(msg2); > - > - return err; > } > static PRL_RESULT > @@ -152,12 +141,12 @@ getJobResultHelper(PRL_HANDLE job, unsigned int timeout, PRL_HANDLE *result, > { > PRL_RESULT ret, retCode; > - if ((ret = PrlJob_Wait(job, timeout))) { > + if (PRL_FAILED(ret = PrlJob_Wait(job, timeout))) { > logPrlErrorHelper(ret, filename, funcname, linenr); > goto cleanup; > } > - if ((ret = PrlJob_GetRetCode(job, &retCode))) { > + if (PRL_FAILED(ret = PrlJob_GetRetCode(job, &retCode))) { > logPrlErrorHelper(ret, filename, funcname, linenr); > goto cleanup; > } > @@ -165,17 +154,24 @@ getJobResultHelper(PRL_HANDLE job, unsigned int timeout, PRL_HANDLE *result, > if (retCode) { > PRL_HANDLE err_handle; > + ret = retCode; > + I think we don't need to store retCode to ret

> /* Sometimes it's possible to get additional error info. */ > - if ((ret = PrlJob_GetError(job, &err_handle))) { > + if (PRL_FAILED(ret = PrlJob_GetError(job, &err_handle))) {

> logPrlErrorHelper(ret, filename, funcname, linenr); > goto cleanup; > } > - if (logPrlEventErrorHelper(err_handle, filename, funcname, linenr)) > - logPrlErrorHelper(retCode, filename, funcname, linenr); > + if (PRL_FAILED(retCode = PrlEvent_GetErrCode(err_handle, &retCode))) { > + logPrlErrorHelper(ret, filename, funcname, linenr); > + if (PRL_ERR_NO_DATA != retCode) > + logPrlError(retCode); > + goto cleanup; > + } > + > + logPrlEventErrorHelper(err_handle, filename, funcname, linenr); > PrlHandle_Free(err_handle); > - ret = retCode; > } else { > ret = PrlJob_GetResult(job, result); > if (PRL_FAILED(ret)) {