[libvirt] [PATCH v3 2/2] security: Refactor virSecurityManagerGenLabel

[Ján Tomko]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150213/762e5f1c/attachment-0001.sig>