[libvirt] [PATCH 0/4] virsh: iothreadinfo fixes
Ján Tomko (4): virsh: reduce the optimism in cmdIOThreadInfo virsh: do not assign negative values to niothreads virsh: remove redundant virshNodeGetCPUCount virsh: initialize info in cmdIOThreadInfo tools/virsh-domain.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) -- 2.19.2
Instead of using niothreads which defaults to zero, use the common pattern with a ret varaible set to true just before the cleanup label. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tools/virsh-domain.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 8b20059335..420ab5e2c3 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -7538,6 +7538,7 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; virshControlPtr priv = ctl->privData; vshTablePtr table = NULL; + bool ret = false; VSH_EXCLUSIVE_OPTIONS_VAR(current, live); VSH_EXCLUSIVE_OPTIONS_VAR(current, config); @@ -7559,6 +7560,7 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) } if (niothreads == 0) { + ret = true; vshPrintExtra(ctl, _("No IOThreads found for the domain")); goto cleanup; } @@ -7582,13 +7584,15 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) vshTablePrintToStdout(table, ctl); + ret = true; + cleanup: for (i = 0; i < niothreads; i++) virDomainIOThreadInfoFree(info[i]); VIR_FREE(info); vshTableFree(table); virshDomainFree(dom); - return niothreads >= 0; + return ret; } /* -- 2.19.2
Use a temporary 'rc' variable to avoid comparing signed and unsigned integers in the cleanup section. Bug introduced by commit 3072ded which added the comparison against the unsigned 'i'. Also make niothreads size_t to mark that it should be unsigned. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tools/virsh-domain.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 420ab5e2c3..17dd992751 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -7531,7 +7531,7 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) bool config = vshCommandOptBool(cmd, "config"); bool live = vshCommandOptBool(cmd, "live"); bool current = vshCommandOptBool(cmd, "current"); - int niothreads = 0; + size_t niothreads = 0; virDomainIOThreadInfoPtr *info; size_t i; int maxcpu; @@ -7539,6 +7539,7 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) virshControlPtr priv = ctl->privData; vshTablePtr table = NULL; bool ret = false; + int rc; VSH_EXCLUSIVE_OPTIONS_VAR(current, live); VSH_EXCLUSIVE_OPTIONS_VAR(current, config); @@ -7554,10 +7555,11 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) if ((maxcpu = virshNodeGetCPUCount(priv->conn)) < 0) goto cleanup; - if ((niothreads = virDomainGetIOThreadInfo(dom, &info, flags)) < 0) { + if ((rc = virDomainGetIOThreadInfo(dom, &info, flags)) < 0) { vshError(ctl, _("Unable to get domain IOThreads information")); goto cleanup; } + niothreads = rc; if (niothreads == 0) { ret = true; -- 2.19.2
Since commit 4c4b821e it is not used for anything. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tools/virsh-domain.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 17dd992751..516b647401 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -7534,9 +7534,7 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) size_t niothreads = 0; virDomainIOThreadInfoPtr *info; size_t i; - int maxcpu; unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; - virshControlPtr priv = ctl->privData; vshTablePtr table = NULL; bool ret = false; int rc; @@ -7552,9 +7550,6 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - if ((maxcpu = virshNodeGetCPUCount(priv->conn)) < 0) - goto cleanup; - if ((rc = virDomainGetIOThreadInfo(dom, &info, flags)) < 0) { vshError(ctl, _("Unable to get domain IOThreads information")); goto cleanup; -- 2.19.2
Although it is not needed at the moment, do not rely on a value being set before the first jump to cleanup. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tools/virsh-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 516b647401..825c07ac96 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -7532,7 +7532,7 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) bool live = vshCommandOptBool(cmd, "live"); bool current = vshCommandOptBool(cmd, "current"); size_t niothreads = 0; - virDomainIOThreadInfoPtr *info; + virDomainIOThreadInfoPtr *info = NULL; size_t i; unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; vshTablePtr table = NULL; -- 2.19.2
On 2/12/19 6:06 AM, Ján Tomko wrote:
Ján Tomko (4): virsh: reduce the optimism in cmdIOThreadInfo virsh: do not assign negative values to niothreads virsh: remove redundant virshNodeGetCPUCount virsh: initialize info in cmdIOThreadInfo
tools/virsh-domain.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> (series) John
participants (2)
-
John Ferlan -
Ján Tomko