This patch refactors a few bigger functions mainly trying to remove too deep indentation and make them more readable and more consistent with the rest of the file. Signed-off-by: Kristina Hanicova <khanicov@xxxxxxxxxx> --- tools/virsh-domain.c | 203 ++++++++++++++++++++----------------------- tools/virsh-host.c | 140 ++++++++++++++--------------- tools/virsh-volume.c | 56 ++++++------ 3 files changed, 190 insertions(+), 209 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index ec427443c4..2881956d78 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -5133,7 +5133,6 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) int nparams = 0; int nupdates = 0; size_t i; - int ret; bool ret_val = false; unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; bool current = vshCommandOptBool(cmd, "current"); @@ -5161,64 +5160,61 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - if (nparams) { - params = g_new0(virTypedParameter, nparams); + if (!nparams) + goto cleanup; - memset(params, 0, sizeof(*params) * nparams); - if (flags || current) { - /* We cannot query both live and config at once, so settle - on current in that case. If we are setting, then the - two values should match when we re-query; otherwise, we - report the error later. */ - ret = virDomainGetSchedulerParametersFlags(dom, params, &nparams, - ((live && config) ? 0 - : flags)); - } else { - ret = virDomainGetSchedulerParameters(dom, params, &nparams); - } - if (ret == -1) + params = g_new0(virTypedParameter, nparams); + memset(params, 0, sizeof(*params) * nparams); + + if (flags || current) { + /* We cannot query both live and config at once, so settle + on current in that case. If we are setting, then the + two values should match when we re-query; otherwise, we + report the error later. */ + if (virDomainGetSchedulerParametersFlags(dom, params, &nparams, + ((live && config) ? 0 : flags)) == -1) goto cleanup; - - /* See if any params are being set */ - if ((nupdates = cmdSchedInfoUpdate(ctl, cmd, params, nparams, - &updates)) < 0) + } else { + if (virDomainGetSchedulerParameters(dom, params, &nparams) == -1) goto cleanup; + } - /* Update parameters & refresh data */ - if (nupdates > 0) { - if (flags || current) - ret = virDomainSetSchedulerParametersFlags(dom, updates, - nupdates, flags); - else - ret = virDomainSetSchedulerParameters(dom, updates, nupdates); + /* See if any params are being set */ + if ((nupdates = cmdSchedInfoUpdate(ctl, cmd, params, nparams, + &updates)) < 0) + goto cleanup; - if (ret == -1) + /* Update parameters & refresh data */ + if (nupdates > 0) { + if (flags || current) { + if (virDomainSetSchedulerParametersFlags(dom, updates, + nupdates, flags) == -1) goto cleanup; - if (flags || current) - ret = virDomainGetSchedulerParametersFlags(dom, params, - &nparams, - ((live && config) ? 0 - : flags)); - else - ret = virDomainGetSchedulerParameters(dom, params, &nparams); - if (ret == -1) + if (virDomainGetSchedulerParametersFlags(dom, params, &nparams, + ((live && config) ? 0 : flags)) == -1) goto cleanup; } else { - /* When not doing --set, --live and --config do not mix. */ - if (live && config) { - vshError(ctl, "%s", - _("cannot query both live and config at once")); + if (virDomainSetSchedulerParameters(dom, updates, nupdates) == -1) goto cleanup; - } - } - ret_val = true; - for (i = 0; i < nparams; i++) { - char *str = vshGetTypedParamValue(ctl, ¶ms[i]); - vshPrint(ctl, "%-15s: %s\n", params[i].field, str); - VIR_FREE(str); + if (virDomainGetSchedulerParameters(dom, params, &nparams) == -1) + goto cleanup; } + } else { + /* When not doing --set, --live and --config do not mix. */ + if (live && config) { + vshError(ctl, "%s", + _("cannot query both live and config at once")); + goto cleanup; + } + } + + ret_val = true; + for (i = 0; i < nparams; i++) { + char *str = vshGetTypedParamValue(ctl, ¶ms[i]); + vshPrint(ctl, "%-15s: %s\n", params[i].field, str); + VIR_FREE(str); } cleanup: @@ -6503,7 +6499,6 @@ virshCPUCountCollect(vshControl *ctl, unsigned int flags, bool checkState) { - int ret = -2; virDomainInfo info; int count; g_autoptr(xmlDoc) xml = NULL; @@ -6524,11 +6519,11 @@ virshCPUCountCollect(vshControl *ctl, /* fallback code */ if (!(last_error->code == VIR_ERR_NO_SUPPORT || last_error->code == VIR_ERR_INVALID_ARG)) - goto cleanup; + return -2; if (flags & VIR_DOMAIN_VCPU_GUEST) { vshError(ctl, "%s", _("Failed to retrieve vCPU count from the guest")); - goto cleanup; + return -2; } if (!(flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG)) && @@ -6538,36 +6533,32 @@ virshCPUCountCollect(vshControl *ctl, vshResetLibvirtError(); if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (flags & VIR_DOMAIN_VCPU_MAXIMUM) { - count = virDomainGetMaxVcpus(dom); - } else { - if (virDomainGetInfo(dom, &info) < 0) - goto cleanup; + if (flags & VIR_DOMAIN_VCPU_MAXIMUM) + return virDomainGetMaxVcpus(dom); - count = info.nrVirtCpu; + if (virDomainGetInfo(dom, &info) < 0) + return -2; + + return info.nrVirtCpu; + } + + if (virshDomainGetXMLFromDom(ctl, dom, VIR_DOMAIN_XML_INACTIVE, + &xml, &ctxt) < 0) + return -2; + + if (flags & VIR_DOMAIN_VCPU_MAXIMUM) { + if (virXPathInt("string(/domain/vcpu)", ctxt, &count) < 0) { + vshError(ctl, "%s", _("Failed to retrieve maximum vcpu count")); + return -2; } } else { - if (virshDomainGetXMLFromDom(ctl, dom, VIR_DOMAIN_XML_INACTIVE, - &xml, &ctxt) < 0) - goto cleanup; - - if (flags & VIR_DOMAIN_VCPU_MAXIMUM) { - if (virXPathInt("string(/domain/vcpu)", ctxt, &count) < 0) { - vshError(ctl, "%s", _("Failed to retrieve maximum vcpu count")); - goto cleanup; - } - } else { - if (virXPathInt("string(/domain/vcpu/@current)", ctxt, &count) < 0) { - vshError(ctl, "%s", _("Failed to retrieve current vcpu count")); - goto cleanup; - } + if (virXPathInt("string(/domain/vcpu/@current)", ctxt, &count) < 0) { + vshError(ctl, "%s", _("Failed to retrieve current vcpu count")); + return -2; } } - ret = count; - cleanup: - - return ret; + return count; } static bool @@ -9588,7 +9579,7 @@ cmdQemuMonitorEvent(vshControl *ctl, const vshCmd *cmd) vshEventCleanup(ctl); if (eventId >= 0 && virConnectDomainQemuMonitorEventDeregister(priv->conn, eventId) < 0) - ret = false; + return false; return ret; } @@ -9790,6 +9781,7 @@ cmdLxcEnterNamespace(vshControl *ctl, const vshCmd *cmd) int nfdlist; int *fdlist; size_t i; + int status; bool setlabel = true; g_autofree virSecurityModelPtr secmodel = NULL; g_autofree virSecurityLabelPtr seclabel = NULL; @@ -9828,40 +9820,8 @@ cmdLxcEnterNamespace(vshControl *ctl, const vshCmd *cmd) */ if ((pid = virFork()) < 0) return false; - if (pid == 0) { - int status; - - if (setlabel && - virDomainLxcEnterSecurityLabel(secmodel, - seclabel, - NULL, - 0) < 0) - _exit(EXIT_CANCELED); - - if (virDomainLxcEnterCGroup(dom, 0) < 0) - _exit(EXIT_CANCELED); - - if (virDomainLxcEnterNamespace(dom, - nfdlist, - fdlist, - NULL, - NULL, - 0) < 0) - _exit(EXIT_CANCELED); - - /* Fork a second time because entering the - * pid namespace only takes effect after fork - */ - if ((pid = virFork()) < 0) - _exit(EXIT_CANCELED); - if (pid == 0) { - execv(cmdargv[0], cmdargv); - _exit(errno == ENOENT ? EXIT_ENOENT : EXIT_CANNOT_INVOKE); - } - if (virProcessWait(pid, &status, true) < 0) - _exit(EXIT_CANNOT_INVOKE); - virProcessExitWithStatus(status); - } else { + + if (pid != 0) { for (i = 0; i < nfdlist; i++) VIR_FORCE_CLOSE(fdlist[i]); VIR_FREE(fdlist); @@ -9869,8 +9829,33 @@ cmdLxcEnterNamespace(vshControl *ctl, const vshCmd *cmd) vshReportError(ctl); return false; } + return true; + } + + if (setlabel && + virDomainLxcEnterSecurityLabel(secmodel, seclabel, NULL, 0) < 0) + _exit(EXIT_CANCELED); + + if (virDomainLxcEnterCGroup(dom, 0) < 0) + _exit(EXIT_CANCELED); + + if (virDomainLxcEnterNamespace(dom, nfdlist, fdlist, NULL, NULL, 0) < 0) + _exit(EXIT_CANCELED); + + /* Fork a second time because entering the + * pid namespace only takes effect after fork + */ + if ((pid = virFork()) < 0) + _exit(EXIT_CANCELED); + + if (pid == 0) { + execv(cmdargv[0], cmdargv); + _exit(errno == ENOENT ? EXIT_ENOENT : EXIT_CANNOT_INVOKE); } + if (virProcessWait(pid, &status, true) < 0) + _exit(EXIT_CANNOT_INVOKE); + virProcessExitWithStatus(status); return true; } diff --git a/tools/virsh-host.c b/tools/virsh-host.c index e6ed4a26ce..591746655b 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -162,7 +162,6 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd) bool cellno = vshCommandOptBool(cmd, "cellno"); size_t i; g_autofree char *cap_xml = NULL; - g_autoptr(xmlDoc) xml = NULL; g_autoptr(xmlXPathContext) ctxt = NULL; virshControl *priv = ctl->privData; @@ -171,68 +170,69 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd) if (cellno && vshCommandOptInt(ctl, cmd, "cellno", &cell) < 0) return false; - if (all) { - if (!(cap_xml = virConnectGetCapabilities(priv->conn))) { - vshError(ctl, "%s", _("unable to get node capabilities")); - return false; - } + if (!all) { + if (cellno) { + if (virNodeGetCellsFreeMemory(priv->conn, &memory, cell, 1) != 1) + return false; - xml = virXMLParseStringCtxt(cap_xml, _("(capabilities)"), &ctxt); - if (!xml) { - vshError(ctl, "%s", _("unable to get node capabilities")); - return false; + vshPrint(ctl, "%d: %llu KiB\n", cell, (memory/1024)); + return true; } - nodes_cnt = virXPathNodeSet("/capabilities/host/topology/cells/cell", - ctxt, &nodes); - - if (nodes_cnt == -1) { - vshError(ctl, "%s", _("could not get information about " - "NUMA topology")); + if ((memory = virNodeGetFreeMemory(priv->conn)) == 0) return false; - } - nodes_free = g_new0(unsigned long long, nodes_cnt); - nodes_id = g_new0(unsigned long, nodes_cnt); + vshPrint(ctl, "%s: %llu KiB\n", _("Total"), (memory/1024)); + return true; + } - for (i = 0; i < nodes_cnt; i++) { - unsigned long id; - g_autofree char *val = virXMLPropString(nodes[i], "id"); - if (virStrToLong_ulp(val, NULL, 10, &id)) { - vshError(ctl, "%s", _("conversion from string failed")); - return false; - } - nodes_id[i] = id; - if (virNodeGetCellsFreeMemory(priv->conn, &(nodes_free[i]), - id, 1) != 1) { - vshError(ctl, _("failed to get free memory for NUMA node " - "number: %lu"), id); - return false; - } - } + if (!(cap_xml = virConnectGetCapabilities(priv->conn))) { + vshError(ctl, "%s", _("unable to get node capabilities")); + return false; + } - for (cell = 0; cell < nodes_cnt; cell++) { - vshPrint(ctl, "%5lu: %10llu KiB\n", nodes_id[cell], - (nodes_free[cell]/1024)); - memory += nodes_free[cell]; - } + if (!virXMLParseStringCtxt(cap_xml, _("(capabilities)"), &ctxt)) { + vshError(ctl, "%s", _("unable to get node capabilities")); + return false; + } - vshPrintExtra(ctl, "--------------------\n"); - vshPrintExtra(ctl, "%5s: %10llu KiB\n", _("Total"), memory/1024); - } else { - if (cellno) { - if (virNodeGetCellsFreeMemory(priv->conn, &memory, cell, 1) != 1) - return false; + nodes_cnt = virXPathNodeSet("/capabilities/host/topology/cells/cell", + ctxt, &nodes); - vshPrint(ctl, "%d: %llu KiB\n", cell, (memory/1024)); - } else { - if ((memory = virNodeGetFreeMemory(priv->conn)) == 0) - return false; + if (nodes_cnt == -1) { + vshError(ctl, "%s", _("could not get information about " + "NUMA topology")); + return false; + } + + nodes_free = g_new0(unsigned long long, nodes_cnt); + nodes_id = g_new0(unsigned long, nodes_cnt); - vshPrint(ctl, "%s: %llu KiB\n", _("Total"), (memory/1024)); + for (i = 0; i < nodes_cnt; i++) { + unsigned long id; + g_autofree char *val = virXMLPropString(nodes[i], "id"); + if (virStrToLong_ulp(val, NULL, 10, &id)) { + vshError(ctl, "%s", _("conversion from string failed")); + return false; + } + nodes_id[i] = id; + if (virNodeGetCellsFreeMemory(priv->conn, &(nodes_free[i]), + id, 1) != 1) { + vshError(ctl, _("failed to get free memory for NUMA node " + "number: %lu"), id); + return false; } } + for (cell = 0; cell < nodes_cnt; cell++) { + vshPrint(ctl, "%5lu: %10llu KiB\n", nodes_id[cell], + (nodes_free[cell]/1024)); + memory += nodes_free[cell]; + } + + vshPrintExtra(ctl, "--------------------\n"); + vshPrintExtra(ctl, "%5s: %10llu KiB\n", _("Total"), memory/1024); + return true; } @@ -804,30 +804,30 @@ cmdNodeCpuStats(vshControl *ctl, const vshCmd *cmd) cpu_stats[i]); } } + return true; + } + + if (present[VIRSH_CPU_USAGE]) { + vshPrint(ctl, "%-15s %5.1llu%%\n", + _("usage:"), cpu_stats[VIRSH_CPU_USAGE]); + vshPrint(ctl, "%-15s %5.1llu%%\n", + _("idle:"), 100 - cpu_stats[VIRSH_CPU_USAGE]); } else { - if (present[VIRSH_CPU_USAGE]) { - vshPrint(ctl, "%-15s %5.1llu%%\n", - _("usage:"), cpu_stats[VIRSH_CPU_USAGE]); - vshPrint(ctl, "%-15s %5.1llu%%\n", - _("idle:"), 100 - cpu_stats[VIRSH_CPU_USAGE]); - } else { - double usage, total_time = 0; - for (i = 0; i < VIRSH_CPU_USAGE; i++) - total_time += cpu_stats[i]; - - usage = (cpu_stats[VIRSH_CPU_USER] + cpu_stats[VIRSH_CPU_SYSTEM]) - / total_time * 100; - - vshPrint(ctl, "%-15s %5.1lf%%\n", _("usage:"), usage); - for (i = 0; i < VIRSH_CPU_USAGE; i++) { - if (present[i]) { - vshPrint(ctl, "%-15s %5.1lf%%\n", _(virshCPUOutput[i]), - cpu_stats[i] / total_time * 100); - } + double usage, total_time = 0; + for (i = 0; i < VIRSH_CPU_USAGE; i++) + total_time += cpu_stats[i]; + + usage = (cpu_stats[VIRSH_CPU_USER] + cpu_stats[VIRSH_CPU_SYSTEM]) + / total_time * 100; + + vshPrint(ctl, "%-15s %5.1lf%%\n", _("usage:"), usage); + for (i = 0; i < VIRSH_CPU_USAGE; i++) { + if (present[i]) { + vshPrint(ctl, "%-15s %5.1lf%%\n", _(virshCPUOutput[i]), + cpu_stats[i] / total_time * 100); } } } - return true; } diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 152f5b0dbe..ef437413df 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -1057,7 +1057,6 @@ cmdVolInfo(vshControl *ctl, const vshCmd *cmd) virStorageVolPtr vol; bool bytes = vshCommandOptBool(cmd, "bytes"); bool physical = vshCommandOptBool(cmd, "physical"); - bool ret = true; int rc; unsigned int flags = 0; @@ -1074,41 +1073,39 @@ cmdVolInfo(vshControl *ctl, const vshCmd *cmd) else rc = virStorageVolGetInfo(vol, &info); - if (rc == 0) { - double val; - const char *unit; + if (rc < 0) { + virStorageVolFree(vol); + return false; + } - vshPrint(ctl, "%-15s %s\n", _("Type:"), - virshVolumeTypeToString(info.type)); + vshPrint(ctl, "%-15s %s\n", _("Type:"), + virshVolumeTypeToString(info.type)); - if (bytes) { - vshPrint(ctl, "%-15s %llu %s\n", _("Capacity:"), - info.capacity, _("bytes")); - } else { - val = vshPrettyCapacity(info.capacity, &unit); - vshPrint(ctl, "%-15s %2.2lf %s\n", _("Capacity:"), val, unit); - } + if (bytes) { + vshPrint(ctl, "%-15s %llu %s\n", _("Capacity:"), + info.capacity, _("bytes")); - if (bytes) { - if (physical) - vshPrint(ctl, "%-15s %llu %s\n", _("Physical:"), - info.allocation, _("bytes")); - else - vshPrint(ctl, "%-15s %llu %s\n", _("Allocation:"), - info.allocation, _("bytes")); - } else { - val = vshPrettyCapacity(info.allocation, &unit); - if (physical) - vshPrint(ctl, "%-15s %2.2lf %s\n", _("Physical:"), val, unit); - else - vshPrint(ctl, "%-15s %2.2lf %s\n", _("Allocation:"), val, unit); - } + if (physical) + vshPrint(ctl, "%-15s %llu %s\n", _("Physical:"), + info.allocation, _("bytes")); + else + vshPrint(ctl, "%-15s %llu %s\n", _("Allocation:"), + info.allocation, _("bytes")); } else { - ret = false; + const char *unit; + double val = vshPrettyCapacity(info.capacity, &unit); + + vshPrint(ctl, "%-15s %2.2lf %s\n", _("Capacity:"), val, unit); + + val = vshPrettyCapacity(info.allocation, &unit); + if (physical) + vshPrint(ctl, "%-15s %2.2lf %s\n", _("Physical:"), val, unit); + else + vshPrint(ctl, "%-15s %2.2lf %s\n", _("Allocation:"), val, unit); } virStorageVolFree(vol); - return ret; + return true; } /* @@ -1203,7 +1200,6 @@ cmdVolResize(vshControl *ctl, const vshCmd *cmd) delta ? _("Failed to change size of volume '%s' by %s") : _("Failed to change size of volume '%s' to %s"), virStorageVolGetName(vol), capacityStr); - ret = false; } cleanup: -- 2.31.1