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

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

On 01/20/2010 03:20 PM, Daniel Veillard wrote:
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...

As someone who normally cringes when seeing large allocations on the stack, I'm embarrassed to admit I authored this! :-P My only excuse is that it was very late, and the virFileMakePath does that, which tricked my at-the-moment feeble mind into thinking it was okay.

   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.

Yup. Patch to follow momentarily, with that suggestion incorporated.

(And I should probably make a patch for virFileMakePath as well, but that's long-standing code, so not as urgent).

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