From: Martin Kletzander <mkletzan@redhat.com>
Date: Wed, 10 Dec 2014 12:54:10 +0000 (+0100)
Subject: storage: unify permission formatting
X-Git-Url: http://xenbits.xensource.com/gitweb?a=commitdiff_plain;h=d2632d60aa86f5f214eba2a10d338beb3b7722ca;p=libvirt.git

storage: unify permission formatting

Volume and pool formatting functions took different approaches to
unspecified uids/gids.  When unknown, it is always parsed as -1, but one
of the functions formatted it as unsigned int (wrong) and one as
int (better).  Due to that, our two of our XML files from tests cannot
be parsed on 32-bit machines.

RNG schema needs to be modified as well, but because both
storagepool.rng and storagevol.rng need same schema for permission
element, save some space by moving it to storagecommon.rng.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---

diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng
index 06b2f81fde..629505f147 100644
--- a/docs/schemas/storagecommon.rng
+++ b/docs/schemas/storagecommon.rng
@@ -93,4 +93,33 @@
     <notAllowed/>
   </define>
 
+  <define name='permissions'>
+    <optional>
+      <element name='permissions'>
+        <interleave>
+          <element name='mode'>
+            <ref name='octalMode'/>
+          </element>
+          <element name='owner'>
+            <choice>
+              <ref name='unsignedInt'/>
+              <value>-1</value>
+            </choice>
+          </element>
+          <element name='group'>
+            <choice>
+              <ref name='unsignedInt'/>
+              <value>-1</value>
+            </choice>
+          </element>
+          <optional>
+            <element name='label'>
+              <text/>
+            </element>
+          </optional>
+        </interleave>
+      </element>
+    </optional>
+  </define>
+
 </grammar>
diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
index 0f05c5cc5f..db6ff49a04 100644
--- a/docs/schemas/storagepool.rng
+++ b/docs/schemas/storagepool.rng
@@ -3,6 +3,7 @@
 <grammar xmlns="http://relaxng.org/ns/structure/1.0"
     datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes">
   <include href='basictypes.rng'/>
+  <include href='storagecommon.rng'/>
   <start>
     <ref name='pool'/>
   </start>
@@ -224,35 +225,6 @@
     </interleave>
   </define>
 
-  <define name='permissions'>
-    <optional>
-      <element name='permissions'>
-        <interleave>
-          <element name='mode'>
-            <ref name='octalMode'/>
-          </element>
-          <element name='owner'>
-            <choice>
-              <ref name='unsignedInt'/>
-              <value>-1</value>
-            </choice>
-          </element>
-          <element name='group'>
-            <choice>
-              <ref name='unsignedInt'/>
-              <value>-1</value>
-            </choice>
-          </element>
-          <optional>
-            <element name='label'>
-              <text/>
-            </element>
-          </optional>
-        </interleave>
-      </element>
-    </optional>
-  </define>
-
   <define name='target'>
     <element name='target'>
       <interleave>
diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng
index 1b2d4cccad..7450547ffa 100644
--- a/docs/schemas/storagevol.rng
+++ b/docs/schemas/storagevol.rng
@@ -59,29 +59,6 @@
     </interleave>
   </define>
 
-  <define name='permissions'>
-    <optional>
-      <element name='permissions'>
-        <interleave>
-          <element name='mode'>
-            <ref name='octalMode'/>
-          </element>
-          <element name='owner'>
-            <ref name='unsignedInt'/>
-          </element>
-          <element name='group'>
-            <ref name='unsignedInt'/>
-          </element>
-          <optional>
-            <element name='label'>
-              <text/>
-            </element>
-          </optional>
-        </interleave>
-      </element>
-    </optional>
-  </define>
-
   <define name='timestamps'>
     <optional>
       <element name='timestamps'>
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 3987470450..e1be06432d 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1203,7 +1203,6 @@ virStoragePoolDefFormat(virStoragePoolDefPtr def)
                           (int) def->target.perms.uid);
         virBufferAsprintf(&buf, "<group>%d</group>\n",
                           (int) def->target.perms.gid);
-
         virBufferEscapeString(&buf, "<label>%s</label>\n",
                               def->target.perms.label);
 
@@ -1527,10 +1526,10 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options,
 
         virBufferAsprintf(buf, "<mode>0%o</mode>\n",
                           def->perms->mode);
-        virBufferAsprintf(buf, "<owner>%u</owner>\n",
-                          (unsigned int) def->perms->uid);
-        virBufferAsprintf(buf, "<group>%u</group>\n",
-                          (unsigned int) def->perms->gid);
+        virBufferAsprintf(buf, "<owner>%d</owner>\n",
+                          (int) def->perms->uid);
+        virBufferAsprintf(buf, "<group>%d</group>\n",
+                          (int) def->perms->gid);
 
 
         virBufferEscapeString(buf, "<label>%s</label>\n",
diff --git a/tests/storagevolxml2xmlin/vol-gluster-dir-neg-uid.xml b/tests/storagevolxml2xmlin/vol-gluster-dir-neg-uid.xml
new file mode 100644
index 0000000000..f188cebd85
--- /dev/null
+++ b/tests/storagevolxml2xmlin/vol-gluster-dir-neg-uid.xml
@@ -0,0 +1,17 @@
+<volume type='netdir'>
+  <name>dir</name>
+  <key>vol/dir</key>
+  <source>
+  </source>
+  <capacity unit='bytes'>0</capacity>
+  <allocation unit='bytes'>0</allocation>
+  <target>
+    <path>gluster://example.com/vol/dir</path>
+    <format type='dir'/>
+    <permissions>
+      <mode>0600</mode>
+      <owner>4294967295</owner>
+      <group>4294967295</group>
+    </permissions>
+  </target>
+</volume>
diff --git a/tests/storagevolxml2xmlout/vol-gluster-dir-neg-uid.xml b/tests/storagevolxml2xmlout/vol-gluster-dir-neg-uid.xml
new file mode 100644
index 0000000000..538b31d08f
--- /dev/null
+++ b/tests/storagevolxml2xmlout/vol-gluster-dir-neg-uid.xml
@@ -0,0 +1,17 @@
+<volume type='netdir'>
+  <name>dir</name>
+  <key>vol/dir</key>
+  <source>
+  </source>
+  <capacity unit='bytes'>0</capacity>
+  <allocation unit='bytes'>0</allocation>
+  <target>
+    <path>gluster://example.com/vol/dir</path>
+    <format type='dir'/>
+    <permissions>
+      <mode>0600</mode>
+      <owner>-1</owner>
+      <group>-1</group>
+    </permissions>
+  </target>
+</volume>
diff --git a/tests/storagevolxml2xmlout/vol-gluster-dir.xml b/tests/storagevolxml2xmlout/vol-gluster-dir.xml
index f188cebd85..538b31d08f 100644
--- a/tests/storagevolxml2xmlout/vol-gluster-dir.xml
+++ b/tests/storagevolxml2xmlout/vol-gluster-dir.xml
@@ -10,8 +10,8 @@
     <format type='dir'/>
     <permissions>
       <mode>0600</mode>
-      <owner>4294967295</owner>
-      <group>4294967295</group>
+      <owner>-1</owner>
+      <group>-1</group>
     </permissions>
   </target>
 </volume>
diff --git a/tests/storagevolxml2xmlout/vol-sheepdog.xml b/tests/storagevolxml2xmlout/vol-sheepdog.xml
index e08e36c1ca..0a1f32c787 100644
--- a/tests/storagevolxml2xmlout/vol-sheepdog.xml
+++ b/tests/storagevolxml2xmlout/vol-sheepdog.xml
@@ -9,8 +9,8 @@
     <format type='unknown'/>
     <permissions>
       <mode>0600</mode>
-      <owner>4294967295</owner>
-      <group>4294967295</group>
+      <owner>-1</owner>
+      <group>-1</group>
     </permissions>
   </target>
 </volume>
diff --git a/tests/storagevolxml2xmltest.c b/tests/storagevolxml2xmltest.c
index 234a2f1e8c..cf4d401bd3 100644
--- a/tests/storagevolxml2xmltest.c
+++ b/tests/storagevolxml2xmltest.c
@@ -122,6 +122,7 @@ mymain(void)
     DO_TEST("pool-logical", "vol-logical-backing");
     DO_TEST("pool-sheepdog", "vol-sheepdog");
     DO_TEST("pool-gluster", "vol-gluster-dir");
+    DO_TEST("pool-gluster", "vol-gluster-dir-neg-uid");
 
     return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }