[libvirt] [PATCH] storage: Inherit permissions of parent pool if they are not specified
Laine Stump
laine at laine.org
Tue Sep 20 17:05:13 UTC 2011
On 09/20/2011 04:38 AM, Osier Yang wrote:
> If permissions (mode, uid, gid) are not specified, a new created vol
> will get the permissions like:
>
> mode = 0600
> uid = -1
> gid = -1
>
> This will be a bit surprised if the user define the pool with a
> non-root uid/gid, but the new created vol is still defined as
> root/root.
>
> This patch changes the behaviour so that the new created vol will
> inherit the permissions of parent pool if permission are not
> specified.
Should this behavior maybe be changed later on when the definition is
used, rather than during parsing? I tend to not like modifying the
incoming data as part of a parse (although I know we're already doing
that in some other places).
(Of course other people may have a different opinion, or there may be a
reason why my suggestion isn't feasible...)
> ---
> src/conf/storage_conf.c | 32 ++++++++++++++++++++------------
> 1 files changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index e893b2d..18675ad 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -539,6 +539,7 @@ static int
> virStorageDefParsePerms(xmlXPathContextPtr ctxt,
> virStoragePermsPtr perms,
> const char *permxpath,
> + virStoragePermsPtr pool_perms,
> int defaultmode) {
> char *mode;
> long v;
> @@ -560,9 +561,8 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt,
> ctxt->node = node;
>
> mode = virXPathString("string(./mode)", ctxt);
> - if (!mode) {
> - perms->mode = defaultmode;
> - } else {
> +
> + if (mode) {
> char *end = NULL;
> perms->mode = strtol(mode,&end, 8);
> if (*end || perms->mode< 0 || perms->mode> 0777) {
> @@ -572,28 +572,32 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt,
> goto error;
> }
> VIR_FREE(mode);
> + } else if (pool_perms) {
> + perms->mode = pool_perms->mode;
> + } else {
> + perms->mode = defaultmode;
> }
>
> - if (virXPathNode("./owner", ctxt) == NULL) {
> - perms->uid = -1;
> - } else {
> + if (virXPathNode("./owner", ctxt)) {
> if (virXPathLong("number(./owner)", ctxt,&v)< 0) {
> virStorageReportError(VIR_ERR_XML_ERROR,
> "%s", _("malformed owner element"));
> goto error;
> }
> perms->uid = (int)v;
> + } else if (pool_perms) {
> + perms->uid = pool_perms->uid;
> }
>
> - if (virXPathNode("./group", ctxt) == NULL) {
> - perms->gid = -1;
> - } else {
> + if (virXPathNode("./group", ctxt)) {
> if (virXPathLong("number(./group)", ctxt,&v)< 0) {
> virStorageReportError(VIR_ERR_XML_ERROR,
> "%s", _("malformed group element"));
> goto error;
> }
> perms->gid = (int)v;
> + } else if (pool_perms) {
> + perms->gid = pool_perms->gid;
> }
>
> /* NB, we're ignoring missing labels here - they'll simply inherit */
> @@ -722,7 +726,7 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) {
>
>
> if (virStorageDefParsePerms(ctxt,&ret->target.perms,
> - "./target/permissions", 0700)< 0)
> + "./target/permissions", NULL, 0700)< 0)
> goto cleanup;
>
> return ret;
> @@ -1069,7 +1073,9 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
> }
>
> if (virStorageDefParsePerms(ctxt,&ret->target.perms,
> - "./target/permissions", 0600)< 0)
> + "./target/permissions",
> +&pool->target.perms,
> + 0600)< 0)
> goto cleanup;
>
> node = virXPathNode("./target/encryption", ctxt);
> @@ -1100,7 +1106,9 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
> }
>
> if (virStorageDefParsePerms(ctxt,&ret->backingStore.perms,
> - "./backingStore/permissions", 0600)< 0)
> + "./backingStore/permissions",
> +&pool->target.perms,
> + 0600)< 0)
> goto cleanup;
>
> return ret;
More information about the libvir-list
mailing list