[libvirt] [PATCH] conf: format runtime DAC seclabel, unless MIGRATABLE

We historically format runtime seclabel selinux/apparmor values, however we skip formatting runtime DAC values. This was added in commit 990e46c4542349f838e001d30638872576c389e9 Author: Marcelo Cerri <mhcerri@linux.vnet.ibm.com> Date: Fri Aug 31 13:40:41 2012 +0200 conf: Avoid formatting auto-generated DAC labels to maintain migration compatibility with libvirt < 0.10.0. However the formatting was skipped unconditionally. Instead only skip formatting in the VIR_DOMAIN_DEF_FORMAT_MIGRATABLE case. https://bugzilla.redhat.com/show_bug.cgi?id=1215833 --- src/conf/domain_conf.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index db567f5..0557912 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18741,7 +18741,8 @@ virDomainEventActionDefFormat(virBufferPtr buf, static void virSecurityLabelDefFormat(virBufferPtr buf, - virSecurityLabelDefPtr def) + virSecurityLabelDefPtr def, + unsigned int flags) { const char *sectype = virDomainSeclabelTypeToString(def->type); @@ -18751,11 +18752,13 @@ virSecurityLabelDefFormat(virBufferPtr buf, if (def->type == VIR_DOMAIN_SECLABEL_DEFAULT) return; - /* To avoid backward compatibility issues, suppress DAC and 'none' labels - * that are automatically generated. + /* libvirt versions prior to 0.10.0 support just a single seclabel element + * in the XML, and that would typically be filled with type=selinux. + * Don't format it in the MIGRATABLE case, for backwards compatibility */ if ((STREQ_NULLABLE(def->model, "dac") || - STREQ_NULLABLE(def->model, "none")) && def->implicit) + STREQ_NULLABLE(def->model, "none")) && def->implicit && + (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE)) return; virBufferAsprintf(buf, "<seclabel type='%s'", @@ -22886,7 +22889,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAddLit(buf, "</devices>\n"); for (n = 0; n < def->nseclabels; n++) - virSecurityLabelDefFormat(buf, def->seclabels[n]); + virSecurityLabelDefFormat(buf, def->seclabels[n], flags); if (def->namespaceData && def->ns.format) { if ((def->ns.format)(buf, def->namespaceData) < 0) -- 2.7.3

On Sat, Apr 23, 2016 at 02:51:26PM -0400, Cole Robinson wrote:
We historically format runtime seclabel selinux/apparmor values, however we skip formatting runtime DAC values. This was added in
commit 990e46c4542349f838e001d30638872576c389e9 Author: Marcelo Cerri <mhcerri@linux.vnet.ibm.com> Date: Fri Aug 31 13:40:41 2012 +0200
conf: Avoid formatting auto-generated DAC labels
to maintain migration compatibility with libvirt < 0.10.0.
However the formatting was skipped unconditionally. Instead only skip formatting in the VIR_DOMAIN_DEF_FORMAT_MIGRATABLE case.
This all makes sense, but when I started tying it I've found it may still cause some problems. Probably. The problem is that I have no idea whether it's safer to use number or a name as the uid/gid. But since we have that parsing code in place and this patch is not about that, so we can deal with that later. ACK

On 04/27/2016 03:30 AM, Martin Kletzander wrote:
On Sat, Apr 23, 2016 at 02:51:26PM -0400, Cole Robinson wrote:
We historically format runtime seclabel selinux/apparmor values, however we skip formatting runtime DAC values. This was added in
commit 990e46c4542349f838e001d30638872576c389e9 Author: Marcelo Cerri <mhcerri@linux.vnet.ibm.com> Date: Fri Aug 31 13:40:41 2012 +0200
conf: Avoid formatting auto-generated DAC labels
to maintain migration compatibility with libvirt < 0.10.0.
However the formatting was skipped unconditionally. Instead only skip formatting in the VIR_DOMAIN_DEF_FORMAT_MIGRATABLE case.
This all makes sense, but when I started tying it I've found it may still cause some problems. Probably. The problem is that I have no idea whether it's safer to use number or a name as the uid/gid. But since we have that parsing code in place and this patch is not about that, so we can deal with that later.
ACK
Thanks, I'll push after the release - Cole
participants (2)
-
Cole Robinson
-
Martin Kletzander