Re: [libvirt] [PATCH v2] conf: Fix parsing of seclabels without model

On Thu, Aug 30, 2012 at 09:39:03PM -0300, Marcelo Cerri wrote: > With this patch libvirt tries to assign a model to a single seclabel > when model is missing. Libvirt will look up at host's capabilities and > assign the first model to seclabel. > > This patch fixes: > > 1. The problem with existing guests that have a seclabel defined in its XML. > 2. A XML parse error when a guest is restored. > > Signed-off-by: Marcelo Cerri <mhcerri(a)linux.vnet.ibm.com&gt; > --- > src/conf/domain_conf.c | 64 +++++++++++++++++++++++++++++--------------------- > 1 file changed, 37 insertions(+), 27 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 224aec5..42c3900 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -3102,22 +3102,10 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr 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(./@model)", > - VIR_SECURITY_MODEL_BUFLEN-1, ctxt); > - if (p == NULL && def->type != VIR_DOMAIN_SECLABEL_NONE) { > - virReportError(VIR_ERR_XML_ERROR, > - "%s", _("missing security model")); > - } > - def->model = p; > - } > + /* Always parse model */ > + p = virXPathStringLimit("string(./@model)", > + VIR_SECURITY_MODEL_BUFLEN-1, ctxt); > + def->model = p; > > return def; > > @@ -3129,10 +3117,12 @@ error: > static int > virSecurityLabelDefsParseXML(virDomainDefPtr def, > xmlXPathContextPtr ctxt, > + virCapsPtr caps, > unsigned int flags) > { > int i = 0, n; > xmlNodePtr *list = NULL, saved_node; > + virCapsHostPtr host = &caps->host; > > /* Check args and save context */ > if (def == NULL || ctxt == NULL) > @@ -3159,18 +3149,38 @@ virSecurityLabelDefsParseXML(virDomainDefPtr def, > ctxt->node = saved_node; > VIR_FREE(list); > > - /* Checking missing model information > - * when there is more than one seclabel */ > - if (n > 1) { > - for(; n; n--) { > - if (def->seclabels[n - 1]->model == NULL) { > - virReportError(VIR_ERR_XML_ERROR, "%s", > - _("missing security model " > - "when using multiple labels")); > - goto error; > - } > + /* libvirt versions prior to 0.10.0 support just a single seclabel element > + * in guest's XML and model attribute can be suppressed if type is none or > + * type is dynamic, baselabel is not defined and INACTIVE flag is set. > + * > + * To avoid compatibility issues, for this specific case the first model > + * defined in host's capabilities is used as model for the seclabel. > + */ > + if (def->nseclabels == 1 && > + def->seclabels[0]->model == NULL && > + def->seclabels[0]->type != VIR_DOMAIN_SECLABEL_STATIC && > + def->seclabels[0]->baselabel == NULL && > + (flags & VIR_DOMAIN_XML_INACTIVE) && > + host->nsecModels > 0) { > + > + /* Copy model from host. */ > + def->seclabels[0]->model = strdup(host->secModels[0].model);

Fails "make check" thinkpad:~/libvirt/tests -> export VIR_TEST_DEBUG=2 thinkpad:~/libvirt/tests -> ./qemuxml2argvtest ... 219) QEMU XML-2-ARGV seclabel-none ... libvir: Domain Config error : XML error: missing security model when using multiple labels FAILED ... thinkpad:~/libvirt/tests -> grep -C2 seclabel qemuxml2argvdata/qemuxml2argv-seclabel-none.xml <memballoon model='virtio'/> </devices> <seclabel type='none'/> </domain> thinkpad:~/libvirt/tests -> So the new code following your patch is unable to parse the construct <seclabel type='none'/>