On 08/24/2012 03:03 AM, Daniel Veillard wrote:
On Fri, Aug 24, 2012 at 02:07:15AM -0400, Laine Stump wrote:
> I noticed this while auditing all calls to virCommandRun that request
> an exit status from virCommandRun. Two functions in the openvz driver
>
> openvzDomainGetBarrierLimit
> openvzDomainSetBarrierLimit
>
> request an exit status from virCommandRun (thus assuring that
> virCommandRun won't log any errors just due to a non-0 exit status),
> but then fail to examine that exit status. This could result in the
> functions believing that the call to "vzlist" was successful, even
> though it may have encountered an error.
> ---
> src/openvz/openvz_driver.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
> index 8257ed5..a1d3b42 100644
> --- a/src/openvz/openvz_driver.c
> +++ b/src/openvz/openvz_driver.c
> @@ -1707,7 +1707,7 @@ openvzDomainGetBarrierLimit(virDomainPtr domain,
> virCommandSetOutputBuffer(cmd, &output);
> virCommandAddArgFormat(cmd, "-o%s.b,%s.l", param, param);
> virCommandAddArg(cmd, domain->name);
> - if (virCommandRun(cmd, &status)) {
> + if (virCommandRun(cmd, &status) < 0 || status != 0) {
> virReportError(VIR_ERR_OPERATION_FAILED,
> _("Failed to get %s for %s: %d"), param,
domain->name,
> status);
> @@ -1758,7 +1758,7 @@ openvzDomainSetBarrierLimit(virDomainPtr domain,
> virCommandAddArgFormat(cmd, "--%s", param);
> virCommandAddArgFormat(cmd, "%llu:%llu", barrier, limit);
> virCommandAddArg(cmd, "--save");
> - if (virCommandRun(cmd, &status)) {
> + if (virCommandRun(cmd, &status) < 0 || status != 0) {
> virReportError(VIR_ERR_OPERATION_FAILED,
> _("Failed to set %s for %s: %d"), param,
domain->name,
> status);
ACK, thanks for checking :-)
Daniel
Pushed. Thanks!