Re: [PATCH 2/3] virsh: Add '--full-seclabels' option for dominfo

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Sep 02, 2021 at 08:29:35PM +0800, Luke Yue wrote:
There is no virsh command uses virDomainGetSecurityLabelList API, so add
an option for dominfo to call it and print full list of security labels.

Also realign some outputs as it's now "Security labels:" instead of
"Security label:".


This should go into separate patch as that makes it nicer to review and
the separation of changes makes it more clear.

Signed-off-by: Luke Yue <lukedyue@xxxxxxxxx>
---
docs/manpages/virsh.rst      |  5 +-
tests/virsh-undefine         |  8 ++--
tests/virshtest.c            | 70 ++++++++++++++--------------
tools/virsh-domain-monitor.c | 89 ++++++++++++++++++++++++------------
4 files changed, 101 insertions(+), 71 deletions(-)

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index f7cf82acdf..2b2746e713 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1299,29 +1304,53 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd)
    } else {
        /* Only print something if a security model is active */
        if (secmodel.model[0] != '\0') {
-            vshPrint(ctl, "%-15s %s\n", _("Security model:"), secmodel.model);
-            vshPrint(ctl, "%-15s %s\n", _("Security DOI:"), secmodel.doi);
-
-            /* Security labels are only valid for active domains */
-            seclabel = g_new0(virSecurityLabel, 1);
+            vshPrint(ctl, "%-16s %s\n", _("Security model:"), secmodel.model);
+            vshPrint(ctl, "%-16s %s\n", _("Security DOI:"), secmodel.doi);
+
+            if (fullseclabels) {
+                int len;
+                size_t i;
+
+                if ((len = virDomainGetSecurityLabelList(dom, &seclabel)) < 0) {
+                    g_clear_pointer(&(seclabel), g_free);
+                    return false;
+                } else {

No need for else branch when the first branch returns.

+                    for (i = 0; i < len; i++)
+                        if (seclabel[i].label[0] != '\0')
+                            vshPrint(ctl, "%-16s %s (%s)\n",
+                                     i == 0 ? _("Security labels:") : "",
+                                     seclabel[i].label,
+                                     seclabel[i].enforcing ?
+                                     "enforcing" :
+                                     "permissive");
+                }

-            if (virDomainGetSecurityLabel(dom, seclabel) == -1) {
-                VIR_FREE(seclabel);
-                return false;
+                g_clear_pointer(&seclabel, g_free);
            } else {
-                if (seclabel->label[0] != '\0')
-                    vshPrint(ctl, "%-15s %s (%s)\n", _("Security label:"),
-                             seclabel->label, seclabel->enforcing ? "enforcing" : "permissive");
-            }
+                /* Security labels are only valid for active domains */
+                seclabel = g_new0(virSecurityLabel, 1);
+
+                if (virDomainGetSecurityLabel(dom, seclabel) == -1) {

You properly used `< 0` before, this should do the same then.

+                    g_clear_pointer(&seclabel, g_free);
+                    return false;
+                } else {

Again, no need for an else branch when if branch returns.

Otherwise it looks good.

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux