[libvirt] [PATCH v2] vz: get additional error information from job correctly
Maxim Nestratov
mnestratov at virtuozzo.com
Tue Jun 14 09:39:51 UTC 2016
14.06.2016 10:43, 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.
Ok.
>> 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
why? something failed while we were trying to get additional information
and we honestly log what and where exactly
>> + logPrlError(retCode);
>> + goto cleanup;
> err_handle leak
thanks, will fix it.
>
>> + }
>> +
>> + logPrlEventErrorHelper(err_handle, filename, funcname, linenr);
>>
>> PrlHandle_Free(err_handle);
>> - ret = retCode;
> retCode of successful PrlEvent_GetErrCode is lost
Actually we do it intentionally and did it before the patch.
The first version stated incorrectly that we have to return this
additional error code.
>
>> } 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.
This is what this function for, as far as I understand its author, as
the intention of this helper is to get a track of original failure,
which is perfectly done. It logs some additional stuff now, when another
error code is returned.
More information about the libvir-list
mailing list