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

[Maxim Nestratov]

12-Aug-16 17:08, Mikhail Feoktistov пишет:

>
>
> 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 at virtuozzo.com>
>> ---
>>   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

Actually returning what we get from  PrlJob_GetRetCode  is one the reasons of this patch.
But you are right that we don't have to rewrite it later two lines below.

>
>>           /* 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))) {

there should have been the following:

+        if (PRL_FAILED(retCode = PrlJob_GetError(job, &err_handle))) {

I messed things up while rebasing

>>               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)) {
>