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@xxxxxxxxxx>
---
Notes:
v2:
- Added test case vol-gluster-dir-neg-uid that shows how 4294967295 is
correctly read as '-1' even on 64bit machines
docs/schemas/storagecommon.rng | 29 +++++++++++++++++++++
docs/schemas/storagepool.rng | 30 +---------------------
docs/schemas/storagevol.rng | 23 -----------------
src/conf/storage_conf.c | 9 +++----
.../vol-gluster-dir-neg-uid.xml} | 0
...gluster-dir.xml => vol-gluster-dir-neg-uid.xml} | 4 +--
tests/storagevolxml2xmlout/vol-gluster-dir.xml | 4 +--
tests/storagevolxml2xmlout/vol-sheepdog.xml | 4 +--
tests/storagevolxml2xmltest.c | 1 +
9 files changed, 41 insertions(+), 63 deletions(-)
copy tests/{storagevolxml2xmlout/vol-gluster-dir.xml => storagevolxml2xmlin/vol-gluster-dir-neg-uid.xml} (100%)
copy tests/storagevolxml2xmlout/{vol-gluster-dir.xml => vol-gluster-dir-neg-uid.xml} (83%)
diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng
index 06b2f81..629505f 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 0f05c5c..db6ff49 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 1b2d4cc..7450547 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 3987470..e1be064 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/storagevolxml2xmlout/vol-gluster-dir.xml b/tests/storagevolxml2xmlin/vol-gluster-dir-neg-uid.xml
similarity index 100%
copy from tests/storagevolxml2xmlout/vol-gluster-dir.xml
copy to tests/storagevolxml2xmlin/vol-gluster-dir-neg-uid.xml
diff --git a/tests/storagevolxml2xmlout/vol-gluster-dir.xml b/tests/storagevolxml2xmlout/vol-gluster-dir-neg-uid.xml
similarity index 83%
copy from tests/storagevolxml2xmlout/vol-gluster-dir.xml
copy to tests/storagevolxml2xmlout/vol-gluster-dir-neg-uid.xml
index f188ceb..538b31d 100644
--- a/tests/storagevolxml2xmlout/vol-gluster-dir.xml
+++ b/tests/storagevolxml2xmlout/vol-gluster-dir-neg-uid.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-gluster-dir.xml b/tests/storagevolxml2xmlout/vol-gluster-dir.xml
index f188ceb..538b31d 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 e08e36c..0a1f32c 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 234a2f1..cf4d401 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;
}
--
2.2.0
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list