From c4d27bdddf23e4026c2b96ff8dfc8a1dec3b8d29 Mon Sep 17 00:00:00 2001
From: Cole Robinson <crobinso@redhat.com>
Date: Tue, 5 May 2015 11:58:12 -0400
Subject: [PATCH] storage: conf: Don't output owner/group -1

-1 is just an internal placeholder and is meaningless to output in the XML.
---
 docs/schemas/storagecommon.rng                | 28 +++++++++++--------
 src/conf/storage_conf.c                       | 23 +++++++++------
 .../storagepoolxml2xmlout/pool-dir-naming.xml |  2 --
 tests/storagepoolxml2xmlout/pool-dir.xml      |  2 --
 .../pool-netfs-gluster.xml                    |  2 --
 .../vol-gluster-dir-neg-uid.xml               |  2 --
 .../storagevolxml2xmlout/vol-gluster-dir.xml  |  2 --
 tests/storagevolxml2xmlout/vol-sheepdog.xml   |  2 --
 8 files changed, 30 insertions(+), 33 deletions(-)

diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng
index 5f71b10b30..6f7d369596 100644
--- a/docs/schemas/storagecommon.rng
+++ b/docs/schemas/storagecommon.rng
@@ -101,18 +101,22 @@
           <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='owner'>
+              <choice>
+                <ref name='unsignedInt'/>
+                <value>-1</value>
+              </choice>
+            </element>
+          </optional>
+          <optional>
+            <element name='group'>
+              <choice>
+                <ref name='unsignedInt'/>
+                <value>-1</value>
+              </choice>
+            </element>
+          </optional>
           <optional>
             <element name='label'>
               <text/>
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 65b2704108..ee6e0cf002 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -759,6 +759,7 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt,
     if (virXPathNode("./owner", ctxt) == NULL) {
         perms->uid = (uid_t) -1;
     } else {
+        /* We previously could output -1, so continue to parse it */
         if (virXPathLongLong("number(./owner)", ctxt, &val) < 0 ||
             ((uid_t)val != val &&
              val != -1)) {
@@ -773,6 +774,7 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt,
     if (virXPathNode("./group", ctxt) == NULL) {
         perms->gid = (gid_t) -1;
     } else {
+        /* We previously could output -1, so continue to parse it */
         if (virXPathLongLong("number(./group)", ctxt, &val) < 0 ||
             ((gid_t) val != val &&
              val != -1)) {
@@ -1187,10 +1189,12 @@ virStoragePoolDefFormatBuf(virBufferPtr buf,
         virBufferAdjustIndent(buf, 2);
         virBufferAsprintf(buf, "<mode>0%o</mode>\n",
                           def->target.perms.mode);
-        virBufferAsprintf(buf, "<owner>%d</owner>\n",
-                          (int) def->target.perms.uid);
-        virBufferAsprintf(buf, "<group>%d</group>\n",
-                          (int) def->target.perms.gid);
+        if (def->target.perms.uid != (uid_t) -1)
+            virBufferAsprintf(buf, "<owner>%d</owner>\n",
+                              (int) def->target.perms.uid);
+        if (def->target.perms.gid != (gid_t) -1)
+            virBufferAsprintf(buf, "<group>%d</group>\n",
+                              (int) def->target.perms.gid);
         virBufferEscapeString(buf, "<label>%s</label>\n",
                               def->target.perms.label);
 
@@ -1522,11 +1526,12 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options,
 
         virBufferAsprintf(buf, "<mode>0%o</mode>\n",
                           def->perms->mode);
-        virBufferAsprintf(buf, "<owner>%d</owner>\n",
-                          (int) def->perms->uid);
-        virBufferAsprintf(buf, "<group>%d</group>\n",
-                          (int) def->perms->gid);
-
+        if (def->perms->uid != (uid_t) -1)
+            virBufferAsprintf(buf, "<owner>%d</owner>\n",
+                              (int) def->perms->uid);
+        if (def->perms->gid != (gid_t) -1)
+            virBufferAsprintf(buf, "<group>%d</group>\n",
+                              (int) def->perms->gid);
 
         virBufferEscapeString(buf, "<label>%s</label>\n",
                               def->perms->label);
diff --git a/tests/storagepoolxml2xmlout/pool-dir-naming.xml b/tests/storagepoolxml2xmlout/pool-dir-naming.xml
index 536f58c7b9..dd6d9b8434 100644
--- a/tests/storagepoolxml2xmlout/pool-dir-naming.xml
+++ b/tests/storagepoolxml2xmlout/pool-dir-naming.xml
@@ -10,8 +10,6 @@
     <path>/var/lib/libvirt/&lt;images&gt;</path>
     <permissions>
       <mode>0700</mode>
-      <owner>-1</owner>
-      <group>-1</group>
       <label>some_label_t</label>
     </permissions>
   </target>
diff --git a/tests/storagepoolxml2xmlout/pool-dir.xml b/tests/storagepoolxml2xmlout/pool-dir.xml
index f81bc1d61b..2054871d3c 100644
--- a/tests/storagepoolxml2xmlout/pool-dir.xml
+++ b/tests/storagepoolxml2xmlout/pool-dir.xml
@@ -10,8 +10,6 @@
     <path>/var/lib/libvirt/images</path>
     <permissions>
       <mode>0700</mode>
-      <owner>-1</owner>
-      <group>-1</group>
       <label>some_label_t</label>
     </permissions>
   </target>
diff --git a/tests/storagepoolxml2xmlout/pool-netfs-gluster.xml b/tests/storagepoolxml2xmlout/pool-netfs-gluster.xml
index bab2a15fb2..90143f919d 100644
--- a/tests/storagepoolxml2xmlout/pool-netfs-gluster.xml
+++ b/tests/storagepoolxml2xmlout/pool-netfs-gluster.xml
@@ -13,8 +13,6 @@
     <path>/mnt/gluster</path>
     <permissions>
       <mode>0755</mode>
-      <owner>-1</owner>
-      <group>-1</group>
     </permissions>
   </target>
 </pool>
diff --git a/tests/storagevolxml2xmlout/vol-gluster-dir-neg-uid.xml b/tests/storagevolxml2xmlout/vol-gluster-dir-neg-uid.xml
index 538b31d08f..0af0be179d 100644
--- a/tests/storagevolxml2xmlout/vol-gluster-dir-neg-uid.xml
+++ b/tests/storagevolxml2xmlout/vol-gluster-dir-neg-uid.xml
@@ -10,8 +10,6 @@
     <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 538b31d08f..0af0be179d 100644
--- a/tests/storagevolxml2xmlout/vol-gluster-dir.xml
+++ b/tests/storagevolxml2xmlout/vol-gluster-dir.xml
@@ -10,8 +10,6 @@
     <format type='dir'/>
     <permissions>
       <mode>0600</mode>
-      <owner>-1</owner>
-      <group>-1</group>
     </permissions>
   </target>
 </volume>
diff --git a/tests/storagevolxml2xmlout/vol-sheepdog.xml b/tests/storagevolxml2xmlout/vol-sheepdog.xml
index 0a1f32c787..d8f34d3806 100644
--- a/tests/storagevolxml2xmlout/vol-sheepdog.xml
+++ b/tests/storagevolxml2xmlout/vol-sheepdog.xml
@@ -9,8 +9,6 @@
     <format type='unknown'/>
     <permissions>
       <mode>0600</mode>
-      <owner>-1</owner>
-      <group>-1</group>
     </permissions>
   </target>
 </volume>
-- 
GitLab