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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list