[PATCH 4/4] qemu_shim: Always pre-create root dir

Michal Privoznik mprivozn at redhat.com
Fri Mar 12 14:38:27 UTC 2021


On 3/12/21 11:51 AM, Andrea Bolognani wrote:
> On Mon, 2021-03-01 at 12:49 +0100, Michal Privoznik wrote:
>> This problem is reproducible only with secret driver. When
>> starting a domain via virt-qemu-run and both secret and
>> (nonexistent) root directory specified this is what happens:
>>
>> 1) virt-qemu-run opens "secret:///embed?root=$rootdir"
>>     connection, which results in the secret driver initialization
>>     (done in secretStateInitialize()). During this process, the
>>     driver creates it's own configDir (derived from $rootdir)
> 
> s/it's own/its own/
> 
>>     including those parents which don't exists yet. This is all
>>     done with the mode S_IRWXU and thus results in the $rootdir
>>     being created with very restrictive mode (specifically, +x is
>>     missing for group and others).
>>
>> 2) now, virt-qemu-run-opens "qemu:///embed?root=$rootdir" and
> 
> s/run-opens/run opens/
> 
>> +++ b/src/qemu/qemu_shim.c
>> @@ -213,11 +213,16 @@ int main(int argc, char **argv)
>>           }
>>           tmproot = true;
>>   
>> -        if (chmod(root, 0755) < 0) {
>> -            g_printerr("%s: cannot chown temporary dir: %s\n",
>> -                       argv[0], g_strerror(errno));
>> -            goto cleanup;
>> -        }
>> +    } else if (g_mkdir_with_parents(root, 0755) < 0) {
>> +        g_printerr("%s: cannot create dir: %s\n",
>> +                   argv[0], g_strerror(errno));
>> +        goto cleanup;
>> +    }
>> +
>> +    if (chmod(root, 0755) < 0) {
>> +        g_printerr("%s: cannot chmod temporary dir: %s\n",
>> +                   argv[0], g_strerror(errno));
>> +        goto cleanup;
>>       }
> 
> Wouldn't it make sense to leave the chmod() bit where it was?
> g_mkdir_with_parents() already accepts the mode as a parameter, so
> calling chmod() again seems unnecessary.

Well, if the dir exists but doesn't have right perms then 
g_mkdir_with_parents() does nothing and we need that explicit chmod().

Michal




More information about the libvir-list mailing list