[libvirt] [PATCH 4/4] Create storage pool directories with proper uid/gid/mode

[Laine Stump]

On 01/20/2010 03:20 PM, Daniel Veillard wrote:
> On Wed, Jan 20, 2010 at 02:29:43AM -0500, Laine Stump wrote:
>    
>> Previously the uid/gid/mode in the xml was ignored when creating new
>> storage pool directories. This commit attempts to honor the requested
>> permissions, and spits out an error if it can't.
>>
>> Note that when creating the directory, the rest of the path leading up
>> to the final element is created using current uid/gid/mode, and the
>> final element gets the settings from xml. It is NOT an error for the
>> directory to already exist; in this case, the perms for the existing
>> directory are just set (if necessary).
>> ---
>>   src/storage/storage_backend_fs.c |   41 +++++++++++++++++++++++++++++++++++--
>>   1 files changed, 38 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
>> index cc26f2f..481e46e 100644
>> --- a/src/storage/storage_backend_fs.c
>> +++ b/src/storage/storage_backend_fs.c
>> @@ -506,9 +506,44 @@ virStorageBackendFileSystemBuild(virConnectPtr conn,
>>                                    unsigned int flags ATTRIBUTE_UNUSED)
>>   {
>>       int err;
>> -    if ((err = virFileMakePath(pool->def->target.path))<  0) {
>> -        virReportSystemError(conn, err,
>> -                             _("cannot create path '%s'"),
>> +    char path[PATH_MAX];
>>      
>    Urgh, I though we we trying to avoid allocating a full page like this
> for an argument on the stack...
>    

As someone who normally cringes when seeing large allocations on the 
stack, I'm embarrassed to admit I authored this! :-P My only excuse is 
that it was very late, and the virFileMakePath does that, which tricked 
my at-the-moment feeble mind into thinking it was okay.

>
>    and considering the handling done with path, I think a simple making
> patch a char * and initializing it with just a simple strdup() should be
> just fine, all we are doing is truncating the path. But it also need to
> be freed.
>    

Yup. Patch to follow momentarily, with that suggestion incorporated.

(And I should probably make a patch for virFileMakePath as well, but 
that's long-standing code, so not as urgent).