On Wed, Jan 04, 2012 at 04:01:24PM -0700, Eric Blake wrote:
> Commit b434329 has a logic bug: seclabel overrides don't set
> def->type, but the default value is 0 (aka static). Restarting
> libvirtd would thus reject the XML for any domain with an
> override of <seclabel relabel='no'/> (which happens quite
> easily if a disk image lives on NFS), with a message:
>
> 2012-01-04 22:29:40.949+0000: 6769: error : virSecurityLabelDefParseXMLHelper:2593 : XML error: security label is missing
>
> Fix the logic to never read from an override's def->type, and
> to allow a missing <label> subelement when relabel is no. There's
> a lot of stupid double-negatives in the code (!norelabel) because
> of the way that we want the zero-initialized defaults to behave.
>
> * src/conf/domain_conf.c (virSecurityLabelDefParseXMLHelper): Use
> type field from correct location.
> ---
> src/conf/domain_conf.c | 16 +++++++++-------
> 1 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 29966f1..dcf23fa 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1,7 +1,7 @@
> /*
> * domain_conf.c: domain XML processing
> *
> - * Copyright (C) 2006-2011 Red Hat, Inc.
> + * Copyright (C) 2006-2012 Red Hat, Inc.
> * Copyright (C) 2006-2008 Daniel P. Berrange
> *
> * This library is free software; you can redistribute it and/or
> @@ -2541,6 +2541,7 @@ virSecurityLabelDefParseXMLHelper(virSecurityLabelDefPtr def,
> char *p;
> xmlNodePtr save_ctxt = ctxt->node;
> int ret = -1;
> + int type = default_seclabel ? default_seclabel->type : def->type;
>
> ctxt->node = node;
>
> @@ -2567,14 +2568,15 @@ virSecurityLabelDefParseXMLHelper(virSecurityLabelDefPtr def,
> }
> VIR_FREE(p);
> if (!default_seclabel &&
> - def->type == VIR_DOMAIN_SECLABEL_DYNAMIC &&
> + type == VIR_DOMAIN_SECLABEL_DYNAMIC &&
> def->norelabel) {
> - virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> - "%s", _("dynamic label type must use resource relabeling"));
> + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("dynamic label type must use resource "
> + "relabeling"));
> goto cleanup;
> }
> } else {
> - if (!default_seclabel && def->type == VIR_DOMAIN_SECLABEL_STATIC)
> + if (!default_seclabel && type == VIR_DOMAIN_SECLABEL_STATIC)
> def->norelabel = true;
> else
> def->norelabel = false;
> @@ -2583,12 +2585,12 @@ virSecurityLabelDefParseXMLHelper(virSecurityLabelDefPtr def,
> /* Only parse label, if using static labels, or
> * if the 'live' VM XML is requested, or if this is a device override
> */
> - if (def->type == VIR_DOMAIN_SECLABEL_STATIC ||
> + if (type == VIR_DOMAIN_SECLABEL_STATIC ||
> !(flags & VIR_DOMAIN_XML_INACTIVE) ||
> (default_seclabel && !def->norelabel)) {
> p = virXPathStringLimit("string(./label[1])",
> VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
> - if (p == NULL) {
> + if (p == NULL && !(default_seclabel && def->norelabel)) {
> virDomainReportError(VIR_ERR_XML_ERROR,
> "%s", _("security label is missing"));
> goto cleanup;
ACK and pushed since I wanted to include it in rc2,
thanks,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list