Re: [PATCH v2 9/9] vfs: Add tmpfs tests for idmap mounts

[Rodrigo Campos <rodrigo@sdfg.com.ar>]

On 3/13/23 11:50, Christian Brauner wrote:
> On Wed, Mar 08, 2023 at 12:13:43PM +0100, Rodrigo Campos wrote:
>> +	/* create separate mount namespace */
>> +	if (unshare(CLONE_NEWNS)) {
>> +		log_stderr("failure: create new mount namespace");
>> +		goto out_no_rm;
>> +	}
> 
> I think you might want to turn off mount propagation here so that the
> tmpfs mount doesn't propagate into the parent mount namespace:
> 
> 	mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0)

Right, great catch!


>> +	// Create a new info to use for test we will call.
>> +	struct vfstest_info nested_test_info = *info;
> 
> nit: We usually don't mix declarations and code in C code so I would
> move that struct vfstest_info to the top of the function.

Heh, I prefer C99 declarations, but sorry I forgot we are not using that 
style in these files. I'll fix it :)

> 
>> +	nested_test_info.t_mountpoint = strdup(path);
>> +	if (nested_test_info.t_mountpoint == NULL) {
> 
> nit: more idiomatic
> if (!nested_test_info.t_mountpoint)

Thanks!

>> +	if(rm_r(info->t_mnt_fd, DIR0)) >
> nit: missing space between "if" and "("

Ouch, probably my scripts for the spacing patch mixed this up... Fixed!