On Fri, Aug 18, 2017 at 12:18:20AM -0600, Jim Fehlig wrote: > On 08/17/2017 02:17 AM, Erik Skultety wrote: > > 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 <jfeh...@suse.com> > > > --- > > > > > > 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. > > Thanks for the review! Sorry, but I'm not sure what you mean by "shorter". > Do you mean the 'if' block should contain fewer lines of code than the > 'else' block?
Yeah, that's exactly what I meant, I'm sorry, not being a native speaker once again resulted in a poor choice of wording :/. Next time I'll do better in expressing myself more precisely, I promise :). Erik > > Regards, > Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
