[libvirt] [PATCH 1/4] storage: Use virFileUnlink instead of rmdir

[Michal Privoznik]

On 18.09.2015 20:20, John Ferlan wrote:
> Similar to commit id '35847860', it's possible to attempt to create
> a 'netfs' directory in an NFS root-squash environment which will cause
> the 'vol-delete' command to fail.  It's also possible error paths from
> the 'vol-create' would result in an error to remove a created directory
> if the permissions were incorrect (and disallowed root access).
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/storage/storage_backend_fs.c | 20 +++++++++-----------
>  src/util/virfile.c               | 23 +++++++++++++++++------
>  2 files changed, 26 insertions(+), 17 deletions(-)
> 


> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index fe9f8d4..7c285d9 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -2316,7 +2316,7 @@ virFileOpenAs(const char *path, int openflags, mode_t mode,
>  
>  
>  /* virFileUnlink:
> - * @path: file to unlink
> + * @path: file to unlink or directory to remove
>   * @uid: uid that was used to create the file (not required)
>   * @gid: gid that was used to create the file (not required)
>   *
> @@ -2341,8 +2341,12 @@ virFileUnlink(const char *path,
>       * the file/volume, then use unlink directly
>       */
>      if ((geteuid() != 0) ||
> -        ((uid == (uid_t) -1) && (gid == (gid_t) -1)))
> -        return unlink(path);
> +        ((uid == (uid_t) -1) && (gid == (gid_t) -1))) {
> +        if (virFileIsDir(path))
> +            return rmdir(path);
> +        else
> +            return unlink(path);
> +    }
>  
>      /* Otherwise, we have to deal with the NFS root-squash craziness
>       * to run under the uid/gid that created the volume in order to
> @@ -2406,9 +2410,16 @@ virFileUnlink(const char *path,
>          goto childerror;
>      }
>  
> -    if (unlink(path) < 0) {
> -        ret = errno;
> -        goto childerror;
> +    if (virFileIsDir(path)) {
> +        if (rmdir(path) < 0) {
> +            ret = errno;
> +            goto childerror;
> +        }
> +    } else {
> +        if (unlink(path) < 0) {
> +            ret = errno;
> +            goto childerror;
> +        }
>      }
>  
>   childerror:
> 

Since this function is now able to work with dirs too, maybe it should
be renamed to something like virFileDirRemove? But that can be saved for
a follow up patch. Not a show stopper to me.

Michal