Re: [LTP] [PATCH v1 2/3] tst_cgroup: Add safe_cg_open and safe_cg_fchown functions

["xuyang2018.jy@fujitsu.com" <xuyang2018.jy@fujitsu.com>]

Hi Richard

> Hello,
> 
> "xuyang2018.jy@fujitsu.com" <xuyang2018.jy@fujitsu.com> writes:
> 
>> Hi Richard
>>> Hello,
>>>
>>> Yang Xu <xuyang2018.jy@fujitsu.com> writes:
>>>
>>>> safe_cg_open is used to open the sub control's file ie cgroup.procs
>>>> and returns the fd.
>>>>
>>>> safe_cg_fchown is used to use fchownat to change file's uid and gid.
>>>>
>>>> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
>>>> ---
>>>>    include/tst_cgroup.h | 15 +++++++++++++++
>>>>    lib/tst_cgroup.c     | 39 +++++++++++++++++++++++++++++++++++++++
>>>>    2 files changed, 54 insertions(+)
>>>>
>>>> diff --git a/include/tst_cgroup.h b/include/tst_cgroup.h
>>>> index d06847cc6..292c9baa4 100644
>>>> --- a/include/tst_cgroup.h
>>>> +++ b/include/tst_cgroup.h
>>>> @@ -188,6 +188,21 @@ ssize_t safe_cg_read(const char *const file, const int lineno,
>>>>    			 char *const out, const size_t len)
>>>>    			 __attribute__ ((nonnull));
>>>>    
>>>> +#define SAFE_CG_OPEN(cg, file_name, flags)			\
>>>> +	safe_cg_open(__FILE__, __LINE__, (cg), (file_name), (flags))
>>>> +
>>>> +int safe_cg_open(const char *const file, const int lineno,
>>>> +		 const struct tst_cg_group *const cg,
>>>> +		 const char *const file_name, int flags);
>>>> +
>>>> +#define SAFE_CG_FCHOWN(cg, file_name, owner, group)		\
>>>> +	safe_cg_fchown(__FILE__, __LINE__,			\
>>>> +			   (cg), (file_name), (owner), (group))
>>>> +
>>>> +void safe_cg_fchown(const char *const file, const int lineno,
>>>> +		    const struct tst_cg_group *const cg,
>>>> +		    const char *const file_name, uid_t owner, gid_t group);
>>>> +
>>>>    #define SAFE_CG_PRINTF(cg, file_name, fmt, ...)			\
>>>>    	safe_cg_printf(__FILE__, __LINE__,				\
>>>>    			   (cg), (file_name), (fmt), __VA_ARGS__)
>>>> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
>>>> index 1cfd79243..dedc0f65b 100644
>>>> --- a/lib/tst_cgroup.c
>>>> +++ b/lib/tst_cgroup.c
>>>> @@ -1297,6 +1297,45 @@ ssize_t safe_cg_read(const char *const file, const int lineno,
>>>>    	return read_ret;
>>>>    }
>>>>    
>>>> +int safe_cg_open(const char *const file, const int lineno,
>>>> +			const struct tst_cg_group *cg,
>>>> +			const char *const file_name, int flags)
>>>> +{
>>>> +	const struct cgroup_file *const cfile =
>>>> +		cgroup_file_find(file, lineno, file_name);
>>>> +	struct cgroup_dir *const *dir;
>>>> +	const char *alias;
>>>> +	int fd;
>>>> +
>>>> +	for_each_dir(cg, cfile->ctrl_indx, dir) {
>>>> +		alias = cgroup_file_alias(cfile, *dir);
>>>> +		if (!alias)
>>>> +			continue;
>>>> +
>>>> +		fd = safe_openat(file, lineno, (*dir)->dir_fd, alias,
>>>> flags);
>>>
>>> This will only return the last fd that gets opened. That's OK if the
>>> file only exists on a single V1 controller, but if it exists on multiple
>>> controllers (e.g. any cgroup.* file) then we will open multiple files
>>> and only return the fd for the last of them.
>>
>> Sorry for the late reply. I just copy these code from safe_cg_printf.
>> So safe_cg_printf have the same situation that write value to muliple
>> files under the created cgroup directory.
>>
>> So what should we do? Add a fd arrat member in  cgroup_file  struct?
> 
> I'm not sure what you mean, but I can see a few options.
> 
> 1. Pass a pointer to a function which takes the fd, for e.g.
> 
> safe_cg_open(..., void (*const on_open)(int fd))
> {
>          ...
>          for_each_dir(...) {
>              ...
>              fd = safe_openat(...);
>              on_open(fd);
>              close(fd);
>          }
>          ...
> }
> 
> 2. Allocate and return an array of open fd's

This way is also in my mind. But we can also filter them by cgroup 
version and detect the root ctrl name. I have sent a RFC v2 patch.

Best Regards
Yang Xu
> 3. Pass an array as large as the maximum number controllers which is
>     populated with open fd's.
> 
> Which option to pick I think depends on what results in the simplest
> test code.
> 
>>
>> Best Regards
>> Yang Xu
>>
>>>
> 
> 

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp