Re: [PATCH v5 04/11] conf: Refactor launch security to allow more types
On Fri, Jul 16, 2021 at 11:44:28AM +0200, Boris Fiuczynski wrote:
> Adding virDomainSecDef for general launch security data
> and moving virDomainSEVDef as an element for SEV data.
>
> Signed-off-by: Boris Fiuczynski
> Reviewed-by: Daniel Henrique Barboza
> ---
> src/conf/domain_conf.c | 133 +---
> src/conf/domain_conf.h | 16 +++--
> src/conf/virconftypes.h | 2 +
> src/qemu/qemu_cgroup.c | 4 +-
> src/qemu/qemu_command.c | 47 ++---
> src/qemu/qemu_driver.c | 3 +-
> src/qemu/qemu_firmware.c| 32 +
> src/qemu/qemu_namespace.c | 20 --
> src/qemu/qemu_process.c | 34 +++--
> src/qemu/qemu_validate.c| 22 --
> src/security/security_dac.c | 6 +-
> 11 files changed, 217 insertions(+), 102 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 74254d505b..af7b4f8ef8 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -3491,17 +3491,25 @@ virDomainResctrlDefFree(virDomainResctrlDef *resctrl)
>
>
> void
> -virDomainSEVDefFree(virDomainSEVDef *def)
> +virDomainSecDefFree(virDomainSecDef *def)
> {
> if (!def)
> return;
>
> -g_free(def->dh_cert);
> -g_free(def->session);
> +switch ((virDomainLaunchSecurity) def->sectype) {
> +case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
> +g_free(def->data.sev.dh_cert);
> +g_free(def->data.sev.session);
> +break;
> +case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
> +case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
> +break;
> +}
>
> g_free(def);
> }
>
> +
Unrelated empty line change.
> static void
> virDomainOSDefClear(virDomainOSDef *os)
> {
> @@ -3703,7 +3711,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);
>
> @@ -14714,68 +14722,82 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr node,
> }
>
>
> -static virDomainSEVDef *
> -virDomainSEVDefParseXML(xmlNodePtr sevNode,
> +static int
> +virDomainSEVDefParseXML(virDomainSEVDef *def,
> +xmlNodePtr sevNode,
No need to pass sevNode attribute ...
> xmlXPathContextPtr ctxt)
> {
> VIR_XPATH_NODE_AUTORESTORE(ctxt)
> -g_autoptr(virDomainSEVDef) def = NULL;
> unsigned long policy;
> int rc;
>
> -def = g_new0(virDomainSEVDef, 1);
> -
> ctxt->node = sevNode;
ctxt->node is already set to correct pointer in virDomainSecDefParseXML
function.
>
> -if (virXMLPropEnum(sevNode, "type",
> virDomainLaunchSecurityTypeFromString,
> - VIR_XML_PROP_NONZERO | VIR_XML_PROP_REQUIRED,
> - >sectype) < 0)
> -return NULL;
> +if (virXPathULongHex("string(./policy)", ctxt, ) < 0) {
> +virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("failed to get launch security policy"));
> +return -1;
> +}
>
> -switch ((virDomainLaunchSecurity) def->sectype) {
> -case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
> -if (virXPathULongHex("string(./policy)", ctxt, ) < 0) {
> -virReportError(VIR_ERR_XML_ERROR, "%s",
> - _("failed to get launch security policy"));
> -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);
> +if (rc == 0) {
> +def->haveCbitpos = true;
> +} else if (rc == -2) {
> +virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("Invalid format for launch security cbitpos"));
> +return -1;
> +}
>
> -/* the following attributes are platform dependent and if missing,
> we can
> - * autofill them from domain capabilities later
> -*/
> -rc = virXPathUInt("string(./cbitpos)", ctxt, >cbitpos);
> -if (rc == 0) {
> -def->haveCbitpos = true;
> -} else if (rc == -2) {
> -virReportError(VIR_ERR_XML_ERROR, "%s",
> - _("Invalid format for launch security cbitpos"));
> -return NULL;
> -}
> +rc = virXPathUInt("string(./reducedPhysBits)", ctxt,
> + >reduced_phys_bits);
> +if (rc == 0) {
> +def->haveReducedPhysBits = true;
> +} else if (rc == -2) {
> +virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("Invalid format for launch security "
> + "reduced-phys-bits"));
> +return -1;
> +}
> +
> +def->policy = policy;
> +def->dh_cert = virXPathString("string(./dhCert)", ctxt);
> +def->session = virXPathString("string(./session)", ctxt);
> +
[PATCH v5 04/11] conf: Refactor launch security to allow more types
Adding virDomainSecDef for general launch security data
and moving virDomainSEVDef as an element for SEV data.
Signed-off-by: Boris Fiuczynski
Reviewed-by: Daniel Henrique Barboza
---
src/conf/domain_conf.c | 133 +---
src/conf/domain_conf.h | 16 +++--
src/conf/virconftypes.h | 2 +
src/qemu/qemu_cgroup.c | 4 +-
src/qemu/qemu_command.c | 47 ++---
src/qemu/qemu_driver.c | 3 +-
src/qemu/qemu_firmware.c| 32 +
src/qemu/qemu_namespace.c | 20 --
src/qemu/qemu_process.c | 34 +++--
src/qemu/qemu_validate.c| 22 --
src/security/security_dac.c | 6 +-
11 files changed, 217 insertions(+), 102 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 74254d505b..af7b4f8ef8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3491,17 +3491,25 @@ virDomainResctrlDefFree(virDomainResctrlDef *resctrl)
void
-virDomainSEVDefFree(virDomainSEVDef *def)
+virDomainSecDefFree(virDomainSecDef *def)
{
if (!def)
return;
-g_free(def->dh_cert);
-g_free(def->session);
+switch ((virDomainLaunchSecurity) def->sectype) {
+case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
+g_free(def->data.sev.dh_cert);
+g_free(def->data.sev.session);
+break;
+case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
+case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
+break;
+}
g_free(def);
}
+
static void
virDomainOSDefClear(virDomainOSDef *os)
{
@@ -3703,7 +3711,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);
@@ -14714,68 +14722,82 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr node,
}
-static virDomainSEVDef *
-virDomainSEVDefParseXML(xmlNodePtr sevNode,
+static int
+virDomainSEVDefParseXML(virDomainSEVDef *def,
+xmlNodePtr sevNode,
xmlXPathContextPtr ctxt)
{
VIR_XPATH_NODE_AUTORESTORE(ctxt)
-g_autoptr(virDomainSEVDef) def = NULL;
unsigned long policy;
int rc;
-def = g_new0(virDomainSEVDef, 1);
-
ctxt->node = sevNode;
-if (virXMLPropEnum(sevNode, "type", virDomainLaunchSecurityTypeFromString,
- VIR_XML_PROP_NONZERO | VIR_XML_PROP_REQUIRED,
- >sectype) < 0)
-return NULL;
+if (virXPathULongHex("string(./policy)", ctxt, ) < 0) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("failed to get launch security policy"));
+return -1;
+}
-switch ((virDomainLaunchSecurity) def->sectype) {
-case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
-if (virXPathULongHex("string(./policy)", ctxt, ) < 0) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("failed to get launch security policy"));
-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);
+if (rc == 0) {
+def->haveCbitpos = true;
+} else if (rc == -2) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Invalid format for launch security cbitpos"));
+return -1;
+}
-/* the following attributes are platform dependent and if missing, we
can
- * autofill them from domain capabilities later
-*/
-rc = virXPathUInt("string(./cbitpos)", ctxt, >cbitpos);
-if (rc == 0) {
-def->haveCbitpos = true;
-} else if (rc == -2) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Invalid format for launch security cbitpos"));
-return NULL;
-}
+rc = virXPathUInt("string(./reducedPhysBits)", ctxt,
+ >reduced_phys_bits);
+if (rc == 0) {
+def->haveReducedPhysBits = true;
+} else if (rc == -2) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Invalid format for launch security "
+ "reduced-phys-bits"));
+return -1;
+}
+
+def->policy = policy;
+def->dh_cert = virXPathString("string(./dhCert)", ctxt);
+def->session = virXPathString("string(./session)", ctxt);
+
+return 0;
+}
-rc = virXPathUInt("string(./reducedPhysBits)", ctxt,
- >reduced_phys_bits);
-if (rc == 0) {
-def->haveReducedPhysBits = true;
-} else if (rc == -2) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Invalid format for launch security "
- "reduced-phys-bits"));
-return NULL;
-}
-def->policy = policy;
-def->dh_cert
