[libvirt] [PATCH] seclabel: fix regression in libvirtd restart

[Daniel Veillard]

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 at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/