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@xxxxxxxx> > --- > > 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. > + if (seclabel->type == VIR_DOMAIN_SECLABEL_DEFAULT) { > + if (virSecurityManagerGetDefaultConfined(sec_managers[i])) { > + seclabel->type = VIR_DOMAIN_SECLABEL_DYNAMIC; > + } else { > + seclabel->type = VIR_DOMAIN_SECLABEL_NONE; > + seclabel->relabel = false; > + } > + } > > - if (seclabel->type == VIR_DOMAIN_SECLABEL_DEFAULT) { > + if (seclabel->type == VIR_DOMAIN_SECLABEL_NONE) { > + if (virSecurityManagerGetRequireConfined(sec_managers[i])) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("Unconfined guests are not allowed on this host")); > + goto cleanup; > + } > + } > + } else { > + /* Only generate seclabel if confinement is enabled */ > if (virSecurityManagerGetDefaultConfined(sec_managers[i])) { The same applies to this nested if-else block. > + if (!(seclabel = virSecurityLabelDefNew(sec_managers[i]->drv->name))) > + goto cleanup; > + generated = seclabel->implicit = true; > seclabel->type = VIR_DOMAIN_SECLABEL_DYNAMIC; > } else { > - seclabel->type = VIR_DOMAIN_SECLABEL_NONE; > - seclabel->relabel = false; > - } > - } > - > - if (seclabel->type == VIR_DOMAIN_SECLABEL_NONE) { > - if (virSecurityManagerGetRequireConfined(sec_managers[i])) { > - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > - _("Unconfined guests are not allowed on this host")); > - goto cleanup; > - } else if (vm->nseclabels && generated) { > - VIR_DEBUG("Skipping auto generated seclabel of type none"); > - virSecurityLabelDefFree(seclabel); > - seclabel = NULL; > + VIR_DEBUG("Skipping auto generated seclabel"); > continue; > } > } ACK with the adjustment. Erik -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
