On Tue, Feb 2, 2021 at 2:16 PM KP Singh <kpsingh@xxxxxxxxxx> wrote:
>
> The script runs the BPF selftests locally on the same kernel image
> as they would run post submit in the BPF continuous integration
> framework.
>
> The goal of the script is to allow contributors to run selftests locally
> in the same environment to check if their changes would end up breaking
> the BPF CI and reduce the back-and-forth between the maintainers and the
> developers.
>
> Tested-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> Signed-off-by: KP Singh <kpsingh@xxxxxxxxxx>
> ---
I almost applied it :) But found two problems still, which ruins
experience in my environment, see below.
Also, do you mind renaming the script (and updating the doc in patch
#2)to vmtest.sh for a shorter name without underscores?
First problem is that it still doesn't propagate exit codes properly.
Try ./run_in_vm.sh -- false, followed by echo $? It should print 1,
but currently it prints zero.
> tools/testing/selftests/bpf/run_in_vm.sh | 368 +++++++++++++++++++++++
> 1 file changed, 368 insertions(+)
> create mode 100755 tools/testing/selftests/bpf/run_in_vm.sh
>
[...]
> +
> +update_kconfig()
> +{
> + local kconfig_file="$1"
> + local update_command="curl -sLf ${KCONFIG_URL} -o ${kconfig_file}"
> + # Github does not return the "last-modified" header when retrieving the
> + # raw contents of the file. Use the API call to get the last-modified
> + # time of the kernel config and only update the config if it has been
> + # updated after the previously cached config was created. This avoids
> + # unnecessarily compiling the kernel and selftests.
> + if [[ -f "${kconfig_file}" ]]; then
> + local last_modified_date="$(curl -sL -D - "${KCONFIG_API_URL}" -o /dev/null | \
> + grep "last-modified" | awk -F ': ' '{print $2}')"
> + local remote_modified_timestamp="$(date -d "${last_modified_date}" +"%s")"
> + local local_creation_timestamp="$(-c %W "${kconfig_file}")"
> +
%W breaks the entire experience for me. stat -c %W returns 0 in my
environment, don't know why. But it's also not clear why %W (file
creation time) was used instead of %Y (file modification time)? When
we overwrite latest.config, it will get updated modification time, but
old creation time, so this whole idea with %W seems wrong?
So, do you mind switching to local_modification_timestamp with %Y? I
checked locally, it finally allowed to skip rebuilding both the kernel
and selftests.
> + if [[ "${remote_modified_timestamp}" -gt "${local_creation_timestamp}" ]]; then
> + ${update_command}
> + fi
> + else
> + ${update_command}
> + fi
> +}
> +
[...]
