[libvirt] [PATCH] hooks: let virCommand do the error reporting
The code was reporting raw exit status without decoding it into normal vs. signal exit. virCommandRun already does this, but with a different error type, so all we have to do is recast the error to the correct type. Reported by li guang. * src/util/hooks.c (virHookCall): Simplify. --- In response to: https://www.redhat.com/archives/libvir-list/2012-October/msg00308.html src/util/hooks.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/util/hooks.c b/src/util/hooks.c index f5890d2..8817a4e 100644 --- a/src/util/hooks.c +++ b/src/util/hooks.c @@ -1,7 +1,7 @@ /* * hooks.c: implementation of the synchronous hooks support * - * Copyright (C) 2010-2011 Red Hat, Inc. + * Copyright (C) 2010-2012 Red Hat, Inc. * Copyright (C) 2010 Daniel Veillard * * This library is free software; you can redistribute it and/or @@ -206,7 +206,6 @@ virHookCall(int driver, char **output) { int ret; - int exitstatus; char *path; virCommandPtr cmd; const char *drvstr; @@ -279,12 +278,11 @@ virHookCall(int driver, if (output) virCommandSetOutputBuffer(cmd, output); - ret = virCommandRun(cmd, &exitstatus); - if (ret == 0 && exitstatus != 0) { - virReportError(VIR_ERR_HOOK_SCRIPT_FAILED, - _("Hook script %s %s failed with error code %d"), - path, drvstr, exitstatus); - ret = -1; + ret = virCommandRun(cmd, NULL); + if (ret < 0) { + /* Convert INTERNAL_ERROR into known error. */ + virErrorPtr err = virGetLastError(); + virReportError(VIR_ERR_HOOK_SCRIPT_FAILED, "%s", err->message); } virCommandFree(cmd); -- 1.7.11.4
On 10/09/2012 05:47 AM, Eric Blake wrote:
The code was reporting raw exit status without decoding it into normal vs. signal exit. virCommandRun already does this, but with a different error type, so all we have to do is recast the error to the correct type. Reported by li guang.
* src/util/hooks.c (virHookCall): Simplify. ---
In response to: https://www.redhat.com/archives/libvir-list/2012-October/msg00308.html
src/util/hooks.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
ACK, Martin
On 10/09/2012 12:32 AM, Martin Kletzander wrote:
On 10/09/2012 05:47 AM, Eric Blake wrote:
The code was reporting raw exit status without decoding it into normal vs. signal exit. virCommandRun already does this, but with a different error type, so all we have to do is recast the error to the correct type. Reported by li guang.
* src/util/hooks.c (virHookCall): Simplify. ---
In response to: https://www.redhat.com/archives/libvir-list/2012-October/msg00308.html
src/util/hooks.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
ACK,
Pushed. Hmm, I wonder if we should add a new function, virCommandSetErrorNumber(cmd, VIR_ERR_...); if it is not called, then command failure continues to give INTERNAL_ERROR; but if it is called, then failures of that particular command object are reported with that error category. Then we wouldn't have to rewrap the error just to change the category. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 10/09/2012 04:45 PM, Eric Blake wrote:
On 10/09/2012 12:32 AM, Martin Kletzander wrote:
On 10/09/2012 05:47 AM, Eric Blake wrote:
The code was reporting raw exit status without decoding it into normal vs. signal exit. virCommandRun already does this, but with a different error type, so all we have to do is recast the error to the correct type. Reported by li guang.
* src/util/hooks.c (virHookCall): Simplify. ---
In response to: https://www.redhat.com/archives/libvir-list/2012-October/msg00308.html
src/util/hooks.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
ACK,
Pushed.
Hmm, I wonder if we should add a new function, virCommandSetErrorNumber(cmd, VIR_ERR_...); if it is not called, then command failure continues to give INTERNAL_ERROR; but if it is called, then failures of that particular command object are reported with that error category. Then we wouldn't have to rewrap the error just to change the category.
Interesting idea. I must say, I spent a while on checking where the error gets saved and if it bubbles up through the code properly, so maybe that would simplify it a little bit. OTOH, it would probably mean more code than less and, when somebody is familiar with current functions, it is already clear whether the caller makes differences between exit status and other errors. Martin
participants (2)
-
Eric Blake -
Martin Kletzander