[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/3] New utility functions virFileCreate and virDirCreate



On Mon, Jan 11, 2010 at 12:46:11AM -0500, Laine Stump wrote:
> These functions create a new file or directory with the given
> uid/gid. If the flag VIR_FILE_CREATE_AS_UID is given, they do this by
> forking a new process, calling setuid/setgid in the new process, and
> then creating the file. This is better than simply calling open then
> fchown, because in the latter case, a root-squashing nfs server would
> create the new file as user nobody, then refuse to allow fchown.
> 
> If VIR_FILE_CREATE_AS_UID is not specified, the simpler tactic of
> creating the file/dir, then chowning is is used. This gives better
> results in cases where the parent directory isn't on a root-squashing
> NFS server, but doesn't give permission for the specified uid/gid to
> create files. (Note that if the fork/setuid method fails to create the
> file due to access privileges, the parent process will make a second
> attempt using this simpler method.)
> 
> Return from both of these functions is 0 on success, or the value of
> errno if there was a failure.
> ---
>  src/util/util.c |  247 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/util/util.h |    9 ++
>  2 files changed, 256 insertions(+), 0 deletions(-)
> 
> diff --git a/src/util/util.c b/src/util/util.c
> index 1d493de..6d31f47 100644
> --- a/src/util/util.c
> +++ b/src/util/util.c
> @@ -1126,6 +1126,253 @@ int virFileExists(const char *path)
>      return(0);
>  }
>  
> +
> +static int virFileCreateSimple(const char *path, mode_t mode, uid_t uid, gid_t gid) {
> +    int fd = -1;
> +    int ret = 0;
> +
> +    if ((fd = open(path, O_RDWR | O_CREAT | O_EXCL, mode)) < 0) {
> +        ret = errno;
> +        virReportSystemError(NULL, errno, _("failed to create file %s"),
> +                             path);

Can you just put single quotes around the '%s'  placeholder for the path.
Every now & then someone uses a crazy path with whitespace and quotes help
identify that kind of issue. Likewise for the rest of the calls.
Functionally the patch looks good to me.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]