[PATCH 4/5] virsh: domain, host & volume: refactor bigger functions
Kristina Hanicova
khanicov at redhat.com
Thu Sep 23 15:08:03 UTC 2021
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 at redhat.com>
---
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
More information about the libvir-list
mailing list