5:14 a.m.
Maxim Nestratov (4):
vz: remove unused macro logPrlEventError
vz: return event error code in logPrlEventErrorHelper and use it
vz: fix getJobResultHelper
vz: return correct result for unimplemented ChangeState actions
src/vz/vz_sdk.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
--
1.8.3.1
5:14 a.m.
New subject: [libvirt] [PATCH 1/4] vz: remove unused macro logPrlEventError
Signed-off-by: Maxim Nestratov <mnestratov(a)virtuozzo.com>
---
src/vz/vz_sdk.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 7eb78ca..e5c56a5 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -138,10 +138,6 @@ logPrlEventErrorHelper(PRL_HANDLE event, const char *filename,
return err;
}
-#define logPrlEventError(event) \
- logPrlEventErrorHelper(event, __FILE__, \
- __FUNCTION__, __LINE__)
-
static PRL_RESULT
getJobResultHelper(PRL_HANDLE job, unsigned int timeout, PRL_HANDLE *result,
const char *filename, const char *funcname,
--
1.8.3.1
3:39 a.m.
New subject: [libvirt] [PATCH 1/4] vz: remove unused macro logPrlEventError
On 26.05.2016 13:14, Maxim Nestratov wrote:
Signed-off-by: Maxim Nestratov <mnestratov(a)virtuozzo.com>
---
src/vz/vz_sdk.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 7eb78ca..e5c56a5 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -138,10 +138,6 @@ logPrlEventErrorHelper(PRL_HANDLE event, const char *filename,
return err;
}
-#define logPrlEventError(event) \
- logPrlEventErrorHelper(event, __FILE__, \
- __FUNCTION__, __LINE__)
-
static PRL_RESULT
getJobResultHelper(PRL_HANDLE job, unsigned int timeout, PRL_HANDLE *result,
const char *filename, const char *funcname,
ACK
10:23 a.m.
New subject: [libvirt] [PATCH 1/4] vz: remove unused macro logPrlEventError
27.05.2016 11:39, Nikolay Shirokovskiy пишет:
On 26.05.2016 13:14, Maxim Nestratov wrote:
> Signed-off-by: Maxim Nestratov <mnestratov(a)virtuozzo.com>
> ---
> src/vz/vz_sdk.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index 7eb78ca..e5c56a5 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -138,10 +138,6 @@ logPrlEventErrorHelper(PRL_HANDLE event, const char *filename,
> return err;
> }
>
> -#define logPrlEventError(event) \
> - logPrlEventErrorHelper(event, __FILE__, \
> - __FUNCTION__, __LINE__)
> -
> static PRL_RESULT
> getJobResultHelper(PRL_HANDLE job, unsigned int timeout, PRL_HANDLE *result,
> const char *filename, const char *funcname,
>
ACK
Pushed now.
5:14 a.m.
New subject: [libvirt] [PATCH 2/4] vz: return event error code in logPrlEventErrorHelper and use it
If PrlEvent_GetErrCode returns an error code this is what we should
use as an error code for the whole action
Signed-off-by: Maxim Nestratov <mnestratov(a)virtuozzo.com>
---
src/vz/vz_sdk.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index e5c56a5..7fc7d97 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -100,14 +100,17 @@ logPrlErrorHelper(PRL_RESULT err, const char *filename,
static PRL_RESULT
logPrlEventErrorHelper(PRL_HANDLE event, const char *filename,
- const char *funcname, size_t linenr)
+ const char *funcname, size_t linenr, PRL_RESULT* retCode)
{
- PRL_RESULT ret, retCode;
+ PRL_RESULT ret;
char *msg1 = NULL, *msg2 = NULL;
PRL_UINT32 len = 0;
int err = -1;
- if ((ret = PrlEvent_GetErrCode(event, &retCode))) {
+ if (!retCode)
+ return -1;
+
+ if ((ret = PrlEvent_GetErrCode(event, retCode))) {
logPrlError(ret);
return ret;
}
@@ -164,7 +167,8 @@ getJobResultHelper(PRL_HANDLE job, unsigned int timeout, PRL_HANDLE
*result,
goto cleanup;
}
- if (logPrlEventErrorHelper(err_handle, filename, funcname, linenr))
+ ret = logPrlEventErrorHelper(err_handle, filename, funcname, linenr,
&retCode);
+ if (ret)
logPrlErrorHelper(retCode, filename, funcname, linenr);
PrlHandle_Free(err_handle);
--
1.8.3.1
4:14 a.m.
New subject: [libvirt] [PATCH 2/4] vz: return event error code in logPrlEventErrorHelper and use it
On 26.05.2016 13:14, Maxim Nestratov wrote:
If PrlEvent_GetErrCode returns an error code this is what we should
use as an error code for the whole action
Signed-off-by: Maxim Nestratov <mnestratov(a)virtuozzo.com>
---
src/vz/vz_sdk.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index e5c56a5..7fc7d97 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -100,14 +100,17 @@ logPrlErrorHelper(PRL_RESULT err, const char *filename,
static PRL_RESULT
logPrlEventErrorHelper(PRL_HANDLE event, const char *filename,
- const char *funcname, size_t linenr)
+ const char *funcname, size_t linenr, PRL_RESULT* retCode)
I would put retCode before caller position parameters(filename etc)
{
- PRL_RESULT ret, retCode;
+ PRL_RESULT ret;
char *msg1 = NULL, *msg2 = NULL;
PRL_UINT32 len = 0;
int err = -1;
- if ((ret = PrlEvent_GetErrCode(event, &retCode))) {
+ if (!retCode)
+ return -1;
Supplying NULL retCode is a big usage mistake as you get wrong result code in this
case. Thus I would instead don't fail but give warning and continue. This way
correct error will be logged at least.
Eventually I think simpliest would be move PrlEvent_GetErrCode out of this function.
Also I would make it void as it is a logging function.
+
+ if ((ret = PrlEvent_GetErrCode(event, retCode))) {
logPrlError(ret);
return ret;
}
@@ -164,7 +167,8 @@ getJobResultHelper(PRL_HANDLE job, unsigned int timeout, PRL_HANDLE
*result,
goto cleanup;
}
- if (logPrlEventErrorHelper(err_handle, filename, funcname, linenr))
+ ret = logPrlEventErrorHelper(err_handle, filename, funcname, linenr,
&retCode);
+ if (ret)
logPrlErrorHelper(retCode, filename, funcname, linenr);
PrlHandle_Free(err_handle);
10:35 a.m.
New subject: [libvirt] [PATCH 2/4] vz: return event error code in logPrlEventErrorHelper and use it
27.05.2016 12:14, Nikolay Shirokovskiy пишет:
On 26.05.2016 13:14, Maxim Nestratov wrote:
> If PrlEvent_GetErrCode returns an error code this is what we should
> use as an error code for the whole action
>
> Signed-off-by: Maxim Nestratov <mnestratov(a)virtuozzo.com>
> ---
> src/vz/vz_sdk.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index e5c56a5..7fc7d97 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -100,14 +100,17 @@ logPrlErrorHelper(PRL_RESULT err, const char *filename,
>
> static PRL_RESULT
> logPrlEventErrorHelper(PRL_HANDLE event, const char *filename,
> - const char *funcname, size_t linenr)
> + const char *funcname, size_t linenr, PRL_RESULT* retCode)
I would put retCode before caller position parameters(filename etc)
> {
> - PRL_RESULT ret, retCode;
> + PRL_RESULT ret;
> char *msg1 = NULL, *msg2 = NULL;
> PRL_UINT32 len = 0;
> int err = -1;
>
> - if ((ret = PrlEvent_GetErrCode(event, &retCode))) {
> + if (!retCode)
> + return -1;
Supplying NULL retCode is a big usage mistake as you get wrong result code in this
case. Thus I would instead don't fail but give warning and continue. This way
correct error will be logged at least.
Eventually I think simpliest would be move PrlEvent_GetErrCode out of this function.
Also I would make it void as it is a logging function.
Makes sense. Thanks.
I'll resend this patch.
Maxim
5:14 a.m.
New subject: [libvirt] [PATCH 3/4] vz: fix getJobResultHelper
when additional information is asked PrlJob_GetError an error
PRL_ERR_NO_DATA should not be treated as error
Signed-off-by: Maxim Nestratov <mnestratov(a)virtuozzo.com>
---
src/vz/vz_sdk.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 7fc7d97..bad4ddd 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -162,7 +162,8 @@ getJobResultHelper(PRL_HANDLE job, unsigned int timeout, PRL_HANDLE
*result,
PRL_HANDLE err_handle;
/* Sometimes it's possible to get additional error info. */
- if ((ret = PrlJob_GetError(job, &err_handle))) {
+ ret = PrlJob_GetError(job, &err_handle);
+ if (ret && ret != PRL_ERR_NO_DATA) {
logPrlErrorHelper(ret, filename, funcname, linenr);
goto cleanup;
}
--
1.8.3.1
4:20 a.m.
New subject: [libvirt] [PATCH 3/4] vz: fix getJobResultHelper
On 26.05.2016 13:14, Maxim Nestratov wrote:
when additional information is asked PrlJob_GetError an error
PRL_ERR_NO_DATA should not be treated as error
Signed-off-by: Maxim Nestratov <mnestratov(a)virtuozzo.com>
---
src/vz/vz_sdk.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 7fc7d97..bad4ddd 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -162,7 +162,8 @@ getJobResultHelper(PRL_HANDLE job, unsigned int timeout, PRL_HANDLE
*result,
PRL_HANDLE err_handle;
/* Sometimes it's possible to get additional error info. */
- if ((ret = PrlJob_GetError(job, &err_handle))) {
+ ret = PrlJob_GetError(job, &err_handle);
+ if (ret && ret != PRL_ERR_NO_DATA) {
logPrlErrorHelper(ret, filename, funcname, linenr);
goto cleanup;
}
I would log and return in case of PRL_ERR_NO_DATA or it is rather compilicated
to call logPrlEventErrorHelper only to return immediately.
10:37 a.m.
New subject: [libvirt] [PATCH 3/4] vz: fix getJobResultHelper
27.05.2016 12:20, Nikolay Shirokovskiy пишет:
On 26.05.2016 13:14, Maxim Nestratov wrote:
> when additional information is asked PrlJob_GetError an error
> PRL_ERR_NO_DATA should not be treated as error
>
> Signed-off-by: Maxim Nestratov <mnestratov(a)virtuozzo.com>
> ---
> src/vz/vz_sdk.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index 7fc7d97..bad4ddd 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -162,7 +162,8 @@ getJobResultHelper(PRL_HANDLE job, unsigned int timeout,
PRL_HANDLE *result,
> PRL_HANDLE err_handle;
>
> /* Sometimes it's possible to get additional error info. */
> - if ((ret = PrlJob_GetError(job, &err_handle))) {
> + ret = PrlJob_GetError(job, &err_handle);
> + if (ret && ret != PRL_ERR_NO_DATA) {
> logPrlErrorHelper(ret, filename, funcname, linenr);
> goto cleanup;
> }
>
I would log and return in case of PRL_ERR_NO_DATA or it is rather compilicated
to call logPrlEventErrorHelper only to return immediately.
I squashed this patch with the previous and reworked it a bit.
Sent a new version.
Maxim
5:15 a.m.
New subject: [libvirt] [PATCH 4/4] vz: return correct result for unimplemented ChangeState actions
Map PRL_ERR_UNIMPLEMENTED to VIR_ERR_OPERATION_INVALID
Signed-off-by: Maxim Nestratov <mnestratov(a)virtuozzo.com>
---
src/vz/vz_sdk.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index bad4ddd..94ed35e 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -2052,6 +2052,7 @@ prlsdkDomainChangeStateLocked(vzDriverPtr driver,
case PRL_ERR_DISP_VM_IS_NOT_STARTED:
case PRL_ERR_DISP_VM_IS_NOT_STOPPED:
case PRL_ERR_INVALID_ACTION_REQUESTED:
+ case PRL_ERR_UNIMPLEMENTED:
virerr = VIR_ERR_OPERATION_INVALID;
break;
default:
--
1.8.3.1
4:38 a.m.
New subject: [libvirt] [PATCH 4/4] vz: return correct result for unimplemented ChangeState actions
On 26.05.2016 13:15, Maxim Nestratov wrote:
Map PRL_ERR_UNIMPLEMENTED to VIR_ERR_OPERATION_INVALID
Signed-off-by: Maxim Nestratov <mnestratov(a)virtuozzo.com>
---
src/vz/vz_sdk.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index bad4ddd..94ed35e 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -2052,6 +2052,7 @@ prlsdkDomainChangeStateLocked(vzDriverPtr driver,
case PRL_ERR_DISP_VM_IS_NOT_STARTED:
case PRL_ERR_DISP_VM_IS_NOT_STOPPED:
case PRL_ERR_INVALID_ACTION_REQUESTED:
+ case PRL_ERR_UNIMPLEMENTED:
virerr = VIR_ERR_OPERATION_INVALID;
break;
default:
ACK
10:23 a.m.
New subject: [libvirt] [PATCH 4/4] vz: return correct result for unimplemented ChangeState actions
27.05.2016 12:38, Nikolay Shirokovskiy пишет:
On 26.05.2016 13:15, Maxim Nestratov wrote:
> Map PRL_ERR_UNIMPLEMENTED to VIR_ERR_OPERATION_INVALID
>
> Signed-off-by: Maxim Nestratov <mnestratov(a)virtuozzo.com>
> ---
> src/vz/vz_sdk.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index bad4ddd..94ed35e 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -2052,6 +2052,7 @@ prlsdkDomainChangeStateLocked(vzDriverPtr driver,
> case PRL_ERR_DISP_VM_IS_NOT_STARTED:
> case PRL_ERR_DISP_VM_IS_NOT_STOPPED:
> case PRL_ERR_INVALID_ACTION_REQUESTED:
> + case PRL_ERR_UNIMPLEMENTED:
> virerr = VIR_ERR_OPERATION_INVALID;
> break;
> default:
>
ACK
Pushed now.
Maxim
3086
days inactive
3102
days old
12 comments
2 participants
participants (2)
-
Maxim Nestratov -
Nikolay Shirokovskiy