Re: [PATCH v2] storage: Set the perms if the pool target already exists for fs pools

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

 



On 2012年06月21日 01:04, Eric Blake wrote:
On 06/19/2012 02:15 AM, Osier Yang wrote:
The comment says:

/* 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.
  */

However, virDirCreate is only invoked if the target path doesn't
exist yet (which is opposite with the comment), or the uid from
the config is not -1 (I don't understand why, think it's just
another mistake). And the result is the perms of the pool won't
be changed if one tries to build the pool with different perms
again.

Besides these logic error fix, if no uid and gid are specified in
the config, the practical used uid, gid are reflected.
---
  src/storage/storage_backend_fs.c |   44 +++++++++++++++++++------------------
  1 files changed, 23 insertions(+), 21 deletions(-)

ACK.  But I wonder if a followup patch should improve things...


+    uid = (pool->def->target.perms.uid == (uid_t) -1)
+        ? getuid() : pool->def->target.perms.uid;
+    gid = (pool->def->target.perms.gid == (gid_t) -1)
+        ? getgid() : pool->def->target.perms.gid;
+
+    if ((err = virDirCreate(pool->def->target.path,
+                            pool->def->target.perms.mode,
+                            uid, gid,

Instead of making callers use getuid(), we could move that logic into
virDirCreate(), similarly to how we recently taught virFileOpenAs to
honor incoming -1 arguments in commit 90e4d68.


Thanks, pushed, yeah, agreed, I will do it in a follow up patch.

Osier

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]