On Thu, Feb 12, 2015 at 06:32:41PM +0100, Erik Skultety wrote: > if (mgr == NULL || mgr->drv == NULL) > return ret; > > This check isn't really necessary, security manager cannot be a NULL > pointer as it is either selinux (by default) or 'none', if no other driver is > set in the config. Even with no config file driver name yields 'none'. > > The other hunk checks for domain's security model validity, but we should > also check devices' security model as well, therefore this hunk is moved into > a separate function which is called by virSecurityManagerCheckAllLabel that > checks both the domain's security model and devices' security model. > > https://bugzilla.redhat.com/show_bug.cgi?id=1165485 > --- > src/security/security_manager.c | 41 ++++++++++++++++++++++------------------- > 1 file changed, 22 insertions(+), 19 deletions(-) ACK > @@ -731,6 +713,22 @@ static int virSecurityManagerCheckSecurityModel(char *secmodel, > > > static int > +virSecurityManagerCheckSecurityDomainLabel(virDomainDefPtr def, > + void *opaque) Same comments as for v1 regarding the use of void and the repeating extra word. > @@ -776,6 +774,11 @@ int virSecurityManagerCheckAllLabel(virSecurityManagerPtr mgr, > { > size_t i; > > + /* first check per-domain seclabels */ These comments don't seem very helpful - a function named CheckDomainLabel should do exactly that. I fixed the nits and pushed the patch. Jan
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list