Re: [PATCH v2 3/3] test: support trace-autotest when enable trace

[David Marchand <david.marchand@redhat.com>]

On Fri, Jun 17, 2022 at 4:43 AM fengchengwen <fengchengwen@huawei.com> wrote:
>
> Hi David, thanks for your review.
>
> On 2022/6/15 22:00, David Marchand wrote:
> > On Tue, Jun 14, 2022 at 8:06 AM Chengwen Feng <fengchengwen@huawei.com> wrote:
> >>
> >> There are a bug[1] when exit application while enable tracing, this
> >> bug has not been discovered for a long time, to quickly detect such
> >> bugs, this patch was introduced.
> >>
> >> This patch adds a testcase with trace enabling, it also depends on
> >> patch[2] because it has a long file-prefix.
> >>
> >> [1] eal: fix segment fault when exit trace
> >> [2] eal: fix trace init fail with long file-prefix
> >
> > This commitlog feels more like a cover letter.
> >
> > Please describe what the impact of the patch is, like mention that the
> > trace_autotest unit test is being called twice, once with traces
> > disabled, and once with traces enabled.
> > And that the traces file is being written in a directory part of the
> > build directory.
> >
> >
> >>
> >> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> >> ---
> >>  app/test/meson.build | 11 +++++++++++
> >>  1 file changed, 11 insertions(+)
> >>
> >> diff --git a/app/test/meson.build b/app/test/meson.build
> >> index 7fe261cae8..eb37aa632a 100644
> >> --- a/app/test/meson.build
> >> +++ b/app/test/meson.build
> >> @@ -509,6 +509,17 @@ foreach arg : fast_tests
> >>                  is_parallel : false,
> >>                  suite : 'fast-tests')
> >>      endif
> >> +
> >> +    if run_test and arg[0] == 'trace_autotest' and (not is_windows)
> >
> > Calling this test on Windows should not be an issue.
> > Is it not the case?
>
> I notice that eal_parse_common_option still keep windows macro for trace parameter parse:
>
> #ifndef RTE_EXEC_ENV_WINDOWS
>         case OPT_TRACE_NUM: {
>                 ...
>         case OPT_TRACE_MODE_NUM: {
>                 if (eal_trace_mode_args_save(optarg) < 0) {
>                         RTE_LOG(ERR, EAL, "invalid parameters for --"
>                                 OPT_TRACE_MODE "\n");
>                         return -1;
>                 }
>                 break;
>         }
> #endif /* !RTE_EXEC_ENV_WINDOWS */
>
> Maybe trace already support in windows, but I havn't the windows enviorment to verify it.
>
> So I think we should keep '(not is_windows)' currently.

Indeed.


-- 
David Marchand