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
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
