21.09.2015 14:56, Michal Privoznik пишет:

On 21.09.2015 13:08, Maxim Nestratov wrote:

...

From: Maxim Nestratov <mnestratov@virtuozzo.com>

As far as not every call of prlsdkUUIDParse assume correct UUID supplied, there is no use to complain about wrong format in it. Otherwise our log is flooded with false error messages. For instance, calling prlsdkUUIDParse from prlsdkEventsHandler works as a filter and in case of uuid absence for event issuer, we simply know that we shouldn't continue further processing. Instead of error logging for all calls we should explicitly take into accaunt where it is called from.

Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_sdk.c | 11 +++++++---- 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 744b58a..32ca1ef 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -329,8 +329,6 @@ prlsdkUUIDParse(const char *uuidstr, unsigned char *uuid)

/* trim curly braces */ if (virUUIDParse(tmp + 1, uuid) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("UUID in config file malformed")); ret = -1; This line is redundant too. I mean, @ret is already initialized to -1.

...

goto error; } @@ -365,8 +363,11 @@ prlsdkGetDomainIds(PRL_HANDLE sdkdom, PrlVmCfg_GetUuid(sdkdom, uuidstr, &len); prlsdkCheckRetGoto(pret, error);

- if (prlsdkUUIDParse(uuidstr, uuid) < 0) + if (prlsdkUUIDParse(uuidstr, uuid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Domain UUID is malformed or empty")); goto error; + }

Hm. This will mask the original error. If, for instance, there's been an OOM in prlsdkUUIDParse(), the error is overwritten with this generic error message. Well, in either cases we don't want to continue anyway ...

...

return 0;

@@ -1724,8 +1725,10 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque) pret = PrlEvent_GetType(prlEvent, &prlEventType); prlsdkCheckRetGoto(pret, cleanup);

- if (prlsdkUUIDParse(uuidstr, uuid) < 0) + if (prlsdkUUIDParse(uuidstr, uuid) < 0) { + VIR_DEBUG("Skipping event type %d", prlEventType); goto cleanup; + }

switch (prlEventType) { case PET_DSP_EVT_VM_STATE_CHANGED:

ACKed with the obvious fix and pushed.

Michal

Thanks!

21.09.2015 14:59, Dmitry Guryanov пишет:

On 09/21/2015 02:08 PM, Maxim Nestratov wrote:

...

From: Maxim Nestratov <mnestratov@virtuozzo.com>

As far as not every call of prlsdkUUIDParse assume correct UUID supplied, there is no use to complain about wrong format in it. Otherwise our log is flooded with false error messages. For instance, calling prlsdkUUIDParse from prlsdkEventsHandler works as a filter and in case of uuid absence for event issuer, we simply know that we shouldn't continue further processing. What does uuid string contain in this case? Maybe it's better to explicitly check for null for example? Usually an empty string. But I can't bet it is always so.

...

Instead of error logging for all calls we should explicitly take into accaunt where it is called from. Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_sdk.c | 11 +++++++---- 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 744b58a..32ca1ef 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -329,8 +329,6 @@ prlsdkUUIDParse(const char *uuidstr, unsigned char *uuid) /* trim curly braces */ if (virUUIDParse(tmp + 1, uuid) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("UUID in config file malformed")); ret = -1; goto error; } @@ -365,8 +363,11 @@ prlsdkGetDomainIds(PRL_HANDLE sdkdom, PrlVmCfg_GetUuid(sdkdom, uuidstr, &len); prlsdkCheckRetGoto(pret, error); - if (prlsdkUUIDParse(uuidstr, uuid) < 0) + if (prlsdkUUIDParse(uuidstr, uuid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Domain UUID is malformed or empty")); goto error; + } return 0; @@ -1724,8 +1725,10 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque) pret = PrlEvent_GetType(prlEvent, &prlEventType); prlsdkCheckRetGoto(pret, cleanup); - if (prlsdkUUIDParse(uuidstr, uuid) < 0) + if (prlsdkUUIDParse(uuidstr, uuid) < 0) { + VIR_DEBUG("Skipping event type %d", prlEventType); goto cleanup; + } switch (prlEventType) { case PET_DSP_EVT_VM_STATE_CHANGED: