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

[Martin Kletzander]

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 at gmail.com>
>---
> 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20211129/11e7c891/attachment-0001.sig>