Re: [libvirt] [PATCH 4/7] Re-add domain device seclabel parsing / formatting

On 01/11/2012 09:33 AM, Daniel P. Berrange wrote: > From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; > > This re-introduces parsing & formatting for per device seclabels. > There is a new virDomainDeviceSeclabelPtr struct and corresponding > APIs for parsing/formatting. > --- > src/conf/domain_conf.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++-- > src/conf/domain_conf.h | 12 +++++- > 2 files changed, 122 insertions(+), 5 deletions(-) Complex enough that I agree with your decision to separate the revert from the new implementation, even if it means a potential for a failed 'git bisect'.

Do we want to be saving this label if norelabel is true (that is, if the user passes: <seclabel relabel='no'> <label>...</label> </seclabel> do we reject that as invalid, or silently ignore the useless <label>, instead of your behavior of accepting it)? > @@ -9767,6 +9844,19 @@ cleanup: > } > > > +static void > +virSecurityDeviceLabelDefFormat(virBufferPtr buf, > + virSecurityDeviceLabelDefPtr def) > +{ > + virBufferAsprintf(buf, "<seclabel relabel='%s'>\n", > + def->norelabel ? "no" : "yes"); > + if (def->label) > + virBufferEscapeString(buf, " <label>%s</label>\n", > + def->label); > + virBufferAddLit(buf, "</seclabel>\n"); With norelabel, this would generate the odd-looking: <seclabel relabel='no'> </seclabel> How about instead doing: virBufferAsprintf(buf, "<seclabel relabel='%s'", def->norelabel ? "no" : "yes"); if (def->label) { virBufferAddLit(buf, ">\n"); virBufferEscapeString(buf, " <label>%s</label>\n", def->label); virBufferAddLit(buf, "</seclabel>\n"); } else { virBufferAddLit(buf, "/>\n"); }