[libvirt] [PATCH v2] storage: Set the perms if the pool target already exists for fs pools
Osier Yang
jyang at redhat.com
Thu Jun 21 03:09:09 UTC 2012
On 2012年06月21日 01:04, Eric Blake wrote:
> On 06/19/2012 02:15 AM, Osier Yang wrote:
>> The comment says:
>>
>> /* Now create the final dir in the path with the uid/gid/mode
>> * requested in the config. If the dir already exists, just set
>> * the perms.
>> */
>>
>> However, virDirCreate is only invoked if the target path doesn't
>> exist yet (which is opposite with the comment), or the uid from
>> the config is not -1 (I don't understand why, think it's just
>> another mistake). And the result is the perms of the pool won't
>> be changed if one tries to build the pool with different perms
>> again.
>>
>> Besides these logic error fix, if no uid and gid are specified in
>> the config, the practical used uid, gid are reflected.
>> ---
>> src/storage/storage_backend_fs.c | 44 +++++++++++++++++++------------------
>> 1 files changed, 23 insertions(+), 21 deletions(-)
>
> ACK. But I wonder if a followup patch should improve things...
>
>
>> + uid = (pool->def->target.perms.uid == (uid_t) -1)
>> + ? getuid() : pool->def->target.perms.uid;
>> + gid = (pool->def->target.perms.gid == (gid_t) -1)
>> + ? getgid() : pool->def->target.perms.gid;
>> +
>> + if ((err = virDirCreate(pool->def->target.path,
>> + pool->def->target.perms.mode,
>> + uid, gid,
>
> Instead of making callers use getuid(), we could move that logic into
> virDirCreate(), similarly to how we recently taught virFileOpenAs to
> honor incoming -1 arguments in commit 90e4d68.
>
Thanks, pushed, yeah, agreed, I will do it in a follow up patch.
Osier
More information about the libvir-list
mailing list