[libvirt PATCH v2 06/12] tools: add 'nodesevinfo' virsh command

Daniel P. Berrangé posted 12 patches 4 years, 2 months ago
There is a newer version of this series
[libvirt PATCH v2 06/12] tools: add 'nodesevinfo' virsh command
Posted by Daniel P. Berrangé 4 years, 2 months ago
While some SEV info is reported in the domain capabilities,
for reasons of size, this excludes the certificates. The
nodesevinfo command provides the full set of information.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 docs/manpages/virsh.rst | 14 +++++++++++++
 tools/virsh-host.c      | 45 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 1a74217625..e828f7ef68 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -479,6 +479,20 @@ Returns memory stats of the node.
 If *cell* is specified, this will print the specified cell statistics only.
 
 
+nodesevinfo
+-----------
+
+**Syntax:**
+
+::
+
+   nodesevinfo
+
+Reports information about the AMD SEV launch security features for
+the node, if any. Some of this information is also reported in the
+domain capabilities XML document.
+
+
 nodesuspend
 -----------
 
diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index 5da1346a9c..5ee3834de2 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -888,6 +888,45 @@ cmdNodeMemStats(vshControl *ctl, const vshCmd *cmd)
     return true;
 }
 
+/*
+ * "nodesevinfo" command
+ */
+static const vshCmdInfo info_nodesevinfo[] = {
+    {.name = "help",
+     .data = N_("node SEV information")
+    },
+    {.name = "desc",
+     .data = N_("Returns basic SEV information about the node.")
+    },
+    {.name = NULL}
+};
+
+static bool
+cmdNodeSEVInfo(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED)
+{
+    virshControl *priv = ctl->privData;
+    size_t i;
+    int nparams = 0;
+    virTypedParameterPtr params = NULL;
+    bool ret = false;
+
+    if (virNodeGetSEVInfo(priv->conn, &params, &nparams, 0) != 0) {
+        vshError(ctl, "%s", _("Unable to get host SEV information"));
+        goto cleanup;
+    }
+
+    for (i = 0; i < nparams; i++) {
+        g_autofree char *str = vshGetTypedParamValue(ctl, &params[i]);
+        vshPrint(ctl, "%-18s: %s\n", params[i].field, str);
+    }
+
+    ret = true;
+
+ cleanup:
+    virTypedParamsFree(params, nparams);
+    return ret;
+}
+
 /*
  * "nodesuspend" command
  */
@@ -1828,6 +1867,12 @@ const vshCmdDef hostAndHypervisorCmds[] = {
      .info = info_nodememstats,
      .flags = 0
     },
+    {.name = "nodesevinfo",
+     .handler = cmdNodeSEVInfo,
+     .opts = NULL,
+     .info = info_nodesevinfo,
+     .flags = 0
+    },
     {.name = "nodesuspend",
      .handler = cmdNodeSuspend,
      .opts = opts_node_suspend,
-- 
2.33.1

Re: [libvirt PATCH v2 06/12] tools: add 'nodesevinfo' virsh command
Posted by Peter Krempa 4 years, 2 months ago
On Fri, Dec 10, 2021 at 11:37:29 +0000, Daniel P. Berrangé wrote:
> While some SEV info is reported in the domain capabilities,
> for reasons of size, this excludes the certificates. The
> nodesevinfo command provides the full set of information.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  docs/manpages/virsh.rst | 14 +++++++++++++
>  tools/virsh-host.c      | 45 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 59 insertions(+)
> 
> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
> index 1a74217625..e828f7ef68 100644
> --- a/docs/manpages/virsh.rst
> +++ b/docs/manpages/virsh.rst
> @@ -479,6 +479,20 @@ Returns memory stats of the node.
>  If *cell* is specified, this will print the specified cell statistics only.
>  
>  
> +nodesevinfo
> +-----------
> +
> +**Syntax:**
> +
> +::
> +
> +   nodesevinfo
> +
> +Reports information about the AMD SEV launch security features for
> +the node, if any. Some of this information is also reported in the
> +domain capabilities XML document.

In this instance it wouldbe IMO higly beneficial to mention the
individual macros [1] that can be returned in the API docs:

https://www.libvirt.org/html/libvirt-libvirt-host.html#virNodeGetSEVInfo

And link to the API docs here. Duplicating the docs isn't great but not
documenting what you get isn't either.


[1]:
https://www.libvirt.org/html/libvirt-libvirt-host.html#VIR_NODE_SEV_CBITPOS
https://www.libvirt.org/html/libvirt-libvirt-host.html#VIR_NODE_SEV_CERT_CHAIN
https://www.libvirt.org/html/libvirt-libvirt-host.html#VIR_NODE_SEV_PDH
https://www.libvirt.org/html/libvirt-libvirt-host.html#VIR_NODE_SEV_REDUCED_PHYS_BITS

(It's unfortunate that these don't have a common anchor to link to)


Reviewed-by: Peter Krempa <pkrempa@redhat.com>

Re: [libvirt PATCH v2 06/12] tools: add 'nodesevinfo' virsh command
Posted by Daniel P. Berrangé 4 years, 1 month ago
On Fri, Dec 10, 2021 at 02:52:55PM +0100, Peter Krempa wrote:
> On Fri, Dec 10, 2021 at 11:37:29 +0000, Daniel P. Berrangé wrote:
> > While some SEV info is reported in the domain capabilities,
> > for reasons of size, this excludes the certificates. The
> > nodesevinfo command provides the full set of information.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  docs/manpages/virsh.rst | 14 +++++++++++++
> >  tools/virsh-host.c      | 45 +++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 59 insertions(+)
> > 
> > diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
> > index 1a74217625..e828f7ef68 100644
> > --- a/docs/manpages/virsh.rst
> > +++ b/docs/manpages/virsh.rst
> > @@ -479,6 +479,20 @@ Returns memory stats of the node.
> >  If *cell* is specified, this will print the specified cell statistics only.
> >  
> >  
> > +nodesevinfo
> > +-----------
> > +
> > +**Syntax:**
> > +
> > +::
> > +
> > +   nodesevinfo
> > +
> > +Reports information about the AMD SEV launch security features for
> > +the node, if any. Some of this information is also reported in the
> > +domain capabilities XML document.
> 
> In this instance it wouldbe IMO higly beneficial to mention the
> individual macros [1] that can be returned in the API docs:
> 
> https://www.libvirt.org/html/libvirt-libvirt-host.html#virNodeGetSEVInfo
> 
> And link to the API docs here. Duplicating the docs isn't great but not
> documenting what you get isn't either.

I didn't do any of this in the v3, but I'll look at doing a separate
series to improve the docs here.

> 
> 
> [1]:
> https://www.libvirt.org/html/libvirt-libvirt-host.html#VIR_NODE_SEV_CBITPOS
> https://www.libvirt.org/html/libvirt-libvirt-host.html#VIR_NODE_SEV_CERT_CHAIN
> https://www.libvirt.org/html/libvirt-libvirt-host.html#VIR_NODE_SEV_PDH
> https://www.libvirt.org/html/libvirt-libvirt-host.html#VIR_NODE_SEV_REDUCED_PHYS_BITS
> 
> (It's unfortunate that these don't have a common anchor to link to)
> 
> 
> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|