[libvirt] [PATCH 3/7] Revert changes to sec label parsing

Daniel P. Berrange berrange at redhat.com
Wed Jan 11 16:33:32 UTC 2012


From: "Daniel P. Berrange" <berrange at redhat.com>

Revert parsing changes:

  commit 302fe95ffa1bc5f1c61c0beb31a1adfbc38c668e
  Author: Eric Blake <eblake at redhat.com>
  Date:   Wed Jan 4 16:01:24 2012 -0700

    seclabel: fix regression in libvirtd restart

  commit b43432931aef92325920953ff92beabfbe5224c8
  Author: Eric Blake <eblake at redhat.com>
  Date:   Thu Dec 22 17:47:50 2011 -0700

    seclabel: allow a seclabel override on a disk src

These two commits changed the sec label parsing code so that
the same code dealt with both the VM level sec label, and the
per device label. Unfortunately, as we add more options to the
VM level sec label, the logic required to use the same parsing
code for the per device label becomes unintelligable.

* src/conf/domain_conf.c: Remove support for parsing per
  device sec labels
---
 src/conf/domain_conf.c |  190 ++++++++++++-----------------------------------
 1 files changed, 49 insertions(+), 141 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 180dd2b..8db674b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1,7 +1,7 @@
 /*
  * domain_conf.c: domain XML processing
  *
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2011 Red Hat, Inc.
  * Copyright (C) 2006-2008 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -798,15 +798,6 @@ virSecurityLabelDefClear(virSecurityLabelDefPtr def)
     VIR_FREE(def->baselabel);
 }
 
-static void
-virSecurityLabelDefFree(virSecurityLabelDefPtr def)
-{
-    if (!def)
-        return;
-    virSecurityLabelDefClear(def);
-    VIR_FREE(def);
-}
-
 void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def)
 {
     int ii;
@@ -876,7 +867,6 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
 
     VIR_FREE(def->serial);
     VIR_FREE(def->src);
-    virSecurityLabelDefFree(def->seclabel);
     VIR_FREE(def->dst);
     VIR_FREE(def->driverName);
     VIR_FREE(def->driverType);
@@ -2527,33 +2517,31 @@ virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def)
     return 0;
 }
 
-/* Parse the portion of a SecurityLabel that is common to both the
- * top-level <seclabel> and to a per-device override.
- * default_seclabel is NULL for top-level, or points to the top-level
- * when parsing an override.  */
 static int
-virSecurityLabelDefParseXMLHelper(virSecurityLabelDefPtr def,
-                                  xmlNodePtr node,
-                                  xmlXPathContextPtr ctxt,
-                                  virSecurityLabelDefPtr default_seclabel,
-                                  unsigned int flags)
+virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
+                            xmlXPathContextPtr ctxt,
+                            unsigned int flags)
 {
     char *p;
-    xmlNodePtr save_ctxt = ctxt->node;
-    int ret = -1;
-    int type = default_seclabel ? default_seclabel->type : def->type;
 
-    ctxt->node = node;
+    if (virXPathNode("./seclabel", ctxt) == NULL)
+        return 0;
 
-    /* Can't use overrides if top-level doesn't allow relabeling.  */
-    if (default_seclabel && default_seclabel->norelabel) {
-        virDomainReportError(VIR_ERR_XML_ERROR, "%s",
-                             _("label overrides require relabeling to be "
-                               "enabled at the domain level"));
-            goto cleanup;
+    p = virXPathStringLimit("string(./seclabel/@type)",
+                            VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
+    if (p == NULL) {
+        virDomainReportError(VIR_ERR_XML_ERROR,
+                             "%s", _("missing security type"));
+        goto error;
     }
-
-    p = virXPathStringLimit("string(./@relabel)",
+    def->type = virDomainSeclabelTypeFromString(p);
+    VIR_FREE(p);
+    if (def->type < 0) {
+        virDomainReportError(VIR_ERR_XML_ERROR,
+                             "%s", _("invalid security type"));
+        goto error;
+    }
+    p = virXPathStringLimit("string(./seclabel/@relabel)",
                             VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
     if (p != NULL) {
         if (STREQ(p, "yes")) {
@@ -2564,77 +2552,38 @@ virSecurityLabelDefParseXMLHelper(virSecurityLabelDefPtr def,
             virDomainReportError(VIR_ERR_XML_ERROR,
                                  _("invalid security relabel value %s"), p);
             VIR_FREE(p);
-            goto cleanup;
+            goto error;
         }
         VIR_FREE(p);
-        if (!default_seclabel &&
-            type == VIR_DOMAIN_SECLABEL_DYNAMIC &&
+        if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC &&
             def->norelabel) {
-            virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                                 _("dynamic label type must use resource "
-                                   "relabeling"));
-            goto cleanup;
+            virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                                 "%s", _("dynamic label type must use resource relabeling"));
+            goto error;
         }
     } else {
-        if (!default_seclabel && type == VIR_DOMAIN_SECLABEL_STATIC)
+        if (def->type == VIR_DOMAIN_SECLABEL_STATIC)
             def->norelabel = true;
         else
             def->norelabel = false;
     }
 
     /* Only parse label, if using static labels, or
-     * if the 'live' VM XML is requested, or if this is a device override
+     * if the 'live' VM XML is requested
      */
-    if (type == VIR_DOMAIN_SECLABEL_STATIC ||
-        !(flags & VIR_DOMAIN_XML_INACTIVE) ||
-        (default_seclabel && !def->norelabel)) {
-        p = virXPathStringLimit("string(./label[1])",
+    if (def->type == VIR_DOMAIN_SECLABEL_STATIC ||
+        !(flags & VIR_DOMAIN_XML_INACTIVE)) {
+        p = virXPathStringLimit("string(./seclabel/label[1])",
                                 VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
-        if (p == NULL && !(default_seclabel && def->norelabel)) {
+        if (p == NULL) {
             virDomainReportError(VIR_ERR_XML_ERROR,
                                  "%s", _("security label is missing"));
-            goto cleanup;
+            goto error;
         }
 
         def->label = p;
     }
 
-    ret = 0;
-cleanup:
-    ctxt->node = save_ctxt;
-    return ret;
-}
-
-/* Parse the top-level <seclabel>, if present.  */
-static int
-virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
-                            xmlXPathContextPtr ctxt,
-                            unsigned int flags)
-{
-    char *p;
-    xmlNodePtr node = virXPathNode("./seclabel", ctxt);
-
-    if (node == NULL)
-        return 0;
-
-    p = virXPathStringLimit("string(./seclabel/@type)",
-                            VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
-    if (p == NULL) {
-        virDomainReportError(VIR_ERR_XML_ERROR,
-                             "%s", _("missing security type"));
-        goto error;
-    }
-    def->type = virDomainSeclabelTypeFromString(p);
-    VIR_FREE(p);
-    if (def->type < 0) {
-        virDomainReportError(VIR_ERR_XML_ERROR,
-                             "%s", _("invalid security type"));
-        goto error;
-    }
-
-    if (virSecurityLabelDefParseXMLHelper(def, node, ctxt, NULL, flags) < 0)
-        goto error;
-
     /* Only parse imagelabel, if requested live XML with relabeling */
     if (!def->norelabel &&
         !(flags & VIR_DOMAIN_XML_INACTIVE)) {
@@ -2760,7 +2709,6 @@ virDomainDiskDefParseXML(virCapsPtr caps,
                          xmlNodePtr node,
                          xmlXPathContextPtr ctxt,
                          virBitmapPtr bootMap,
-                         virSecurityLabelDefPtr default_seclabel,
                          unsigned int flags)
 {
     virDomainDiskDefPtr def;
@@ -3068,16 +3016,6 @@ virDomainDiskDefParseXML(virCapsPtr caps,
         goto error;
     }
 
-    /* If source is present, check for an optional seclabel override.  */
-    if (source) {
-        xmlNodePtr seclabel = virXPathNode("./source/seclabel", ctxt);
-        if (seclabel &&
-            (VIR_ALLOC(def->seclabel) < 0 ||
-             virSecurityLabelDefParseXMLHelper(def->seclabel, seclabel, ctxt,
-                                               default_seclabel, flags) < 0))
-            goto error;
-    }
-
     if (target == NULL) {
         virDomainReportError(VIR_ERR_NO_TARGET,
                              source ? "%s" : NULL, source);
@@ -6400,8 +6338,7 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps,
     if (xmlStrEqual(node->name, BAD_CAST "disk")) {
         dev->type = VIR_DOMAIN_DEVICE_DISK;
         if (!(dev->data.disk = virDomainDiskDefParseXML(caps, node, ctxt,
-                                                        NULL, &def->seclabel,
-                                                        flags)))
+                                                        NULL, flags)))
             goto error;
     } else if (xmlStrEqual(node->name, BAD_CAST "lease")) {
         dev->type = VIR_DOMAIN_DEVICE_LEASE;
@@ -7503,7 +7440,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
                                                             nodes[i],
                                                             ctxt,
                                                             bootMap,
-                                                            &def->seclabel,
                                                             flags);
         if (!disk)
             goto error;
@@ -9807,32 +9743,23 @@ virSecurityLabelDefFormat(virBufferPtr buf, virSecurityLabelDefPtr def,
     if (!sectype)
         goto cleanup;
 
-    if (def->model &&
-        def->type == VIR_DOMAIN_SECLABEL_DYNAMIC &&
+    if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC &&
         !def->baselabel &&
         (flags & VIR_DOMAIN_XML_INACTIVE)) {
         /* This is the default for inactive xml, so nothing to output.  */
     } else {
-        virBufferAddLit(buf, "<seclabel");
-        if (def->model)
-            virBufferAsprintf(buf, " type='%s' model='%s'",
-                              sectype, def->model);
-        virBufferAsprintf(buf, " relabel='%s'",
+        virBufferAsprintf(buf, "<seclabel type='%s' model='%s' relabel='%s'>\n",
+                          sectype, def->model,
                           def->norelabel ? "no" : "yes");
-        if (def->label || def->baselabel) {
-            virBufferAddLit(buf, ">\n");
-            virBufferEscapeString(buf, "  <label>%s</label>\n",
-                                  def->label);
-            if (!def->norelabel)
-                virBufferEscapeString(buf, "  <imagelabel>%s</imagelabel>\n",
-                                      def->imagelabel);
-            if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC)
-                virBufferEscapeString(buf, "  <baselabel>%s</baselabel>\n",
-                                      def->baselabel);
-            virBufferAddLit(buf, "</seclabel>\n");
-        } else {
-            virBufferAddLit(buf, "/>\n");
-        }
+        virBufferEscapeString(buf, "  <label>%s</label>\n",
+                              def->label);
+        if (!def->norelabel)
+            virBufferEscapeString(buf, "  <imagelabel>%s</imagelabel>\n",
+                                  def->imagelabel);
+        if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC)
+            virBufferEscapeString(buf, "  <baselabel>%s</baselabel>\n",
+                                  def->baselabel);
+        virBufferAddLit(buf, "</seclabel>\n");
     }
     ret = 0;
 cleanup:
@@ -9952,36 +9879,17 @@ virDomainDiskDefFormat(virBufferPtr buf,
         def->startupPolicy) {
         switch (def->type) {
         case VIR_DOMAIN_DISK_TYPE_FILE:
-            virBufferAddLit(buf, "      <source");
+            virBufferAsprintf(buf,"      <source");
             if (def->src)
                 virBufferEscapeString(buf, " file='%s'", def->src);
             if (def->startupPolicy)
                 virBufferEscapeString(buf, " startupPolicy='%s'",
                                       startupPolicy);
-            if (def->seclabel) {
-                virBufferAddLit(buf, ">\n");
-                virBufferAdjustIndent(buf, 8);
-                if (virSecurityLabelDefFormat(buf, def->seclabel, flags) < 0)
-                    return -1;
-                virBufferAdjustIndent(buf, -8);
-                virBufferAddLit(buf, "      </source>\n");
-            } else {
-                virBufferAddLit(buf, "/>\n");
-            }
+            virBufferAsprintf(buf, "/>\n");
             break;
         case VIR_DOMAIN_DISK_TYPE_BLOCK:
-            if (def->src && def->seclabel) {
-                virBufferEscapeString(buf, "      <source dev='%s'>\n",
-                                      def->src);
-                virBufferAdjustIndent(buf, 8);
-                if (virSecurityLabelDefFormat(buf, def->seclabel, flags) < 0)
-                    return -1;
-                virBufferAdjustIndent(buf, -8);
-                virBufferAddLit(buf, "      </source>\n");
-            } else {
-                virBufferEscapeString(buf, "      <source dev='%s'/>\n",
-                                      def->src);
-            }
+            virBufferEscapeString(buf, "      <source dev='%s'/>\n",
+                                  def->src);
             break;
         case VIR_DOMAIN_DISK_TYPE_DIR:
             virBufferEscapeString(buf, "      <source dir='%s'/>\n",
-- 
1.7.7.5




More information about the libvir-list mailing list