Re: [PATCH v3 2/6] conf: modernize SEV XML parse and format methods

2021-06-28 Thread Boris Fiuczynski 

On 6/25/21 10:39 AM, Pavel Hrdina wrote:

On Tue, Jun 22, 2021 at 03:10:45PM +0200, Boris Fiuczynski wrote:

Make use of virDomainLaunchSecurity enum and automatic memory freeing.

Signed-off-by: Boris Fiuczynski 
---
  src/conf/domain_conf.c | 123 +
  src/conf/domain_conf.h |   2 +
  2 files changed, 64 insertions(+), 61 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index af2fd03d3c..93ec50ff5d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3490,7 +3490,7 @@ virDomainResctrlDefFree(virDomainResctrlDef *resctrl)
  }
  
  
-static void

+void
  virDomainSEVDefFree(virDomainSEVDef *def)
  {
  if (!def)
@@ -14719,73 +14719,66 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
  xmlXPathContextPtr ctxt)
  {
  VIR_XPATH_NODE_AUTORESTORE(ctxt)
-virDomainSEVDef *def;
  unsigned long policy;
  g_autofree char *type = NULL;
  int rc = -1;
  


No need for this empty line.


-def = g_new0(virDomainSEVDef, 1);
+g_autoptr(virDomainSEVDef) 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;
+return NULL;
  }
  
  def->sectype = virDomainLaunchSecurityTypeFromString(type);

  switch ((virDomainLaunchSecurity) def->sectype) {
  case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
-break;
+if (virXPathULongHex("string(./policy)", ctxt, ) < 0) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("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);
+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 NULL;
+}
+
+def->policy = policy;
+def->dh_cert = virXPathString("string(./dhCert)", ctxt);
+def->session = virXPathString("string(./session)", ctxt);
+
+return g_steal_pointer();
  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 for "
- "launch security type SEV"));
-goto error;
-}
-
-/* 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"));
-goto error;
-}
-
-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"));
-goto error;
+return NULL;
  }
-
-def->policy = policy;
-def->dh_cert = virXPathString("string(./dhCert)", ctxt);
-def->session = virXPathString("string(./session)", ctxt);
-
-return def;
-
- error:
-virDomainSEVDefFree(def);
-return NULL;
  }
  
  
@@ -26844,25 +26837,33 @@ virDomainSEVDefFormat(virBuffer *buf, virDomainSEVDef *sev)

  if (!sev)
  return;
  
-virBufferAsprintf(buf, "\n",

-  virDomainLaunchSecurityTypeToString(sev->sectype));
-virBufferAdjustIndent(buf, 2);
+switch ((virDomainLaunchSecurity) sev->sectype) {
+case VIR_DOMAIN_LAUNCH_SECURITY_SEV: {
+virBufferAsprintf(buf, "\n",
+

Re: [PATCH v3 2/6] conf: modernize SEV XML parse and format methods

2021-06-25 Thread Pavel Hrdina 
On Tue, Jun 22, 2021 at 03:10:45PM +0200, Boris Fiuczynski wrote:
> Make use of virDomainLaunchSecurity enum and automatic memory freeing.
> 
> Signed-off-by: Boris Fiuczynski 
> ---
>  src/conf/domain_conf.c | 123 +
>  src/conf/domain_conf.h |   2 +
>  2 files changed, 64 insertions(+), 61 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index af2fd03d3c..93ec50ff5d 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -3490,7 +3490,7 @@ virDomainResctrlDefFree(virDomainResctrlDef *resctrl)
>  }
>  
>  
> -static void
> +void
>  virDomainSEVDefFree(virDomainSEVDef *def)
>  {
>  if (!def)
> @@ -14719,73 +14719,66 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
>  xmlXPathContextPtr ctxt)
>  {
>  VIR_XPATH_NODE_AUTORESTORE(ctxt)
> -virDomainSEVDef *def;
>  unsigned long policy;
>  g_autofree char *type = NULL;
>  int rc = -1;
>  

No need for this empty line.

> -def = g_new0(virDomainSEVDef, 1);
> +g_autoptr(virDomainSEVDef) 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;
> +return NULL;
>  }
>  
>  def->sectype = virDomainLaunchSecurityTypeFromString(type);
>  switch ((virDomainLaunchSecurity) def->sectype) {
>  case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
> -break;
> +if (virXPathULongHex("string(./policy)", ctxt, ) < 0) {
> +virReportError(VIR_ERR_XML_ERROR, "%s",
> +   _("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);
> +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 NULL;
> +}
> +
> +def->policy = policy;
> +def->dh_cert = virXPathString("string(./dhCert)", ctxt);
> +def->session = virXPathString("string(./session)", ctxt);
> +
> +return g_steal_pointer();
>  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 for "
> - "launch security type SEV"));
> -goto error;
> -}
> -
> -/* 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"));
> -goto error;
> -}
> -
> -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"));
> -goto error;
> +return NULL;
>  }
> -
> -def->policy = policy;
> -def->dh_cert = virXPathString("string(./dhCert)", ctxt);
> -def->session = virXPathString("string(./session)", ctxt);
> -
> -return def;
> -
> - error:
> -virDomainSEVDefFree(def);
> -return NULL;
>  }
>  
>  
> @@ -26844,25 +26837,33 @@ virDomainSEVDefFormat(virBuffer *buf, 
> virDomainSEVDef *sev)
>  if (!sev)
>  return;
>  
> -virBufferAsprintf(buf, "\n",
> -

Re: [PATCH v3 2/6] conf: modernize SEV XML parse and format methods

2021-06-22 Thread Daniel Henrique Barboza 




On 6/22/21 10:10 AM, Boris Fiuczynski wrote:

Make use of virDomainLaunchSecurity enum and automatic memory freeing.

Signed-off-by: Boris Fiuczynski 
---


Reviewed-by: Daniel Henrique Barboza 


  src/conf/domain_conf.c | 123 +
  src/conf/domain_conf.h |   2 +
  2 files changed, 64 insertions(+), 61 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index af2fd03d3c..93ec50ff5d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3490,7 +3490,7 @@ virDomainResctrlDefFree(virDomainResctrlDef *resctrl)
  }
  
  
-static void

+void
  virDomainSEVDefFree(virDomainSEVDef *def)
  {
  if (!def)
@@ -14719,73 +14719,66 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
  xmlXPathContextPtr ctxt)
  {
  VIR_XPATH_NODE_AUTORESTORE(ctxt)
-virDomainSEVDef *def;
  unsigned long policy;
  g_autofree char *type = NULL;
  int rc = -1;
  
-def = g_new0(virDomainSEVDef, 1);

+g_autoptr(virDomainSEVDef) 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;
+return NULL;
  }
  
  def->sectype = virDomainLaunchSecurityTypeFromString(type);

  switch ((virDomainLaunchSecurity) def->sectype) {
  case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
-break;
+if (virXPathULongHex("string(./policy)", ctxt, ) < 0) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("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);
+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 NULL;
+}
+
+def->policy = policy;
+def->dh_cert = virXPathString("string(./dhCert)", ctxt);
+def->session = virXPathString("string(./session)", ctxt);
+
+return g_steal_pointer();
  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 for "
- "launch security type SEV"));
-goto error;
-}
-
-/* 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"));
-goto error;
-}
-
-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"));
-goto error;
+return NULL;
  }
-
-def->policy = policy;
-def->dh_cert = virXPathString("string(./dhCert)", ctxt);
-def->session = virXPathString("string(./session)", ctxt);
-
-return def;
-
- error:
-virDomainSEVDefFree(def);
-return NULL;
  }
  
  
@@ -26844,25 +26837,33 @@ virDomainSEVDefFormat(virBuffer *buf, virDomainSEVDef *sev)

  if (!sev)
  return;
  
-virBufferAsprintf(buf, "\n",

-  virDomainLaunchSecurityTypeToString(sev->sectype));
-virBufferAdjustIndent(buf, 2);
+switch ((virDomainLaunchSecurity) sev->sectype) {
+case VIR_DOMAIN_LAUNCH_SECURITY_SEV: {
+virBufferAsprintf(buf, "\n",
+