On Tue, Nov 10, 2020 at 01:52:56PM -0800, Ben Gardon wrote: > On Tue, Nov 10, 2020 at 12:48 PM Andrew Jones <drjones@xxxxxxxxxx> wrote: > > > > demand_paging_test, dirty_log_test, and dirty_log_perf_test have > > redundant guest mode code. Factor it out. > > > > Also, while adding a new include, remove the ones we don't need. > > > > Signed-off-by: Andrew Jones <drjones@xxxxxxxxxx> > > --- > > tools/testing/selftests/kvm/Makefile | 2 +- > > .../selftests/kvm/demand_paging_test.c | 107 ++++----------- > > .../selftests/kvm/dirty_log_perf_test.c | 119 +++++------------ > > tools/testing/selftests/kvm/dirty_log_test.c | 125 ++++++------------ > > .../selftests/kvm/include/guest_modes.h | 21 +++ > > tools/testing/selftests/kvm/lib/guest_modes.c | 70 ++++++++++ > > 6 files changed, 188 insertions(+), 256 deletions(-) > > create mode 100644 tools/testing/selftests/kvm/include/guest_modes.h > > create mode 100644 tools/testing/selftests/kvm/lib/guest_modes.c > > > > diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile > > index 3d14ef77755e..ca6b64d9ab64 100644 > > --- a/tools/testing/selftests/kvm/Makefile > > +++ b/tools/testing/selftests/kvm/Makefile > > @@ -33,7 +33,7 @@ ifeq ($(ARCH),s390) > > UNAME_M := s390x > > endif > > > > -LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/sparsebit.c lib/test_util.c > > +LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/sparsebit.c lib/test_util.c lib/guest_modes.c > > LIBKVM_x86_64 = lib/x86_64/processor.c lib/x86_64/vmx.c lib/x86_64/svm.c lib/x86_64/ucall.c lib/x86_64/handlers.S > > LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c > > LIBKVM_s390x = lib/s390x/processor.c lib/s390x/ucall.c > > diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c > > index 3d96a7bfaff3..946161a9ce2d 100644 > > --- a/tools/testing/selftests/kvm/demand_paging_test.c > > +++ b/tools/testing/selftests/kvm/demand_paging_test.c > > @@ -7,23 +7,20 @@ > > * Copyright (C) 2019, Google, Inc. > > */ > > > > -#define _GNU_SOURCE /* for program_invocation_name */ > > +#define _GNU_SOURCE /* for program_invocation_name and pipe2 */ > > What is the purpose of pipe2 in this patch / why add it to this > comment but not the comments in the other files modified here? Only this file uses pipe2. If we do a later cleanup removing the program_invocation_name usage from this file, then I want to point out that we need to keep _GNU_SOURCE defined for pipe2. Actually, the only reason we still need program_invocation_name at this point in the series is because program_invocation_name is used in perf_test_util.h in a function we include in this file. I should have removed program_invocation_name from the comment with the "KVM: selftests: Use vm_create_with_vcpus in create_vm" patch, but I forgot. Thanks, drew
