Re: [libvirt] [PATCH] Don't autogenerate seclabels of type 'none'

On Mon, Aug 14, 2017 at 06:07:10PM -0600, Jim Fehlig wrote: > When security drivers are active and domain def contains > no <seclabel> elements, there is no need to autogenerate > seclabels when starting the domain, e.g. > > <seclabel type='none' model='apparmor'/> > > In fact, autogenerating the label can result in needless > save/restore and migration failures when the security driver > is not active on the restore/migration target. > > The virSecurityManagerGenLabel function in src/security_manager.c > even has logic to skip autogenerated labels, but the logic > is a bit flawed. Autogeneration should be skipped when the > domain has not seclabels, i.e. vm->nseclabels == 0. > > Resolves: https://bugzilla.opensuse.org/show_bug.cgi?id=1051017 > Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; > --- > src/security/security_manager.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/security/security_manager.c b/src/security/security_manager.c > index 013bbc37e..441c4d1fd 100644 > --- a/src/security/security_manager.c > +++ b/src/security/security_manager.c > @@ -670,7 +670,7 @@ virSecurityManagerGenLabel(virSecurityManagerPtr mgr, > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > _("Unconfined guests are not allowed on this host")); > goto cleanup; > - } else if (vm->nseclabels && generated) { > + } else if (vm->nseclabels == 0 && generated) { This would likely cause a regression like we did prior to commit e4a28a3281 which introduced the condition you're changing, IOW if you specify a seclabel specifically, you're still going to autogenerate one of type='none'.

So my question is, what's the point of autogenerating seclabel of type='none' anyway? Shouldn't we just skip type='none' altogether when it's us who generated it?