Re: [libvirt] [PATCH] util: Don't try to fchown files opened as non-root

On 07/08/2011 07:30 AM, Jiri Denemark wrote: > When virFileOpenAs is called with VIR_FILE_OPEN_AS_UID flag and uid/gid > different from root/root while libvirtd is running as root, we fork a > new child, change its effective UID/GID to uid/gid and run > virFileOpenAsNoFork. It doesn't make any sense to fchown() the opened > file in this case since we already know that uid/gid can access the file > when open succeeds and one of the following situations may happen: > > - the file is already owned by uid/gid and we skip fchown even before > this patch > - the file is owned by uid but not gid because it was created in a > directory with SETGID set, in which case it is desirable not to change > the group > - the file may be owned by a completely different user and/or group > because it was created on a root-squashed or even all-squashed NFS > filesystem, in which case fchown would most likely fail anyway > --- > src/util/util.c | 31 +++++++++++++++++++------------ > 1 files changed, 19 insertions(+), 12 deletions(-) ACK. > + /* VIR_FILE_OPEN_AS_UID in flags means we are running in a child process > + * owned by uid and gid */ > + if (!(flags & VIR_FILE_OPEN_AS_UID)) { > + struct stat st; > + if (fstat(fd, &st) == -1) { Style nit - add a newline between the declaration of st and the first statement (the nested if).