[libvirt] [PATCH] qemu: Fix return value of qemuDomainGetBlockJobInfo

While reviewing 1b43885d1784640 I've noticed a virReportError() followed by a goto endjob; without setting the correct return value. Problem is, if block job is so fast that it's bandwidth does not fit into ulong, an error is reported. However, by that time @ret is already set to 1 which means success. Since the scenario can be hardly considered successful, we should return a value meaning error. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 304165c..1161aa0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16546,6 +16546,7 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom, virReportError(VIR_ERR_OVERFLOW, _("bandwidth %llu cannot be represented in result"), rawInfo.bandwidth); + ret = -1; goto endjob; } -- 2.4.10

On Mon, Jan 04, 2016 at 15:53:32 +0100, Michal Privoznik wrote:
While reviewing 1b43885d1784640 I've noticed a virReportError() followed by a goto endjob; without setting the correct return value. Problem is, if block job is so fast that it's bandwidth does not fit into ulong, an error is reported. However, by that time @ret is already set to 1 which means success. Since the scenario can be hardly considered successful, we should return a value meaning error.
I'd prefer if you dropped the beginning of the commit message describing how you located this bug, just describing the bug itself.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 304165c..1161aa0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16546,6 +16546,7 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom, virReportError(VIR_ERR_OVERFLOW, _("bandwidth %llu cannot be represented in result"), rawInfo.bandwidth); + ret = -1; goto endjob;
ACK, Peter
participants (2)
-
Michal Privoznik
-
Peter Krempa