[libvirt] [PATCH] seclabel: fix regression in libvirtd restart
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; -- 1.7.7.5
On 05.01.2012 00:01, 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(-)
ACK. Although, I'd split the long log line. But that's just cosmetic Michal
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@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (3)
-
Daniel Veillard -
Eric Blake -
Michal Privoznik