[libvirt] [PATCH v2 1/5] Internal refactory of data structures
Michal Privoznik
mprivozn at redhat.com
Wed Jul 18 16:31:57 UTC 2012
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>?
More information about the libvir-list
mailing list