uid_t and gid_t are opaque types, ranging from s32 to u32 to u64.
Unfortunately libvirt uses the value -1 to mean "current process", which
on Linux32 gets converted to UINT_MAX := +(2^32-1) = 4294967295.
Different libvirt versions used different formatting in the past, which
break one or the other parsing function:
virXPathLong(): (signed long)-1 != (double)UINT_MAX
virXPathULong(): (unsigned long)-1 != (double)-1
Roll our own version of virXPathULong(), which also accepts -1, which is
silently converted to UINT_MAX.
For output: do the reverse and print -1 instead of UINT_MAX.
Allow -1 for owner and group in schema for storage volumes.
Change UINT_MAX in one test case to -1 to follow the changes from above.
Signed-off-by: Philipp Hahn <hahn@xxxxxxxxxxxxx>
---
[v2]
- change schema for storage volume to accept -1 for owner and group
- Use UINT_MAX instead of ALLONE
- Windows64 uses s64, mention s16 and u16 as well
- Fix Sheepdog test case to use -1 instead of UINT_MAX
---
docs/schemas/storagevol.rng | 10 +++-
src/conf/storage_conf.c | 76 ++++++++++++++++++++++-----
tests/storagevolxml2xmlout/vol-sheepdog.xml | 4 +-
3 files changed, 72 insertions(+), 18 deletions(-)
diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng
index d4a29c7..ba95c55 100644
--- a/docs/schemas/storagevol.rng
+++ b/docs/schemas/storagevol.rng
@@ -49,10 +49,16 @@
<ref name='octalMode'/>
</element>
<element name='owner'>
- <ref name='unsignedInt'/>
+ <choice>
+ <ref name='unsignedInt'/>
+ <value>-1</value>
+ </choice>
</element>
<element name='group'>
- <ref name='unsignedInt'/>
+ <choice>
+ <ref name='unsignedInt'/>
+ <value>-1</value>
+ </choice>
</element>
<optional>
<element name='label'>
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 9134a22..617b19f 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -659,13 +659,14 @@ cleanup:
return ret;
}
+
static int
virStorageDefParsePerms(xmlXPathContextPtr ctxt,
virStoragePermsPtr perms,
const char *permxpath,
int defaultmode) {
char *mode;
- long v;
+ double d;
int ret = -1;
xmlNodePtr relnode;
xmlNodePtr node;
@@ -699,26 +700,58 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt,
VIR_FREE(mode);
}
+ /* uid_t and gid_t are *opaque* types: on Linux they're u32, on Windows64
+ * they're s64, on Solaris they were s32 in the past, and u16 and s16 have
+ * been used as well.
+ *
+ * Unfortunately libvirt uses the value -1 to mean "current process", which
+ * on Linux32 gets converted to UINT_MAX := +(2^32-1).
+ *
+ * Different libvirt versions used different formatting in the past, which
+ * break one or the other parsing function:
+ * virXPathLong(): (signed long)-1 != (double)UINT_MAX
+ * virXPathULong(): (unsigned long)-1 != (double)-1
+ */
if (virXPathNode("./owner", ctxt) == NULL) {
perms->uid = (uid_t) -1;
} else {
- if (virXPathLong("number(./owner)", ctxt, &v) < 0) {
+ ret = virXPathNumber("number(./owner)", ctxt, &d);
+ if (ret == 0) {
+ if (d == (double) -1) {
+ perms->uid = (uid_t) -1;
+ } else {
+ perms->uid = (uid_t) (unsigned long) d;
+ if (perms->uid != d) {
+ ret = -2;
+ }
+ }
+ }
+ if (ret < 0) {
virReportError(VIR_ERR_XML_ERROR,
"%s", _("malformed owner element"));
goto error;
}
- perms->uid = (int)v;
}
if (virXPathNode("./group", ctxt) == NULL) {
perms->gid = (gid_t) -1;
} else {
- if (virXPathLong("number(./group)", ctxt, &v) < 0) {
+ ret = virXPathNumber("number(./group)", ctxt, &d);
+ if (ret == 0) {
+ if (d == (double) -1) {
+ perms->gid = (gid_t)-1;
+ } else {
+ perms->gid = (gid_t) (unsigned long) d;
+ if (perms->gid != d) {
+ ret = -2;
+ }
+ }
+ }
+ if (ret < 0) {
virReportError(VIR_ERR_XML_ERROR,
"%s", _("malformed group element"));
goto error;
}
- perms->gid = (int)v;
}
/* NB, we're ignoring missing labels here - they'll simply inherit */
@@ -1060,10 +1093,18 @@ virStoragePoolDefFormat(virStoragePoolDefPtr def) {
virBufferAddLit(&buf," <permissions>\n");
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) {
+ virBufferAddLit(&buf, " <owner>-1</owner>\n");
+ } else {
+ virBufferAsprintf(&buf, " <owner>%u</owner>\n",
+ (unsigned int) def->target.perms.uid);
+ }
+ if (def->target.perms.gid == (gid_t) -1) {
+ virBufferAddLit(&buf, " <group>-1</group>\n");
+ } else {
+ virBufferAsprintf(&buf, " <group>%u</group>\n",
+ (unsigned int) def->target.perms.gid);
+ }
if (def->target.perms.label)
virBufferAsprintf(&buf," <label>%s</label>\n",
@@ -1313,11 +1354,18 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options,
virBufferAddLit(buf," <permissions>\n");
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);
-
+ if (def->perms.uid == (uid_t) -1) {
+ virBufferAddLit(buf, " <owner>-1</owner>\n");
+ } else {
+ virBufferAsprintf(buf, " <owner>%u</owner>\n",
+ (unsigned int) def->perms.uid);
+ }
+ if (def->perms.gid == (gid_t) -1) {
+ virBufferAddLit(buf, " <group>-1</group>\n");
+ } else {
+ virBufferAsprintf(buf, " <group>%u</group>\n",
+ (unsigned int) def->perms.gid);
+ }
if (def->perms.label)
virBufferAsprintf(buf," <label>%s</label>\n",
diff --git a/tests/storagevolxml2xmlout/vol-sheepdog.xml b/tests/storagevolxml2xmlout/vol-sheepdog.xml
index 2f19af8..fc04f19 100644
--- a/tests/storagevolxml2xmlout/vol-sheepdog.xml
+++ b/tests/storagevolxml2xmlout/vol-sheepdog.xml
@@ -10,8 +10,8 @@
<format type='unknown'/>
<permissions>
<mode>0600</mode>
- <owner>4294967295</owner>
- <group>4294967295</group>
+ <owner>-1</owner>
+ <group>-1</group>
</permissions>
</target>
</volume>
--
1.7.10.4
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list