[libvirt] [PATCH v3 1/5] Internal refactory of data structures
Michal Privoznik
mprivozn at redhat.com
Mon Aug 6 13:30:06 UTC 2012
On 03.08.2012 16:18, 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.
>
> Signed-off-by: Marcelo Cerri <mhcerri at linux.vnet.ibm.com>
> ---
> src/conf/capabilities.c | 17 ++++--
> src/conf/capabilities.h | 6 ++-
> src/conf/domain_audit.c | 14 +++--
> src/conf/domain_conf.c | 81 ++++++++++++++++-------
> src/conf/domain_conf.h | 9 ++-
> src/lxc/lxc_conf.c | 8 ++-
> src/lxc/lxc_controller.c | 8 +-
> src/lxc/lxc_driver.c | 11 ++--
> src/lxc/lxc_process.c | 23 ++++---
> src/qemu/qemu_driver.c | 15 +++--
> src/qemu/qemu_process.c | 18 +++---
> src/security/security_manager.c | 12 ++--
> src/security/security_selinux.c | 133 ++++++++++++++++++++-------------------
> src/test/test_driver.c | 11 ++-
> 14 files changed, 216 insertions(+), 150 deletions(-)
>
> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> index 8b9b516..9a0bc3d 100644
> --- a/src/conf/capabilities.c
> +++ b/src/conf/capabilities.c
> @@ -181,8 +181,13 @@ virCapabilitiesFree(virCapsPtr caps) {
> VIR_FREE(caps->host.migrateTrans);
>
> VIR_FREE(caps->host.arch);
> - VIR_FREE(caps->host.secModel.model);
> - VIR_FREE(caps->host.secModel.doi);
> +
> + for (i = 0; i < caps->host.nsecModels; i++) {
> + VIR_FREE(caps->host.secModels[i].model);
> + VIR_FREE(caps->host.secModels[i].doi);
> + }
> + VIR_FREE(caps->host.secModels);
> +
> virCPUDefFree(caps->host.cpu);
>
> VIR_FREE(caps);
> @@ -767,10 +772,12 @@ virCapabilitiesFormatXML(virCapsPtr caps)
> virBufferAddLit(&xml, " </topology>\n");
> }
>
> - if (caps->host.secModel.model) {
> + if (caps->host.nsecModels) {
> virBufferAddLit(&xml, " <secmodel>\n");
> - virBufferAsprintf(&xml, " <model>%s</model>\n", caps->host.secModel.model);
> - virBufferAsprintf(&xml, " <doi>%s</doi>\n", caps->host.secModel.doi);
> + virBufferAsprintf(&xml, " <model>%s</model>\n",
> + caps->host.secModels[0].model);
> + virBufferAsprintf(&xml, " <doi>%s</doi>\n",
> + caps->host.secModels[0].doi);
> virBufferAddLit(&xml, " </secmodel>\n");
> }
>
> diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
> index 460b273..e43f404 100644
> --- a/src/conf/capabilities.h
> +++ b/src/conf/capabilities.h
> @@ -93,6 +93,7 @@ struct _virCapsHostNUMACell {
> };
>
> typedef struct _virCapsHostSecModel virCapsHostSecModel;
> +typedef virCapsHostSecModel *virCapsHostSecModelPtr;
> struct _virCapsHostSecModel {
> char *model;
> char *doi;
> @@ -116,7 +117,10 @@ struct _virCapsHost {
> size_t nnumaCell;
> size_t nnumaCell_max;
> virCapsHostNUMACellPtr *numaCell;
> - virCapsHostSecModel secModel;
> +
> + size_t nsecModels;
> + virCapsHostSecModelPtr secModels;
> +
> virCPUDefPtr cpu;
> unsigned char host_uuid[VIR_UUID_BUFLEN];
> };
> diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
> index 5300371..59db649 100644
> --- a/src/conf/domain_audit.c
> +++ b/src/conf/domain_audit.c
> @@ -618,6 +618,7 @@ virDomainAuditSecurityLabel(virDomainObjPtr vm, bool success)
> char uuidstr[VIR_UUID_STRING_BUFLEN];
> char *vmname;
> const char *virt;
> + int i;
>
> virUUIDFormat(vm->def->uuid, uuidstr);
> if (!(vmname = virAuditEncode("vm", vm->def->name))) {
> @@ -630,11 +631,14 @@ virDomainAuditSecurityLabel(virDomainObjPtr vm, bool success)
> virt = "?";
> }
>
> - VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_ID, success,
> - "virt=%s %s uuid=%s vm-ctx=%s img-ctx=%s",
> - virt, vmname, uuidstr,
> - VIR_AUDIT_STR(vm->def->seclabel.label),
> - VIR_AUDIT_STR(vm->def->seclabel.imagelabel));
> + for (i = 0; i < vm->def->nseclabels; i++) {
> + VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_ID, success,
> + "virt=%s %s uuid=%s vm-ctx=%s img-ctx=%s model=%s",
> + virt, vmname, uuidstr,
> + VIR_AUDIT_STR(vm->def->seclabels[i]->label),
> + VIR_AUDIT_STR(vm->def->seclabels[i]->imagelabel),
> + VIR_AUDIT_STR(vm->def->seclabels[i]->model));
> + }
>
> VIR_FREE(vmname);
> }
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 58603a3..1c318a0 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -855,12 +855,15 @@ virDomainGraphicsListenDefClear(virDomainGraphicsListenDefPtr def)
> }
>
> static void
> -virSecurityLabelDefClear(virSecurityLabelDefPtr def)
> +virSecurityLabelDefFree(virSecurityLabelDefPtr def)
> {
> + if (!def)
> + return;
> VIR_FREE(def->model);
> VIR_FREE(def->label);
> VIR_FREE(def->imagelabel);
> VIR_FREE(def->baselabel);
> + VIR_FREE(def);
> }
>
>
> @@ -869,6 +872,7 @@ virSecurityDeviceLabelDefFree(virSecurityDeviceLabelDefPtr def)
> {
> if (!def)
> return;
> + VIR_FREE(def->model);
> VIR_FREE(def->label);
> VIR_FREE(def);
> }
> @@ -954,7 +958,11 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
> virStorageEncryptionFree(def->encryption);
> virDomainDeviceInfoClear(&def->info);
>
> - virSecurityDeviceLabelDefFree(def->seclabel);
> + if (def->seclabels) {
> + for (i = 0; i < def->nseclabels; i++)
> + virSecurityDeviceLabelDefFree(def->seclabels[i]);
> + VIR_FREE(def->seclabels);
> + }
>
> for (i = 0 ; i < def->nhosts ; i++)
> virDomainDiskHostDefFree(&def->hosts[i]);
> @@ -1620,7 +1628,9 @@ void virDomainDefFree(virDomainDefPtr def)
>
> virDomainMemballoonDefFree(def->memballoon);
>
> - virSecurityLabelDefClear(&def->seclabel);
> + for (i = 0; i < def->nseclabels; i++)
> + virSecurityLabelDefFree(def->seclabels[i]);
> + VIR_FREE(def->seclabels);
>
> virCPUDefFree(def->cpu);
>
> @@ -3075,10 +3085,10 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
> {
> char *p;
>
> - if (virXPathNode("./seclabel", ctxt) == NULL)
> + if (virXPathNode("./seclabel[1]", ctxt) == NULL)
> return 0;
>
> - p = virXPathStringLimit("string(./seclabel/@type)",
> + p = virXPathStringLimit("string(./seclabel[1]/@type)",
> VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
> if (p == NULL) {
> def->type = VIR_DOMAIN_SECLABEL_DYNAMIC;
> @@ -3092,7 +3102,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
> }
> }
>
> - p = virXPathStringLimit("string(./seclabel/@relabel)",
> + p = virXPathStringLimit("string(./seclabel[1]/@relabel)",
> VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
> if (p != NULL) {
> if (STREQ(p, "yes")) {
> @@ -3132,7 +3142,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/label[1])",
> + p = virXPathStringLimit("string(./seclabel[1]/label[1])",
> VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
> if (p == NULL) {
> virReportError(VIR_ERR_XML_ERROR,
> @@ -3147,7 +3157,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
> if (!def->norelabel &&
> (!(flags & VIR_DOMAIN_XML_INACTIVE) &&
> def->type != VIR_DOMAIN_SECLABEL_NONE)) {
> - p = virXPathStringLimit("string(./seclabel/imagelabel[1])",
> + p = virXPathStringLimit("string(./seclabel[1]/imagelabel[1])",
> VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
> if (p == NULL) {
> virReportError(VIR_ERR_XML_ERROR,
> @@ -3159,7 +3169,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
>
> /* Only parse baselabel for dynamic label type */
> if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC) {
> - p = virXPathStringLimit("string(./seclabel/baselabel[1])",
> + p = virXPathStringLimit("string(./seclabel[1]/baselabel[1])",
> VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
> def->baselabel = p;
> }
> @@ -3171,7 +3181,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
> def->baselabel ||
> (!(flags & VIR_DOMAIN_XML_INACTIVE) &&
> def->type != VIR_DOMAIN_SECLABEL_NONE)) {
> - p = virXPathStringLimit("string(./seclabel/@model)",
> + p = virXPathStringLimit("string(./seclabel[1]/@model)",
> VIR_SECURITY_MODEL_BUFLEN-1, ctxt);
> if (p == NULL) {
> virReportError(VIR_ERR_XML_ERROR,
> @@ -3184,7 +3194,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
> return 0;
>
> error:
> - virSecurityLabelDefClear(def);
> + virSecurityLabelDefFree(def);
> return -1;
> }
>
> @@ -3198,7 +3208,7 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr *def,
>
> *def = NULL;
>
> - if (virXPathNode("./seclabel", ctxt) == NULL)
> + if (virXPathNode("./seclabel[1]", ctxt) == NULL)
> return 0;
>
> /* Can't use overrides if top-level doesn't allow relabeling. */
> @@ -3214,7 +3224,7 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr *def,
> return -1;
> }
>
> - p = virXPathStringLimit("string(./seclabel/@relabel)",
> + p = virXPathStringLimit("string(./seclabel[1]/@relabel)",
> VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
> if (p != NULL) {
> if (STREQ(p, "yes")) {
> @@ -3233,7 +3243,7 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr *def,
> (*def)->norelabel = false;
> }
>
> - p = virXPathStringLimit("string(./seclabel/label[1])",
> + p = virXPathStringLimit("string(./seclabel[1]/label[1])",
> VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
> (*def)->label = p;
>
> @@ -3667,10 +3677,16 @@ 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)) {
> + virReportOOMError();
> + goto error;
> + }
> + if (virSecurityDeviceLabelDefParseXML(&def->seclabels[0],
> vmSeclabel,
> ctxt) < 0)
> goto error;
> + def->nseclabels = 1;
> ctxt->node = saved_node;
> }
>
> @@ -7120,10 +7136,17 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps,
> goto error;
> }
>
> + if ((VIR_ALLOC(def->seclabels) < 0) ||
> + (VIR_ALLOC(def->seclabels[0])) < 0 ) {
> + virReportOOMError();
> + goto error;
> + }
> +
'def' is passed as 'const virDomainDefPtr def' here. I guess we could
preserve const correctness here.
Moreover, are we guaranteed that these haven't been allocated yet? I
mean, virDomainDeviceDefParse is called from virDomainDeviceDefCopy()
too which is called when doing live+config device attach 'virsh
attach-devoce --persistent device.xml'
> 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)))
> goto error;
> } else if (xmlStrEqual(node->name, BAD_CAST "lease")) {
> dev->type = VIR_DOMAIN_DEVICE_LEASE;
> @@ -8061,7 +8084,13 @@ 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;
> + }
> + def->nseclabels = 1;
> + if (virSecurityLabelDefParseXML(def->seclabels[0], ctxt, flags) == -1)
> goto error;
>
> /* Extract domain memory */
> @@ -8661,7 +8690,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
> nodes[i],
> ctxt,
> bootMap,
> - &def->seclabel,
> + def->seclabels[0],
> flags);
> if (!disk)
> goto error;
> @@ -11181,10 +11210,10 @@ virDomainDiskDefFormat(virBufferPtr buf,
> if (def->startupPolicy)
> virBufferEscapeString(buf, " startupPolicy='%s'",
> startupPolicy);
> - if (def->seclabel) {
> + if (def->seclabels && def->seclabels[0]) {
I guess def->nseclabels would be sufficient here.
> virBufferAddLit(buf, ">\n");
> virBufferAdjustIndent(buf, 8);
> - virSecurityDeviceLabelDefFormat(buf, def->seclabel);
> + virSecurityDeviceLabelDefFormat(buf, def->seclabels[0]);
> virBufferAdjustIndent(buf, -8);
> virBufferAddLit(buf, " </source>\n");
> } else {
> @@ -11194,10 +11223,10 @@ virDomainDiskDefFormat(virBufferPtr buf,
> case VIR_DOMAIN_DISK_TYPE_BLOCK:
> virBufferEscapeString(buf, " <source dev='%s'",
> def->src);
> - if (def->seclabel) {
> + if (def->seclabels && def->seclabels[0]) {
ditto.
> virBufferAddLit(buf, ">\n");
> virBufferAdjustIndent(buf, 8);
> - virSecurityDeviceLabelDefFormat(buf, def->seclabel);
> + virSecurityDeviceLabelDefFormat(buf, def->seclabels[0]);
> virBufferAdjustIndent(buf, -8);
> virBufferAddLit(buf, " </source>\n");
> } else {
Otherwise I haven't spotted anything wrong.
ACK modulo problem with 'def' within virDomainDeviceDefParse().
Michal
More information about the libvir-list
mailing list