[LTP] [PATCH v4 11/19] controllers: Expand cgroup_lib shell library

[Luke Nowakowski-Krijger]

Hi Petr and Cyril,

On Fri, May 13, 2022 at 1:15 PM Luke Nowakowski-Krijger <
luke.nowakowskikrijger@canonical.com> wrote:

> Hi Petr and Cyril,
>
> On Tue, May 10, 2022 at 4:19 AM Petr Vorel <pvorel@suse.cz> wrote:
>
>> Hi Luke, Cyril,
>>
>> > Hi!
>> > > > +_cgroup_check_return()
>> > > > +{
>> > > > + local ret="$1"
>> > > > + local msg="$2"
>> > > > +
>> > > > + tst_flag2mask TBROK
>> > > > + [ "$ret" = "$?" ] && tst_brk TBROK "$msg"
>> > > > +
>> > > > + tst_flag2mask TCONF
>> > > > + [ "$ret" = "$?" ] && tst_brk TCONF "$msg"
>> > > > +}
>> > > As I wrote in previous patch likely we can avoid tst_flag2mask in new
>> API.
>>
>>
I submitted a new version of the changes, however I found that ROD does not
seem to play nice with sending output to a variable (e.g. var=$(command)).

Also reflecting on it, I think it would be better to propagate the TCONF
that comes from tst_cgroup.c so that the errors make a little more sense
instead of getting a TBROK when the error previous was TCONF. This also
makes sense as test frameworks (at canonical) usually parse TBROK as fails
and TCONF as skips in our testing framework, so having a TCONF would just
propagate the status in tst_cgctl and make things make a little more sense.

The only issue, like you said, is in hardcoding the 32 return value. If you
guys think its safe I think thats the best way to go.

Let me know. I submitted the patches to the ML. Sorry for the long hiatus
BTW :) Ive been lagging on getting these patches out forever.



> > > In few cases where needed we hardwired numbers (IMHO POSIX shell does
>> not
>> > > support constants, which would be better than variables which can be
>> changed).
>>
>> > > In this case you could probably use ROD() or EXPECT_PASS_BRK().
>>
>> > Or we can just passthrough the result, as long as it's non-zero we can
>> > do exit $ret and be done with it.
>> +1 (that would suggest to use ROD)
>>
>> Please, rebase the code for new version. You'll have to for cgroup_lib.sh
>> put
>> '. tst_test.sh' to the end and also '. cgroup_lib.sh' in the tests also
>> at the
>> end - required by 04021637f ("tst_test.sh: Cleanup getopts usage").
>>
>>
> Thank you for the reviews! I agree with the changes mentioned and will
> submit an update to these patches.
>
> Kind regards,
>> Petr
>>
>
> Thanks,
> - Luke
>

Best

- Luke
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20220721/2905c23a/attachment-0001.htm>