[PATCH 1/5] virsh: checkpoint & domain-monitor: small refactoring
Kristina Hanicova
khanicov at redhat.com
Thu Sep 23 15:08:00 UTC 2021
This patch includes small refactoring such as:
* early return in case of an error - helps with indentation
* removal of 'else' branch after return - unnecessary
* altering code to be more consistent with the rest of the file -
function calls inside of parentheses, etc.
* removal of unnecessary variables - mainly the ones used for
return value instead of returning it directly
* missing parentheses around multi-line block of code
Signed-off-by: Kristina Hanicova <khanicov at redhat.com>
---
tools/virsh-checkpoint.c | 19 +--
tools/virsh-domain-monitor.c | 314 ++++++++++++++++-------------------
2 files changed, 155 insertions(+), 178 deletions(-)
diff --git a/tools/virsh-checkpoint.c b/tools/virsh-checkpoint.c
index 8ad37ece69..c1a9e66a24 100644
--- a/tools/virsh-checkpoint.c
+++ b/tools/virsh-checkpoint.c
@@ -292,12 +292,12 @@ virshLookupCheckpoint(vshControl *ctl,
if (vshCommandOptStringReq(ctl, cmd, arg, &chkname) < 0)
return -1;
- if (chkname) {
- *chk = virDomainCheckpointLookupByName(dom, chkname, 0);
- } else {
+ if (!chkname) {
vshError(ctl, _("--%s is required"), arg);
return -1;
}
+
+ *chk = virDomainCheckpointLookupByName(dom, chkname, 0);
if (!*chk) {
vshReportError(ctl);
return -1;
@@ -931,13 +931,13 @@ cmdCheckpointParent(vshControl *ctl,
if (virshGetCheckpointParent(ctl, checkpoint, &parent) < 0)
return false;
+
if (!parent) {
vshError(ctl, _("checkpoint '%s' has no parent"), name);
return false;
}
vshPrint(ctl, "%s", parent);
-
return true;
}
@@ -1002,16 +1002,15 @@ cmdCheckpointDelete(vshControl *ctl,
if (vshCommandOptBool(cmd, "metadata"))
flags |= VIR_DOMAIN_CHECKPOINT_DELETE_METADATA_ONLY;
- if (virDomainCheckpointDelete(checkpoint, flags) == 0) {
- if (flags & VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN_ONLY)
- vshPrintExtra(ctl, _("Domain checkpoint %s children deleted\n"), name);
- else
- vshPrintExtra(ctl, _("Domain checkpoint %s deleted\n"), name);
- } else {
+ if (virDomainCheckpointDelete(checkpoint, flags) < 0) {
vshError(ctl, _("Failed to delete checkpoint %s"), name);
return false;
}
+ if (flags & VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN_ONLY)
+ vshPrintExtra(ctl, _("Domain checkpoint %s children deleted\n"), name);
+ else
+ vshPrintExtra(ctl, _("Domain checkpoint %s deleted\n"), name);
return true;
}
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index eb3e0ef11a..d9bc869080 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -58,6 +58,7 @@ virshGetDomainDescription(vshControl *ctl, virDomainPtr dom, bool title,
g_autoptr(xmlDoc) doc = NULL;
g_autoptr(xmlXPathContext) ctxt = NULL;
int type;
+ int errCode;
if (title)
type = VIR_DOMAIN_METADATA_TITLE;
@@ -66,19 +67,18 @@ virshGetDomainDescription(vshControl *ctl, virDomainPtr dom, bool title,
if ((desc = virDomainGetMetadata(dom, type, NULL, flags))) {
return desc;
- } else {
- int errCode = virGetLastErrorCode();
-
- if (errCode == VIR_ERR_NO_DOMAIN_METADATA) {
- desc = g_strdup("");
- vshResetLibvirtError();
- return desc;
- }
+ }
+ errCode = virGetLastErrorCode();
- if (errCode != VIR_ERR_NO_SUPPORT)
- return desc;
+ if (errCode == VIR_ERR_NO_DOMAIN_METADATA) {
+ desc = g_strdup("");
+ vshResetLibvirtError();
+ return desc;
}
+ if (errCode != VIR_ERR_NO_SUPPORT)
+ return desc;
+
/* fall back to xml */
if (virshDomainGetXMLFromDom(ctl, dom, flags, &doc, &ctxt) < 0)
return NULL;
@@ -467,79 +467,72 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd)
human = vshCommandOptBool(cmd, "human");
- if (all) {
- bool active = virDomainIsActive(dom) == 1;
- int rc;
+ if (!all) {
+ g_autofree char *cap = NULL;
+ g_autofree char *alloc = NULL;
+ g_autofree char *phy = NULL;
- if (virshDomainGetXML(ctl, cmd, 0, &xmldoc, &ctxt) < 0)
+ if (virDomainGetBlockInfo(dom, device, &info, 0) < 0)
return false;
- ndisks = virXPathNodeSet("./devices/disk", ctxt, &disks);
- if (ndisks < 0)
+ if (!cmdDomblkinfoGet(&info, &cap, &alloc, &phy, human))
return false;
- /* title */
- table = vshTableNew(_("Target"), _("Capacity"), _("Allocation"), _("Physical"), NULL);
- if (!table)
- return false;
+ vshPrint(ctl, "%-15s %s\n", _("Capacity:"), cap);
+ vshPrint(ctl, "%-15s %s\n", _("Allocation:"), alloc);
+ vshPrint(ctl, "%-15s %s\n", _("Physical:"), phy);
+ return true;
+ }
- for (i = 0; i < ndisks; i++) {
- g_autofree char *target = NULL;
- g_autofree char *protocol = NULL;
- g_autofree char *cap = NULL;
- g_autofree char *alloc = NULL;
- g_autofree char *phy = NULL;
-
- ctxt->node = disks[i];
- protocol = virXPathString("string(./source/@protocol)", ctxt);
- target = virXPathString("string(./target/@dev)", ctxt);
-
- if (virXPathBoolean("boolean(./source)", ctxt) == 1) {
-
- rc = virDomainGetBlockInfo(dom, target, &info, 0);
-
- if (rc < 0) {
- /* If protocol is present that's an indication of a
- * networked storage device which cannot provide statistics,
- * so generate 0 based data and get the next disk. */
- if (protocol && !active &&
- virGetLastErrorCode() == VIR_ERR_INTERNAL_ERROR &&
- virGetLastErrorDomain() == VIR_FROM_STORAGE) {
- memset(&info, 0, sizeof(info));
- vshResetLibvirtError();
- } else {
- return false;
- }
- }
- } else {
- /* if we don't call virDomainGetBlockInfo() who clears 'info'
- * we have to do it manually */
- memset(&info, 0, sizeof(info));
- }
+ if (virshDomainGetXML(ctl, cmd, 0, &xmldoc, &ctxt) < 0)
+ return false;
- if (!cmdDomblkinfoGet(&info, &cap, &alloc, &phy, human))
- return false;
- if (vshTableRowAppend(table, target, cap, alloc, phy, NULL) < 0)
- return false;
- }
+ ndisks = virXPathNodeSet("./devices/disk", ctxt, &disks);
+ if (ndisks < 0)
+ return false;
- vshTablePrintToStdout(table, ctl);
+ /* title */
+ table = vshTableNew(_("Target"), _("Capacity"), _("Allocation"), _("Physical"), NULL);
+ if (!table)
+ return false;
- } else {
+ for (i = 0; i < ndisks; i++) {
+ g_autofree char *target = NULL;
+ g_autofree char *protocol = NULL;
g_autofree char *cap = NULL;
g_autofree char *alloc = NULL;
g_autofree char *phy = NULL;
- if (virDomainGetBlockInfo(dom, device, &info, 0) < 0)
- return false;
+ ctxt->node = disks[i];
+ protocol = virXPathString("string(./source/@protocol)", ctxt);
+ target = virXPathString("string(./target/@dev)", ctxt);
+
+ if (virXPathBoolean("boolean(./source)", ctxt) == 1) {
+ if (virDomainGetBlockInfo(dom, target, &info, 0) < 0) {
+ /* If protocol is present that's an indication of a
+ * networked storage device which cannot provide statistics,
+ * so generate 0 based data and get the next disk. */
+ if (!protocol || (virDomainIsActive(dom) != 1) ||
+ virGetLastErrorCode() != VIR_ERR_INTERNAL_ERROR ||
+ virGetLastErrorDomain() != VIR_FROM_STORAGE)
+ return false;
+
+ memset(&info, 0, sizeof(info));
+ vshResetLibvirtError();
+ }
+ } else {
+ /* if we don't call virDomainGetBlockInfo() who clears 'info'
+ * we have to do it manually */
+ memset(&info, 0, sizeof(info));
+ }
if (!cmdDomblkinfoGet(&info, &cap, &alloc, &phy, human))
return false;
- vshPrint(ctl, "%-15s %s\n", _("Capacity:"), cap);
- vshPrint(ctl, "%-15s %s\n", _("Allocation:"), alloc);
- vshPrint(ctl, "%-15s %s\n", _("Physical:"), phy);
+ if (vshTableRowAppend(table, target, cap, alloc, phy, NULL) < 0)
+ return false;
}
+ vshTablePrintToStdout(table, ctl);
return true;
}
@@ -584,8 +577,6 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptBool(cmd, "inactive"))
flags |= VIR_DOMAIN_XML_INACTIVE;
- details = vshCommandOptBool(cmd, "details");
-
if (virshDomainGetXML(ctl, cmd, flags, &xmldoc, &ctxt) < 0)
return false;
@@ -593,7 +584,7 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd)
if (ndisks < 0)
return false;
- if (details)
+ if ((details = vshCommandOptBool(cmd, "details")))
table = vshTableNew(_("Type"), _("Device"), _("Target"), _("Source"), NULL);
else
table = vshTableNew(_("Target"), _("Source"), NULL);
@@ -618,8 +609,7 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd)
}
}
- target = virXPathString("string(./target/@dev)", ctxt);
- if (!target) {
+ if (!(target = virXPathString("string(./target/@dev)", ctxt))) {
vshError(ctl, "unable to query block list");
return false;
}
@@ -648,19 +638,16 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd)
"|./source/@path)", ctxt);
}
- if (details) {
- if (vshTableRowAppend(table, type, device, target,
- NULLSTR_MINUS(source), NULL) < 0)
- return false;
- } else {
- if (vshTableRowAppend(table, target,
- NULLSTR_MINUS(source), NULL) < 0)
- return false;
- }
+ if (details && (vshTableRowAppend(table, type, device, target,
+ NULLSTR_MINUS(source), NULL) < 0))
+ return false;
+
+ if (!details && (vshTableRowAppend(table, target,
+ NULLSTR_MINUS(source), NULL) < 0))
+ return false;
}
vshTablePrintToStdout(table, ctl);
-
return true;
}
@@ -808,16 +795,15 @@ cmdDomIfGetLink(vshControl *ctl, const vshCmd *cmd)
return false;
}
- if (ninterfaces < 1) {
- if (macstr[0])
+ if (ninterfaces != 1) {
+ if (ninterfaces > 1)
+ vshError(ctl, _("multiple matching interfaces found"));
+ else if (macstr[0])
vshError(ctl, _("Interface (mac: %s) not found."), macstr);
else
vshError(ctl, _("Interface (dev: %s) not found."), iface);
return false;
- } else if (ninterfaces > 1) {
- vshError(ctl, _("multiple matching interfaces found"));
- return false;
}
ctxt->node = interfaces[0];
@@ -952,8 +938,7 @@ cmdDomblkstat(vshControl *ctl, const vshCmd *cmd)
virDomainBlockStatsStruct stats;
g_autofree virTypedParameterPtr params = NULL;
virTypedParameterPtr par = NULL;
- const char *field = NULL;
- int rc, nparams = 0;
+ int nparams = 0;
size_t i;
bool human = vshCommandOptBool(cmd, "human"); /* human readable output */
@@ -970,13 +955,11 @@ cmdDomblkstat(vshControl *ctl, const vshCmd *cmd)
if (!device)
device = "";
- rc = virDomainBlockStatsFlags(dom, device, NULL, &nparams, 0);
-
/* It might fail when virDomainBlockStatsFlags is not
* supported on older libvirt, fallback to use virDomainBlockStats
* then.
*/
- if (rc < 0) {
+ if (virDomainBlockStatsFlags(dom, device, NULL, &nparams, 0) < 0) {
/* try older API if newer is not supported */
if (last_error->code != VIR_ERR_NO_SUPPORT)
return false;
@@ -1001,57 +984,58 @@ cmdDomblkstat(vshControl *ctl, const vshCmd *cmd)
DOMBLKSTAT_LEGACY_PRINT(2, stats.wr_req);
DOMBLKSTAT_LEGACY_PRINT(3, stats.wr_bytes);
DOMBLKSTAT_LEGACY_PRINT(4, stats.errs);
- } else {
- params = g_new0(virTypedParameter, nparams);
- if (virDomainBlockStatsFlags(dom, device, params, &nparams, 0) < 0) {
- vshError(ctl, _("Failed to get block stats for domain '%s' device '%s'"), name, device);
- return false;
- }
+ return true;
+ }
- /* set for prettier output */
- if (human) {
- vshPrint(ctl, N_("Device: %s\n"), device);
- device = "";
- }
+ params = g_new0(virTypedParameter, nparams);
+ if (virDomainBlockStatsFlags(dom, device, params, &nparams, 0) < 0) {
+ vshError(ctl, _("Failed to get block stats for domain '%s' device '%s'"), name, device);
+ return false;
+ }
- /* at first print all known values in desired order */
- for (i = 0; domblkstat_output[i].field != NULL; i++) {
- g_autofree char *value = NULL;
+ /* set for prettier output */
+ if (human) {
+ vshPrint(ctl, N_("Device: %s\n"), device);
+ device = "";
+ }
- if (!(par = virTypedParamsGet(params, nparams,
- domblkstat_output[i].field)))
- continue;
+ /* at first print all known values in desired order */
+ for (i = 0; domblkstat_output[i].field != NULL; i++) {
+ g_autofree char *value = NULL;
+ const char *field = NULL;
- value = vshGetTypedParamValue(ctl, par);
+ if (!(par = virTypedParamsGet(params, nparams,
+ domblkstat_output[i].field)))
+ continue;
- /* to print other not supported fields, mark the already printed */
- par->field[0] = '\0'; /* set the name to empty string */
+ value = vshGetTypedParamValue(ctl, par);
- /* translate into human readable or legacy spelling */
- field = NULL;
- if (human)
- field = _(domblkstat_output[i].human);
- else
- field = domblkstat_output[i].legacy;
+ /* to print other not supported fields, mark the already printed */
+ par->field[0] = '\0'; /* set the name to empty string */
- /* use the provided spelling if no translation is available */
- if (!field)
- field = domblkstat_output[i].field;
+ /* translate into human readable or legacy spelling */
+ if (human)
+ field = domblkstat_output[i].human;
+ else
+ field = domblkstat_output[i].legacy;
- vshPrint(ctl, "%s %-*s %s\n", device,
- human ? 31 : 0, field, value);
- }
+ /* use the provided spelling if no translation is available */
+ if (!field)
+ field = domblkstat_output[i].field;
+
+ vshPrint(ctl, "%s %-*s %s\n", device,
+ human ? 31 : 0, field, value);
+ }
- /* go through the fields again, for remaining fields */
- for (i = 0; i < nparams; i++) {
- g_autofree char *value = NULL;
+ /* go through the fields again, for remaining fields */
+ for (i = 0; i < nparams; i++) {
+ g_autofree char *value = NULL;
- if (!*params[i].field)
- continue;
+ if (!*params[i].field)
+ continue;
- value = vshGetTypedParamValue(ctl, params+i);
- vshPrint(ctl, "%s %s %s\n", device, params[i].field, value);
- }
+ value = vshGetTypedParamValue(ctl, params+i);
+ vshPrint(ctl, "%s %s %s\n", device, params[i].field, value);
}
return true;
@@ -1292,11 +1276,9 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd)
/* Security model and label information */
memset(&secmodel, 0, sizeof(secmodel));
if (virNodeGetSecurityModel(priv->conn, &secmodel) == -1) {
- if (last_error->code != VIR_ERR_NO_SUPPORT) {
+ if (last_error->code != VIR_ERR_NO_SUPPORT)
return false;
- } else {
- vshResetLibvirtError();
- }
+ vshResetLibvirtError();
} else {
/* Only print something if a security model is active */
if (secmodel.model[0] != '\0') {
@@ -1309,10 +1291,11 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd)
if (virDomainGetSecurityLabel(dom, seclabel) == -1) {
VIR_FREE(seclabel);
return false;
- } else {
- if (seclabel->label[0] != '\0')
- vshPrint(ctl, "%-15s %s (%s)\n", _("Security label:"),
- seclabel->label, seclabel->enforcing ? "enforcing" : "permissive");
+ }
+
+ if (seclabel->label[0] != '\0') {
+ vshPrint(ctl, "%-15s %s (%s)\n", _("Security label:"),
+ seclabel->label, seclabel->enforcing ? "enforcing" : "permissive");
}
VIR_FREE(seclabel);
@@ -1421,7 +1404,6 @@ cmdDomTime(vshControl *ctl, const vshCmd *cmd)
long long seconds = 0;
unsigned int nseconds = 0;
unsigned int flags = 0;
- bool doSet = false;
int rv;
VSH_EXCLUSIVE_OPTIONS("time", "now");
@@ -1433,15 +1415,12 @@ cmdDomTime(vshControl *ctl, const vshCmd *cmd)
rv = vshCommandOptLongLong(ctl, cmd, "time", &seconds);
- if (rv < 0) {
- /* invalid integer format */
+ /* invalid integer format */
+ if (rv < 0)
return false;
- } else if (rv > 0) {
- /* valid integer to set */
- doSet = true;
- }
- if (doSet || now || rtcSync) {
+ /* valid integer to set */
+ if (rv > 0 || now || rtcSync) {
if (now && ((seconds = time(NULL)) == (time_t) -1)) {
vshError(ctl, _("Unable to get current time"));
return false;
@@ -1453,21 +1432,22 @@ cmdDomTime(vshControl *ctl, const vshCmd *cmd)
if (virDomainSetTime(dom, seconds, nseconds, flags) < 0)
return false;
- } else {
- if (virDomainGetTime(dom, &seconds, &nseconds, flags) < 0)
- return false;
+ return true;
+ }
- if (pretty) {
- g_autoptr(GDateTime) then = NULL;
- g_autofree char *thenstr = NULL;
+ if (virDomainGetTime(dom, &seconds, &nseconds, flags) < 0)
+ return false;
- then = g_date_time_new_from_unix_utc(seconds);
- thenstr = g_date_time_format(then, "%Y-%m-%d %H:%M:%S");
+ if (pretty) {
+ g_autoptr(GDateTime) then = NULL;
+ g_autofree char *thenstr = NULL;
- vshPrint(ctl, _("Time: %s"), thenstr);
- } else {
- vshPrint(ctl, _("Time: %lld"), seconds);
- }
+ then = g_date_time_new_from_unix_utc(seconds);
+ thenstr = g_date_time_format(then, "%Y-%m-%d %H:%M:%S");
+
+ vshPrint(ctl, _("Time: %s"), thenstr);
+ } else {
+ vshPrint(ctl, _("Time: %lld"), seconds);
}
return true;
@@ -1508,17 +1488,15 @@ virshDomainSorter(const void *a, const void *b)
if (ida == inactive && idb == inactive)
return vshStrcasecmp(virDomainGetName(*da), virDomainGetName(*db));
- if (ida != inactive && idb != inactive) {
+ if (ida != inactive && idb != inactive && ida != idb) {
if (ida > idb)
return 1;
- else if (ida < idb)
- return -1;
+ return -1;
}
if (ida != inactive)
return -1;
- else
- return 1;
+ return 1;
}
struct virshDomainList {
@@ -1994,10 +1972,9 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
vshPrint(ctl, "%s%s", sep, id_buf);
sep = " ";
}
- if (optName) {
+ if (optName)
vshPrint(ctl, "%s%s", sep, virDomainGetName(dom));
- sep = " ";
- }
+
vshPrint(ctl, "\n");
}
}
@@ -2372,13 +2349,14 @@ cmdDomIfAddr(vshControl *ctl, const vshCmd *cmd)
ip_addr_str = g_strdup("");
/* Don't repeat interface name */
- if (full || !j)
+ if (full || !j) {
vshPrint(ctl, " %-10s %-17s %s\n",
iface->name,
NULLSTR_EMPTY(iface->hwaddr), ip_addr_str);
- else
+ } else {
vshPrint(ctl, " %-10s %-17s %s\n",
"-", "-", ip_addr_str);
+ }
}
}
--
2.31.1
More information about the libvir-list
mailing list