storage: conf: Don't set any default <mode> in the XML

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.
This commit is contained in:
Cole Robinson 2015-04-27 16:48:05 -04:00
parent fafcc818f1
commit 7c2d65dde2
10 changed files with 49 additions and 34 deletions

View File

@ -406,6 +406,7 @@
namespace. It provides information about the permissions to use for the
final directory when the pool is built. There are 4 child elements.
The <code>mode</code> element contains the octal permission set.
The <code>mode</code> defaults to 0755 when not provided.
The <code>owner</code> element contains the numeric user ID.
The <code>group</code> element contains the numeric group ID.
If <code>owner</code> or <code>group</code> aren't specified when
@ -595,6 +596,7 @@
files. For pools where the volumes are device nodes, the hotplug
scripts determine permissions. There are 4 child elements.
The <code>mode</code> element contains the octal permission set.
The <code>mode</code> defaults to 0600 when not provided.
The <code>owner</code> element contains the numeric user ID.
The <code>group</code> element contains the numeric group ID.
If <code>owner</code> or <code>group</code> aren't specified when

View File

@ -98,9 +98,11 @@
<optional>
<element name='permissions'>
<interleave>
<element name='mode'>
<ref name='octalMode'/>
</element>
<optional>
<element name='mode'>
<ref name='octalMode'/>
</element>
</optional>
<optional>
<element name='owner'>
<choice>

View File

@ -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,10 +736,7 @@ 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)) {
@ -754,6 +747,8 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt,
}
perms->mode = tmp;
VIR_FREE(mode);
} else {
perms->mode = (mode_t) -1;
}
if (virXPathNode("./owner", ctxt) == NULL) {
@ -949,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;
}
@ -1187,8 +1181,9 @@ 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)
virBufferAsprintf(buf, "<mode>0%o</mode>\n",
def->target.perms.mode);
if (def->target.perms.uid != (uid_t) -1)
virBufferAsprintf(buf, "<owner>%d</owner>\n",
(int) def->target.perms.uid);
@ -1319,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;
}
@ -1365,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);
@ -1524,8 +1517,9 @@ 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)
virBufferAsprintf(buf, "<mode>0%o</mode>\n",
def->perms->mode);
if (def->perms->uid != (uid_t) -1)
virBufferAsprintf(buf, "<owner>%d</owner>\n",
(int) def->perms->uid);

View File

@ -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;

View File

@ -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);

View File

@ -801,7 +801,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_ALLOW_EXIST |
@ -1071,7 +1073,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,
(pool->def->type == VIR_STORAGE_POOL_NETFS

View File

@ -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);

View File

@ -12,7 +12,6 @@
<target>
<path>/mnt/gluster</path>
<permissions>
<mode>0755</mode>
</permissions>
</target>
</pool>

View File

@ -9,7 +9,6 @@
<path>gluster://example.com/vol/dir</path>
<format type='dir'/>
<permissions>
<mode>0600</mode>
</permissions>
</target>
</volume>

View File

@ -8,7 +8,6 @@
<path>sheepdog:test2</path>
<format type='unknown'/>
<permissions>
<mode>0600</mode>
</permissions>
</target>
</volume>