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

On Wed, Aug 16, 2017 at 05:54:08PM -0600, Jim Fehlig wrote: > When security drivers are active but confinement is not enabled, > there is no need to autogenerate <seclabel> elements when starting > a domain def that contains no <seclabel> elements. In fact, > autogenerating the elements can result in needless save/restore and > migration failures when the security driver is not active on the > restore/migration target. > > This patch changes the virSecurityManagerGenLabel function in > src/security_manager.c to only autogenerate a <seclabel> element > if none is already defined for the domain *and* default > confinement is enabled. Otherwise the needless <seclabel> > autogeneration is skipped. > > Resolves: https://bugzilla.opensuse.org/show_bug.cgi?id=1051017 > Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; > --- > > V2: Don't autogenerate a seclabel if domain does not contain one > and confinement is disabled. > > src/security/security_manager.c | 42 +++++++++++++++++++++-------------------- > 1 file changed, 22 insertions(+), 20 deletions(-) > > diff --git a/src/security/security_manager.c b/src/security/security_manager.c > index 013bbc37e..10515c314 100644 > --- a/src/security/security_manager.c > +++ b/src/security/security_manager.c > @@ -650,30 +650,32 @@ virSecurityManagerGenLabel(virSecurityManagerPtr mgr, > for (i = 0; sec_managers[i]; i++) { > generated = false; > seclabel = virDomainDefGetSecurityLabelDef(vm, sec_managers[i]->drv->name); > - if (!seclabel) { > - if (!(seclabel = virSecurityLabelDefNew(sec_managers[i]->drv->name))) > - goto cleanup; > - generated = seclabel->implicit = true; > - } > + if (seclabel) { Just a tiny nitpick, generally we prefer the 'if' block to be shorter than the corresponding 'else' block.