[libvirt] [PATCH 4/4] Create storage pool directories with proper uid/gid/mode

[Daniel Veillard]

On Wed, Jan 20, 2010 at 02:29:43AM -0500, Laine Stump wrote:
> Previously the uid/gid/mode in the xml was ignored when creating new
> storage pool directories. This commit attempts to honor the requested
> permissions, and spits out an error if it can't.
> 
> Note that when creating the directory, the rest of the path leading up
> to the final element is created using current uid/gid/mode, and the
> final element gets the settings from xml. It is NOT an error for the
> directory to already exist; in this case, the perms for the existing
> directory are just set (if necessary).
> ---
>  src/storage/storage_backend_fs.c |   41 +++++++++++++++++++++++++++++++++++--
>  1 files changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
> index cc26f2f..481e46e 100644
> --- a/src/storage/storage_backend_fs.c
> +++ b/src/storage/storage_backend_fs.c
> @@ -506,9 +506,44 @@ virStorageBackendFileSystemBuild(virConnectPtr conn,
>                                   unsigned int flags ATTRIBUTE_UNUSED)
>  {
>      int err;
> -    if ((err = virFileMakePath(pool->def->target.path)) < 0) {
> -        virReportSystemError(conn, err,
> -                             _("cannot create path '%s'"),
> +    char path[PATH_MAX];

  Urgh, I though we we trying to avoid allocating a full page like this
for an argument on the stack...

> +    char *p;
> +
> +    if (virStrcpyStatic(path, pool->def->target.path) == NULL) {
> +        virStorageReportError(conn, VIR_ERR_INVALID_ARG,
> +                              _("cannot make copy of path '%s'"),
> +                              pool->def->target.path);
> +        return -1;
> +    }
> +    if (!(p = strrchr(path, '/'))) {
> +        virStorageReportError(conn, VIR_ERR_INVALID_ARG,
> +                              _("path '%s' is not absolute"),
> +                              pool->def->target.path);
> +        return -1;
> +    }
> +
> +    if (p != path) {
> +        /* assure all directories in the path prior to the final dir
> +         * exist, with default uid/gid/mode. */
> +        *p = '\0';
> +        if ((err = virFileMakePath(path)) != 0) {
> +            virReportSystemError(conn, err, _("cannot create path '%s'"),
> +                                 path);
> +            return -1;
> +        }
> +    }

  and considering the handling done with path, I think a simple making
patch a char * and initializing it with just a simple strdup() should be
just fine, all we are doing is truncating the path. But it also need to
be freed.

> +    /* Now create the final dir in the path with the uid/gid/mode
> +     * requested in the config. If the dir already exists, just set
> +     * the perms. */
> +    if ((err = virDirCreate(pool->def->target.path,
> +                            pool->def->target.perms.mode,
> +                            pool->def->target.perms.uid,
> +                            pool->def->target.perms.gid,
> +                            VIR_FILE_CREATE_ALLOW_EXIST |
> +                            (pool->def->type == VIR_STORAGE_POOL_NETFS
> +                             ? VIR_FILE_CREATE_AS_UID : 0)) != 0)) {
> +        virReportSystemError(conn, err, _("cannot create path '%s'"),
>                               pool->def->target.path);
>          return -1;
>      }

  It's probably better to get rid of char path[PATH_MAX] before
  commiting this,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/