[PATCH 2/5] virsh: domain: refactoring of small functions
Kristina Hanicova
khanicov at redhat.com
Thu Sep 23 15:08:01 UTC 2021
This patch includes:
* fix of a mistake in cmdMigrateSetMaxDowntime() - function
returned false when it should have returned true
* use of an early return in case of an error
* removal of unnecessary variables and extra labels
* returning value directly
Signed-off-by: Kristina Hanicova <khanicov at redhat.com>
---
tools/virsh-domain.c | 250 +++++++++++++++++--------------------------
1 file changed, 98 insertions(+), 152 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index f876f30cc5..3232463485 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -245,18 +245,18 @@ static virDomainPtr
virshDomainDefine(virConnectPtr conn, const char *xml, unsigned int flags)
{
virDomainPtr dom;
- if (flags) {
- dom = virDomainDefineXMLFlags(conn, xml, flags);
- /* If validate is the only flag, just drop it and
- * try again.
- */
- if (!dom) {
- if ((virGetLastErrorCode() == VIR_ERR_NO_SUPPORT) &&
- (flags == VIR_DOMAIN_DEFINE_VALIDATE))
- dom = virDomainDefineXML(conn, xml);
- }
- } else {
- dom = virDomainDefineXML(conn, xml);
+
+ if (!flags)
+ return virDomainDefineXML(conn, xml);
+
+ dom = virDomainDefineXMLFlags(conn, xml, flags);
+ /* If validate is the only flag, just drop it and
+ * try again.
+ */
+ if (!dom) {
+ if ((virGetLastErrorCode() == VIR_ERR_NO_SUPPORT) &&
+ (flags == VIR_DOMAIN_DEFINE_VALIDATE))
+ dom = virDomainDefineXML(conn, xml);
}
return dom;
}
@@ -659,12 +659,10 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
return false;
}
- if (mode) {
- if (STRNEQ(mode, "readonly") && STRNEQ(mode, "shareable")) {
- vshError(ctl, _("No support for %s in command 'attach-disk'"),
- mode);
- return false;
- }
+ if (mode && STRNEQ(mode, "readonly") && STRNEQ(mode, "shareable")) {
+ vshError(ctl, _("No support for %s in command 'attach-disk'"),
+ mode);
+ return false;
}
if (wwn && !virValidateWWN(wwn))
@@ -2705,7 +2703,6 @@ virshBlockJobAbort(virDomainPtr dom,
static bool
cmdBlockjob(vshControl *ctl, const vshCmd *cmd)
{
- bool ret = false;
bool raw = vshCommandOptBool(cmd, "raw");
bool bytes = vshCommandOptBool(cmd, "bytes");
bool abortMode = vshCommandOptBool(cmd, "abort");
@@ -2738,13 +2735,10 @@ cmdBlockjob(vshControl *ctl, const vshCmd *cmd)
return false;
if (bandwidth)
- ret = virshBlockJobSetSpeed(ctl, cmd, dom, path, bytes);
- else if (abortMode || pivot || async)
- ret = virshBlockJobAbort(dom, path, pivot, async);
- else
- ret = virshBlockJobInfo(ctl, dom, path, raw, bytes);
-
- return ret;
+ return virshBlockJobSetSpeed(ctl, cmd, dom, path, bytes);
+ if (abortMode || pivot || async)
+ return virshBlockJobAbort(dom, path, pivot, async);
+ return virshBlockJobInfo(ctl, dom, path, raw, bytes);
}
/*
@@ -2930,7 +2924,6 @@ cmdBlockresize(vshControl *ctl, const vshCmd *cmd)
const char *path = NULL;
unsigned long long size = 0;
unsigned int flags = 0;
- bool ret = false;
if (vshCommandOptStringReq(ctl, cmd, "path", (const char **) &path) < 0)
return false;
@@ -2949,12 +2942,11 @@ cmdBlockresize(vshControl *ctl, const vshCmd *cmd)
if (virDomainBlockResize(dom, path, size, flags) < 0) {
vshError(ctl, _("Failed to resize block device '%s'"), path);
- } else {
- vshPrintExtra(ctl, _("Block device '%s' is resized"), path);
- ret = true;
+ return false;
}
- return ret;
+ vshPrintExtra(ctl, _("Block device '%s' is resized"), path);
+ return true;
}
#ifndef WIN32
@@ -3428,19 +3420,17 @@ cmdSuspend(vshControl *ctl, const vshCmd *cmd)
{
g_autoptr(virshDomain) dom = NULL;
const char *name;
- bool ret = true;
if (!(dom = virshCommandOptDomain(ctl, cmd, &name)))
return false;
- if (virDomainSuspend(dom) == 0) {
- vshPrintExtra(ctl, _("Domain '%s' suspended\n"), name);
- } else {
+ if (virDomainSuspend(dom) != 0) {
vshError(ctl, _("Failed to suspend domain '%s'"), name);
- ret = false;
+ return false;
}
- return ret;
+ vshPrintExtra(ctl, _("Domain '%s' suspended\n"), name);
+ return true;
}
/*
@@ -5066,7 +5056,6 @@ cmdSchedInfoUpdateOne(vshControl *ctl,
const char *field, const char *value)
{
virTypedParameterPtr param;
- int ret = -1;
size_t i;
for (i = 0; i < nsrc_params; i++) {
@@ -5081,14 +5070,11 @@ cmdSchedInfoUpdateOne(vshControl *ctl,
vshSaveLibvirtError();
return -1;
}
- ret = 0;
- break;
+ return 0;
}
- if (ret < 0)
- vshError(ctl, _("invalid scheduler option: %s"), field);
-
- return ret;
+ vshError(ctl, _("invalid scheduler option: %s"), field);
+ return -1;
}
static int
@@ -5539,7 +5525,6 @@ virshGenFileName(vshControl *ctl, virDomainPtr dom, const char *mime)
g_autoptr(GDateTime) now = g_date_time_new_now_local();
g_autofree char *nowstr = NULL;
const char *ext = NULL;
- char *ret = NULL;
if (!dom) {
vshError(ctl, "%s", _("Invalid domain supplied"));
@@ -5554,10 +5539,8 @@ virshGenFileName(vshControl *ctl, virDomainPtr dom, const char *mime)
nowstr = g_date_time_format(now, "%Y-%m-%d-%H:%M:%S");
- ret = g_strdup_printf("%s-%s%s", virDomainGetName(dom),
- nowstr, NULLSTR_EMPTY(ext));
-
- return ret;
+ return g_strdup_printf("%s-%s%s", virDomainGetName(dom),
+ nowstr, NULLSTR_EMPTY(ext));
}
static bool
@@ -5696,7 +5679,6 @@ static bool
cmdSetLifecycleAction(vshControl *ctl, const vshCmd *cmd)
{
g_autoptr(virshDomain) dom = NULL;
- bool ret = true;
bool config = vshCommandOptBool(cmd, "config");
bool live = vshCommandOptBool(cmd, "live");
bool current = vshCommandOptBool(cmd, "current");
@@ -5737,10 +5719,9 @@ cmdSetLifecycleAction(vshControl *ctl, const vshCmd *cmd)
if (virDomainSetLifecycleAction(dom, type, action, flags) < 0) {
vshError(ctl, "%s", _("Unable to change lifecycle action."));
- ret = false;
+ return false;
}
-
- return ret;
+ return true;
}
/*
@@ -5825,20 +5806,18 @@ static bool
cmdResume(vshControl *ctl, const vshCmd *cmd)
{
g_autoptr(virshDomain) dom = NULL;
- bool ret = true;
const char *name;
if (!(dom = virshCommandOptDomain(ctl, cmd, &name)))
return false;
- if (virDomainResume(dom) == 0) {
- vshPrintExtra(ctl, _("Domain '%s' resumed\n"), name);
- } else {
+ if (virDomainResume(dom) != 0) {
vshError(ctl, _("Failed to resume domain '%s'"), name);
- ret = false;
+ return false;
}
- return ret;
+ vshPrintExtra(ctl, _("Domain '%s' resumed\n"), name);
+ return true;
}
/*
@@ -5912,13 +5891,13 @@ cmdShutdown(vshControl *ctl, const vshCmd *cmd)
rv = virDomainShutdownFlags(dom, flags);
else
rv = virDomainShutdown(dom);
- if (rv == 0) {
- vshPrintExtra(ctl, _("Domain '%s' is being shutdown\n"), name);
- } else {
+
+ if (rv != 0) {
vshError(ctl, _("Failed to shutdown domain '%s'"), name);
return false;
}
+ vshPrintExtra(ctl, _("Domain '%s' is being shutdown\n"), name);
return true;
}
@@ -5988,13 +5967,12 @@ cmdReboot(vshControl *ctl, const vshCmd *cmd)
if (!(dom = virshCommandOptDomain(ctl, cmd, &name)))
return false;
- if (virDomainReboot(dom, flags) == 0) {
- vshPrintExtra(ctl, _("Domain '%s' is being rebooted\n"), name);
- } else {
+ if (virDomainReboot(dom, flags) != 0) {
vshError(ctl, _("Failed to reboot domain '%s'"), name);
return false;
}
+ vshPrintExtra(ctl, _("Domain '%s' is being rebooted\n"), name);
return true;
}
@@ -6020,20 +5998,18 @@ static bool
cmdReset(vshControl *ctl, const vshCmd *cmd)
{
g_autoptr(virshDomain) dom = NULL;
- bool ret = true;
const char *name;
if (!(dom = virshCommandOptDomain(ctl, cmd, &name)))
return false;
- if (virDomainReset(dom, 0) == 0) {
- vshPrintExtra(ctl, _("Domain '%s' was reset\n"), name);
- } else {
+ if (virDomainReset(dom, 0) != 0) {
vshError(ctl, _("Failed to reset domain '%s'"), name);
- ret = false;
+ return false;
}
- return ret;
+ vshPrintExtra(ctl, _("Domain '%s' was reset\n"), name);
+ return true;
}
/*
@@ -6479,15 +6455,14 @@ static bool
cmdDomjobabort(vshControl *ctl, const vshCmd *cmd)
{
g_autoptr(virshDomain) dom = NULL;
- bool ret = true;
if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
return false;
if (virDomainAbortJob(dom) < 0)
- ret = false;
+ return false;
- return ret;
+ return true;
}
/*
@@ -7042,8 +7017,7 @@ virshParseCPUList(vshControl *ctl, int *cpumaplen,
}
}
- if (virBitmapToData(map, &cpumap, cpumaplen) < 0)
- goto cleanup;
+ virBitmapToData(map, &cpumap, cpumaplen);
cleanup:
virBitmapFree(map);
@@ -8212,7 +8186,6 @@ cmdDefine(vshControl *ctl, const vshCmd *cmd)
{
g_autoptr(virshDomain) dom = NULL;
const char *from = NULL;
- bool ret = true;
char *buffer;
unsigned int flags = 0;
virshControl *priv = ctl->privData;
@@ -8232,14 +8205,14 @@ cmdDefine(vshControl *ctl, const vshCmd *cmd)
dom = virDomainDefineXML(priv->conn, buffer);
VIR_FREE(buffer);
- if (dom != NULL) {
- vshPrintExtra(ctl, _("Domain '%s' defined from %s\n"),
- virDomainGetName(dom), from);
- } else {
+ if (!dom) {
vshError(ctl, _("Failed to define domain from %s"), from);
- ret = false;
+ return false;
}
- return ret;
+
+ vshPrintExtra(ctl, _("Domain '%s' defined from %s\n"),
+ virDomainGetName(dom), from);
+ return true;
}
/*
@@ -8268,7 +8241,6 @@ static bool
cmdDestroy(vshControl *ctl, const vshCmd *cmd)
{
g_autoptr(virshDomain) dom = NULL;
- bool ret = true;
const char *name;
unsigned int flags = 0;
int result;
@@ -8284,14 +8256,13 @@ cmdDestroy(vshControl *ctl, const vshCmd *cmd)
else
result = virDomainDestroy(dom);
- if (result == 0) {
- vshPrintExtra(ctl, _("Domain '%s' destroyed\n"), name);
- } else {
+ if (result < 0) {
vshError(ctl, _("Failed to destroy domain '%s'"), name);
- ret = false;
+ return false;
}
- return ret;
+ vshPrintExtra(ctl, _("Domain '%s' destroyed\n"), name);
+ return true;
}
/*
@@ -9990,7 +9961,6 @@ static bool
cmdDumpXML(vshControl *ctl, const vshCmd *cmd)
{
g_autoptr(virshDomain) dom = NULL;
- bool ret = true;
g_autofree char *dump = NULL;
unsigned int flags = 0;
bool inactive = vshCommandOptBool(cmd, "inactive");
@@ -10010,14 +9980,11 @@ cmdDumpXML(vshControl *ctl, const vshCmd *cmd)
if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
return false;
- dump = virDomainGetXMLDesc(dom, flags);
- if (dump != NULL) {
- vshPrint(ctl, "%s", dump);
- } else {
- ret = false;
- }
+ if (!(dump = virDomainGetXMLDesc(dom, flags)))
+ return false;
- return ret;
+ vshPrint(ctl, "%s", dump);
+ return true;
}
/*
@@ -10051,7 +10018,6 @@ static const vshCmdOptDef opts_domxmlfromnative[] = {
static bool
cmdDomXMLFromNative(vshControl *ctl, const vshCmd *cmd)
{
- bool ret = true;
const char *format = NULL;
const char *configFile = NULL;
g_autofree char *configData = NULL;
@@ -10067,13 +10033,11 @@ cmdDomXMLFromNative(vshControl *ctl, const vshCmd *cmd)
return false;
xmlData = virConnectDomainXMLFromNative(priv->conn, format, configData, flags);
- if (xmlData != NULL) {
- vshPrint(ctl, "%s", xmlData);
- } else {
- ret = false;
- }
+ if (!xmlData)
+ return false;
- return ret;
+ vshPrint(ctl, "%s", xmlData);
+ return true;
}
/*
@@ -11006,25 +10970,19 @@ cmdMigrateSetMaxDowntime(vshControl *ctl, const vshCmd *cmd)
{
g_autoptr(virshDomain) dom = NULL;
unsigned long long downtime = 0;
- bool ret = false;
if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
return false;
if (vshCommandOptULongLong(ctl, cmd, "downtime", &downtime) < 0)
- goto done;
+ return false;
+
if (downtime < 1) {
vshError(ctl, "%s", _("migrate: Invalid downtime"));
- goto done;
+ return false;
}
- if (virDomainMigrateSetMaxDowntime(dom, downtime, 0))
- goto done;
-
- ret = true;
-
- done:
- return ret;
+ return virDomainMigrateSetMaxDowntime(dom, downtime, 0) == 0;
}
@@ -11100,12 +11058,12 @@ cmdMigrateCompCache(vshControl *ctl, const vshCmd *cmd)
return false;
rc = vshCommandOptULongLong(ctl, cmd, "size", &size);
- if (rc < 0) {
+ if (rc < 0)
+ return false;
+
+ if (rc != 0 &&
+ (virDomainMigrateSetCompressionCache(dom, size, 0) < 0))
return false;
- } else if (rc != 0) {
- if (virDomainMigrateSetCompressionCache(dom, size, 0) < 0)
- return false;
- }
if (virDomainMigrateGetCompressionCache(dom, &size, 0) < 0)
return false;
@@ -11486,11 +11444,9 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
/* We got what we came for so return successfully */
ret = true;
- if (!all) {
+ if (!all)
break;
- } else {
- vshPrint(ctl, "\n");
- }
+ vshPrint(ctl, "\n");
}
if (!ret) {
@@ -11947,12 +11903,12 @@ virshDomainDetachInterface(char *doc,
xmlNodePtr cur = NULL, matchNode = NULL;
g_autofree char *detach_xml = NULL;
char buf[64];
- int diff_mac, ret = -1;
+ int diff_mac = -1;
size_t i;
if (!(xml = virXMLParseStringCtxt(doc, _("(domain_definition)"), &ctxt))) {
vshError(ctl, "%s", _("Failed to get interface information"));
- goto cleanup;
+ return false;
}
g_snprintf(buf, sizeof(buf), "/domain/devices/interface[@type='%s']", type);
@@ -11960,13 +11916,13 @@ virshDomainDetachInterface(char *doc,
if (obj == NULL || obj->type != XPATH_NODESET ||
obj->nodesetval == NULL || obj->nodesetval->nodeNr == 0) {
vshError(ctl, _("No interface found whose type is %s"), type);
- goto cleanup;
+ return false;
}
if (!mac && obj->nodesetval->nodeNr > 1) {
vshError(ctl, _("Domain has %d interfaces. Please specify which one "
"to detach using --mac"), obj->nodesetval->nodeNr);
- goto cleanup;
+ return false;
}
if (!mac) {
@@ -11989,7 +11945,7 @@ virshDomainDetachInterface(char *doc,
"MAC address %s. You must use detach-device and "
"specify the device pci address to remove it."),
mac);
- goto cleanup;
+ return false;
}
matchNode = obj->nodesetval->nodeTab[i];
}
@@ -11999,22 +11955,18 @@ virshDomainDetachInterface(char *doc,
}
if (!matchNode) {
vshError(ctl, _("No interface with MAC address %s was found"), mac);
- goto cleanup;
+ return false;
}
hit:
if (!(detach_xml = virXMLNodeToString(xml, matchNode))) {
vshSaveLibvirtError();
- goto cleanup;
+ return false;
}
if (flags != 0 || current)
- ret = virDomainDetachDeviceFlags(dom, detach_xml, flags);
- else
- ret = virDomainDetachDevice(dom, detach_xml);
-
- cleanup:
- return ret == 0;
+ return virDomainDetachDeviceFlags(dom, detach_xml, flags) == 0;
+ return virDomainDetachDevice(dom, detach_xml) == 0;
}
@@ -13666,10 +13618,10 @@ static bool
cmdDomFSFreeze(vshControl *ctl, const vshCmd *cmd)
{
g_autoptr(virshDomain) dom = NULL;
- int ret = -1;
const vshCmdOpt *opt = NULL;
g_autofree const char **mountpoints = NULL;
size_t nmountpoints = 0;
+ int count = 0;
if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
return false;
@@ -13679,16 +13631,13 @@ cmdDomFSFreeze(vshControl *ctl, const vshCmd *cmd)
mountpoints[nmountpoints-1] = opt->data;
}
- ret = virDomainFSFreeze(dom, mountpoints, nmountpoints, 0);
- if (ret < 0) {
+ if ((count = virDomainFSFreeze(dom, mountpoints, nmountpoints, 0)) < 0) {
vshError(ctl, _("Unable to freeze filesystems"));
- goto cleanup;
+ return false;
}
- vshPrintExtra(ctl, _("Froze %d filesystem(s)\n"), ret);
-
- cleanup:
- return ret >= 0;
+ vshPrintExtra(ctl, _("Froze %d filesystem(s)\n"), count);
+ return true;
}
static const vshCmdInfo info_domfsthaw[] = {
@@ -13714,10 +13663,10 @@ static bool
cmdDomFSThaw(vshControl *ctl, const vshCmd *cmd)
{
g_autoptr(virshDomain) dom = NULL;
- int ret = -1;
const vshCmdOpt *opt = NULL;
g_autofree const char **mountpoints = NULL;
size_t nmountpoints = 0;
+ int count = 0;
if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
return false;
@@ -13727,16 +13676,13 @@ cmdDomFSThaw(vshControl *ctl, const vshCmd *cmd)
mountpoints[nmountpoints-1] = opt->data;
}
- ret = virDomainFSThaw(dom, mountpoints, nmountpoints, 0);
- if (ret < 0) {
+ if ((count = virDomainFSThaw(dom, mountpoints, nmountpoints, 0)) < 0) {
vshError(ctl, _("Unable to thaw filesystems"));
- goto cleanup;
+ return false;
}
- vshPrintExtra(ctl, _("Thawed %d filesystem(s)\n"), ret);
-
- cleanup:
- return ret >= 0;
+ vshPrintExtra(ctl, _("Thawed %d filesystem(s)\n"), count);
+ return true;
}
static const vshCmdInfo info_domfsinfo[] = {
--
2.31.1
More information about the libvir-list
mailing list