On Wed, Nov 24, 2021 at 12:25:35PM +0100, Kristina Hanicova wrote:

I have seen this pattern a lot in the project, so I decided to rewrite code I stumbled upon to the same pattern as well.

Signed-off-by: Kristina Hanicova <khanicov@redhat.com> ---

This is v2 of: https://listman.redhat.com/archives/libvir-list/2021-November/msg00747.html

Diff to v1: * adding variable 'rc' to fix buggy code and keep the code equivalent (suggested by Jano)

src/qemu/qemu_driver.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1959b639da..5c4b493f64 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15975,6 +15975,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, g_autofree char *drivealias = NULL; const char *qdevid = NULL; int ret = -1; + int rc = 0;

Since this variable is not used in the function except ...

size_t i; virDomainDiskDef *conf_disk = NULL; virDomainDiskDef *disk; @@ -16229,13 +16230,10 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, !virStorageSourceIsEmpty(disk->src)) {

... in this block, you can safely introduce it here ;)

qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorSetBlockIoThrottle(priv->mon, drivealias, qdevid, - &info); - if (qemuDomainObjExitMonitor(driver, vm) < 0) - ret = -1; - if (ret < 0) + rc = qemuMonitorSetBlockIoThrottle(priv->mon, drivealias, qdevid, &info); + + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) goto endjob; - ret = -1; }

virDomainDiskSetBlockIOTune(disk, &info); @@ -16310,6 +16308,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, g_autofree char *drivealias = NULL; const char *qdevid = NULL; int ret = -1; + int rc = 0; int maxparams;

virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | @@ -16361,10 +16360,9 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, goto endjob; } qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorGetBlockIoThrottle(priv->mon, drivealias, qdevid, &reply); - if (qemuDomainObjExitMonitor(driver, vm) < 0) - goto endjob; - if (ret < 0) + rc = qemuMonitorGetBlockIoThrottle(priv->mon, drivealias, qdevid, &reply); + + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)

Same here. Anyway, since this is just a nitpick I can say this is Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

goto endjob; }

@@ -17375,10 +17373,8 @@ qemuDomainSetTime(virDomainPtr dom, if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_RTC_RESET_REINJECTION)) { qemuDomainObjEnterMonitor(driver, vm); rv = qemuMonitorRTCResetReinjection(priv->mon); - if (qemuDomainObjExitMonitor(driver, vm) < 0) - goto endjob;

- if (rv < 0) + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rv < 0) goto endjob; }

-- 2.31.1