On 11/24/21 13:01, Martin Kletzander wrote: > 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@xxxxxxxxxx> >> --- >> >> 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@xxxxxxxxxx> Fixed and pushed. Michal
