[libvirt] [PATCH v2] vz: get additional error information from job correctly

[Nikolay Shirokovskiy]

On 11.06.2016 20:16, Maxim Nestratov wrote:
> Function logPrlEventErrorHelper is made void and now it only prints
> an information (if any) from event.
> If there is no addition information (i.e. an error returned when
> asked), we don't rewrite original error code with secondary one.

I would mention the original motivation of the patch, that is returning
error code of PrlEvent_GetErrCode. And motivation of checking for 
PRL_ERR_NO_DATA too.

> 
> Signed-off-by: Maxim Nestratov <mnestratov at virtuozzo.com>
> ---
>  src/vz/vz_sdk.c | 33 ++++++++++++++++-----------------
>  1 file changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index 99c5d4a..54c56e9 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -99,19 +99,13 @@ 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);
>  
> @@ -130,13 +124,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
> @@ -146,12 +136,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;
>      }
> @@ -159,17 +149,26 @@ getJobResultHelper(PRL_HANDLE job, unsigned int timeout, PRL_HANDLE *result,
>      if (retCode) {
>          PRL_HANDLE err_handle;
>  
> +        ret = retCode;
> +
>          /* Sometimes it's possible to get additional error info. */
> -        if ((ret = PrlJob_GetError(job, &err_handle))) {
> +        if (PRL_FAILED(retCode = PrlJob_GetError(job, &err_handle))) {
>              logPrlErrorHelper(ret, filename, funcname, linenr);
> +            if (PRL_ERR_NO_DATA != retCode)
> +                logPrlError(retCode);
>              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)

looks like this error code does not matter in case of PrlEvent_GetErrCode

> +                logPrlError(retCode);
> +            goto cleanup;

err_handle leak

> +        }
> +
> +        logPrlEventErrorHelper(err_handle, filename, funcname, linenr);
>  
>          PrlHandle_Free(err_handle);
> -        ret = retCode;

retCode of successful PrlEvent_GetErrCode is lost

>      } else {
>          ret = PrlJob_GetResult(job, result);
>          if (PRL_FAILED(ret)) {
> 


Even after this patch getJobResultHelper will not be good. If some error occurs we have no simple 
way to find what function fails: 

PrlJob_Wait
PrlJob_GetRetCode
PrlJob_GetError
PrlEvent_GetErrCode
PrlJob_GetResult

as they all will print: logPrlErrorHelper(ret, filename, funcname, linenr) with funcname and
linenr of the caller of getJobResultHelper.