From nobody Mon Feb 9 23:05:28 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) client-ip=170.10.133.124; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-124.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1632409714; cv=none; d=zohomail.com; s=zohoarc; b=HjyiccpPs84tOruEAn+DQyFr06PiZssU5NCAshrcBbfujVg7ST0Qq9MjQIH0hEhXB5BCCUmwSVsztqFVnyistUl9N7wX13fywAajvrwnWHhEdi4X5m5OA6iDePDh1h5MLtp74uhXN/7hJZNA86qXzl+rTUsRVzz5Ep6+8lN+exw= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1632409714; h=Content-Type:Content-Transfer-Encoding:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=dVCMt/PnBUMCamRXhJn9ob8tzp3mYH8knPS1DwkCu4E=; b=WIRp/BPhZw4TmNe4iUTyWFv0Wmwk9nHxzth9z4emoqv6AeblSGBz58s+mBhvLk3LgUc0vTUblmESpt5/EpFAXzmVqNtpyIawM6FL4qWkYoz99GbBdKOxTFuUof+ZOeXg0c9hRCzpp9b+iuza0uuq6RDGSQEGx0F//J1I1nsm0Sc= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.zohomail.com with SMTPS id 1632409714673932.0983327361237; Thu, 23 Sep 2021 08:08:34 -0700 (PDT) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-22-TH8GJsRSO_mdK13X3Z1r4Q-1; Thu, 23 Sep 2021 11:08:31 -0400 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 2B9F49127D; Thu, 23 Sep 2021 15:08:26 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 0742C652A5; Thu, 23 Sep 2021 15:08:25 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 6EBB74EA29; Thu, 23 Sep 2021 15:08:22 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 18NF8Lg7030560 for ; Thu, 23 Sep 2021 11:08:21 -0400 Received: by smtp.corp.redhat.com (Postfix) id 3EB53177C0; Thu, 23 Sep 2021 15:08:21 +0000 (UTC) Received: from fedora.redhat.com (unknown [10.43.2.78]) by smtp.corp.redhat.com (Postfix) with ESMTP id 578425F707 for ; Thu, 23 Sep 2021 15:08:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1632409713; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=dVCMt/PnBUMCamRXhJn9ob8tzp3mYH8knPS1DwkCu4E=; b=NOqT5bQEfgyKC90vTqclC+PBHqLadoEK/IWxAodIBVl9d3qJrekLe91uz/1ffQPDstujYb 2UR4jqyX0L4tKcVenYV2ccNI7Rh1cMzlEyP+2kMJkW4wuYuKIYkvPHI0ZpYNrAJg3YlpJQ xPXfsSlFb3FNw0pMkVJZD8NlCb+T+/U= X-MC-Unique: TH8GJsRSO_mdK13X3Z1r4Q-1 From: Kristina Hanicova To: libvir-list@redhat.com Subject: [PATCH 1/5] virsh: checkpoint & domain-monitor: small refactoring Date: Thu, 23 Sep 2021 17:08:00 +0200 Message-Id: In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-loop: libvir-list@redhat.com X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=libvir-list-bounces@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) X-ZM-MESSAGEID: 1632409715887100001 Content-Type: text/plain; charset="utf-8" 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 --- 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; =20 - if (chkname) { - *chk =3D virDomainCheckpointLookupByName(dom, chkname, 0); - } else { + if (!chkname) { vshError(ctl, _("--%s is required"), arg); return -1; } + + *chk =3D virDomainCheckpointLookupByName(dom, chkname, 0); if (!*chk) { vshReportError(ctl); return -1; @@ -931,13 +931,13 @@ cmdCheckpointParent(vshControl *ctl, =20 if (virshGetCheckpointParent(ctl, checkpoint, &parent) < 0) return false; + if (!parent) { vshError(ctl, _("checkpoint '%s' has no parent"), name); return false; } =20 vshPrint(ctl, "%s", parent); - return true; } =20 @@ -1002,16 +1002,15 @@ cmdCheckpointDelete(vshControl *ctl, if (vshCommandOptBool(cmd, "metadata")) flags |=3D VIR_DOMAIN_CHECKPOINT_DELETE_METADATA_ONLY; =20 - if (virDomainCheckpointDelete(checkpoint, flags) =3D=3D 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; } =20 + if (flags & VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN_ONLY) + vshPrintExtra(ctl, _("Domain checkpoint %s children deleted\n"), n= ame); + else + vshPrintExtra(ctl, _("Domain checkpoint %s deleted\n"), name); return true; } =20 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 d= om, bool title, g_autoptr(xmlDoc) doc =3D NULL; g_autoptr(xmlXPathContext) ctxt =3D NULL; int type; + int errCode; =20 if (title) type =3D VIR_DOMAIN_METADATA_TITLE; @@ -66,19 +67,18 @@ virshGetDomainDescription(vshControl *ctl, virDomainPtr= dom, bool title, =20 if ((desc =3D virDomainGetMetadata(dom, type, NULL, flags))) { return desc; - } else { - int errCode =3D virGetLastErrorCode(); - - if (errCode =3D=3D VIR_ERR_NO_DOMAIN_METADATA) { - desc =3D g_strdup(""); - vshResetLibvirtError(); - return desc; - } + } + errCode =3D virGetLastErrorCode(); =20 - if (errCode !=3D VIR_ERR_NO_SUPPORT) - return desc; + if (errCode =3D=3D VIR_ERR_NO_DOMAIN_METADATA) { + desc =3D g_strdup(""); + vshResetLibvirtError(); + return desc; } =20 + if (errCode !=3D 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) =20 human =3D vshCommandOptBool(cmd, "human"); =20 - if (all) { - bool active =3D virDomainIsActive(dom) =3D=3D 1; - int rc; + if (!all) { + g_autofree char *cap =3D NULL; + g_autofree char *alloc =3D NULL; + g_autofree char *phy =3D NULL; =20 - if (virshDomainGetXML(ctl, cmd, 0, &xmldoc, &ctxt) < 0) + if (virDomainGetBlockInfo(dom, device, &info, 0) < 0) return false; =20 - ndisks =3D virXPathNodeSet("./devices/disk", ctxt, &disks); - if (ndisks < 0) + if (!cmdDomblkinfoGet(&info, &cap, &alloc, &phy, human)) return false; =20 - /* title */ - table =3D 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; + } =20 - for (i =3D 0; i < ndisks; i++) { - g_autofree char *target =3D NULL; - g_autofree char *protocol =3D NULL; - g_autofree char *cap =3D NULL; - g_autofree char *alloc =3D NULL; - g_autofree char *phy =3D NULL; - - ctxt->node =3D disks[i]; - protocol =3D virXPathString("string(./source/@protocol)", ctxt= ); - target =3D virXPathString("string(./target/@dev)", ctxt); - - if (virXPathBoolean("boolean(./source)", ctxt) =3D=3D 1) { - - rc =3D virDomainGetBlockInfo(dom, target, &info, 0); - - if (rc < 0) { - /* If protocol is present that's an indication of a - * networked storage device which cannot provide stati= stics, - * so generate 0 based data and get the next disk. */ - if (protocol && !active && - virGetLastErrorCode() =3D=3D VIR_ERR_INTERNAL_ERRO= R && - virGetLastErrorDomain() =3D=3D VIR_FROM_STORAGE) { - memset(&info, 0, sizeof(info)); - vshResetLibvirtError(); - } else { - return false; - } - } - } else { - /* if we don't call virDomainGetBlockInfo() who clears 'in= fo' - * we have to do it manually */ - memset(&info, 0, sizeof(info)); - } + if (virshDomainGetXML(ctl, cmd, 0, &xmldoc, &ctxt) < 0) + return false; =20 - if (!cmdDomblkinfoGet(&info, &cap, &alloc, &phy, human)) - return false; - if (vshTableRowAppend(table, target, cap, alloc, phy, NULL) < = 0) - return false; - } + ndisks =3D virXPathNodeSet("./devices/disk", ctxt, &disks); + if (ndisks < 0) + return false; =20 - vshTablePrintToStdout(table, ctl); + /* title */ + table =3D vshTableNew(_("Target"), _("Capacity"), _("Allocation"), _("= Physical"), NULL); + if (!table) + return false; =20 - } else { + for (i =3D 0; i < ndisks; i++) { + g_autofree char *target =3D NULL; + g_autofree char *protocol =3D NULL; g_autofree char *cap =3D NULL; g_autofree char *alloc =3D NULL; g_autofree char *phy =3D NULL; =20 - if (virDomainGetBlockInfo(dom, device, &info, 0) < 0) - return false; + ctxt->node =3D disks[i]; + protocol =3D virXPathString("string(./source/@protocol)", ctxt); + target =3D virXPathString("string(./target/@dev)", ctxt); + + if (virXPathBoolean("boolean(./source)", ctxt) =3D=3D 1) { + if (virDomainGetBlockInfo(dom, target, &info, 0) < 0) { + /* If protocol is present that's an indication of a + * networked storage device which cannot provide statistic= s, + * so generate 0 based data and get the next disk. */ + if (!protocol || (virDomainIsActive(dom) !=3D 1) || + virGetLastErrorCode() !=3D VIR_ERR_INTERNAL_ERROR || + virGetLastErrorDomain() !=3D 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)); + } =20 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; } =20 + vshTablePrintToStdout(table, ctl); return true; } =20 @@ -584,8 +577,6 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "inactive")) flags |=3D VIR_DOMAIN_XML_INACTIVE; =20 - details =3D vshCommandOptBool(cmd, "details"); - if (virshDomainGetXML(ctl, cmd, flags, &xmldoc, &ctxt) < 0) return false; =20 @@ -593,7 +584,7 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd) if (ndisks < 0) return false; =20 - if (details) + if ((details =3D vshCommandOptBool(cmd, "details"))) table =3D vshTableNew(_("Type"), _("Device"), _("Target"), _("Sour= ce"), NULL); else table =3D vshTableNew(_("Target"), _("Source"), NULL); @@ -618,8 +609,7 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd) } } =20 - target =3D virXPathString("string(./target/@dev)", ctxt); - if (!target) { + if (!(target =3D 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); } =20 - 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; } =20 vshTablePrintToStdout(table, ctl); - return true; } =20 @@ -808,16 +795,15 @@ cmdDomIfGetLink(vshControl *ctl, const vshCmd *cmd) return false; } =20 - if (ninterfaces < 1) { - if (macstr[0]) + if (ninterfaces !=3D 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); =20 return false; - } else if (ninterfaces > 1) { - vshError(ctl, _("multiple matching interfaces found")); - return false; } =20 ctxt->node =3D interfaces[0]; @@ -952,8 +938,7 @@ cmdDomblkstat(vshControl *ctl, const vshCmd *cmd) virDomainBlockStatsStruct stats; g_autofree virTypedParameterPtr params =3D NULL; virTypedParameterPtr par =3D NULL; - const char *field =3D NULL; - int rc, nparams =3D 0; + int nparams =3D 0; size_t i; bool human =3D vshCommandOptBool(cmd, "human"); /* human readable outp= ut */ =20 @@ -970,13 +955,11 @@ cmdDomblkstat(vshControl *ctl, const vshCmd *cmd) if (!device) device =3D ""; =20 - rc =3D 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 !=3D 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 =3D g_new0(virTypedParameter, nparams); - if (virDomainBlockStatsFlags(dom, device, params, &nparams, 0) < 0= ) { - vshError(ctl, _("Failed to get block stats for domain '%s' dev= ice '%s'"), name, device); - return false; - } + return true; + } =20 - /* set for prettier output */ - if (human) { - vshPrint(ctl, N_("Device: %s\n"), device); - device =3D ""; - } + params =3D 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; + } =20 - /* at first print all known values in desired order */ - for (i =3D 0; domblkstat_output[i].field !=3D NULL; i++) { - g_autofree char *value =3D NULL; + /* set for prettier output */ + if (human) { + vshPrint(ctl, N_("Device: %s\n"), device); + device =3D ""; + } =20 - if (!(par =3D virTypedParamsGet(params, nparams, - domblkstat_output[i].field))) - continue; + /* at first print all known values in desired order */ + for (i =3D 0; domblkstat_output[i].field !=3D NULL; i++) { + g_autofree char *value =3D NULL; + const char *field =3D NULL; =20 - value =3D vshGetTypedParamValue(ctl, par); + if (!(par =3D virTypedParamsGet(params, nparams, + domblkstat_output[i].field))) + continue; =20 - /* to print other not supported fields, mark the already print= ed */ - par->field[0] =3D '\0'; /* set the name to empty string */ + value =3D vshGetTypedParamValue(ctl, par); =20 - /* translate into human readable or legacy spelling */ - field =3D NULL; - if (human) - field =3D _(domblkstat_output[i].human); - else - field =3D domblkstat_output[i].legacy; + /* to print other not supported fields, mark the already printed */ + par->field[0] =3D '\0'; /* set the name to empty string */ =20 - /* use the provided spelling if no translation is available */ - if (!field) - field =3D domblkstat_output[i].field; + /* translate into human readable or legacy spelling */ + if (human) + field =3D domblkstat_output[i].human; + else + field =3D domblkstat_output[i].legacy; =20 - vshPrint(ctl, "%s %-*s %s\n", device, - human ? 31 : 0, field, value); - } + /* use the provided spelling if no translation is available */ + if (!field) + field =3D domblkstat_output[i].field; + + vshPrint(ctl, "%s %-*s %s\n", device, + human ? 31 : 0, field, value); + } =20 - /* go through the fields again, for remaining fields */ - for (i =3D 0; i < nparams; i++) { - g_autofree char *value =3D NULL; + /* go through the fields again, for remaining fields */ + for (i =3D 0; i < nparams; i++) { + g_autofree char *value =3D NULL; =20 - if (!*params[i].field) - continue; + if (!*params[i].field) + continue; =20 - value =3D vshGetTypedParamValue(ctl, params+i); - vshPrint(ctl, "%s %s %s\n", device, params[i].field, value); - } + value =3D vshGetTypedParamValue(ctl, params+i); + vshPrint(ctl, "%s %s %s\n", device, params[i].field, value); } =20 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) =3D=3D -1) { - if (last_error->code !=3D VIR_ERR_NO_SUPPORT) { + if (last_error->code !=3D VIR_ERR_NO_SUPPORT) return false; - } else { - vshResetLibvirtError(); - } + vshResetLibvirtError(); } else { /* Only print something if a security model is active */ if (secmodel.model[0] !=3D '\0') { @@ -1309,10 +1291,11 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd) if (virDomainGetSecurityLabel(dom, seclabel) =3D=3D -1) { VIR_FREE(seclabel); return false; - } else { - if (seclabel->label[0] !=3D '\0') - vshPrint(ctl, "%-15s %s (%s)\n", _("Security label:"), - seclabel->label, seclabel->enforcing ? "enfor= cing" : "permissive"); + } + + if (seclabel->label[0] !=3D '\0') { + vshPrint(ctl, "%-15s %s (%s)\n", _("Security label:"), + seclabel->label, seclabel->enforcing ? "enforcing= " : "permissive"); } =20 VIR_FREE(seclabel); @@ -1421,7 +1404,6 @@ cmdDomTime(vshControl *ctl, const vshCmd *cmd) long long seconds =3D 0; unsigned int nseconds =3D 0; unsigned int flags =3D 0; - bool doSet =3D false; int rv; =20 VSH_EXCLUSIVE_OPTIONS("time", "now"); @@ -1433,15 +1415,12 @@ cmdDomTime(vshControl *ctl, const vshCmd *cmd) =20 rv =3D vshCommandOptLongLong(ctl, cmd, "time", &seconds); =20 - if (rv < 0) { - /* invalid integer format */ + /* invalid integer format */ + if (rv < 0) return false; - } else if (rv > 0) { - /* valid integer to set */ - doSet =3D true; - } =20 - if (doSet || now || rtcSync) { + /* valid integer to set */ + if (rv > 0 || now || rtcSync) { if (now && ((seconds =3D time(NULL)) =3D=3D (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; =20 - } else { - if (virDomainGetTime(dom, &seconds, &nseconds, flags) < 0) - return false; + return true; + } =20 - if (pretty) { - g_autoptr(GDateTime) then =3D NULL; - g_autofree char *thenstr =3D NULL; + if (virDomainGetTime(dom, &seconds, &nseconds, flags) < 0) + return false; =20 - then =3D g_date_time_new_from_unix_utc(seconds); - thenstr =3D g_date_time_format(then, "%Y-%m-%d %H:%M:%S"); + if (pretty) { + g_autoptr(GDateTime) then =3D NULL; + g_autofree char *thenstr =3D NULL; =20 - vshPrint(ctl, _("Time: %s"), thenstr); - } else { - vshPrint(ctl, _("Time: %lld"), seconds); - } + then =3D g_date_time_new_from_unix_utc(seconds); + thenstr =3D g_date_time_format(then, "%Y-%m-%d %H:%M:%S"); + + vshPrint(ctl, _("Time: %s"), thenstr); + } else { + vshPrint(ctl, _("Time: %lld"), seconds); } =20 return true; @@ -1508,17 +1488,15 @@ virshDomainSorter(const void *a, const void *b) if (ida =3D=3D inactive && idb =3D=3D inactive) return vshStrcasecmp(virDomainGetName(*da), virDomainGetName(*db)); =20 - if (ida !=3D inactive && idb !=3D inactive) { + if (ida !=3D inactive && idb !=3D inactive && ida !=3D idb) { if (ida > idb) return 1; - else if (ida < idb) - return -1; + return -1; } =20 if (ida !=3D inactive) return -1; - else - return 1; + return 1; } =20 struct virshDomainList { @@ -1994,10 +1972,9 @@ cmdList(vshControl *ctl, const vshCmd *cmd) vshPrint(ctl, "%s%s", sep, id_buf); sep =3D " "; } - if (optName) { + if (optName) vshPrint(ctl, "%s%s", sep, virDomainGetName(dom)); - sep =3D " "; - } + vshPrint(ctl, "\n"); } } @@ -2372,13 +2349,14 @@ cmdDomIfAddr(vshControl *ctl, const vshCmd *cmd) ip_addr_str =3D g_strdup(""); =20 /* 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); + } } } =20 --=20 2.31.1 From nobody Mon Feb 9 23:05:28 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 216.205.24.124 as permitted sender) client-ip=216.205.24.124; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-124.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 216.205.24.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1632409746; cv=none; d=zohomail.com; s=zohoarc; b=VwsGQoJzNsZArt93p5e4RDRpfSKoAOqcR99aXtWzpsxfT0PXrzMIUwnA3gvdeba/2Cnw1LRaL85CiMu/3CObMTQ/6IQ/eTU1Sso94mjqUaWTygnd+KX/TI5GRJy/KKoFfCm44k+nuU9Ll1BrZPjsFmgioMl8xTVk/KBZ1A2p2rI= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1632409746; h=Content-Type:Content-Transfer-Encoding:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=hh+m1gp3vvt5WEKRpxvj4/dgkGdLf6F6oyu0tEHLUoA=; b=PSzJU+Xrvcr8sR4wv74VQU7dDXkXHMYmJ+OFKwn3ETfIgTAY6+CCQD3Y7UaHkkQu5B9+WU3rXYsz9kZM2tkYD+P8xPIskIo7rCXc2spg9N8UpvhD7Z0s6dtDIzffhGthUAEma6xo8nt6qAH3A2XJg++N884AJY++6D5q6FFEr2g= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 216.205.24.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.zohomail.com with SMTPS id 1632409746579790.1129544839323; Thu, 23 Sep 2021 08:09:06 -0700 (PDT) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-209-AX0uwJQhPLSv6p23uz8GOA-1; Thu, 23 Sep 2021 11:08:45 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 9934E835DED; Thu, 23 Sep 2021 15:08:39 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7B8CF5D6CF; Thu, 23 Sep 2021 15:08:39 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 1EA5B180BAD2; Thu, 23 Sep 2021 15:08:39 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 18NF8M6C030569 for ; Thu, 23 Sep 2021 11:08:22 -0400 Received: by smtp.corp.redhat.com (Postfix) id 40E0C171FF; Thu, 23 Sep 2021 15:08:22 +0000 (UTC) Received: from fedora.redhat.com (unknown [10.43.2.78]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8B9DB5FC25 for ; Thu, 23 Sep 2021 15:08:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1632409745; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=hh+m1gp3vvt5WEKRpxvj4/dgkGdLf6F6oyu0tEHLUoA=; b=CqWN6HOWI8VFheUsZCLrKhHr/FpMy3kCWTutZl9vF1OvMghkYis0FqISW1yO8Ev42lIVto jWsZ+6DLbnI7LGYHKZfvndbkC/DAUKuZ1CHqfwoOn/9KSDZC7CRjygZh1pjcuyT3VC9rrH 93uwARjacDqqtJK1KnHL3f0UoAmBXsE= X-MC-Unique: AX0uwJQhPLSv6p23uz8GOA-1 From: Kristina Hanicova To: libvir-list@redhat.com Subject: [PATCH 2/5] virsh: domain: refactoring of small functions Date: Thu, 23 Sep 2021 17:08:01 +0200 Message-Id: <490cfcfa238430be8521fbe4ca37a2cb62c54b77.1632409613.git.khanicov@redhat.com> In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-loop: libvir-list@redhat.com X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=libvir-list-bounces@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) X-ZM-MESSAGEID: 1632409747528100001 Content-Type: text/plain; charset="utf-8" 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 --- 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 =3D virDomainDefineXMLFlags(conn, xml, flags); - /* If validate is the only flag, just drop it and - * try again. - */ - if (!dom) { - if ((virGetLastErrorCode() =3D=3D VIR_ERR_NO_SUPPORT) && - (flags =3D=3D VIR_DOMAIN_DEFINE_VALIDATE)) - dom =3D virDomainDefineXML(conn, xml); - } - } else { - dom =3D virDomainDefineXML(conn, xml); + + if (!flags) + return virDomainDefineXML(conn, xml); + + dom =3D virDomainDefineXMLFlags(conn, xml, flags); + /* If validate is the only flag, just drop it and + * try again. + */ + if (!dom) { + if ((virGetLastErrorCode() =3D=3D VIR_ERR_NO_SUPPORT) && + (flags =3D=3D VIR_DOMAIN_DEFINE_VALIDATE)) + dom =3D virDomainDefineXML(conn, xml); } return dom; } @@ -659,12 +659,10 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) return false; } =20 - 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; } =20 if (wwn && !virValidateWWN(wwn)) @@ -2705,7 +2703,6 @@ virshBlockJobAbort(virDomainPtr dom, static bool cmdBlockjob(vshControl *ctl, const vshCmd *cmd) { - bool ret =3D false; bool raw =3D vshCommandOptBool(cmd, "raw"); bool bytes =3D vshCommandOptBool(cmd, "bytes"); bool abortMode =3D vshCommandOptBool(cmd, "abort"); @@ -2738,13 +2735,10 @@ cmdBlockjob(vshControl *ctl, const vshCmd *cmd) return false; =20 if (bandwidth) - ret =3D virshBlockJobSetSpeed(ctl, cmd, dom, path, bytes); - else if (abortMode || pivot || async) - ret =3D virshBlockJobAbort(dom, path, pivot, async); - else - ret =3D 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); } =20 /* @@ -2930,7 +2924,6 @@ cmdBlockresize(vshControl *ctl, const vshCmd *cmd) const char *path =3D NULL; unsigned long long size =3D 0; unsigned int flags =3D 0; - bool ret =3D false; =20 if (vshCommandOptStringReq(ctl, cmd, "path", (const char **) &path) < = 0) return false; @@ -2949,12 +2942,11 @@ cmdBlockresize(vshControl *ctl, const vshCmd *cmd) =20 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 =3D true; + return false; } =20 - return ret; + vshPrintExtra(ctl, _("Block device '%s' is resized"), path); + return true; } =20 #ifndef WIN32 @@ -3428,19 +3420,17 @@ cmdSuspend(vshControl *ctl, const vshCmd *cmd) { g_autoptr(virshDomain) dom =3D NULL; const char *name; - bool ret =3D true; =20 if (!(dom =3D virshCommandOptDomain(ctl, cmd, &name))) return false; =20 - if (virDomainSuspend(dom) =3D=3D 0) { - vshPrintExtra(ctl, _("Domain '%s' suspended\n"), name); - } else { + if (virDomainSuspend(dom) !=3D 0) { vshError(ctl, _("Failed to suspend domain '%s'"), name); - ret =3D false; + return false; } =20 - return ret; + vshPrintExtra(ctl, _("Domain '%s' suspended\n"), name); + return true; } =20 /* @@ -5066,7 +5056,6 @@ cmdSchedInfoUpdateOne(vshControl *ctl, const char *field, const char *value) { virTypedParameterPtr param; - int ret =3D -1; size_t i; =20 for (i =3D 0; i < nsrc_params; i++) { @@ -5081,14 +5070,11 @@ cmdSchedInfoUpdateOne(vshControl *ctl, vshSaveLibvirtError(); return -1; } - ret =3D 0; - break; + return 0; } =20 - if (ret < 0) - vshError(ctl, _("invalid scheduler option: %s"), field); - - return ret; + vshError(ctl, _("invalid scheduler option: %s"), field); + return -1; } =20 static int @@ -5539,7 +5525,6 @@ virshGenFileName(vshControl *ctl, virDomainPtr dom, c= onst char *mime) g_autoptr(GDateTime) now =3D g_date_time_new_now_local(); g_autofree char *nowstr =3D NULL; const char *ext =3D NULL; - char *ret =3D NULL; =20 if (!dom) { vshError(ctl, "%s", _("Invalid domain supplied")); @@ -5554,10 +5539,8 @@ virshGenFileName(vshControl *ctl, virDomainPtr dom, = const char *mime) =20 nowstr =3D g_date_time_format(now, "%Y-%m-%d-%H:%M:%S"); =20 - ret =3D 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)); } =20 static bool @@ -5696,7 +5679,6 @@ static bool cmdSetLifecycleAction(vshControl *ctl, const vshCmd *cmd) { g_autoptr(virshDomain) dom =3D NULL; - bool ret =3D true; bool config =3D vshCommandOptBool(cmd, "config"); bool live =3D vshCommandOptBool(cmd, "live"); bool current =3D vshCommandOptBool(cmd, "current"); @@ -5737,10 +5719,9 @@ cmdSetLifecycleAction(vshControl *ctl, const vshCmd = *cmd) =20 if (virDomainSetLifecycleAction(dom, type, action, flags) < 0) { vshError(ctl, "%s", _("Unable to change lifecycle action.")); - ret =3D false; + return false; } - - return ret; + return true; } =20 /* @@ -5825,20 +5806,18 @@ static bool cmdResume(vshControl *ctl, const vshCmd *cmd) { g_autoptr(virshDomain) dom =3D NULL; - bool ret =3D true; const char *name; =20 if (!(dom =3D virshCommandOptDomain(ctl, cmd, &name))) return false; =20 - if (virDomainResume(dom) =3D=3D 0) { - vshPrintExtra(ctl, _("Domain '%s' resumed\n"), name); - } else { + if (virDomainResume(dom) !=3D 0) { vshError(ctl, _("Failed to resume domain '%s'"), name); - ret =3D false; + return false; } =20 - return ret; + vshPrintExtra(ctl, _("Domain '%s' resumed\n"), name); + return true; } =20 /* @@ -5912,13 +5891,13 @@ cmdShutdown(vshControl *ctl, const vshCmd *cmd) rv =3D virDomainShutdownFlags(dom, flags); else rv =3D virDomainShutdown(dom); - if (rv =3D=3D 0) { - vshPrintExtra(ctl, _("Domain '%s' is being shutdown\n"), name); - } else { + + if (rv !=3D 0) { vshError(ctl, _("Failed to shutdown domain '%s'"), name); return false; } =20 + vshPrintExtra(ctl, _("Domain '%s' is being shutdown\n"), name); return true; } =20 @@ -5988,13 +5967,12 @@ cmdReboot(vshControl *ctl, const vshCmd *cmd) if (!(dom =3D virshCommandOptDomain(ctl, cmd, &name))) return false; =20 - if (virDomainReboot(dom, flags) =3D=3D 0) { - vshPrintExtra(ctl, _("Domain '%s' is being rebooted\n"), name); - } else { + if (virDomainReboot(dom, flags) !=3D 0) { vshError(ctl, _("Failed to reboot domain '%s'"), name); return false; } =20 + vshPrintExtra(ctl, _("Domain '%s' is being rebooted\n"), name); return true; } =20 @@ -6020,20 +5998,18 @@ static bool cmdReset(vshControl *ctl, const vshCmd *cmd) { g_autoptr(virshDomain) dom =3D NULL; - bool ret =3D true; const char *name; =20 if (!(dom =3D virshCommandOptDomain(ctl, cmd, &name))) return false; =20 - if (virDomainReset(dom, 0) =3D=3D 0) { - vshPrintExtra(ctl, _("Domain '%s' was reset\n"), name); - } else { + if (virDomainReset(dom, 0) !=3D 0) { vshError(ctl, _("Failed to reset domain '%s'"), name); - ret =3D false; + return false; } =20 - return ret; + vshPrintExtra(ctl, _("Domain '%s' was reset\n"), name); + return true; } =20 /* @@ -6479,15 +6455,14 @@ static bool cmdDomjobabort(vshControl *ctl, const vshCmd *cmd) { g_autoptr(virshDomain) dom =3D NULL; - bool ret =3D true; =20 if (!(dom =3D virshCommandOptDomain(ctl, cmd, NULL))) return false; =20 if (virDomainAbortJob(dom) < 0) - ret =3D false; + return false; =20 - return ret; + return true; } =20 /* @@ -7042,8 +7017,7 @@ virshParseCPUList(vshControl *ctl, int *cpumaplen, } } =20 - if (virBitmapToData(map, &cpumap, cpumaplen) < 0) - goto cleanup; + virBitmapToData(map, &cpumap, cpumaplen); =20 cleanup: virBitmapFree(map); @@ -8212,7 +8186,6 @@ cmdDefine(vshControl *ctl, const vshCmd *cmd) { g_autoptr(virshDomain) dom =3D NULL; const char *from =3D NULL; - bool ret =3D true; char *buffer; unsigned int flags =3D 0; virshControl *priv =3D ctl->privData; @@ -8232,14 +8205,14 @@ cmdDefine(vshControl *ctl, const vshCmd *cmd) dom =3D virDomainDefineXML(priv->conn, buffer); VIR_FREE(buffer); =20 - if (dom !=3D 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 =3D false; + return false; } - return ret; + + vshPrintExtra(ctl, _("Domain '%s' defined from %s\n"), + virDomainGetName(dom), from); + return true; } =20 /* @@ -8268,7 +8241,6 @@ static bool cmdDestroy(vshControl *ctl, const vshCmd *cmd) { g_autoptr(virshDomain) dom =3D NULL; - bool ret =3D true; const char *name; unsigned int flags =3D 0; int result; @@ -8284,14 +8256,13 @@ cmdDestroy(vshControl *ctl, const vshCmd *cmd) else result =3D virDomainDestroy(dom); =20 - if (result =3D=3D 0) { - vshPrintExtra(ctl, _("Domain '%s' destroyed\n"), name); - } else { + if (result < 0) { vshError(ctl, _("Failed to destroy domain '%s'"), name); - ret =3D false; + return false; } =20 - return ret; + vshPrintExtra(ctl, _("Domain '%s' destroyed\n"), name); + return true; } =20 /* @@ -9990,7 +9961,6 @@ static bool cmdDumpXML(vshControl *ctl, const vshCmd *cmd) { g_autoptr(virshDomain) dom =3D NULL; - bool ret =3D true; g_autofree char *dump =3D NULL; unsigned int flags =3D 0; bool inactive =3D vshCommandOptBool(cmd, "inactive"); @@ -10010,14 +9980,11 @@ cmdDumpXML(vshControl *ctl, const vshCmd *cmd) if (!(dom =3D virshCommandOptDomain(ctl, cmd, NULL))) return false; =20 - dump =3D virDomainGetXMLDesc(dom, flags); - if (dump !=3D NULL) { - vshPrint(ctl, "%s", dump); - } else { - ret =3D false; - } + if (!(dump =3D virDomainGetXMLDesc(dom, flags))) + return false; =20 - return ret; + vshPrint(ctl, "%s", dump); + return true; } =20 /* @@ -10051,7 +10018,6 @@ static const vshCmdOptDef opts_domxmlfromnative[] = =3D { static bool cmdDomXMLFromNative(vshControl *ctl, const vshCmd *cmd) { - bool ret =3D true; const char *format =3D NULL; const char *configFile =3D NULL; g_autofree char *configData =3D NULL; @@ -10067,13 +10033,11 @@ cmdDomXMLFromNative(vshControl *ctl, const vshCmd= *cmd) return false; =20 xmlData =3D virConnectDomainXMLFromNative(priv->conn, format, configDa= ta, flags); - if (xmlData !=3D NULL) { - vshPrint(ctl, "%s", xmlData); - } else { - ret =3D false; - } + if (!xmlData) + return false; =20 - return ret; + vshPrint(ctl, "%s", xmlData); + return true; } =20 /* @@ -11006,25 +10970,19 @@ cmdMigrateSetMaxDowntime(vshControl *ctl, const v= shCmd *cmd) { g_autoptr(virshDomain) dom =3D NULL; unsigned long long downtime =3D 0; - bool ret =3D false; =20 if (!(dom =3D virshCommandOptDomain(ctl, cmd, NULL))) return false; =20 if (vshCommandOptULongLong(ctl, cmd, "downtime", &downtime) < 0) - goto done; + return false; + if (downtime < 1) { vshError(ctl, "%s", _("migrate: Invalid downtime")); - goto done; + return false; } =20 - if (virDomainMigrateSetMaxDowntime(dom, downtime, 0)) - goto done; - - ret =3D true; - - done: - return ret; + return virDomainMigrateSetMaxDowntime(dom, downtime, 0) =3D=3D 0; } =20 =20 @@ -11100,12 +11058,12 @@ cmdMigrateCompCache(vshControl *ctl, const vshCmd= *cmd) return false; =20 rc =3D vshCommandOptULongLong(ctl, cmd, "size", &size); - if (rc < 0) { + if (rc < 0) + return false; + + if (rc !=3D 0 && + (virDomainMigrateSetCompressionCache(dom, size, 0) < 0)) return false; - } else if (rc !=3D 0) { - if (virDomainMigrateSetCompressionCache(dom, size, 0) < 0) - return false; - } =20 if (virDomainMigrateGetCompressionCache(dom, &size, 0) < 0) return false; @@ -11486,11 +11444,9 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) =20 /* We got what we came for so return successfully */ ret =3D true; - if (!all) { + if (!all) break; - } else { - vshPrint(ctl, "\n"); - } + vshPrint(ctl, "\n"); } =20 if (!ret) { @@ -11947,12 +11903,12 @@ virshDomainDetachInterface(char *doc, xmlNodePtr cur =3D NULL, matchNode =3D NULL; g_autofree char *detach_xml =3D NULL; char buf[64]; - int diff_mac, ret =3D -1; + int diff_mac =3D -1; size_t i; =20 if (!(xml =3D virXMLParseStringCtxt(doc, _("(domain_definition)"), &ct= xt))) { vshError(ctl, "%s", _("Failed to get interface information")); - goto cleanup; + return false; } =20 g_snprintf(buf, sizeof(buf), "/domain/devices/interface[@type=3D'%s']"= , type); @@ -11960,13 +11916,13 @@ virshDomainDetachInterface(char *doc, if (obj =3D=3D NULL || obj->type !=3D XPATH_NODESET || obj->nodesetval =3D=3D NULL || obj->nodesetval->nodeNr =3D=3D 0) { vshError(ctl, _("No interface found whose type is %s"), type); - goto cleanup; + return false; } =20 if (!mac && obj->nodesetval->nodeNr > 1) { vshError(ctl, _("Domain has %d interfaces. Please specify which on= e " "to detach using --mac"), obj->nodesetval->nodeNr); - goto cleanup; + return false; } =20 if (!mac) { @@ -11989,7 +11945,7 @@ virshDomainDetachInterface(char *doc, "MAC address %s. You must use deta= ch-device and " "specify the device pci address to= remove it."), mac); - goto cleanup; + return false; } matchNode =3D 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; } =20 hit: if (!(detach_xml =3D virXMLNodeToString(xml, matchNode))) { vshSaveLibvirtError(); - goto cleanup; + return false; } =20 if (flags !=3D 0 || current) - ret =3D virDomainDetachDeviceFlags(dom, detach_xml, flags); - else - ret =3D virDomainDetachDevice(dom, detach_xml); - - cleanup: - return ret =3D=3D 0; + return virDomainDetachDeviceFlags(dom, detach_xml, flags) =3D=3D 0; + return virDomainDetachDevice(dom, detach_xml) =3D=3D 0; } =20 =20 @@ -13666,10 +13618,10 @@ static bool cmdDomFSFreeze(vshControl *ctl, const vshCmd *cmd) { g_autoptr(virshDomain) dom =3D NULL; - int ret =3D -1; const vshCmdOpt *opt =3D NULL; g_autofree const char **mountpoints =3D NULL; size_t nmountpoints =3D 0; + int count =3D 0; =20 if (!(dom =3D virshCommandOptDomain(ctl, cmd, NULL))) return false; @@ -13679,16 +13631,13 @@ cmdDomFSFreeze(vshControl *ctl, const vshCmd *cmd) mountpoints[nmountpoints-1] =3D opt->data; } =20 - ret =3D virDomainFSFreeze(dom, mountpoints, nmountpoints, 0); - if (ret < 0) { + if ((count =3D virDomainFSFreeze(dom, mountpoints, nmountpoints, 0)) <= 0) { vshError(ctl, _("Unable to freeze filesystems")); - goto cleanup; + return false; } =20 - vshPrintExtra(ctl, _("Froze %d filesystem(s)\n"), ret); - - cleanup: - return ret >=3D 0; + vshPrintExtra(ctl, _("Froze %d filesystem(s)\n"), count); + return true; } =20 static const vshCmdInfo info_domfsthaw[] =3D { @@ -13714,10 +13663,10 @@ static bool cmdDomFSThaw(vshControl *ctl, const vshCmd *cmd) { g_autoptr(virshDomain) dom =3D NULL; - int ret =3D -1; const vshCmdOpt *opt =3D NULL; g_autofree const char **mountpoints =3D NULL; size_t nmountpoints =3D 0; + int count =3D 0; =20 if (!(dom =3D virshCommandOptDomain(ctl, cmd, NULL))) return false; @@ -13727,16 +13676,13 @@ cmdDomFSThaw(vshControl *ctl, const vshCmd *cmd) mountpoints[nmountpoints-1] =3D opt->data; } =20 - ret =3D virDomainFSThaw(dom, mountpoints, nmountpoints, 0); - if (ret < 0) { + if ((count =3D virDomainFSThaw(dom, mountpoints, nmountpoints, 0)) < 0= ) { vshError(ctl, _("Unable to thaw filesystems")); - goto cleanup; + return false; } =20 - vshPrintExtra(ctl, _("Thawed %d filesystem(s)\n"), ret); - - cleanup: - return ret >=3D 0; + vshPrintExtra(ctl, _("Thawed %d filesystem(s)\n"), count); + return true; } =20 static const vshCmdInfo info_domfsinfo[] =3D { --=20 2.31.1 From nobody Mon Feb 9 23:05:28 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 170.10.129.124 as permitted sender) client-ip=170.10.129.124; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-124.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1632409728; cv=none; d=zohomail.com; s=zohoarc; b=gfdgkqcbAjNeTfZ/qMkJ2GCh00R9j0ffOheJ92IBeHDtP33x2Yd0J6jiOMzzeO7DbFOdY4/4wGmBUdu2qWY/k2g1J0X3nkzfDzOssiQA0K9XqdW2CQjBzGA8TKgDv2+/YoJwm2yFKTwpz0LoGScw4kpC5VUxLmvAV9o/76lWiho= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1632409728; h=Content-Type:Content-Transfer-Encoding:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=o7EievYVY90JhfjRQcjb/9Y7xUY0oL8gcVXICzT8yCk=; b=aXzoWJ0G1T7bv3yOPIMMiWv3j7UbFgVE3+TQXBitgFQkuG5bmRljayuVLcMlR2Q1k5xz0uuOxIikLXxNpXTXSeUTAmrvK1REiSjG5KM759tPhnsjnmY/lgGHkNHwtG5uncY4Jwl4Y4t6Jkyu+cDjPwpp7xtibvDhEV7eOhkD8H0= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mx.zohomail.com with SMTPS id 163240972869673.6040225837512; Thu, 23 Sep 2021 08:08:48 -0700 (PDT) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-25-zN0mK268M_S0zz6FtwfHdw-1; Thu, 23 Sep 2021 11:08:45 -0400 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 58AB89126F; Thu, 23 Sep 2021 15:08:32 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 3B837652A5; Thu, 23 Sep 2021 15:08:32 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id CE1281806D03; Thu, 23 Sep 2021 15:08:31 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 18NF8Np3030580 for ; Thu, 23 Sep 2021 11:08:23 -0400 Received: by smtp.corp.redhat.com (Postfix) id 285B1171FF; Thu, 23 Sep 2021 15:08:23 +0000 (UTC) Received: from fedora.redhat.com (unknown [10.43.2.78]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8D8245F707 for ; Thu, 23 Sep 2021 15:08:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1632409727; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=o7EievYVY90JhfjRQcjb/9Y7xUY0oL8gcVXICzT8yCk=; b=Gk5O7CFcalQrjVEAImDflezv2MSUDukgNybuWVeK7CMsI4VsNK5+2qGVxU3gXcmzarVIYe vo7OnRcD+otOZFbFYWuk55XnNsKllFvOXTJKmhDiGO8s4Ayz6qHA4HDbQCe7h483hhG/3J rorkzkfnN+adnUwSWsJ0rgb0xx6KafU= X-MC-Unique: zN0mK268M_S0zz6FtwfHdw-1 From: Kristina Hanicova To: libvir-list@redhat.com Subject: [PATCH 3/5] virsh: domain: change the way functions return bool value Date: Thu, 23 Sep 2021 17:08:02 +0200 Message-Id: In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-loop: libvir-list@redhat.com X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=libvir-list-bounces@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) X-ZM-MESSAGEID: 1632409729828100001 Content-Type: text/plain; charset="utf-8" This patch changes the way functions return bool value from pattern: if (functionCall() < 0) return false; return true; into a more readable and modern way of direct return: return functionCall() =3D=3D 0; I know that not everybody will agree this is more readable so I am open to discussion and I leave merging this patch up to the reviewer. Signed-off-by: Kristina Hanicova --- tools/virsh-domain.c | 114 ++++++++++--------------------------------- 1 file changed, 26 insertions(+), 88 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 3232463485..ec427443c4 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -765,7 +765,6 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) return false; } =20 - vshPrintExtra(ctl, "%s", _("Disk attached successfully\n")); return true; } @@ -1432,7 +1431,6 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) goto save_error; } =20 - if (nparams =3D=3D 0) { if (virDomainGetBlockIoTune(dom, NULL, NULL, &nparams, flags) !=3D= 0) { vshError(ctl, "%s", @@ -2673,10 +2671,7 @@ virshBlockJobSetSpeed(vshControl *ctl, if (vshBlockJobOptionBandwidth(ctl, cmd, bytes, &bandwidth) < 0) return false; =20 - if (virDomainBlockJobSetSpeed(dom, path, bandwidth, flags) < 0) - return false; - - return true; + return virDomainBlockJobSetSpeed(dom, path, bandwidth, flags) =3D=3D 0; } =20 =20 @@ -2693,10 +2688,7 @@ virshBlockJobAbort(virDomainPtr dom, if (pivot) flags |=3D VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT; =20 - if (virDomainBlockJobAbort(dom, path, flags) < 0) - return false; - - return true; + return virDomainBlockJobAbort(dom, path, flags) =3D=3D 0; } =20 =20 @@ -3010,10 +3002,8 @@ cmdRunConsole(vshControl *ctl, virDomainPtr dom, vshPrintExtra(ctl, " (Ctrl + %c)", priv->escapeChar[1]); vshPrintExtra(ctl, "\n"); fflush(stdout); - if (virshRunConsole(ctl, dom, name, flags) =3D=3D 0) - return true; =20 - return false; + return virshRunConsole(ctl, dom, name, flags) =3D=3D 0; } =20 static bool @@ -7079,15 +7069,10 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd) return false; =20 /* use old API without any explicit flags */ - if (flags =3D=3D VIR_DOMAIN_AFFECT_CURRENT && !current) { - if (virDomainPinVcpu(dom, vcpu, cpumap, cpumaplen) !=3D 0) - return false; - } else { - if (virDomainPinVcpuFlags(dom, vcpu, cpumap, cpumaplen, flags) != =3D 0) - return false; - } + if (flags =3D=3D VIR_DOMAIN_AFFECT_CURRENT && !current) + return virDomainPinVcpu(dom, vcpu, cpumap, cpumaplen) =3D=3D 0; =20 - return true; + return virDomainPinVcpuFlags(dom, vcpu, cpumap, cpumaplen, flags) =3D= =3D 0; } =20 /* @@ -7184,10 +7169,7 @@ cmdEmulatorPin(vshControl *ctl, const vshCmd *cmd) if (flags =3D=3D -1) flags =3D VIR_DOMAIN_AFFECT_LIVE; =20 - if (virDomainPinEmulator(dom, cpumap, cpumaplen, flags) !=3D 0) - return false; - - return true; + return virDomainPinEmulator(dom, cpumap, cpumaplen, flags) =3D=3D 0; } =20 /* @@ -7270,15 +7252,10 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd) } =20 /* none of the options were specified */ - if (!current && flags =3D=3D 0) { - if (virDomainSetVcpus(dom, count) !=3D 0) - return false; - } else { - if (virDomainSetVcpusFlags(dom, count, flags) < 0) - return false; - } + if (!current && flags =3D=3D 0) + return virDomainSetVcpus(dom, count) =3D=3D 0; =20 - return true; + return virDomainSetVcpusFlags(dom, count, flags) =3D=3D 0; } =20 =20 @@ -7439,10 +7416,7 @@ cmdSetvcpu(vshControl *ctl, const vshCmd *cmd) if (enable) state =3D 1; =20 - if (virDomainSetVcpu(dom, vcpulist, state, flags) < 0) - return false; - - return true; + return virDomainSetVcpu(dom, vcpulist, state, flags) =3D=3D 0; } =20 =20 @@ -7493,10 +7467,7 @@ cmdDomblkthreshold(vshControl *ctl, const vshCmd *cm= d) if (!(dom =3D virshCommandOptDomain(ctl, cmd, NULL))) return false; =20 - if (virDomainSetBlockThreshold(dom, dev, threshold, 0) < 0) - return false; - - return true; + return virDomainSetBlockThreshold(dom, dev, threshold, 0) =3D=3D 0; } =20 =20 @@ -7656,11 +7627,8 @@ cmdIOThreadPin(vshControl *ctl, const vshCmd *cmd) if (!(cpumap =3D virshParseCPUList(ctl, &cpumaplen, cpulist, maxcpu))) return false; =20 - if (virDomainPinIOThread(dom, iothread_id, - cpumap, cpumaplen, flags) !=3D 0) - return false; - - return true; + return virDomainPinIOThread(dom, iothread_id, cpumap, + cpumaplen, flags) =3D=3D 0; } =20 /* @@ -7717,10 +7685,7 @@ cmdIOThreadAdd(vshControl *ctl, const vshCmd *cmd) return false; } =20 - if (virDomainAddIOThread(dom, iothread_id, flags) < 0) - return false; - - return true; + return virDomainAddIOThread(dom, iothread_id, flags) =3D=3D 0; } =20 =20 @@ -7877,15 +7842,13 @@ cmdIOThreadDel(vshControl *ctl, const vshCmd *cmd) =20 if (vshCommandOptInt(ctl, cmd, "id", &iothread_id) < 0) return false; + if (iothread_id <=3D 0) { vshError(ctl, _("Invalid IOThread id value: '%d'"), iothread_id); return false; } =20 - if (virDomainDelIOThread(dom, iothread_id, flags) < 0) - return false; - - return true; + return virDomainDelIOThread(dom, iothread_id, flags) =3D=3D 0; } =20 /* @@ -8593,10 +8556,7 @@ cmdInjectNMI(vshControl *ctl, const vshCmd *cmd) if (!(dom =3D virshCommandOptDomain(ctl, cmd, NULL))) return false; =20 - if (virDomainInjectNMI(dom, 0) < 0) - return false; - - return true; + return virDomainInjectNMI(dom, 0) =3D=3D 0; } =20 /* @@ -8693,10 +8653,7 @@ cmdSendKey(vshControl *ctl, const vshCmd *cmd) count++; } =20 - if (virDomainSendKey(dom, codeset, holdtime, keycodes, count, 0) < 0) - return false; - - return true; + return virDomainSendKey(dom, codeset, holdtime, keycodes, count, 0) = =3D=3D 0; } =20 /* @@ -8788,10 +8745,7 @@ cmdSendProcessSignal(vshControl *ctl, const vshCmd *= cmd) return false; } =20 - if (virDomainSendProcessSignal(dom, pid_value, signum, 0) < 0) - return false; - - return true; + return virDomainSendProcessSignal(dom, pid_value, signum, 0) =3D=3D 0; } =20 /* @@ -8862,10 +8816,7 @@ cmdSetmem(vshControl *ctl, const vshCmd *cmd) return false; kibibytes =3D VIR_DIV_UP(bytes, 1024); =20 - if (virDomainSetMemoryFlags(dom, kibibytes, flags) < 0) - return false; - - return true; + return virDomainSetMemoryFlags(dom, kibibytes, flags) =3D=3D 0; } =20 /* @@ -11118,10 +11069,7 @@ cmdMigrateSetMaxSpeed(vshControl *ctl, const vshCm= d *cmd) if (vshCommandOptBool(cmd, "postcopy")) flags |=3D VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY; =20 - if (virDomainMigrateSetMaxSpeed(dom, bandwidth, flags) < 0) - return false; - - return true; + return virDomainMigrateSetMaxSpeed(dom, bandwidth, flags) =3D=3D 0; } =20 /* @@ -11194,10 +11142,7 @@ cmdMigratePostCopy(vshControl *ctl, const vshCmd *= cmd) if (!(dom =3D virshCommandOptDomain(ctl, cmd, NULL))) return false; =20 - if (virDomainMigrateStartPostCopy(dom, 0) < 0) - return false; - - return true; + return virDomainMigrateStartPostCopy(dom, 0) =3D=3D 0; } =20 /* @@ -13801,10 +13746,7 @@ cmdGuestAgentTimeout(vshControl *ctl, const vshCmd= *cmd) if (vshCommandOptInt(ctl, cmd, "timeout", &timeout) < 0) return false; =20 - if (virDomainAgentSetResponseTimeout(dom, timeout, flags) < 0) - return false; - - return true; + return virDomainAgentSetResponseTimeout(dom, timeout, flags) =3D=3D 0; } =20 /* @@ -14032,12 +13974,8 @@ cmdSetUserSSHKeys(vshControl *ctl, const vshCmd *c= md) } } =20 - if (virDomainAuthorizedSSHKeysSet(dom, user, - (const char **) keys, nkeys, flags) = < 0) { - return false; - } - - return true; + return virDomainAuthorizedSSHKeysSet(dom, user, (const char **) keys, + nkeys, flags) =3D=3D 0; } =20 =20 --=20 2.31.1 From nobody Mon Feb 9 23:05:28 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 216.205.24.124 as permitted sender) client-ip=216.205.24.124; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-124.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 216.205.24.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1632409731; cv=none; d=zohomail.com; s=zohoarc; b=Voyx/Zc76iYTOx8nJikoIjTEe6C4fcubxlK5ZWk5A4qiu0/BF82Qf8LOBhRfwIeSUwrgid0YEfkS6DmtlmUjTH4/7mbU2NTlbfUkgW9RIMA/5hG+ENitusC5VUvuCB1NmF1fqVwvqnsnfzxRHkHjgcKIW00r/CSfzdN8s4v2YeQ= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1632409731; h=Content-Type:Content-Transfer-Encoding:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=7Sc8K3w9h3UnWXuLkJ4gBq+zyeuzOWMeE5uEwiK0KC4=; b=eCU3S36b+Jx103EdEyMItTJ2JBi7WIaMCFv71EImGNVAjrqf63wUgoxvS+zKDMICmDKwIefPoe0zc/BUyhTyiReuHO/pE9Y1i7RNx/JD2OWT+mmmQkSofXS/syR44QvhUDA2lVemGloOGvX7q1mC4u/gvHEqqhJ8B7KAoPrVRjg= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 216.205.24.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.zohomail.com with SMTPS id 1632409731434927.2723028941981; Thu, 23 Sep 2021 08:08:51 -0700 (PDT) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-471-e0DjzqucNjmH8FKV4tu3EQ-1; Thu, 23 Sep 2021 11:08:48 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 6B2851808328; Thu, 23 Sep 2021 15:08:42 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 1981B5D9DC; Thu, 23 Sep 2021 15:08:42 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id DA631180BAD2; Thu, 23 Sep 2021 15:08:41 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 18NF8O8N030587 for ; Thu, 23 Sep 2021 11:08:24 -0400 Received: by smtp.corp.redhat.com (Postfix) id 0FCF2171FF; Thu, 23 Sep 2021 15:08:24 +0000 (UTC) Received: from fedora.redhat.com (unknown [10.43.2.78]) by smtp.corp.redhat.com (Postfix) with ESMTP id 760017621C for ; Thu, 23 Sep 2021 15:08:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1632409730; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=7Sc8K3w9h3UnWXuLkJ4gBq+zyeuzOWMeE5uEwiK0KC4=; b=Yo52qO6BPzPDTGfzm3ycW6hcKpbjEhSNptFX51lPgdIKco+PAOtmSuJw+NViTD0ntaFPt7 VAheZR9su5oXIhmR8aW9r01+t9AwnPPxK/O5cwn/R6Gho3E57rciKJu2s8nR8eeuk0jIpV ADmYwHt8aFhoCVFAqra388/S0PBQUaM= X-MC-Unique: e0DjzqucNjmH8FKV4tu3EQ-1 From: Kristina Hanicova To: libvir-list@redhat.com Subject: [PATCH 4/5] virsh: domain, host & volume: refactor bigger functions Date: Thu, 23 Sep 2021 17:08:03 +0200 Message-Id: In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-loop: libvir-list@redhat.com X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=libvir-list-bounces@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) X-ZM-MESSAGEID: 1632409732530100001 Content-Type: text/plain; charset="utf-8" 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 --- 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 =3D 0; int nupdates =3D 0; size_t i; - int ret; bool ret_val =3D false; unsigned int flags =3D VIR_DOMAIN_AFFECT_CURRENT; bool current =3D vshCommandOptBool(cmd, "current"); @@ -5161,64 +5160,61 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) goto cleanup; } =20 - if (nparams) { - params =3D g_new0(virTypedParameter, nparams); + if (!nparams) + goto cleanup; =20 - 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 =3D virDomainGetSchedulerParametersFlags(dom, params, &npa= rams, - ((live && config) ?= 0 - : flags)); - } else { - ret =3D virDomainGetSchedulerParameters(dom, params, &nparams); - } - if (ret =3D=3D -1) + params =3D 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 : f= lags)) =3D=3D -1) goto cleanup; - - /* See if any params are being set */ - if ((nupdates =3D cmdSchedInfoUpdate(ctl, cmd, params, nparams, - &updates)) < 0) + } else { + if (virDomainGetSchedulerParameters(dom, params, &nparams) =3D=3D = -1) goto cleanup; + } =20 - /* Update parameters & refresh data */ - if (nupdates > 0) { - if (flags || current) - ret =3D virDomainSetSchedulerParametersFlags(dom, updates, - nupdates, flags= ); - else - ret =3D virDomainSetSchedulerParameters(dom, updates, nupd= ates); + /* See if any params are being set */ + if ((nupdates =3D cmdSchedInfoUpdate(ctl, cmd, params, nparams, + &updates)) < 0) + goto cleanup; =20 - if (ret =3D=3D -1) + /* Update parameters & refresh data */ + if (nupdates > 0) { + if (flags || current) { + if (virDomainSetSchedulerParametersFlags(dom, updates, + nupdates, flags) =3D= =3D -1) goto cleanup; =20 - if (flags || current) - ret =3D virDomainGetSchedulerParametersFlags(dom, params, - &nparams, - ((live && confi= g) ? 0 - : flags)); - else - ret =3D virDomainGetSchedulerParameters(dom, params, &npar= ams); - if (ret =3D=3D -1) + if (virDomainGetSchedulerParametersFlags(dom, params, &nparams, + ((live && config) ? 0= : flags)) =3D=3D -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) = =3D=3D -1) goto cleanup; - } - } =20 - ret_val =3D true; - for (i =3D 0; i < nparams; i++) { - char *str =3D vshGetTypedParamValue(ctl, ¶ms[i]); - vshPrint(ctl, "%-15s: %s\n", params[i].field, str); - VIR_FREE(str); + if (virDomainGetSchedulerParameters(dom, params, &nparams) =3D= =3D -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 =3D true; + for (i =3D 0; i < nparams; i++) { + char *str =3D vshGetTypedParamValue(ctl, ¶ms[i]); + vshPrint(ctl, "%-15s: %s\n", params[i].field, str); + VIR_FREE(str); } =20 cleanup: @@ -6503,7 +6499,6 @@ virshCPUCountCollect(vshControl *ctl, unsigned int flags, bool checkState) { - int ret =3D -2; virDomainInfo info; int count; g_autoptr(xmlDoc) xml =3D NULL; @@ -6524,11 +6519,11 @@ virshCPUCountCollect(vshControl *ctl, /* fallback code */ if (!(last_error->code =3D=3D VIR_ERR_NO_SUPPORT || last_error->code =3D=3D VIR_ERR_INVALID_ARG)) - goto cleanup; + return -2; =20 if (flags & VIR_DOMAIN_VCPU_GUEST) { vshError(ctl, "%s", _("Failed to retrieve vCPU count from the gues= t")); - goto cleanup; + return -2; } =20 if (!(flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG)) && @@ -6538,36 +6533,32 @@ virshCPUCountCollect(vshControl *ctl, vshResetLibvirtError(); =20 if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (flags & VIR_DOMAIN_VCPU_MAXIMUM) { - count =3D virDomainGetMaxVcpus(dom); - } else { - if (virDomainGetInfo(dom, &info) < 0) - goto cleanup; + if (flags & VIR_DOMAIN_VCPU_MAXIMUM) + return virDomainGetMaxVcpus(dom); =20 - count =3D 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 cou= nt")); - goto cleanup; - } - } else { - if (virXPathInt("string(/domain/vcpu/@current)", ctxt, &count)= < 0) { - vshError(ctl, "%s", _("Failed to retrieve current vcpu cou= nt")); - goto cleanup; - } + if (virXPathInt("string(/domain/vcpu/@current)", ctxt, &count) < 0= ) { + vshError(ctl, "%s", _("Failed to retrieve current vcpu count")= ); + return -2; } } =20 - ret =3D count; - cleanup: - - return ret; + return count; } =20 static bool @@ -9588,7 +9579,7 @@ cmdQemuMonitorEvent(vshControl *ctl, const vshCmd *cm= d) vshEventCleanup(ctl); if (eventId >=3D 0 && virConnectDomainQemuMonitorEventDeregister(priv->conn, eventId) < = 0) - ret =3D false; + return false; =20 return ret; } @@ -9790,6 +9781,7 @@ cmdLxcEnterNamespace(vshControl *ctl, const vshCmd *c= md) int nfdlist; int *fdlist; size_t i; + int status; bool setlabel =3D true; g_autofree virSecurityModelPtr secmodel =3D NULL; g_autofree virSecurityLabelPtr seclabel =3D NULL; @@ -9828,40 +9820,8 @@ cmdLxcEnterNamespace(vshControl *ctl, const vshCmd *= cmd) */ if ((pid =3D virFork()) < 0) return false; - if (pid =3D=3D 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 =3D virFork()) < 0) - _exit(EXIT_CANCELED); - if (pid =3D=3D 0) { - execv(cmdargv[0], cmdargv); - _exit(errno =3D=3D ENOENT ? EXIT_ENOENT : EXIT_CANNOT_INVOKE); - } - if (virProcessWait(pid, &status, true) < 0) - _exit(EXIT_CANNOT_INVOKE); - virProcessExitWithStatus(status); - } else { + + if (pid !=3D 0) { for (i =3D 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 =3D virFork()) < 0) + _exit(EXIT_CANCELED); + + if (pid =3D=3D 0) { + execv(cmdargv[0], cmdargv); + _exit(errno =3D=3D ENOENT ? EXIT_ENOENT : EXIT_CANNOT_INVOKE); } =20 + if (virProcessWait(pid, &status, true) < 0) + _exit(EXIT_CANNOT_INVOKE); + virProcessExitWithStatus(status); return true; } =20 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 =3D vshCommandOptBool(cmd, "cellno"); size_t i; g_autofree char *cap_xml =3D NULL; - g_autoptr(xmlDoc) xml =3D NULL; g_autoptr(xmlXPathContext) ctxt =3D NULL; virshControl *priv =3D ctl->privData; =20 @@ -171,68 +170,69 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd) if (cellno && vshCommandOptInt(ctl, cmd, "cellno", &cell) < 0) return false; =20 - if (all) { - if (!(cap_xml =3D virConnectGetCapabilities(priv->conn))) { - vshError(ctl, "%s", _("unable to get node capabilities")); - return false; - } + if (!all) { + if (cellno) { + if (virNodeGetCellsFreeMemory(priv->conn, &memory, cell, 1) != =3D 1) + return false; =20 - xml =3D 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; } =20 - nodes_cnt =3D virXPathNodeSet("/capabilities/host/topology/cells/c= ell", - ctxt, &nodes); - - if (nodes_cnt =3D=3D -1) { - vshError(ctl, "%s", _("could not get information about " - "NUMA topology")); + if ((memory =3D virNodeGetFreeMemory(priv->conn)) =3D=3D 0) return false; - } =20 - nodes_free =3D g_new0(unsigned long long, nodes_cnt); - nodes_id =3D g_new0(unsigned long, nodes_cnt); + vshPrint(ctl, "%s: %llu KiB\n", _("Total"), (memory/1024)); + return true; + } =20 - for (i =3D 0; i < nodes_cnt; i++) { - unsigned long id; - g_autofree char *val =3D virXMLPropString(nodes[i], "id"); - if (virStrToLong_ulp(val, NULL, 10, &id)) { - vshError(ctl, "%s", _("conversion from string failed")); - return false; - } - nodes_id[i] =3D id; - if (virNodeGetCellsFreeMemory(priv->conn, &(nodes_free[i]), - id, 1) !=3D 1) { - vshError(ctl, _("failed to get free memory for NUMA node " - "number: %lu"), id); - return false; - } - } + if (!(cap_xml =3D virConnectGetCapabilities(priv->conn))) { + vshError(ctl, "%s", _("unable to get node capabilities")); + return false; + } =20 - for (cell =3D 0; cell < nodes_cnt; cell++) { - vshPrint(ctl, "%5lu: %10llu KiB\n", nodes_id[cell], - (nodes_free[cell]/1024)); - memory +=3D nodes_free[cell]; - } + if (!virXMLParseStringCtxt(cap_xml, _("(capabilities)"), &ctxt)) { + vshError(ctl, "%s", _("unable to get node capabilities")); + return false; + } =20 - vshPrintExtra(ctl, "--------------------\n"); - vshPrintExtra(ctl, "%5s: %10llu KiB\n", _("Total"), memory/1024); - } else { - if (cellno) { - if (virNodeGetCellsFreeMemory(priv->conn, &memory, cell, 1) != =3D 1) - return false; + nodes_cnt =3D virXPathNodeSet("/capabilities/host/topology/cells/cell", + ctxt, &nodes); =20 - vshPrint(ctl, "%d: %llu KiB\n", cell, (memory/1024)); - } else { - if ((memory =3D virNodeGetFreeMemory(priv->conn)) =3D=3D 0) - return false; + if (nodes_cnt =3D=3D -1) { + vshError(ctl, "%s", _("could not get information about " + "NUMA topology")); + return false; + } + + nodes_free =3D g_new0(unsigned long long, nodes_cnt); + nodes_id =3D g_new0(unsigned long, nodes_cnt); =20 - vshPrint(ctl, "%s: %llu KiB\n", _("Total"), (memory/1024)); + for (i =3D 0; i < nodes_cnt; i++) { + unsigned long id; + g_autofree char *val =3D virXMLPropString(nodes[i], "id"); + if (virStrToLong_ulp(val, NULL, 10, &id)) { + vshError(ctl, "%s", _("conversion from string failed")); + return false; + } + nodes_id[i] =3D id; + if (virNodeGetCellsFreeMemory(priv->conn, &(nodes_free[i]), + id, 1) !=3D 1) { + vshError(ctl, _("failed to get free memory for NUMA node " + "number: %lu"), id); + return false; } } =20 + for (cell =3D 0; cell < nodes_cnt; cell++) { + vshPrint(ctl, "%5lu: %10llu KiB\n", nodes_id[cell], + (nodes_free[cell]/1024)); + memory +=3D nodes_free[cell]; + } + + vshPrintExtra(ctl, "--------------------\n"); + vshPrintExtra(ctl, "%5s: %10llu KiB\n", _("Total"), memory/1024); + return true; } =20 @@ -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 =3D 0; - for (i =3D 0; i < VIRSH_CPU_USAGE; i++) - total_time +=3D cpu_stats[i]; - - usage =3D (cpu_stats[VIRSH_CPU_USER] + cpu_stats[VIRSH_CPU_SYS= TEM]) - / total_time * 100; - - vshPrint(ctl, "%-15s %5.1lf%%\n", _("usage:"), usage); - for (i =3D 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 =3D 0; + for (i =3D 0; i < VIRSH_CPU_USAGE; i++) + total_time +=3D cpu_stats[i]; + + usage =3D (cpu_stats[VIRSH_CPU_USER] + cpu_stats[VIRSH_CPU_SYSTEM]) + / total_time * 100; + + vshPrint(ctl, "%-15s %5.1lf%%\n", _("usage:"), usage); + for (i =3D 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; } =20 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 =3D vshCommandOptBool(cmd, "bytes"); bool physical =3D vshCommandOptBool(cmd, "physical"); - bool ret =3D true; int rc; unsigned int flags =3D 0; =20 @@ -1074,41 +1073,39 @@ cmdVolInfo(vshControl *ctl, const vshCmd *cmd) else rc =3D virStorageVolGetInfo(vol, &info); =20 - if (rc =3D=3D 0) { - double val; - const char *unit; + if (rc < 0) { + virStorageVolFree(vol); + return false; + } =20 - vshPrint(ctl, "%-15s %s\n", _("Type:"), - virshVolumeTypeToString(info.type)); + vshPrint(ctl, "%-15s %s\n", _("Type:"), + virshVolumeTypeToString(info.type)); =20 - if (bytes) { - vshPrint(ctl, "%-15s %llu %s\n", _("Capacity:"), - info.capacity, _("bytes")); - } else { - val =3D 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")); =20 - 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 =3D vshPrettyCapacity(info.allocation, &unit); - if (physical) - vshPrint(ctl, "%-15s %2.2lf %s\n", _("Physical:"), val, un= it); - 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 =3D false; + const char *unit; + double val =3D vshPrettyCapacity(info.capacity, &unit); + + vshPrint(ctl, "%-15s %2.2lf %s\n", _("Capacity:"), val, unit); + + val =3D 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= ); } =20 virStorageVolFree(vol); - return ret; + return true; } =20 /* @@ -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 =3D false; } =20 cleanup: --=20 2.31.1 From nobody Mon Feb 9 23:05:28 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) client-ip=170.10.133.124; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-124.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1632409816; cv=none; d=zohomail.com; s=zohoarc; b=lvY2NEgA/NQITi53o6Lk5QnyZJ7jPY3feI1c26BtzUm3QvTd+GyRruZPYkVDg1Jv6ZA3YmPcI2QBObdybg2u/JvTYeDeGK/RefOXZa/Typ04ZhXKEvuBPZKFQkBDTic2hFDLhPHcFvF8K1nn1i6hs7NHUEbCKbGqVvegdJE/p+w= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1632409816; h=Content-Type:Content-Transfer-Encoding:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=VYvKuuE+jGC00A2BZzZS+YAVkUmPW97sgHnXjxcVDiY=; b=aW2c+TzGhUQpBTRlYKqsWjvI9lk3vx/sitfNU829uPmBHt7wN+Nn7ZlEVx/zo7j9gicFTZDeRdGRo5yZ3GCsImYNv7sDC8nJ1tYnHeC5/KhcBiMKiewJxZIC+IgTFMHmvJY8EPqA7P2xFMpI16g0t4eb3tmwDWi2ih4Ir+7vz48= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.zohomail.com with SMTPS id 1632409816149593.1311589280368; Thu, 23 Sep 2021 08:10:16 -0700 (PDT) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-346-DfJhpoQ3Ne6kpExZSSaUpw-1; Thu, 23 Sep 2021 11:08:50 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 0C1D2802CBD; Thu, 23 Sep 2021 15:08:45 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C053F60622; Thu, 23 Sep 2021 15:08:44 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 8F5C74EA30; Thu, 23 Sep 2021 15:08:44 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 18NF8S3T030611 for ; Thu, 23 Sep 2021 11:08:28 -0400 Received: by smtp.corp.redhat.com (Postfix) id E26DE171FF; Thu, 23 Sep 2021 15:08:28 +0000 (UTC) Received: from fedora.redhat.com (unknown [10.43.2.78]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6B5925FC25 for ; Thu, 23 Sep 2021 15:08:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1632409814; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=VYvKuuE+jGC00A2BZzZS+YAVkUmPW97sgHnXjxcVDiY=; b=OEA1uGgmj84fBYLoVPL1FVnjEIsZ8RBqYAIulMXYBc47o81KQPCn8cTTDrgXgwYLqsp4tE lkTAqms6GLhpV83Cl8WPnESac6dgHTeve4SLm8DIilpgNte/dHhKLyMwKGfPJrTIXVeezk OxtWQlYjkZn+R43aYfsyFUIv/OAqEHk= X-MC-Unique: DfJhpoQ3Ne6kpExZSSaUpw-1 From: Kristina Hanicova To: libvir-list@redhat.com Subject: [PATCH 5/5] virsh: refactor functions to not use 'ret' variable Date: Thu, 23 Sep 2021 17:08:04 +0200 Message-Id: <1e957c4b72cfef56e62eac56441631bb5a34a5bb.1632409613.git.khanicov@redhat.com> In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-loop: libvir-list@redhat.com X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=libvir-list-bounces@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) X-ZM-MESSAGEID: 1632409817052100001 Content-Type: text/plain; charset="utf-8" This patch targets smaller functions and rewrites them into the pattern using an early return without needing redundant 'ret' variable (if possible). Patch also includes removal of 'else' branch after 'return' and other small alterations. Signed-off-by: Kristina Hanicova Reviewed-by: J=C3=A1n Tomko --- tools/virsh-host.c | 26 ++++++----------- tools/virsh-interface.c | 65 ++++++++++++++++++----------------------- tools/virsh-network.c | 47 +++++++++++++---------------- tools/virsh-nodedev.c | 32 ++++++++------------ tools/virsh-nwfilter.c | 15 +++++----- tools/virsh-util.c | 3 +- 6 files changed, 77 insertions(+), 111 deletions(-) diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 591746655b..66cd844263 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -1243,7 +1243,6 @@ static bool cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) { const char *from =3D NULL; - bool ret =3D false; g_autofree char *result =3D NULL; g_auto(GStrv) list =3D NULL; unsigned int flags =3D 0; @@ -1260,16 +1259,13 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) if (!(list =3D vshExtractCPUDefXMLs(ctl, from))) return false; =20 - result =3D virConnectBaselineCPU(priv->conn, (const char **)list, - g_strv_length(list), - flags); - - if (result) { - vshPrint(ctl, "%s", result); - ret =3D true; - } + if (!(result =3D virConnectBaselineCPU(priv->conn, (const char **)list, + g_strv_length(list), + flags))) + return false; =20 - return ret; + vshPrint(ctl, "%s", result); + return true; } =20 /* @@ -1355,7 +1351,6 @@ cmdVersion(vshControl *ctl, const vshCmd *cmd G_GNUC_= UNUSED) unsigned long includeVersion; unsigned long apiVersion; unsigned long daemonVersion; - int ret; unsigned int major; unsigned int minor; unsigned int rel; @@ -1375,8 +1370,7 @@ cmdVersion(vshControl *ctl, const vshCmd *cmd G_GNUC_= UNUSED) vshPrint(ctl, _("Compiled against library: libvirt %d.%d.%d\n"), major, minor, rel); =20 - ret =3D virGetVersion(&libVersion, hvType, &apiVersion); - if (ret < 0) { + if (virGetVersion(&libVersion, hvType, &apiVersion) < 0) { vshError(ctl, "%s", _("failed to get the library version")); return false; } @@ -1394,8 +1388,7 @@ cmdVersion(vshControl *ctl, const vshCmd *cmd G_GNUC_= UNUSED) vshPrint(ctl, _("Using API: %s %d.%d.%d\n"), hvType, major, minor, rel); =20 - ret =3D virConnectGetVersion(priv->conn, &hvVersion); - if (ret < 0) { + if (virConnectGetVersion(priv->conn, &hvVersion) < 0) { vshError(ctl, "%s", _("failed to get the hypervisor version")); return false; } @@ -1413,8 +1406,7 @@ cmdVersion(vshControl *ctl, const vshCmd *cmd G_GNUC_= UNUSED) } =20 if (vshCommandOptBool(cmd, "daemon")) { - ret =3D virConnectGetLibVersion(priv->conn, &daemonVersion); - if (ret < 0) { + if (virConnectGetLibVersion(priv->conn, &daemonVersion) < 0) { vshError(ctl, "%s", _("failed to get the daemon version")); } else { major =3D daemonVersion / 1000000; diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c index 46af45c97b..f402119b68 100644 --- a/tools/virsh-interface.c +++ b/tools/virsh-interface.c @@ -485,26 +485,23 @@ static bool cmdInterfaceDumpXML(vshControl *ctl, const vshCmd *cmd) { virInterfacePtr iface; - bool ret =3D true; g_autofree char *dump =3D NULL; unsigned int flags =3D 0; - bool inactive =3D vshCommandOptBool(cmd, "inactive"); =20 - if (inactive) + if (vshCommandOptBool(cmd, "inactive")) flags |=3D VIR_INTERFACE_XML_INACTIVE; =20 if (!(iface =3D virshCommandOptInterface(ctl, cmd, NULL))) return false; =20 - dump =3D virInterfaceGetXMLDesc(iface, flags); - if (dump !=3D NULL) { - vshPrint(ctl, "%s", dump); - } else { - ret =3D false; + if (!(dump =3D virInterfaceGetXMLDesc(iface, flags))) { + virInterfaceFree(iface); + return false; } =20 + vshPrint(ctl, "%s", dump); virInterfaceFree(iface); - return ret; + return true; } =20 /* @@ -535,7 +532,6 @@ cmdInterfaceDefine(vshControl *ctl, const vshCmd *cmd) { virInterfacePtr iface; const char *from =3D NULL; - bool ret =3D true; g_autofree char *buffer =3D NULL; unsigned int flags =3D 0; virshControl *priv =3D ctl->privData; @@ -549,17 +545,15 @@ cmdInterfaceDefine(vshControl *ctl, const vshCmd *cmd) if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) return false; =20 - iface =3D virInterfaceDefineXML(priv->conn, buffer, flags); - - if (iface !=3D NULL) { - vshPrintExtra(ctl, _("Interface %s defined from %s\n"), - virInterfaceGetName(iface), from); - virInterfaceFree(iface); - } else { + if (!(iface =3D virInterfaceDefineXML(priv->conn, buffer, flags))) { vshError(ctl, _("Failed to define interface from %s"), from); - ret =3D false; + return false; } - return ret; + + vshPrintExtra(ctl, _("Interface %s defined from %s\n"), + virInterfaceGetName(iface), from); + virInterfaceFree(iface); + return true; } =20 /* @@ -584,21 +578,20 @@ static bool cmdInterfaceUndefine(vshControl *ctl, const vshCmd *cmd) { virInterfacePtr iface; - bool ret =3D true; const char *name; =20 if (!(iface =3D virshCommandOptInterface(ctl, cmd, &name))) return false; =20 - if (virInterfaceUndefine(iface) =3D=3D 0) { - vshPrintExtra(ctl, _("Interface %s undefined\n"), name); - } else { + if (virInterfaceUndefine(iface) < 0) { vshError(ctl, _("Failed to undefine interface %s"), name); - ret =3D false; + virInterfaceFree(iface); + return false; } =20 + vshPrintExtra(ctl, _("Interface %s undefined\n"), name); virInterfaceFree(iface); - return ret; + return true; } =20 /* @@ -623,21 +616,20 @@ static bool cmdInterfaceStart(vshControl *ctl, const vshCmd *cmd) { virInterfacePtr iface; - bool ret =3D true; const char *name; =20 if (!(iface =3D virshCommandOptInterface(ctl, cmd, &name))) return false; =20 - if (virInterfaceCreate(iface, 0) =3D=3D 0) { - vshPrintExtra(ctl, _("Interface %s started\n"), name); - } else { + if (virInterfaceCreate(iface, 0) < 0) { vshError(ctl, _("Failed to start interface %s"), name); - ret =3D false; + virInterfaceFree(iface); + return false; } =20 + vshPrintExtra(ctl, _("Interface %s started\n"), name); virInterfaceFree(iface); - return ret; + return true; } =20 /* @@ -662,21 +654,20 @@ static bool cmdInterfaceDestroy(vshControl *ctl, const vshCmd *cmd) { virInterfacePtr iface; - bool ret =3D true; const char *name; =20 if (!(iface =3D virshCommandOptInterface(ctl, cmd, &name))) return false; =20 - if (virInterfaceDestroy(iface, 0) =3D=3D 0) { - vshPrintExtra(ctl, _("Interface %s destroyed\n"), name); - } else { + if (virInterfaceDestroy(iface, 0) < 0) { vshError(ctl, _("Failed to destroy interface %s"), name); - ret =3D false; + virInterfaceFree(iface); + return false; } =20 + vshPrintExtra(ctl, _("Interface %s destroyed\n"), name); virInterfaceFree(iface); - return ret; + return false; } =20 /* diff --git a/tools/virsh-network.c b/tools/virsh-network.c index 8651265909..1442210278 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -209,7 +209,6 @@ cmdNetworkCreate(vshControl *ctl, const vshCmd *cmd) { virNetworkPtr network; const char *from =3D NULL; - bool ret =3D true; g_autofree char *buffer =3D NULL; unsigned int flags =3D 0; virshControl *priv =3D ctl->privData; @@ -228,15 +227,15 @@ cmdNetworkCreate(vshControl *ctl, const vshCmd *cmd) else network =3D virNetworkCreateXML(priv->conn, buffer); =20 - if (network !=3D NULL) { - vshPrintExtra(ctl, _("Network %s created from %s\n"), - virNetworkGetName(network), from); - virNetworkFree(network); - } else { + if (!network) { vshError(ctl, _("Failed to create network from %s"), from); - ret =3D false; + return false; } - return ret; + + vshPrintExtra(ctl, _("Network %s created from %s\n"), + virNetworkGetName(network), from); + virNetworkFree(network); + return true; } =20 /* @@ -267,7 +266,6 @@ cmdNetworkDefine(vshControl *ctl, const vshCmd *cmd) { virNetworkPtr network; const char *from =3D NULL; - bool ret =3D true; g_autofree char *buffer =3D NULL; unsigned int flags =3D 0; virshControl *priv =3D ctl->privData; @@ -286,15 +284,15 @@ cmdNetworkDefine(vshControl *ctl, const vshCmd *cmd) else network =3D virNetworkDefineXML(priv->conn, buffer); =20 - if (network !=3D NULL) { - vshPrintExtra(ctl, _("Network %s defined from %s\n"), - virNetworkGetName(network), from); - virNetworkFree(network); - } else { + if (!network) { vshError(ctl, _("Failed to define network from %s"), from); - ret =3D false; + return false; } - return ret; + + vshPrintExtra(ctl, _("Network %s defined from %s\n"), + virNetworkGetName(network), from); + virNetworkFree(network); + return true; } =20 /* @@ -362,28 +360,23 @@ static bool cmdNetworkDumpXML(vshControl *ctl, const vshCmd *cmd) { virNetworkPtr network; - bool ret =3D true; g_autofree char *dump =3D NULL; unsigned int flags =3D 0; - int inactive; =20 if (!(network =3D virshCommandOptNetwork(ctl, cmd, NULL))) return false; =20 - inactive =3D vshCommandOptBool(cmd, "inactive"); - if (inactive) + if (vshCommandOptBool(cmd, "inactive")) flags |=3D VIR_NETWORK_XML_INACTIVE; =20 - dump =3D virNetworkGetXMLDesc(network, flags); - - if (dump !=3D NULL) { - vshPrint(ctl, "%s", dump); - } else { - ret =3D false; + if (!(dump =3D virNetworkGetXMLDesc(network, flags))) { + virNetworkFree(network); + return false; } =20 + vshPrint(ctl, "%s", dump); virNetworkFree(network); - return ret; + return true; } =20 /* diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index f1b4eb94bf..f72359121f 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -57,7 +57,6 @@ cmdNodeDeviceCreate(vshControl *ctl, const vshCmd *cmd) { virNodeDevicePtr dev =3D NULL; const char *from =3D NULL; - bool ret =3D true; g_autofree char *buffer =3D NULL; virshControl *priv =3D ctl->privData; =20 @@ -67,18 +66,15 @@ cmdNodeDeviceCreate(vshControl *ctl, const vshCmd *cmd) if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) return false; =20 - dev =3D virNodeDeviceCreateXML(priv->conn, buffer, 0); - - if (dev !=3D NULL) { - vshPrintExtra(ctl, _("Node device %s created from %s\n"), - virNodeDeviceGetName(dev), from); - virNodeDeviceFree(dev); - } else { + if (!(dev =3D virNodeDeviceCreateXML(priv->conn, buffer, 0))) { vshError(ctl, _("Failed to create node device from %s"), from); - ret =3D false; + return false; } =20 - return ret; + vshPrintExtra(ctl, _("Node device %s created from %s\n"), + virNodeDeviceGetName(dev), from); + virNodeDeviceFree(dev); + return true; } =20 =20 @@ -1077,7 +1073,6 @@ cmdNodeDeviceDefine(vshControl *ctl, const vshCmd *cm= d G_GNUC_UNUSED) { virNodeDevice *dev =3D NULL; const char *from =3D NULL; - bool ret =3D true; g_autofree char *buffer =3D NULL; virshControl *priv =3D ctl->privData; =20 @@ -1087,18 +1082,15 @@ cmdNodeDeviceDefine(vshControl *ctl, const vshCmd *= cmd G_GNUC_UNUSED) if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) return false; =20 - dev =3D virNodeDeviceDefineXML(priv->conn, buffer, 0); - - if (dev !=3D NULL) { - vshPrintExtra(ctl, _("Node device '%s' defined from '%s'\n"), - virNodeDeviceGetName(dev), from); - virNodeDeviceFree(dev); - } else { + if (!(dev =3D virNodeDeviceDefineXML(priv->conn, buffer, 0))) { vshError(ctl, _("Failed to define node device from '%s'"), from); - ret =3D false; + return false; } =20 - return ret; + vshPrintExtra(ctl, _("Node device '%s' defined from '%s'\n"), + virNodeDeviceGetName(dev), from); + virNodeDeviceFree(dev); + return true; } =20 =20 diff --git a/tools/virsh-nwfilter.c b/tools/virsh-nwfilter.c index 77f211d031..33164f623f 100644 --- a/tools/virsh-nwfilter.c +++ b/tools/virsh-nwfilter.c @@ -515,7 +515,6 @@ cmdNWFilterBindingCreate(vshControl *ctl, const vshCmd = *cmd) { virNWFilterBindingPtr binding; const char *from =3D NULL; - bool ret =3D true; char *buffer; unsigned int flags =3D 0; virshControl *priv =3D ctl->privData; @@ -532,15 +531,15 @@ cmdNWFilterBindingCreate(vshControl *ctl, const vshCm= d *cmd) binding =3D virNWFilterBindingCreateXML(priv->conn, buffer, flags); VIR_FREE(buffer); =20 - if (binding !=3D NULL) { - vshPrintExtra(ctl, _("Network filter binding on %s created from %s= \n"), - virNWFilterBindingGetPortDev(binding), from); - virNWFilterBindingFree(binding); - } else { + if (!binding) { vshError(ctl, _("Failed to create network filter from %s"), from); - ret =3D false; + return false; } - return ret; + + vshPrintExtra(ctl, _("Network filter binding on %s created from %s\n"), + virNWFilterBindingGetPortDev(binding), from); + virNWFilterBindingFree(binding); + return true; } =20 =20 diff --git a/tools/virsh-util.c b/tools/virsh-util.c index 19cd0bcb99..fb74514b3c 100644 --- a/tools/virsh-util.c +++ b/tools/virsh-util.c @@ -137,8 +137,7 @@ virshDomainState(vshControl *ctl, /* fall back to virDomainGetInfo if virDomainGetState is not supported= */ if (virDomainGetInfo(dom, &info) < 0) return -1; - else - return info.state; + return info.state; } =20 =20 --=20 2.31.1