Re: [PATCH v7 2/2] Update bench-code for addressing CI problem

[Juan Quintela <quintela@redhat.com>]

Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> On 16/11/22 16:29, ling xu wrote:
>> Unit test code is in test-xbzrle.c, and benchmark code is in xbzrle-bench.c
>> for performance benchmarking. we have modified xbzrle-bench.c to address
>> CI problem.
>> Signed-off-by: ling xu <ling1.xu@intel.com>
>> Co-authored-by: Zhou Zhao <zhou.zhao@intel.com>
>> Co-authored-by: Jun Jin <jun.i.jin@intel.com>
>> ---
>>   tests/bench/meson.build    |   4 +
>>   tests/bench/xbzrle-bench.c | 469 +++++++++++++++++++++++++++++++++++++
>>   tests/unit/test-xbzrle.c   |  39 ++-
>>   3 files changed, 507 insertions(+), 5 deletions(-)
>>   create mode 100644 tests/bench/xbzrle-bench.c
>
>
>> diff --git a/tests/bench/xbzrle-bench.c b/tests/bench/xbzrle-bench.c
>> new file mode 100644
>> index 0000000000..8848a3a32d
>> --- /dev/null
>> +++ b/tests/bench/xbzrle-bench.c
>> @@ -0,0 +1,469 @@
>> +/*
>> + * Xor Based Zero Run Length Encoding unit tests.
>> + *
>> + * Copyright 2013 Red Hat, Inc. and/or its affiliates
>> + *
>> + * Authors:
>> + *  Orit Wasserman  <owasserm@redhat.com>
>
> Is Orit the real author? Or is it based on migration/xbzrle.c?

Based on as far as I can se.

>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + *
>> + */
>> +#include "qemu/osdep.h"
>> +#include "qemu/cutils.h"
>> +#include "../migration/xbzrle.h"
>
> Interesting migration include path. Similarly:

xbzrle.h is only exported for migration.  Nothing else can use them.
So we can't put that on include/migration/*

> $ git grep -F '#include "../' tests | egrep -v
> '(../libqtest.h|tests/tcg/mips|../multiarch)'
> tests/qtest/netdev-socket.c:12:#include "../unit/socket-helpers.h"
> tests/unit/test-qgraph.c:20:#include "../qtest/libqos/qgraph.h"
> tests/unit/test-qgraph.c:21:#include "../qtest/libqos/qgraph_internal.h"
>
> tests/migration/aarch64/a-b-kernel.S:14:#include "../migration-test.h"
> tests/unit/test-vmstate.c:27:#include "../migration/migration.h"
> tests/unit/test-vmstate.c:30:#include "../migration/qemu-file.h"
> tests/unit/test-vmstate.c:31:#include "../migration/savevm.h"
> tests/unit/test-xbzrle.c:15:#include "../migration/xbzrle.h"
>
> $ ls -1 migration/*.h
> migration/block.h
> migration/channel-block.h
> migration/channel.h
> migration/dirtyrate.h
> migration/exec.h
> migration/fd.h
> migration/migration.h      [*]
> migration/multifd.h
> migration/page_cache.h
> migration/postcopy-ram.h
> migration/qemu-file.h      [*]
> migration/ram.h
> migration/rdma.h
> migration/savevm.h         [*]
> migration/socket.h
> migration/threadinfo.h
> migration/tls.h
> migration/trace.h
> migration/xbzrle.h         [*]
> migration/yank_functions.h
>
> $ ls -1 include/migration/*.h
> include/migration/blocker.h
> include/migration/colo.h
> include/migration/cpu.h
> include/migration/failover.h
> include/migration/global_state.h
> include/migration/misc.h
> include/migration/qemu-file-types.h
> include/migration/register.h
> include/migration/snapshot.h
> include/migration/vmstate.h
>
> Do the 4 files marked [*] belong to include/migration/?

The split is:
include/migration/* <- exported for everybody to use
migration/*.h       <- Only for migration

Now, doing tests for migration makes this difference complicated,
because some tests really need things that are not exported.

This is the way that it is normally used in the tree, no?

Later, Juan.