[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 2/5] multiple security drivers in XML data



On 18.07.2012 03:28, Marcelo Cerri wrote:
> This patch updates the domain 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.
> ---
>  docs/schemas/domaincommon.rng                      |   30 ++-
>  src/conf/domain_conf.c                             |  339 ++++++++++++++------
>  src/conf/domain_conf.h                             |    9 +
>  .../qemuxml2argv-seclabel-dynamic-baselabel.xml    |    2 +-
>  .../qemuxml2argv-seclabel-dynamic-override.xml     |    6 +-
>  .../qemuxml2argv-seclabel-dynamic.xml              |    2 +-
>  .../qemuxml2argv-seclabel-static.xml               |    2 +-
>  7 files changed, 270 insertions(+), 120 deletions(-)
> 
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index b7562ad..72ea54e 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/domain_conf.c b/src/conf/domain_conf.c
> index b468174..a63f36f 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -3080,17 +3080,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;
> @@ -3104,7 +3106,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")) {
> @@ -3121,13 +3123,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 {
> @@ -3144,7 +3148,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) {
>              virDomainReportError(VIR_ERR_XML_ERROR,
> @@ -3159,7 +3163,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) {
>              virDomainReportError(VIR_ERR_XML_ERROR,
> @@ -3171,93 +3175,162 @@ 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) {
> -            virDomainReportError(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) {
> +        virDomainReportError(VIR_ERR_XML_ERROR,
> +                             "%s", _("missing security model"));
> +        goto error;
>      }
> +    def->model = p;
>  
> -    return 0;
> +    return def;
>  
>  error:
>      virSecurityLabelDefFree(def);
> -    return -1;
> +    return NULL;
>  }
>  
> -
>  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[1]", 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;
>      }

'list' must be VIR_FREE()d after use. And here^^ is just leaked.

>  
> -    if (VIR_ALLOC(*def) < 0) {
> +    /* 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);

> +    return 0;
> +
> +error:
> +    ctxt->node = saved_node;
> +    for (i = 0; i < n; i++) {

We can just take reverse steps. By the time we get here,
'i' represents the real position in 'def->seclabels' so we don't
need to go through whole 'n'; But that's really a premature optimization.

> +        virSecurityLabelDefFree(def->seclabels[i]);
> +    }
> +    VIR_FREE(def->seclabels);

VIR_FREE(list);

> +    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)

Again, 'list' must be VIR_FREE()d at the end.

> +        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[1]/@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) {
> +            virDomainReportError(VIR_ERR_XML_ERROR, "%s",
> +                _("invalid security model"));
> +            goto error;
>          } 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 */

s/overrided/overridden/

> +            for (j = 0; j < nvmSeclabels; j++) {
> +                if (STREQ(vmSeclabels[j]->model, model)) {
> +                    vmDef = vmSeclabels[j];
> +                    break;
> +                }
> +            }
> +            def->seclabels[i]->model = model;
>          }
> -        VIR_FREE(p);
> -    } else {
> -        (*def)->norelabel = false;
> -    }
>  
> -    p = virXPathStringLimit("string(./seclabel[1]/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(./label)",
> +                                    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;
>  }
>  
>  
> @@ -3341,7 +3414,8 @@ virDomainDiskDefParseXML(virCapsPtr caps,
>                           xmlNodePtr node,
>                           xmlXPathContextPtr ctxt,
>                           virBitmapPtr bootMap,
> -                         virSecurityLabelDefPtr vmSeclabel,
> +                         virSecurityLabelDefPtr* vmSeclabels,
> +                         int nvmSeclabels,
>                           unsigned int flags)
>  {
>      virDomainDiskDefPtr def;
> @@ -3679,15 +3753,9 @@ 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)) {
> -            virReportOOMError();
> -            goto error;
> -        }
> -        if (virSecurityDeviceLabelDefParseXML(&def->seclabels[0],
> -                                              vmSeclabel,
> -                                              ctxt) < 0)
> +        if (virSecurityDeviceLabelDefParseXML(def, vmSeclabels,
> +                                              nvmSeclabels, ctxt) < 0)
>              goto error;
> -        def->nseclabels = 1;
>          ctxt->node = saved_node;
>      }
>  
> @@ -7130,16 +7198,12 @@ 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->seclabels[0], flags)))
> +                                                        NULL, def->seclabels,
> +                                                        def->nseclabels,
> +                                                        flags)))
>              goto error;
>      } else if (xmlStrEqual(node->name, BAD_CAST "lease")) {
>          dev->type = VIR_DOMAIN_DEVICE_LEASE;
> @@ -8038,12 +8102,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 ((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)
> +    if (virSecurityLabelDefsParseXML(def, ctxt, flags) == -1)
>          goto error;
>  
>      /* Extract domain memory */
> @@ -8642,7 +8701,8 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>                                                              nodes[i],
>                                                              ctxt,
>                                                              bootMap,
> -                                                            def->seclabels[0],
> +                                                            def->seclabels,
> +                                                            def->nseclabels,
>                                                              flags);
>          if (!disk)
>              goto error;
> @@ -10935,16 +10995,19 @@ virSecurityLabelDefFormat(virBufferPtr buf, virSecurityLabelDefPtr def)
>      if (def->type == VIR_DOMAIN_SECLABEL_DEFAULT)
>          return;
>  
> -    virBufferAsprintf(buf, "<seclabel type='%s'",
> -                      sectype);
> +    virBufferAsprintf(buf, "<seclabel");
> +
> +    if (def->model) {
> +        virBufferEscapeString(buf, " model='%s'", def->model);
> +    }
> +
> +    virBufferAsprintf(buf," type='%s'", sectype);


If you haven't swapped 'model' and 'type' attributes you wouldn't need
this [1]. Therefore I suggest keeps things in the order they were.

>  
>      if (def->type == VIR_DOMAIN_SECLABEL_NONE) {
>          virBufferAddLit(buf, "/>\n");
>          return;
>      }
>  
> -    virBufferEscapeString(buf, " model='%s'", def->model);
> -
>      virBufferAsprintf(buf, " relabel='%s'",
>                        def->norelabel ? "no" : "yes");
>  
> @@ -10970,8 +11033,8 @@ static void
>  virSecurityDeviceLabelDefFormat(virBufferPtr buf,
>                                  virSecurityDeviceLabelDefPtr def)
>  {
> -    virBufferAsprintf(buf, "<seclabel relabel='%s'",
> -                      def->norelabel ? "no" : "yes");
> +    virBufferAsprintf(buf, "<seclabel model='%s' relabel='%s'",
> +                      def->model, def->norelabel ? "no" : "yes");
>      if (def->label) {
>          virBufferAddLit(buf, ">\n");
>          virBufferEscapeString(buf, "  <label>%s</label>\n",
> @@ -11016,6 +11079,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) {
> @@ -11111,10 +11175,11 @@ virDomainDiskDefFormat(virBufferPtr buf,
>              if (def->startupPolicy)
>                  virBufferEscapeString(buf, " startupPolicy='%s'",
>                                        startupPolicy);
> -            if (def->seclabels && def->seclabels[0]) {
> +            if (def->nseclabels) {
>                  virBufferAddLit(buf, ">\n");
>                  virBufferAdjustIndent(buf, 8);
> -                virSecurityDeviceLabelDefFormat(buf, def->seclabels[0]);
> +                for (n = 0; n < def->nseclabels; n++)
> +                    virSecurityDeviceLabelDefFormat(buf, def->seclabels[n]);
>                  virBufferAdjustIndent(buf, -8);
>                  virBufferAddLit(buf, "      </source>\n");
>              } else {
> @@ -11124,10 +11189,11 @@ virDomainDiskDefFormat(virBufferPtr buf,
>          case VIR_DOMAIN_DISK_TYPE_BLOCK:
>              virBufferEscapeString(buf, "      <source dev='%s'",
>                                    def->src);
> -            if (def->seclabels && def->seclabels[0]) {
> +            if (def->nseclabels) {
>                  virBufferAddLit(buf, ">\n");
>                  virBufferAdjustIndent(buf, 8);
> -                virSecurityDeviceLabelDefFormat(buf, def->seclabels[0]);
> +                for (n = 0; n < def->nseclabels; n++)
> +                    virSecurityDeviceLabelDefFormat(buf, def->seclabels[n]);
>                  virBufferAdjustIndent(buf, -8);
>                  virBufferAddLit(buf, "      </source>\n");
>              } else {
> @@ -13153,11 +13219,10 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>  
>      virBufferAddLit(buf, "  </devices>\n");
>  
> -    if (def->nseclabels && def->seclabels) {
> -        virBufferAdjustIndent(buf, 2);
> -        virSecurityLabelDefFormat(buf, def->seclabels[0]);
> -        virBufferAdjustIndent(buf, -2);
> -    }
> +    virBufferAdjustIndent(buf, 2);
> +    for (n = 0; n < def->nseclabels; n++)
> +        virSecurityLabelDefFormat(buf, def->seclabels[n]);
> +    virBufferAdjustIndent(buf, -2);
>  
>      if (def->namespaceData && def->ns.format) {
>          if ((def->ns.format)(buf, def->namespaceData) < 0)
> @@ -15282,3 +15347,65 @@ 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;
> +}
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 9a2189a..faa760a 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2155,6 +2155,15 @@ virDomainState
>  virDomainObjGetState(virDomainObjPtr obj, int *reason)
>          ATTRIBUTE_NONNULL(1);
>  
> +virSecurityLabelDefPtr
> +virDomainDefGetSecurityLabelDef(virDomainDefPtr def, const char *model);
> +
> +virSecurityDeviceLabelDefPtr
> +virDomainDiskDefGetSecurityLabelDef(virDomainDiskDefPtr def, const char *model);
> +
> +virSecurityLabelDefPtr
> +virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char *model);
> +

I think these should be included in libvirt_private.syms within this
patch rather than the next one.

>  typedef const char* (*virLifecycleToStringFunc)(int type);
>  typedef int (*virLifecycleFromStringFunc)(const char *type);
>  
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-baselabel.xml b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-baselabel.xml
> index 98362a7..171dd47 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-baselabel.xml
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-baselabel.xml
> @@ -23,7 +23,7 @@
>      <controller type='ide' index='0'/>
>      <memballoon model='virtio'/>
>    </devices>
> -  <seclabel type='dynamic' model='selinux' relabel='yes'>
> +  <seclabel model='selinux' type='dynamic' relabel='yes'>

[1]: ^^ (here and the rest of this patch).

>      <baselabel>system_u:system_r:svirt_custom_t:s0</baselabel>
>    </seclabel>
>  </domain>
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.xml b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.xml
> index 4de435b..769caeb 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.xml
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.xml
> @@ -16,14 +16,14 @@
>      <emulator>/usr/bin/qemu</emulator>
>      <disk type='block' device='disk'>
>        <source dev='/dev/HostVG/QEMUGuest1'>
> -        <seclabel relabel='no'/>
> +        <seclabel model='selinux' relabel='no'/>
>        </source>
>        <target dev='hda' bus='ide'/>
>        <address type='drive' controller='0' bus='0' target='0' unit='0'/>
>      </disk>
>      <disk type='block' device='disk'>
>        <source dev='/dev/HostVG/QEMUGuest2'>
> -        <seclabel relabel='yes'>
> +        <seclabel model='selinux' relabel='yes'>
>            <label>system_u:system_r:public_content_t:s0</label>
>          </seclabel>
>        </source>
> @@ -35,7 +35,7 @@
>      <controller type='ide' index='0'/>
>      <memballoon model='virtio'/>
>    </devices>
> -  <seclabel type='dynamic' model='selinux' relabel='yes'>
> +  <seclabel model='selinux' type='dynamic' relabel='yes'>
>      <baselabel>system_u:system_r:svirt_custom_t:s0</baselabel>
>    </seclabel>
>  </domain>
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic.xml b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic.xml
> index 78a6b6a..36df9d4 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic.xml
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic.xml
> @@ -22,5 +22,5 @@
>      <controller type='ide' index='0'/>
>      <memballoon model='virtio'/>
>    </devices>
> -  <seclabel type='dynamic' relabel='yes'/>
> +  <seclabel model='selinux' type='dynamic' relabel='yes'/>
>  </domain>
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-static.xml b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-static.xml
> index 31d5f58..23ddef1 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-static.xml
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-static.xml
> @@ -23,7 +23,7 @@
>      <controller type='ide' index='0'/>
>      <memballoon model='virtio'/>
>    </devices>
> -  <seclabel type='static' model='selinux' relabel='no'>
> +  <seclabel model='selinux' type='static' relabel='no'>
>      <label>system_u:system_r:svirt_custom_t:s0:c192,c392</label>
>    </seclabel>
>  </domain>
> 


Otherwise looking good.

Side note: sometimes the patches are more readable when produced with
--patience especially when moving blocks of code around.

This is the farest I can go for today. I'll continue tomorrow.

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]