On 16.08.2012 00:10, Marcelo Cerri wrote:
This patch updates the domain and capability XML parser and formatter
to
support more than one "seclabel" element for each domain and device. The
RNG schema and the tests related to this are also updated by this patch.
Signed-off-by: Marcelo Cerri <mhcerri(a)linux.vnet.ibm.com>
---
docs/formatdomain.html.in | 11 +-
docs/schemas/capability.rng | 18 +-
docs/schemas/domaincommon.rng | 30 ++-
src/conf/capabilities.c | 6 +-
src/conf/domain_conf.c | 353 ++++++++++++++------
src/conf/domain_conf.h | 9 +
src/libvirt_private.syms | 3 +
.../qemuxml2argv-seclabel-dynamic-override.xml | 4 +-
.../qemuxml2argv-seclabel-dynamic.xml | 2 +-
9 files changed, 304 insertions(+), 132 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index fb25cb8..85d2515 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1275,8 +1275,8 @@
path to the file holding the disk. If the disk
<code>type</code> is "block", then the
<code>dev</code>
attribute specifies the path to the host device to serve as
- the disk. With both "file" and "block", an optional
- sub-element <code>seclabel</code>, <a
href="#seclabel">described
+ the disk. With both "file" and "block", one or more
optional
+ sub-elements <code>seclabel</code>, <a
href="#seclabel">described
below</a> (and <span class="since">since
0.9.9</span>), can be
used to override the domain security labeling policy for just
that source file. If the disk <code>type</code> is "dir",
then the
@@ -3880,6 +3880,13 @@ qemu-kvm -net nic,model=? /dev/null
</p>
<p>
+ If more than one security driver is used by libvirt, multiple
+ <code>seclabel</code> tags can be used, one for each driver and
+ the security driver referenced by each tag can be defined using
+ the attribute <code>model</code>
+ </p>
+
+ <p>
Valid input XML configurations for the top-level security label
are:
</p>
diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
index c392e44..8c928bc 100644
--- a/docs/schemas/capability.rng
+++ b/docs/schemas/capability.rng
@@ -44,20 +44,22 @@
<optional>
<ref name='topology'/>
</optional>
- <optional>
+ <zeroOrMore>
<ref name='secmodel'/>
- </optional>
+ </zeroOrMore>
</element>
</define>
<define name='secmodel'>
<element name='secmodel'>
- <element name='model'>
- <text/>
- </element>
- <element name='doi'>
- <text/>
- </element>
+ <interleave>
+ <element name='model'>
+ <text/>
+ </element>
+ <element name='doi'>
+ <text/>
+ </element>
+ </interleave>
</element>
</define>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 1b94155..dd93f4d 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -55,9 +55,9 @@
<optional>
<ref name="devices"/>
</optional>
- <optional>
+ <zeroOrMore>
<ref name="seclabel"/>
- </optional>
+ </zeroOrMore>
<optional>
<ref name='qemucmdline'/>
</optional>
@@ -148,18 +148,32 @@
<!-- A per-device seclabel override is more limited, either
relabel=no or a <label> must be present. -->
<choice>
- <attribute name='relabel'>
- <value>no</value>
- </attribute>
<group>
<optional>
+ <attribute name='model'>
+ <text/>
+ </attribute>
+ </optional>
+ <attribute name='relabel'>
+ <value>no</value>
+ </attribute>
+ </group>
+ <group>
+ <optional>
+ <attribute name='model'>
+ <text/>
+ </attribute>
+ </optional>
+ <optional>
<attribute name='relabel'>
<value>yes</value>
</attribute>
</optional>
- <element name='label'>
- <text/>
- </element>
+ <zeroOrMore>
+ <element name='label'>
+ <text/>
+ </element>
+ </zeroOrMore>
</group>
</choice>
</element>
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 9a0bc3d..bc6266e 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -772,12 +772,12 @@ virCapabilitiesFormatXML(virCapsPtr caps)
virBufferAddLit(&xml, " </topology>\n");
}
- if (caps->host.nsecModels) {
+ for (i = 0; i < caps->host.nsecModels; i++) {
virBufferAddLit(&xml, " <secmodel>\n");
virBufferAsprintf(&xml, " <model>%s</model>\n",
- caps->host.secModels[0].model);
+ caps->host.secModels[i].model);
virBufferAsprintf(&xml, " <doi>%s</doi>\n",
- caps->host.secModels[0].doi);
+ caps->host.secModels[i].doi);
virBufferAddLit(&xml, " </secmodel>\n");
}
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 98daf32..2dea33e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3067,17 +3067,19 @@ virDomainDiskDefAssignAddress(virCapsPtr caps,
virDomainDiskDefPtr def)
return 0;
}
-static int
-virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
- xmlXPathContextPtr ctxt,
+static virSecurityLabelDefPtr
+virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt,
unsigned int flags)
{
char *p;
+ virSecurityLabelDefPtr def = NULL;
- if (virXPathNode("./seclabel[1]", ctxt) == NULL)
- return 0;
+ if (VIR_ALLOC(def) < 0) {
+ virReportOOMError();
+ goto error;
+ }
- p = virXPathStringLimit("string(./seclabel[1]/@type)",
+ p = virXPathStringLimit("string(./@type)",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
if (p == NULL) {
def->type = VIR_DOMAIN_SECLABEL_DYNAMIC;
@@ -3091,7 +3093,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
}
}
- p = virXPathStringLimit("string(./seclabel[1]/@relabel)",
+ p = virXPathStringLimit("string(./@relabel)",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
if (p != NULL) {
if (STREQ(p, "yes")) {
@@ -3131,7 +3133,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
if (def->type == VIR_DOMAIN_SECLABEL_STATIC ||
(!(flags & VIR_DOMAIN_XML_INACTIVE) &&
def->type != VIR_DOMAIN_SECLABEL_NONE)) {
- p = virXPathStringLimit("string(./seclabel[1]/label[1])",
+ p = virXPathStringLimit("string(./label[1])",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
if (p == NULL) {
virReportError(VIR_ERR_XML_ERROR,
@@ -3146,7 +3148,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
if (!def->norelabel &&
(!(flags & VIR_DOMAIN_XML_INACTIVE) &&
def->type != VIR_DOMAIN_SECLABEL_NONE)) {
- p = virXPathStringLimit("string(./seclabel[1]/imagelabel[1])",
+ p = virXPathStringLimit("string(./imagelabel[1])",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
if (p == NULL) {
virReportError(VIR_ERR_XML_ERROR,
@@ -3158,93 +3160,179 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
/* Only parse baselabel for dynamic label type */
if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC) {
- p = virXPathStringLimit("string(./seclabel[1]/baselabel[1])",
+ p = virXPathStringLimit("string(./baselabel[1])",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
def->baselabel = p;
}
- /* Only parse model, if static labelling, or a base
- * label is set, or doing active XML
- */
- if (def->type == VIR_DOMAIN_SECLABEL_STATIC ||
- def->baselabel ||
- (!(flags & VIR_DOMAIN_XML_INACTIVE) &&
- def->type != VIR_DOMAIN_SECLABEL_NONE)) {
- p = virXPathStringLimit("string(./seclabel[1]/@model)",
- VIR_SECURITY_MODEL_BUFLEN-1, ctxt);
- if (p == NULL) {
- virReportError(VIR_ERR_XML_ERROR,
- "%s", _("missing security model"));
- goto error;
- }
- def->model = p;
+ /* Always parse model */
+ p = virXPathStringLimit("string(./@model)",
+ VIR_SECURITY_MODEL_BUFLEN-1, ctxt);
+ if (p == NULL && def->type != VIR_DOMAIN_SECLABEL_NONE) {
+ virReportError(VIR_ERR_XML_ERROR,
+ "%s", _("missing security model"));
}
+ def->model = p;
- return 0;
+ return def;
error:
virSecurityLabelDefFree(def);
+ return NULL;
+}
+
+static int
+virSecurityLabelDefsParseXML(virDomainDefPtr def,
+ xmlXPathContextPtr ctxt,
+ unsigned int flags)
+{
+ int i, n;
'i' needs to be initialized, otherwise ..
+ xmlNodePtr *list = NULL, saved_node;
+
+ /* Check args and save context */
+ if (def == NULL || ctxt == NULL)
+ return 0;
+ saved_node = ctxt->node;
+
+ /* Allocate a security labels based on XML */
+ if ((n = virXPathNodeSet("./seclabel", ctxt, &list)) == 0)
+ return 0;
+
+ if (VIR_ALLOC_N(def->seclabels, n) < 0) {
+ virReportOOMError();
+ goto error;
.. we might jump to error here ..
+ }
+
+ /* Parse each "seclabel" tag */
+ for (i = 0; i < n; i++) {
+ ctxt->node = list[i];
+ def->seclabels[i] = virSecurityLabelDefParseXML(ctxt, flags);
+ if (def->seclabels[i] == NULL)
+ goto error;
+ }
+ def->nseclabels = n;
+ ctxt->node = saved_node;
+ VIR_FREE(list);
+
+ /* Checking missing model information
+ * when there is more than one seclabel */
+ if (n > 1) {
+ for(; n; n--) {
+ if (def->seclabels[n - 1]->model == NULL) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing security model "
+ "when using multiple labels"));
+ goto error;
+ }
+ }
+ }
+ return 0;
+
+error:
+ ctxt->node = saved_node;
+ for (; i > 0; i--) {
and use 'i' uninitialized.
side note: It took me quite some time to identify this as I had hit gcc
bug here. For some reason it was reporting uninitialized variable 'i'
but in totally different function virDomainDefParseXML. I guess since
virSecurityLabelDefsParseXML is static, gcc might have inlined it and
hence the wrong function name.
+ virSecurityLabelDefFree(def->seclabels[i - 1]);
+ }
+ VIR_FREE(def->seclabels);
+ VIR_FREE(list);
return -1;
}
ACK with this squashed in:
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8f9f6de..c9f5a3c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3104,7 +3104,7 @@ virSecurityLabelDefsParseXML(virDomainDefPtr def,
xmlXPathContextPtr ctxt,
unsigned int flags)
{
- int i, n;
+ int i = 0, n;
xmlNodePtr *list = NULL, saved_node;
/* Check args and save context */