[libvirt] [PATCH 1/2] conf: Add oldlabel field to virSecurityDeviceLabelDef

[Daniel P. Berrange]

On Mon, Feb 18, 2013 at 12:29:03PM +0100, Michal Privoznik wrote:
> The field is there to store the original label of device,
> so we can restore it when domain is shutting down.
> ---
>  src/conf/domain_conf.c | 20 +++++++++++++++-----
>  src/conf/domain_conf.h |  1 +
>  2 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 7a2b012..d83330a 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -957,6 +957,7 @@ virSecurityDeviceLabelDefFree(virSecurityDeviceLabelDefPtr def)
>          return;
>      VIR_FREE(def->model);
>      VIR_FREE(def->label);
> +    VIR_FREE(def->oldlabel);
>      VIR_FREE(def);
>  }
>  
> @@ -3639,14 +3640,15 @@ static int
>  virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn,
>                                    size_t *nseclabels_rtn,
>                                    virSecurityLabelDefPtr *vmSeclabels,
> -                                  int nvmSeclabels, xmlXPathContextPtr ctxt)
> +                                  int nvmSeclabels, xmlXPathContextPtr ctxt,
> +                                  unsigned int flags)
>  {
>      virSecurityDeviceLabelDefPtr *seclabels;
>      size_t nseclabels = 0;
>      int n, i, j;
>      xmlNodePtr *list = NULL;
>      virSecurityLabelDefPtr vmDef = NULL;
> -    char *model, *relabel, *label;
> +    char *model, *relabel, *label, *oldlabel;
>  
>      if ((n = virXPathNodeSet("./seclabel", ctxt, &list)) < 0)
>          goto error;
> @@ -3717,6 +3719,13 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn,
>                               NULLSTR(seclabels[i]->model));
>              goto error;
>          }
> +
> +        /* only parse oldlabel when parsing domain status XML */
> +        if (flags & VIR_DOMAIN_XML_INTERNAL_STATUS) {
> +            oldlabel = virXPathStringLimit("string(./oldlabel)",
> +                                           VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
> +            seclabels[i]->oldlabel = oldlabel;
> +        }
>      }
>      VIR_FREE(list);
>  
> @@ -4299,7 +4308,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
>                                                &def->nseclabels,
>                                                vmSeclabels,
>                                                nvmSeclabels,
> -                                              ctxt) < 0)
> +                                              ctxt, flags) < 0)
>              goto error;
>          ctxt->node = saved_node;
>      }
> @@ -5926,7 +5935,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
>                                                            &chr_def->nseclabels,
>                                                            vmSeclabels,
>                                                            nvmSeclabels,
> -                                                          ctxt) < 0) {
> +                                                          ctxt, flags) < 0) {
>                          ctxt->node = saved_node;
>                          goto error;
>                      }
> @@ -12344,10 +12353,11 @@ virSecurityDeviceLabelDefFormat(virBufferPtr buf,
>  
>      virBufferAsprintf(buf, " relabel='%s'", def->norelabel ? "no" : "yes");
>  
> -    if (def->label) {
> +    if (def->label || def->oldlabel) {
>          virBufferAddLit(buf, ">\n");
>          virBufferEscapeString(buf, "  <label>%s</label>\n",
>                                def->label);
> +        virBufferEscapeString(buf, "  <oldlabel>%s</oldlabel>\n", def->oldlabel);
>          virBufferAddLit(buf, "</seclabel>\n");
>      } else {
>          virBufferAddLit(buf, "/>\n");
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 9232ff9..b7f4b38 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -327,6 +327,7 @@ typedef virSecurityDeviceLabelDef *virSecurityDeviceLabelDefPtr;
>  struct _virSecurityDeviceLabelDef {
>      char *model;
>      char *label;        /* image label string */
> +    char *oldlabel;     /* the original label to return to */
>      bool norelabel;
>  };

This is storing driver specific state in the XML configuration
description which is bad practice in general. Any such state
should be maintained inside the DAC driver itself.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|