11:31 a.m.
On 18.07.2012 03:28, Marcelo Cerri wrote:
This patch updates the structures that store information about each
domain and each hypervisor to support multiple security labels and
drivers. It also updates all the remaining code to use the new fields.
---
src/conf/capabilities.c | 17 ++++--
src/conf/capabilities.h | 6 ++-
src/conf/domain_audit.c | 14 +++--
src/conf/domain_conf.c | 86 +++++++++++++++++---------
src/conf/domain_conf.h | 9 ++-
src/lxc/lxc_conf.c | 8 ++-
src/lxc/lxc_controller.c | 8 +-
src/lxc/lxc_driver.c | 35 ++++++-----
src/qemu/qemu_driver.c | 15 +++--
src/qemu/qemu_process.c | 18 +++---
src/security/security_manager.c | 10 ++--
src/security/security_selinux.c | 132 +++++++++++++++++++-------------------
src/test/test_driver.c | 11 ++-
13 files changed, 217 insertions(+), 152 deletions(-)
We must update XML schema as well as we are going to allow more
<model> and <doi> elements under <secmodel>. And maybe we want to add a
test case. But that can be a follow up patch.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 398f630..b468174 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -857,12 +857,15 @@ virDomainGraphicsListenDefClear(virDomainGraphicsListenDefPtr def)
}
static void
-virSecurityLabelDefClear(virSecurityLabelDefPtr def)
+virSecurityLabelDefFree(virSecurityLabelDefPtr def)
{
- VIR_FREE(def->model);
- VIR_FREE(def->label);
- VIR_FREE(def->imagelabel);
- VIR_FREE(def->baselabel);
+ if (def) {
+ VIR_FREE(def->model);
+ VIR_FREE(def->label);
+ VIR_FREE(def->imagelabel);
+ VIR_FREE(def->baselabel);
+ VIR_FREE(def);
+ }
}
We tend to write this a slightly different:
{
if (!def)
return;
VIR_FREE(def->model);
...
}
@@ -3669,10 +3679,15 @@ virDomainDiskDefParseXML(virCapsPtr caps,
if (sourceNode) {
xmlNodePtr saved_node = ctxt->node;
ctxt->node = sourceNode;
- if (virSecurityDeviceLabelDefParseXML(&def->seclabel,
+ if ((VIR_ALLOC(def->seclabels) < 0) || (VIR_ALLOC(def->seclabels[0])
< 0)) {
Long line.
+ virReportOOMError();
+ goto error;
+ }
+ if (virSecurityDeviceLabelDefParseXML(&def->seclabels[0],
vmSeclabel,
ctxt) < 0)
goto error;
+ def->nseclabels = 1;
ctxt->node = saved_node;
}
@@ -7115,10 +7130,16 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps,
goto error;
}
+ if ((VIR_ALLOC(def->seclabels) < 0) ||
+ (VIR_ALLOC(def->seclabels[0])) < 0 ) {
+ virReportOOMError();
+ goto error;
+ }
+
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, def->seclabels[0],
flags)))
Long line.
goto error;
} else if (xmlStrEqual(node->name, BAD_CAST "lease")) {
dev->type = VIR_DOMAIN_DEVICE_LEASE;
@@ -8017,7 +8038,12 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
/* analysis of security label, done early even though we format it
* late, so devices can refer to this for defaults */
- if (virSecurityLabelDefParseXML(&def->seclabel, ctxt, flags) == -1)
+ if ((VIR_ALLOC(def->seclabels) < 0) || (VIR_ALLOC(def->seclabels[0]) <
0)) {
+ virReportOOMError();
+ goto error;
+ }
Again, I'd split this into two lines.
+ def->nseclabels = 1;
+ if (virSecurityLabelDefParseXML(def->seclabels[0], ctxt, flags) == -1)
goto error;
/* Extract domain memory */
I think needs to be squashed in:
diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
index 06ff685..be9d295 100644
--- a/docs/schemas/capability.rng
+++ b/docs/schemas/capability.rng
@@ -52,12 +52,14 @@
<define name='secmodel'>
<element name='secmodel'>
- <element name='model'>
- <text/>
- </element>
- <element name='doi'>
- <text/>
- </element>
+ <oneOrMore>
+ <element name='model'>
+ <text/>
+ </element>
+ <element name='doi'>
+ <text/>
+ </element>
+ </oneOrMore>
</element>
</define>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b468174..719efea 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -859,13 +859,14 @@ virDomainGraphicsListenDefClear(virDomainGraphicsListenDefPtr def)
static void
virSecurityLabelDefFree(virSecurityLabelDefPtr def)
{
- if (def) {
- VIR_FREE(def->model);
- VIR_FREE(def->label);
- VIR_FREE(def->imagelabel);
- VIR_FREE(def->baselabel);
- VIR_FREE(def);
- }
+ if (!def)
+ return;
+
+ VIR_FREE(def->model);
+ VIR_FREE(def->label);
+ VIR_FREE(def->imagelabel);
+ VIR_FREE(def->baselabel);
+ VIR_FREE(def);
}
@@ -3679,7 +3680,8 @@ virDomainDiskDefParseXML(virCapsPtr caps,
if (sourceNode) {
xmlNodePtr saved_node = ctxt->node;
ctxt->node = sourceNode;
- if ((VIR_ALLOC(def->seclabels) < 0) || (VIR_ALLOC(def->seclabels[0])
< 0)) {
+ if ((VIR_ALLOC(def->seclabels) < 0) ||
+ (VIR_ALLOC(def->seclabels[0]) < 0)) {
virReportOOMError();
goto error;
}
@@ -7138,8 +7140,9 @@ 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->seclabels[0],
flags)))
+ if (!(dev->data.disk = virDomainDiskDefParseXML(caps, node, ctxt, NULL,
+ def->seclabels[0],
+ flags)))
goto error;
} else if (xmlStrEqual(node->name, BAD_CAST "lease")) {
dev->type = VIR_DOMAIN_DEVICE_LEASE;
@@ -8038,7 +8041,8 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
/* analysis of security label, done early even though we format it
* late, so devices can refer to this for defaults */
- if ((VIR_ALLOC(def->seclabels) < 0) || (VIR_ALLOC(def->seclabels[0]) <
0)) {
+ if ((VIR_ALLOC(def->seclabels) < 0) ||
+ (VIR_ALLOC(def->seclabels[0]) < 0)) {
virReportOOMError();
goto error;
}
But don't we rather want multiple <secmodel> elements than multiple <doi>
and <model> inside one <secmodel>?