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

Re: [libvirt] [PATCH] storage: Inherit permissions of parent pool if they are not specified



On 09/20/2011 04:38 AM, Osier Yang wrote:
If permissions (mode, uid, gid) are not specified, a new created vol
will get the permissions like:

     mode = 0600
     uid = -1
     gid = -1

This will be a bit surprised if the user define the pool with a
non-root uid/gid, but the new created vol is still defined as
root/root.

This patch changes the behaviour so that the new created vol will
inherit the permissions of parent pool if permission are not
specified.

Should this behavior maybe be changed later on when the definition is used, rather than during parsing? I tend to not like modifying the incoming data as part of a parse (although I know we're already doing that in some other places).

(Of course other people may have a different opinion, or there may be a reason why my suggestion isn't feasible...)


---
  src/conf/storage_conf.c |   32 ++++++++++++++++++++------------
  1 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index e893b2d..18675ad 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -539,6 +539,7 @@ static int
  virStorageDefParsePerms(xmlXPathContextPtr ctxt,
                          virStoragePermsPtr perms,
                          const char *permxpath,
+                        virStoragePermsPtr pool_perms,
                          int defaultmode) {
      char *mode;
      long v;
@@ -560,9 +561,8 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt,
      ctxt->node = node;

      mode = virXPathString("string(./mode)", ctxt);
-    if (!mode) {
-        perms->mode = defaultmode;
-    } else {
+
+    if (mode) {
          char *end = NULL;
          perms->mode = strtol(mode,&end, 8);
          if (*end || perms->mode<  0 || perms->mode>  0777) {
@@ -572,28 +572,32 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt,
              goto error;
          }
          VIR_FREE(mode);
+    } else if (pool_perms) {
+        perms->mode = pool_perms->mode;
+    } else {
+        perms->mode = defaultmode;
      }

-    if (virXPathNode("./owner", ctxt) == NULL) {
-        perms->uid = -1;
-    } else {
+    if (virXPathNode("./owner", ctxt)) {
          if (virXPathLong("number(./owner)", ctxt,&v)<  0) {
              virStorageReportError(VIR_ERR_XML_ERROR,
                                    "%s", _("malformed owner element"));
              goto error;
          }
          perms->uid = (int)v;
+    } else if (pool_perms)  {
+        perms->uid = pool_perms->uid;
      }

-    if (virXPathNode("./group", ctxt) == NULL) {
-        perms->gid = -1;
-    } else {
+    if (virXPathNode("./group", ctxt)) {
          if (virXPathLong("number(./group)", ctxt,&v)<  0) {
              virStorageReportError(VIR_ERR_XML_ERROR,
                                    "%s", _("malformed group element"));
              goto error;
          }
          perms->gid = (int)v;
+    } else if (pool_perms) {
+        perms->gid = pool_perms->gid;
      }

      /* NB, we're ignoring missing labels here - they'll simply inherit */
@@ -722,7 +726,7 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) {


      if (virStorageDefParsePerms(ctxt,&ret->target.perms,
-                                "./target/permissions", 0700)<  0)
+                                "./target/permissions", NULL, 0700)<  0)
          goto cleanup;

      return ret;
@@ -1069,7 +1073,9 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
      }

      if (virStorageDefParsePerms(ctxt,&ret->target.perms,
-                                "./target/permissions", 0600)<  0)
+                                "./target/permissions",
+&pool->target.perms,
+                                0600)<  0)
          goto cleanup;

      node = virXPathNode("./target/encryption", ctxt);
@@ -1100,7 +1106,9 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
      }

      if (virStorageDefParsePerms(ctxt,&ret->backingStore.perms,
-                                "./backingStore/permissions", 0600)<  0)
+                                "./backingStore/permissions",
+&pool->target.perms,
+                                0600)<  0)
          goto cleanup;

      return ret;


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