Re: [PATCH v2 1/4] conf: refactor launch security to allow more types

2021-06-18 Thread Peter Krempa
On Fri, Jun 18, 2021 at 15:20:24 +0200, Boris Fiuczynski wrote:
> To allow other types of launch security the SEV type specific
> parameters like e.g. policy need to be optional and be separated
> from other new launch security types. A test is added to ensure
> the previously required and now optional launch security policy
> remains required when launch security type is SEV.
> 
> Signed-off-by: Boris Fiuczynski 
> Reviewed-by: Daniel Henrique Barboza 
> ---
>  docs/schemas/domaincommon.rng |  12 +-
>  src/conf/domain_conf.c| 156 +++---
>  src/conf/domain_conf.h|  13 +-
>  src/conf/virconftypes.h   |   2 +
>  src/qemu/qemu_cgroup.c|   4 +-
>  src/qemu/qemu_command.c   |  43 -
>  src/qemu/qemu_driver.c|   2 +-
>  src/qemu/qemu_firmware.c  |   8 +-
>  src/qemu/qemu_namespace.c |  20 ++-
>  src/qemu/qemu_process.c   |  35 +++-
>  src/qemu/qemu_validate.c  |  22 ++-
>  src/security/security_dac.c   |   4 +-
>  ...urity-sev-missing-policy.x86_64-2.12.0.err |   1 +
>  .../launch-security-sev-missing-policy.xml|  34 
>  tests/qemuxml2argvtest.c  |   1 +

There's a bit too much going on in this single commit. Please split it
into appropriate parts.



[PATCH v2 1/4] conf: refactor launch security to allow more types

2021-06-18 Thread Boris Fiuczynski
To allow other types of launch security the SEV type specific
parameters like e.g. policy need to be optional and be separated
from other new launch security types. A test is added to ensure
the previously required and now optional launch security policy
remains required when launch security type is SEV.

Signed-off-by: Boris Fiuczynski 
Reviewed-by: Daniel Henrique Barboza 
---
 docs/schemas/domaincommon.rng |  12 +-
 src/conf/domain_conf.c| 156 +++---
 src/conf/domain_conf.h|  13 +-
 src/conf/virconftypes.h   |   2 +
 src/qemu/qemu_cgroup.c|   4 +-
 src/qemu/qemu_command.c   |  43 -
 src/qemu/qemu_driver.c|   2 +-
 src/qemu/qemu_firmware.c  |   8 +-
 src/qemu/qemu_namespace.c |  20 ++-
 src/qemu/qemu_process.c   |  35 +++-
 src/qemu/qemu_validate.c  |  22 ++-
 src/security/security_dac.c   |   4 +-
 ...urity-sev-missing-policy.x86_64-2.12.0.err |   1 +
 .../launch-security-sev-missing-policy.xml|  34 
 tests/qemuxml2argvtest.c  |   1 +
 15 files changed, 257 insertions(+), 100 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/launch-security-sev-missing-policy.x86_64-2.12.0.err
 create mode 100644 
tests/qemuxml2argvdata/launch-security-sev-missing-policy.xml

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 5ea14b6dbf..8c1b6c3a09 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -483,7 +483,9 @@
   
 
   
-sev
+
+  sev
+
   
   
 
@@ -496,9 +498,11 @@
 
   
 
-
-  
-
+
+  
+
+  
+
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f65509d8ec..8c2f4fd227 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3490,8 +3490,7 @@ virDomainResctrlDefFree(virDomainResctrlDef *resctrl)
 }
 
 
-static void
-virDomainSEVDefFree(virDomainSEVDef *def)
+void virDomainSEVDefFree(virDomainSEVDef *def)
 {
 if (!def)
 return;
@@ -3502,6 +3501,17 @@ virDomainSEVDefFree(virDomainSEVDef *def)
 g_free(def);
 }
 
+void virDomainSecDefFree(virDomainSecDef *def)
+{
+if (!def)
+return;
+
+virDomainSEVDefFree(def->sev);
+
+g_free(def);
+}
+
+
 static void
 virDomainOSDefClear(virDomainOSDef *os)
 {
@@ -3703,7 +3713,7 @@ void virDomainDefFree(virDomainDef *def)
 if (def->namespaceData && def->ns.free)
 (def->ns.free)(def->namespaceData);
 
-virDomainSEVDefFree(def->sev);
+virDomainSecDefFree(def->sec);
 
 xmlFreeNode(def->metadata);
 
@@ -14719,72 +14729,80 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
 xmlXPathContextPtr ctxt)
 {
 VIR_XPATH_NODE_AUTORESTORE(ctxt)
-virDomainSEVDef *def;
+g_autoptr(virDomainSEVDef) sev = g_new0(virDomainSEVDef, 1);
 unsigned long policy;
-g_autofree char *type = NULL;
 int rc = -1;
 
-def = g_new0(virDomainSEVDef, 1);
-
 ctxt->node = sevNode;
 
-if (!(type = virXMLPropString(sevNode, "type"))) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
-   _("missing launch security type"));
-goto error;
-}
-
-def->sectype = virDomainLaunchSecurityTypeFromString(type);
-switch ((virDomainLaunchSecurity) def->sectype) {
-case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
-break;
-case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
-case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
-default:
-virReportError(VIR_ERR_XML_ERROR,
-   _("unsupported launch security type '%s'"),
-   type);
-goto error;
-}
-
 if (virXPathULongHex("string(./policy)", ctxt, ) < 0) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
-   _("failed to get launch security policy"));
-goto error;
+   _("Failed to get launch security policy for "
+ "launch security type SEV"));
+return NULL;
 }
 
 /* the following attributes are platform dependent and if missing, we can
  * autofill them from domain capabilities later
  */
-rc = virXPathUInt("string(./cbitpos)", ctxt, >cbitpos);
+rc = virXPathUInt("string(./cbitpos)", ctxt, >cbitpos);
 if (rc == 0) {
-def->haveCbitpos = true;
+sev->haveCbitpos = true;
 } else if (rc == -2) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("Invalid format for launch security cbitpos"));
-goto error;
+return NULL;
 }
 
 rc = virXPathUInt("string(./reducedPhysBits)", ctxt,
-  >reduced_phys_bits);
+  >reduced_phys_bits);