[libvirt] [PATCH 5/6] storage: conf: Don't set any default <mode> in the XML
Cole Robinson
crobinso at redhat.com
Mon Apr 27 20:48:43 UTC 2015
The XML parser sets a default <mode> if none is explicitly passed in.
This is then used at pool/vol creation time, and unconditionally reported
in the XML.
The problem with this approach is that it's impossible for other code
to determine if the user explicitly requested a storage mode. There
are some cases where we want to make this distinction, but we currently
can't.
Handle <mode> parsing like we handle <owner>/<group>: if no value is
passed in, set it to -1, and adjust the internal consumers to handle
it.
---
docs/schemas/storagecommon.rng | 5 ++-
src/conf/storage_conf.c | 42 +++++++++++-----------
src/storage/storage_backend.c | 20 ++++++++---
src/storage/storage_backend.h | 3 ++
src/storage/storage_backend_fs.c | 9 +++--
src/storage/storage_backend_logical.c | 4 ++-
tests/storagepoolxml2xmlin/pool-dir.xml | 2 +-
tests/storagepoolxml2xmlout/pool-dir.xml | 2 +-
tests/storagepoolxml2xmlout/pool-netfs-gluster.xml | 2 +-
tests/storagevolxml2xmlin/vol-file.xml | 6 ++--
tests/storagevolxml2xmlout/vol-file.xml | 6 ++--
tests/storagevolxml2xmlout/vol-gluster-dir.xml | 2 +-
tests/storagevolxml2xmlout/vol-sheepdog.xml | 2 +-
13 files changed, 64 insertions(+), 41 deletions(-)
diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng
index 5f71b10..e4e8a51 100644
--- a/docs/schemas/storagecommon.rng
+++ b/docs/schemas/storagecommon.rng
@@ -99,7 +99,10 @@
<element name='permissions'>
<interleave>
<element name='mode'>
- <ref name='octalMode'/>
+ <choice>
+ <ref name='octalMode'/>
+ <value>-1</value>
+ </choice>
</element>
<element name='owner'>
<choice>
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 4852dfb..7131242 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -50,9 +50,6 @@
VIR_LOG_INIT("conf.storage_conf");
-#define DEFAULT_POOL_PERM_MODE 0755
-#define DEFAULT_VOL_PERM_MODE 0600
-
VIR_ENUM_IMPL(virStorageVol,
VIR_STORAGE_VOL_LAST,
"file", "block", "dir", "network", "netdir")
@@ -718,8 +715,7 @@ virStoragePoolDefParseSourceString(const char *srcSpec,
static int
virStorageDefParsePerms(xmlXPathContextPtr ctxt,
virStoragePermsPtr perms,
- const char *permxpath,
- int defaultmode)
+ const char *permxpath)
{
char *mode;
long long val;
@@ -730,7 +726,7 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt,
node = virXPathNode(permxpath, ctxt);
if (node == NULL) {
/* Set default values if there is not <permissions> element */
- perms->mode = defaultmode;
+ perms->mode = (mode_t) -1;
perms->uid = (uid_t) -1;
perms->gid = (gid_t) -1;
perms->label = NULL;
@@ -740,13 +736,12 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt,
relnode = ctxt->node;
ctxt->node = node;
- mode = virXPathString("string(./mode)", ctxt);
- if (!mode) {
- perms->mode = defaultmode;
- } else {
+ if ((mode = virXPathString("string(./mode)", ctxt))) {
int tmp;
- if (virStrToLong_i(mode, NULL, 8, &tmp) < 0 || (tmp & ~0777)) {
+ if (virStrToLong_i(mode, NULL, 8, &tmp) < 0 ||
+ ((tmp & ~0777) &&
+ tmp != -1)) {
VIR_FREE(mode);
virReportError(VIR_ERR_XML_ERROR, "%s",
_("malformed octal mode"));
@@ -754,6 +749,8 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt,
}
perms->mode = tmp;
VIR_FREE(mode);
+ } else {
+ perms->mode = (mode_t) -1;
}
if (virXPathNode("./owner", ctxt) == NULL) {
@@ -947,8 +944,7 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
goto error;
if (virStorageDefParsePerms(ctxt, &ret->target.perms,
- "./target/permissions",
- DEFAULT_POOL_PERM_MODE) < 0)
+ "./target/permissions") < 0)
goto error;
}
@@ -1185,8 +1181,11 @@ virStoragePoolDefFormatBuf(virBufferPtr buf,
virBufferAddLit(buf, "<permissions>\n");
virBufferAdjustIndent(buf, 2);
- virBufferAsprintf(buf, "<mode>0%o</mode>\n",
- def->target.perms.mode);
+ if (def->target.perms.mode == (mode_t) -1)
+ virBufferAddLit(buf, "<mode>-1</mode>\n");
+ else
+ 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",
@@ -1315,8 +1314,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
if (VIR_ALLOC(ret->target.backingStore->perms) < 0)
goto error;
if (virStorageDefParsePerms(ctxt, ret->target.backingStore->perms,
- "./backingStore/permissions",
- DEFAULT_VOL_PERM_MODE) < 0)
+ "./backingStore/permissions") < 0)
goto error;
}
@@ -1361,8 +1359,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
if (VIR_ALLOC(ret->target.perms) < 0)
goto error;
if (virStorageDefParsePerms(ctxt, ret->target.perms,
- "./target/permissions",
- DEFAULT_VOL_PERM_MODE) < 0)
+ "./target/permissions") < 0)
goto error;
node = virXPathNode("./target/encryption", ctxt);
@@ -1520,8 +1517,11 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options,
virBufferAddLit(buf, "<permissions>\n");
virBufferAdjustIndent(buf, 2);
- virBufferAsprintf(buf, "<mode>0%o</mode>\n",
- def->perms->mode);
+ if (def->perms->mode == (mode_t) -1)
+ virBufferAddLit(buf, "<mode>-1</mode>\n");
+ else
+ 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",
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index e0311e1..1bf79db 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -318,6 +318,7 @@ virStorageBackendCreateBlockFrom(virConnectPtr conn ATTRIBUTE_UNUSED,
struct stat st;
gid_t gid;
uid_t uid;
+ mode_t mode;
bool reflink_copy = false;
virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA |
@@ -367,10 +368,13 @@ virStorageBackendCreateBlockFrom(virConnectPtr conn ATTRIBUTE_UNUSED,
(unsigned int) gid);
goto cleanup;
}
- if (fchmod(fd, vol->target.perms->mode) < 0) {
+
+ mode = (vol->target.perms->mode == (mode_t) -1 ?
+ VIR_STORAGE_DEFAULT_VOL_PERM_MODE : vol->target.perms->mode);
+ if (fchmod(fd, mode) < 0) {
virReportSystemError(errno,
_("cannot set mode of '%s' to %04o"),
- vol->target.path, vol->target.perms->mode);
+ vol->target.path, mode);
goto cleanup;
}
if (VIR_CLOSE(fd) < 0) {
@@ -509,7 +513,9 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED,
if ((fd = virFileOpenAs(vol->target.path,
O_RDWR | O_CREAT | O_EXCL,
- vol->target.perms->mode,
+ (vol->target.perms->mode ?
+ VIR_STORAGE_DEFAULT_VOL_PERM_MODE :
+ vol->target.perms->mode),
vol->target.perms->uid,
vol->target.perms->gid,
operation_flags)) < 0) {
@@ -664,6 +670,7 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool,
struct stat st;
gid_t gid;
uid_t uid;
+ mode_t mode;
bool filecreated = false;
if ((pool->def->type == VIR_STORAGE_POOL_NETFS)
@@ -709,10 +716,13 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool,
(unsigned int) gid);
return -1;
}
- if (chmod(vol->target.path, vol->target.perms->mode) < 0) {
+
+ mode = (vol->target.perms->mode == (mode_t) -1 ?
+ VIR_STORAGE_DEFAULT_VOL_PERM_MODE : vol->target.perms->mode);
+ if (chmod(vol->target.path, mode) < 0) {
virReportSystemError(errno,
_("cannot set mode of '%s' to %04o"),
- vol->target.path, vol->target.perms->mode);
+ vol->target.path, mode);
return -1;
}
return 0;
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 85a8a4b..39c00b1 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -177,6 +177,9 @@ int virStorageBackendVolOpen(const char *path, struct stat *sb,
ATTRIBUTE_RETURN_CHECK
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+# define VIR_STORAGE_DEFAULT_POOL_PERM_MODE 0755
+# define VIR_STORAGE_DEFAULT_VOL_PERM_MODE 0600
+
int virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol,
bool withBlockVolFormat,
unsigned int openflags);
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 804b7c3..344ee4c 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -806,7 +806,9 @@ virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED,
* requested in the config. If the dir already exists, just set
* the perms. */
if ((err = virDirCreate(pool->def->target.path,
- pool->def->target.perms.mode,
+ (pool->def->target.perms.mode == (mode_t) -1 ?
+ VIR_STORAGE_DEFAULT_POOL_PERM_MODE :
+ pool->def->target.perms.mode),
pool->def->target.perms.uid,
pool->def->target.perms.gid,
VIR_DIR_CREATE_FORCE_PERMS |
@@ -1077,7 +1079,10 @@ static int createFileDir(virConnectPtr conn ATTRIBUTE_UNUSED,
}
- if ((err = virDirCreate(vol->target.path, vol->target.perms->mode,
+ if ((err = virDirCreate(vol->target.path,
+ (vol->target.perms->mode == (mode_t) -1 ?
+ VIR_STORAGE_DEFAULT_VOL_PERM_MODE :
+ vol->target.perms->mode),
vol->target.perms->uid,
vol->target.perms->gid,
VIR_DIR_CREATE_FORCE_PERMS |
diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
index 11c5683..9c77b4c 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -787,7 +787,9 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
goto error;
}
}
- if (fchmod(fd, vol->target.perms->mode) < 0) {
+ if (fchmod(fd, (vol->target.perms->mode == (mode_t) -1 ?
+ VIR_STORAGE_DEFAULT_VOL_PERM_MODE :
+ vol->target.perms->mode)) < 0) {
virReportSystemError(errno,
_("cannot set file mode '%s'"),
vol->target.path);
diff --git a/tests/storagepoolxml2xmlin/pool-dir.xml b/tests/storagepoolxml2xmlin/pool-dir.xml
index e10ccb7..6b30053 100644
--- a/tests/storagepoolxml2xmlin/pool-dir.xml
+++ b/tests/storagepoolxml2xmlin/pool-dir.xml
@@ -9,7 +9,7 @@
<target>
<path>///var/////lib/libvirt/images//</path>
<permissions>
- <mode>0700</mode>
+ <mode>-1</mode>
<owner>-1</owner>
<group>-1</group>
<label>some_label_t</label>
diff --git a/tests/storagepoolxml2xmlout/pool-dir.xml b/tests/storagepoolxml2xmlout/pool-dir.xml
index f81bc1d..9b926fe 100644
--- a/tests/storagepoolxml2xmlout/pool-dir.xml
+++ b/tests/storagepoolxml2xmlout/pool-dir.xml
@@ -9,7 +9,7 @@
<target>
<path>/var/lib/libvirt/images</path>
<permissions>
- <mode>0700</mode>
+ <mode>-1</mode>
<owner>-1</owner>
<group>-1</group>
<label>some_label_t</label>
diff --git a/tests/storagepoolxml2xmlout/pool-netfs-gluster.xml b/tests/storagepoolxml2xmlout/pool-netfs-gluster.xml
index bab2a15..d91fe03 100644
--- a/tests/storagepoolxml2xmlout/pool-netfs-gluster.xml
+++ b/tests/storagepoolxml2xmlout/pool-netfs-gluster.xml
@@ -12,7 +12,7 @@
<target>
<path>/mnt/gluster</path>
<permissions>
- <mode>0755</mode>
+ <mode>-1</mode>
<owner>-1</owner>
<group>-1</group>
</permissions>
diff --git a/tests/storagevolxml2xmlin/vol-file.xml b/tests/storagevolxml2xmlin/vol-file.xml
index d3f65f6..ffb472e 100644
--- a/tests/storagevolxml2xmlin/vol-file.xml
+++ b/tests/storagevolxml2xmlin/vol-file.xml
@@ -6,9 +6,9 @@
<target>
<path>/var/lib/libvirt/images/sparse.img</path>
<permissions>
- <mode>0</mode>
- <owner>0744</owner>
- <group>0</group>
+ <mode>-1</mode>
+ <owner>-1</owner>
+ <group>-1</group>
<label>virt_image_t</label>
</permissions>
<timestamps>
diff --git a/tests/storagevolxml2xmlout/vol-file.xml b/tests/storagevolxml2xmlout/vol-file.xml
index 2923188..aa9ac6b 100644
--- a/tests/storagevolxml2xmlout/vol-file.xml
+++ b/tests/storagevolxml2xmlout/vol-file.xml
@@ -8,9 +8,9 @@
<path>/var/lib/libvirt/images/sparse.img</path>
<format type='raw'/>
<permissions>
- <mode>00</mode>
- <owner>744</owner>
- <group>0</group>
+ <mode>-1</mode>
+ <owner>-1</owner>
+ <group>-1</group>
<label>virt_image_t</label>
</permissions>
</target>
diff --git a/tests/storagevolxml2xmlout/vol-gluster-dir.xml b/tests/storagevolxml2xmlout/vol-gluster-dir.xml
index 538b31d..067be0f 100644
--- a/tests/storagevolxml2xmlout/vol-gluster-dir.xml
+++ b/tests/storagevolxml2xmlout/vol-gluster-dir.xml
@@ -9,7 +9,7 @@
<path>gluster://example.com/vol/dir</path>
<format type='dir'/>
<permissions>
- <mode>0600</mode>
+ <mode>-1</mode>
<owner>-1</owner>
<group>-1</group>
</permissions>
diff --git a/tests/storagevolxml2xmlout/vol-sheepdog.xml b/tests/storagevolxml2xmlout/vol-sheepdog.xml
index 0a1f32c..6b8355f 100644
--- a/tests/storagevolxml2xmlout/vol-sheepdog.xml
+++ b/tests/storagevolxml2xmlout/vol-sheepdog.xml
@@ -8,7 +8,7 @@
<path>sheepdog:test2</path>
<format type='unknown'/>
<permissions>
- <mode>0600</mode>
+ <mode>-1</mode>
<owner>-1</owner>
<group>-1</group>
</permissions>
--
2.3.6
More information about the libvir-list
mailing list