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 <fiuczy@xxxxxxxxxxxxx>
---
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, &policy) < 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, &def->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,
+ &def->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(&def);
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, &policy) < 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, &def->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,
- &def->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, "<launchSecurity type='%s'>\n",
- virDomainLaunchSecurityTypeToString(sev->sectype));
- virBufferAdjustIndent(buf, 2);
+ switch ((virDomainLaunchSecurity) sev->sectype) {
+ case VIR_DOMAIN_LAUNCH_SECURITY_SEV: {
+ virBufferAsprintf(buf, "<launchSecurity type='%s'>\n",
+ virDomainLaunchSecurityTypeToString(sev->sectype));
I would keep this line out out of the switch as it can be reused by
other launchSecurity types.
In the spirit of modernizing the parse/format code you can user the
following pattern witch will make addition of other types with
sub-elements easier:
g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf);
g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER;
virBufferAsprintf(buf, " type='%s'",
virDomainLaunchSecurityTypeToString(sev->sectype));
switch ((virDomainLaunchSecurity) sev->sectype) {
case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
if (sev->haveCbitpos)
virBufferAsprintf(childBuf, "<cbitpos>%d</cbitpos>\n", sev->cbitpos);
...
}
virXMLFormatElement(buf, "launchSecurity", &attrBuf, &childBuf);
With this helper we don't have to duplicate the code formatting
launchSecurity type and it will also correctly handle if there are any
sub-elements or not.
Otherwise looks good.
Pavel
+ virBufferAdjustIndent(buf, 2);
- if (sev->haveCbitpos)
- virBufferAsprintf(buf, "<cbitpos>%d</cbitpos>\n", sev->cbitpos);
+ if (sev->haveCbitpos)
+ virBufferAsprintf(buf, "<cbitpos>%d</cbitpos>\n", sev->cbitpos);
- if (sev->haveReducedPhysBits)
- virBufferAsprintf(buf, "<reducedPhysBits>%d</reducedPhysBits>\n",
- sev->reduced_phys_bits);
- virBufferAsprintf(buf, "<policy>0x%04x</policy>\n", sev->policy);
- if (sev->dh_cert)
- virBufferEscapeString(buf, "<dhCert>%s</dhCert>\n", sev->dh_cert);
+ if (sev->haveReducedPhysBits)
+ virBufferAsprintf(buf, "<reducedPhysBits>%d</reducedPhysBits>\n",
+ sev->reduced_phys_bits);
+ virBufferAsprintf(buf, "<policy>0x%04x</policy>\n", sev->policy);
+ if (sev->dh_cert)
+ virBufferEscapeString(buf, "<dhCert>%s</dhCert>\n", sev->dh_cert);
- if (sev->session)
- virBufferEscapeString(buf, "<session>%s</session>\n", sev->session);
+ if (sev->session)
+ virBufferEscapeString(buf, "<session>%s</session>\n", sev->session);
- virBufferAdjustIndent(buf, -2);
- virBufferAddLit(buf, "</launchSecurity>\n");
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</launchSecurity>\n");
+ break;
+ }
+ case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
+ case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
+ break;
+ }
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index f706c498ff..512c7c8bd7 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3285,6 +3285,8 @@ void virDomainRedirdevDefFree(virDomainRedirdevDef *def);
void virDomainRedirFilterDefFree(virDomainRedirFilterDef *def);
void virDomainShmemDefFree(virDomainShmemDef *def);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainShmemDef, virDomainShmemDefFree);
+void virDomainSEVDefFree(virDomainSEVDef *def);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainSEVDef, virDomainSEVDefFree);
void virDomainDeviceDefFree(virDomainDeviceDef *def);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainDeviceDef, virDomainDeviceDefFree);
--
2.30.2
