[libvirt] [PATCH 1/7] Update internal data structures to support multiple security label definitions
Michal Privoznik
mprivozn at redhat.com
Mon Jun 4 14:07:35 UTC 2012
On 21.05.2012 15:39, Marcelo Cerri wrote:
> ---
> src/conf/capabilities.c | 17 ++-
> src/conf/capabilities.h | 6 +-
> src/conf/domain_audit.c | 14 ++-
> src/conf/domain_conf.c | 310 +++++++++++++++++++++++++++++++++++------------
> src/conf/domain_conf.h | 18 +++-
> 5 files changed, 272 insertions(+), 93 deletions(-)
Needs a v2. Test driver needs to be fixed:
CC libvirt_driver_test_la-test_driver.lo
test/test_driver.c: In function 'testBuildCapabilities':
test/test_driver.c:213:15: error: 'virCapsHost' has no member named
'secModel'
test/test_driver.c:214:20: error: 'virCapsHost' has no member named
'secModel'
test/test_driver.c:217:15: error: 'virCapsHost' has no member named
'secModel'
test/test_driver.c:218:20: error: 'virCapsHost' has no member named
'secModel'
>
> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> index 542bf03..7e786c5 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) {
> + for (i = 0; i < caps->host.nsecModels; i++) {
> 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[i].model);
> + virBufferAsprintf(&xml, " <doi>%s</doi>\n",
> + caps->host.secModels[i].doi);
> virBufferAddLit(&xml, " </secmodel>\n");
> }
>
> diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
> index 421030d..9a0092d 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 653657b..ac296d5 100644
> --- a/src/conf/domain_audit.c
> +++ b/src/conf/domain_audit.c
> @@ -619,6 +619,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))) {
> @@ -631,11 +632,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 9def71d..91ffb6f 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -852,12 +852,13 @@ 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);
> + VIR_FREE(def);
> }
>
>
> @@ -866,6 +867,7 @@ virSecurityDeviceLabelDefFree(virSecurityDeviceLabelDefPtr def)
> {
> if (!def)
> return;
> + VIR_FREE(def->model);
> VIR_FREE(def->label);
> VIR_FREE(def);
> }
> @@ -951,7 +953,9 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
> virStorageEncryptionFree(def->encryption);
> virDomainDeviceInfoClear(&def->info);
>
> - virSecurityDeviceLabelDefFree(def->seclabel);
> + 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]);
> @@ -1617,7 +1621,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);
>
> @@ -3071,10 +3077,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
> {
> char *p;
Since we are returning only a boolean value here (0 vs. -1) and seems
the only caller pre-allocates memory for us I wonder if we can change
the prototype so we are returning virSecurityLabelDefPtr instead of int
and therefore allocating return variable in here instead of doing [1]
>
> - if (virXPathNode("./seclabel", ctxt) == NULL)
> - return 0;
> -
> - p = virXPathStringLimit("string(./seclabel/@type)",
> + p = virXPathStringLimit("string(./@type)",
> VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
> if (p == NULL) {
> def->type = VIR_DOMAIN_SECLABEL_DYNAMIC;
> @@ -3088,7 +3091,7 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
> }
> }
>
> - p = virXPathStringLimit("string(./seclabel/@relabel)",
> + p = virXPathStringLimit("string(./@relabel)",
> VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
> if (p != NULL) {
> if (STREQ(p, "yes")) {
> @@ -3105,13 +3108,15 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def,
> if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC &&
> def->norelabel) {
> virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> - "%s", _("dynamic label type must use resource relabeling"));
> + "%s", _("dynamic label type must use "
> + "resource relabeling"));
> goto error;
> }
> if (def->type == VIR_DOMAIN_SECLABEL_NONE &&
> !def->norelabel) {
> virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> - "%s", _("resource relabeling is not compatible with 'none' label type"));
> + "%s", _("resource relabeling is not "
> + "compatible with 'none' label type"));
> goto error;
> }
> } else {
> @@ -3128,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/label[1])",
> + p = virXPathStringLimit("string(./label[1])",
> VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
> if (p == NULL) {
> virDomainReportError(VIR_ERR_XML_ERROR,
> @@ -3143,7 +3148,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(./imagelabel[1])",
> VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
> if (p == NULL) {
> virDomainReportError(VIR_ERR_XML_ERROR,
> @@ -3155,93 +3160,166 @@ 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(./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/@model)",
> - VIR_SECURITY_MODEL_BUFLEN-1, ctxt);
> - if (p == NULL) {
> - virDomainReportError(VIR_ERR_XML_ERROR,
> - "%s", _("missing security model"));
> - goto error;
> - }
> - def->model = p;
> + /* TODO: check */
> + /* Always parse model */
> + p = virXPathStringLimit("string(./@model)",
> + VIR_SECURITY_MODEL_BUFLEN-1, ctxt);
> + if (p == NULL) {
> + virDomainReportError(VIR_ERR_XML_ERROR,
> + "%s", _("missing security model"));
> + goto error;
> }
> + def->model = p;
>
> return 0;
>
> error:
> - virSecurityLabelDefClear(def);
> return -1;
> }
>
> -
> static int
> -virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr *def,
> - virSecurityLabelDefPtr vmDef,
> - xmlXPathContextPtr ctxt)
> +virSecurityLabelDefsParseXML(virDomainDefPtr def,
> + xmlXPathContextPtr ctxt,
> + unsigned int flags)
> {
> - char *p;
> + int i, n;
> + xmlNodePtr *list, saved_node;
>
> - *def = NULL;
> + /* Check args and save context */
> + if (def == NULL || ctxt == NULL)
> + return 0;
> + saved_node = ctxt->node;
>
> - if (virXPathNode("./seclabel", ctxt) == NULL)
> + /* Allocate a security labels based on XML */
> + if ((n = virXPathNodeSet("./seclabel", ctxt, &list)) == 0)
> return 0;
>
> - /* Can't use overrides if top-level doesn't allow relabeling. */
> - if (vmDef && vmDef->norelabel) {
> - virDomainReportError(VIR_ERR_XML_ERROR, "%s",
> - _("label overrides require relabeling to be "
> - "enabled at the domain level"));
> + if (VIR_ALLOC_N(def->seclabels, n) < 0) {
> + virReportOOMError();
> return -1;
> }
> + for (i = 0; i < n; i++) {
> + if (VIR_ALLOC(def->seclabels[i]) < 0) {
> + virReportOOMError();
> + goto error;
> + }
> + }
1: this ^^. On an error NULL would be returned which means changing ...
> +
> + /* Parse each "seclabel" tag */
> + for (i = 0; i < n; i++) {
> + ctxt->node = list[i];
> + if (virSecurityLabelDefParseXML(def->seclabels[i], ctxt, flags))
1: this condition of course.
> + goto error;
> + }
> + def->nseclabels = n;
> + ctxt->node = saved_node;
> + return 0;
>
> - if (VIR_ALLOC(*def) < 0) {
> +error:
> + ctxt->node = saved_node;
> + for (i = 0; i < n; i++) {
> + virSecurityLabelDefFree(def->seclabels[i]);
> + }
> + VIR_FREE(def->seclabels);
> + return -1;
> +}
> +
> +static int
> +virSecurityDeviceLabelDefParseXML(virDomainDiskDefPtr def,
> + virSecurityLabelDefPtr *vmSeclabels,
> + int nvmSeclabels, xmlXPathContextPtr ctxt)
> +{
> + int n, i, j;
> + xmlNodePtr *list;
> + virSecurityLabelDefPtr vmDef = NULL;
> + char *model, *relabel, *label;
> +
> + if (def == NULL)
> + return 0;
> +
> + if ((n = virXPathNodeSet("./seclabel", ctxt, &list)) == 0)
> + return 0;
> +
> + def->nseclabels = n;
> + if (VIR_ALLOC_N(def->seclabels, n) < 0) {
> virReportOOMError();
> return -1;
> }
> + for (i = 0; i < n; i++) {
> + if (VIR_ALLOC(def->seclabels[i]) < 0) {
> + virReportOOMError();
> + goto error;
> + }
> + }
>
> - p = virXPathStringLimit("string(./seclabel/@relabel)",
> - VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
> - if (p != NULL) {
> - if (STREQ(p, "yes")) {
> - (*def)->norelabel = false;
> - } else if (STREQ(p, "no")) {
> - (*def)->norelabel = true;
> + for (i = 0; i < n; i++) {
> + /* get model associated to this override */
> + model = virXMLPropString(list[i], "model");
> + if (model == NULL) {
> + // TODO primary ?
> + // vmDef = ?
> } else {
> - virDomainReportError(VIR_ERR_XML_ERROR,
> - _("invalid security relabel value %s"), p);
> - VIR_FREE(p);
> - VIR_FREE(*def);
> - return -1;
> + /* find the security label that it's being overrided */
> + for (j = 0; j < nvmSeclabels; j++) {
> + if (STREQ(vmSeclabels[j]->model, model)) {
> + vmDef = vmSeclabels[j];
> + break;
> + }
> + }
> + VIR_FREE(model);
> }
> - VIR_FREE(p);
> - } else {
> - (*def)->norelabel = false;
> - }
>
> - p = virXPathStringLimit("string(./seclabel/label[1])",
> - VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
> - (*def)->label = p;
> + /* Can't use overrides if top-level doesn't allow relabeling. */
> + if (vmDef && vmDef->norelabel) {
> + virDomainReportError(VIR_ERR_XML_ERROR, "%s",
> + _("label overrides require relabeling to be "
> + "enabled at the domain level"));
> + goto error;
> + }
>
> - if ((*def)->label && (*def)->norelabel) {
> - virDomainReportError(VIR_ERR_XML_ERROR,
> - _("Cannot specify a label if relabelling is turned off"));
> - VIR_FREE((*def)->label);
> - VIR_FREE(*def);
> - return -1;
> - }
> + relabel = virXMLPropString(list[i], "relabel");
> + if (relabel != NULL) {
> + if (STREQ(relabel, "yes")) {
> + def->seclabels[i]->norelabel = false;
> + } else if (STREQ(relabel, "no")) {
> + def->seclabels[i]->norelabel = true;
> + } else {
> + virDomainReportError(VIR_ERR_XML_ERROR,
> + _("invalid security relabel value %s"),
> + relabel);
> + VIR_FREE(relabel);
> + goto error;
> + }
> + VIR_FREE(relabel);
> + } else {
> + def->seclabels[i]->norelabel = false;
> + }
>
> + ctxt->node = list[i];
> + label = virXPathStringLimit("string(./seclabel/label[1])",
> + VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
> + def->seclabels[i]->label = label;
> +
> + if (label && def->seclabels[i]->norelabel) {
> + virDomainReportError(VIR_ERR_XML_ERROR,
> + _("Cannot specify a label if relabelling is "
> + "turned off"));
> + goto error;
> + }
> + }
> return 0;
> +
> +error:
> + for (i = 0; i < n; i++) {
> + virSecurityDeviceLabelDefFree(def->seclabels[i]);
> + }
> + VIR_FREE(def->seclabels);
> + return -1;
> }
>
>
> @@ -3325,7 +3403,8 @@ virDomainDiskDefParseXML(virCapsPtr caps,
> xmlNodePtr node,
> xmlXPathContextPtr ctxt,
> virBitmapPtr bootMap,
> - virSecurityLabelDefPtr vmSeclabel,
> + virSecurityLabelDefPtr* vmSeclabels,
> + int nvmSeclabels,
> unsigned int flags)
> {
> virDomainDiskDefPtr def;
> @@ -3663,9 +3742,8 @@ virDomainDiskDefParseXML(virCapsPtr caps,
> if (sourceNode) {
> xmlNodePtr saved_node = ctxt->node;
> ctxt->node = sourceNode;
> - if (virSecurityDeviceLabelDefParseXML(&def->seclabel,
> - vmSeclabel,
> - ctxt) < 0)
> + if (virSecurityDeviceLabelDefParseXML(def, vmSeclabels,
> + nvmSeclabels, ctxt) < 0)
> goto error;
> ctxt->node = saved_node;
> }
> @@ -7011,7 +7089,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->seclabel, flags)))
> + NULL, def->seclabels,
> + def->nseclabels,
> + flags)))
> goto error;
> } else if (xmlStrEqual(node->name, BAD_CAST "lease")) {
> dev->type = VIR_DOMAIN_DEVICE_LEASE;
> @@ -7939,7 +8019,7 @@ 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 (virSecurityLabelDefsParseXML(def, ctxt, flags) == -1)
> goto error;
>
> /* Extract domain memory */
> @@ -8538,7 +8618,8 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
> nodes[i],
> ctxt,
> bootMap,
> - &def->seclabel,
> + def->seclabels,
> + def->nseclabels,
> flags);
> if (!disk)
> goto error;
> @@ -10843,6 +10924,8 @@ virSecurityLabelDefFormat(virBufferPtr buf, virSecurityLabelDefPtr def)
> virBufferAsprintf(buf, " relabel='%s'",
> def->norelabel ? "no" : "yes");
>
> +VIR_DEBUG("FMT %s: %s %s %s", def->model, def->label, def->imagelabel, def->baselabel); // TODO remove
> +
> if (def->label || def->imagelabel || def->baselabel) {
> virBufferAddLit(buf, ">\n");
>
> @@ -10911,6 +10994,7 @@ virDomainDiskDefFormat(virBufferPtr buf,
> const char *copy_on_read = virDomainVirtioEventIdxTypeToString(def->copy_on_read);
> const char *startupPolicy = virDomainStartupPolicyTypeToString(def->startupPolicy);
>
> + int n;
> char uuidstr[VIR_UUID_STRING_BUFLEN];
>
> if (!type) {
> @@ -11006,10 +11090,11 @@ virDomainDiskDefFormat(virBufferPtr buf,
> if (def->startupPolicy)
> virBufferEscapeString(buf, " startupPolicy='%s'",
> startupPolicy);
> - if (def->seclabel) {
> + if (def->nseclabels) {
> virBufferAddLit(buf, ">\n");
> virBufferAdjustIndent(buf, 8);
> - virSecurityDeviceLabelDefFormat(buf, def->seclabel);
> + for (n = 0; n < def->nseclabels; n++)
> + virSecurityDeviceLabelDefFormat(buf, def->seclabels[n]);
> virBufferAdjustIndent(buf, -8);
> virBufferAddLit(buf, " </source>\n");
> } else {
> @@ -11019,10 +11104,11 @@ virDomainDiskDefFormat(virBufferPtr buf,
> case VIR_DOMAIN_DISK_TYPE_BLOCK:
> virBufferEscapeString(buf, " <source dev='%s'",
> def->src);
> - if (def->seclabel) {
> + if (def->nseclabels) {
> virBufferAddLit(buf, ">\n");
> virBufferAdjustIndent(buf, 8);
> - virSecurityDeviceLabelDefFormat(buf, def->seclabel);
> + for (n = 0; n < def->nseclabels; n++)
> + virSecurityDeviceLabelDefFormat(buf, def->seclabels[n]);
> virBufferAdjustIndent(buf, -8);
> virBufferAddLit(buf, " </source>\n");
> } else {
> @@ -13020,7 +13106,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
> virBufferAddLit(buf, " </devices>\n");
>
> virBufferAdjustIndent(buf, 2);
> - virSecurityLabelDefFormat(buf, &def->seclabel);
> + for (n = 0; n < def->nseclabels; n++)
> + virSecurityLabelDefFormat(buf, def->seclabels[n]);
> virBufferAdjustIndent(buf, -2);
>
> if (def->namespaceData && def->ns.format) {
> @@ -15217,3 +15304,66 @@ cleanup:
> VIR_FREE(xmlStr);
> return ret;
> }
> +
> +virSecurityLabelDefPtr
> +virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char *model)
> +{
> + int i;
> +
> + if (def == NULL || model == NULL)
> + return NULL;
> +
> + for (i = 0; i < def->nseclabels; i++) {
> + if (def->seclabels[i]->model == NULL)
> + continue;
> + if (STREQ(def->seclabels[i]->model, model))
> + return def->seclabels[i];
> + }
> +
> + return virDomainDefAddSecurityLabelDef(def, model);
> +}
> +
> +virSecurityDeviceLabelDefPtr
> +virDomainDiskDefGetSecurityLabelDef(virDomainDiskDefPtr def, const char *model)
> +{
> + int i;
> +
> + if (def == NULL)
> + return NULL;
> +
> + for (i = 0; i < def->nseclabels; i++) {
> + if (STREQ(def->seclabels[i]->model, model))
> + return def->seclabels[i];
> + }
> + return NULL;
> +}
> +
> +virSecurityLabelDefPtr
> +virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char *model)
> +{
> + virSecurityLabelDefPtr seclabel = NULL;
> +
> + if (VIR_ALLOC(seclabel) < 0) {
> + virReportOOMError();
> + return NULL;
> + }
> +
> + if (model) {
> + seclabel->model = strdup(model);
> + if (seclabel->model == NULL) {
> + virReportOOMError();
> + virSecurityLabelDefFree(seclabel);
> + return NULL;
> + }
> + }
> +
> + if (VIR_EXPAND_N(def->seclabels, def->nseclabels, 1) < 0) {
> + virReportOOMError();
> + virSecurityLabelDefFree(seclabel);
> + return NULL;
> + }
> + def->seclabels[def->nseclabels - 1] = seclabel;
> +
> + return seclabel;
> +}
> +
We don't like empty lines at the end of a file.
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 4c56902..49a7120 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -309,6 +309,7 @@ struct _virSecurityLabelDef {
> typedef struct _virSecurityDeviceLabelDef virSecurityDeviceLabelDef;
> typedef virSecurityDeviceLabelDef *virSecurityDeviceLabelDefPtr;
> struct _virSecurityDeviceLabelDef {
> + char *model;
> char *label; /* image label string */
> bool norelabel;
> };
> @@ -553,7 +554,6 @@ struct _virDomainDiskDef {
> int device;
> int bus;
> char *src;
> - virSecurityDeviceLabelDefPtr seclabel;
And you we need to adapt src/security/security_manager.c and
security/security_selinux.c too. I know it's the next patch, but in
libvirt we have this policy of 'being able to compile after each patch'.
But frankly, we fail sometimes. On the other hand, it would turn this
one into unbearable huge patch. So I'd say it's okay in this case.
Otherwise looking good. But can't give you my ACK unless you fix the
test driver.
Michal
More information about the libvir-list
mailing list