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

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.