[libvirt] [PATCH v1 03/23] virSecurityDACSetOwnershipInternal: Don't chown so often

[John Ferlan]

On 10/12/2015 06:25 AM, Michal Privoznik wrote:
> It's better if we stat() file that we are about to chown() at
> first and check if there's something we need to change. Not that
> it would make much difference, but for the upcoming patches we
> need to be doing stat() anyway.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/security/security_dac.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
> index 0dfe570..9b079e0 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c
> @@ -271,17 +271,18 @@ virSecurityDACSetOwnershipInternal(virSecurityDACDataPtr priv,
>              path = src->path;
>          }
>  
> +        if (stat(path, &sb) < 0) {
> +            virReportSystemError(errno, _("unable to stat: %s"), path);
> +            return -1;
> +        }
> +
> +        if (sb.st_uid == uid && sb.st_gid == gid) {
> +            /* nothing to chown */
> +            return 0;
> +        }
> +
>          rc = chown(path, uid, gid);
>          chown_errno = errno;
> -
> -        if (rc < 0 &&
> -            stat(path, &sb) >= 0) {
> -            if (sb.st_uid == uid &&
> -                sb.st_gid == gid) {
> -                /* It's alright, there's nothing to change anyway. */
> -                return 0;
> -            }
> -        }

I don't believe chown_errno is necessary anymore either since a failure
in stat() won't overwrite errno.

Your call to keep it or not since it's also part of the "if" portion
(although that path doesn't overwrite either).

ACK -

John



>      }
>  
>      if (rc < 0) {
>