[LTP] [PATCH v4 1/3] lib: alter find_free_loopdev()

Yang Xu
2019-07-11T04:00:13Z

> Hi!
>> Alter find_free_loopdev() to return the free loopdev minor
>> (and -1 for no free loopdev) and then WE can safely use the
>> minor number that find_free_loopdev() returned in test cases.
>>
>> Signed-off-by: Yang Xu<xuyang2018.jy@cn.fujitsu.com>
>> Reviewed-by: Amir Goldstein<amir73il@gmail.com>
>> ---
>> doc/test-writing-guidelines.txt | 9 +++++++++
>> include/tst_device.h | 6 ++++++
>> lib/tst_device.c | 12 ++++++------
>> 3 files changed, 21 insertions(+), 6 deletions(-)
>>
>> diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt
>> index c6d4e001d..887801e68 100644
>> --- a/doc/test-writing-guidelines.txt
>> +++ b/doc/test-writing-guidelines.txt
>> @@ -1045,6 +1045,15 @@ IMPORTANT: All testcases should use 'tst_umount()' instead of 'umount(2)' to
>> -------------------------------------------------------------------------------
>> #include "tst_test.h"
>>
>> +int find_free_loopdev();
> Once this is exported as public API it should be prefixed with tst_.
>
OK.

>> -static int find_free_loopdev(void)
>> +int find_free_loopdev(void)
>> {
>> int ctl_fd, dev_fd, rc, i;
>> struct loop_info loopinfo;
>> @@ -82,10 +82,10 @@ static int find_free_loopdev(void)
>> if (rc>= 0) {
>> set_dev_path(rc);
>> tst_resm(TINFO, "Found free device '%s'", dev_path);
>> - return 0;
>> + return rc;
>> }
>> tst_resm(TINFO, "Couldn't find free loop device");
>> - return 1;
>> + return -1;
>> }
>>
>> switch (errno) {
>> @@ -121,7 +121,7 @@ static int find_free_loopdev(void)
>> continue;
>> tst_resm(TINFO, "Found free device '%s'", dev_path);
>> close(dev_fd);
>> - return 0;
>> + return i;
>> }
>>
>> close(dev_fd);
>> @@ -129,7 +129,7 @@ static int find_free_loopdev(void)
>>
>> tst_resm(TINFO, "No free devices found");
>>
>> - return 1;
>> + return -1;
>> }
> This needs more changes than this.
>
> The problem here is that the function modifies dev_path which is
> returned by tst_acquire_device() so if you call this function after
> tst_acquire_device() it will rewrite the dev_path which means that the
> test would end up with wrong device path in tst_device->dev.
>
> I guess that the easiest solution would be changing the function to get
> buffer parameter which, when non-NULL, is filled with the path.
>
> I.e. the function prototype would became:
>
> int tst_find_free_loopdev(char *path, size_t path_len);
>
> And we would pass the dev_path inside of the tst_device.c and NULL from
> the copy_file_range() tests.
Hi Cyril

This is a good comment. But I doubt why we don't use a set_devpath_flag todistinguish it.
Or you have a future plan(in different directory ,/dev,/dev/loop/,/dev/block)?

Thanks
Yang Xu