Re: [PATCH v4 8/9] KVM: selftests: Add rule to generate default tests for KVM selftests runner

Sean Christopherson <[email protected]>
Newsgroups org.kernel.vger.kvm,dev.linux.lists.kvmarm,org.infradead.lists.kvm-riscv
Message-ID <[email protected]>
On Thu, Aug 20, 2026, Vipin Sharma wrote:
> On Wed, Jul 29, 2026 at 12:07:35PM -0700, Sean Christopherson wrote:
> > @@ -347,15 +345,13 @@ $(SPLIT_TEST_GEN_PROGS): $(OUTPUT)/%: $(OUTPUT)/%.o $(OUTPUT)/$(ARCH)/%.o
> >  $(SPLIT_TEST_GEN_OBJ): $(OUTPUT)/$(ARCH)/%.o: $(ARCH)/%.c
> >         $(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c $< -o $@
> >  
> > -# Default testcases for KVM selftests runner will be generated in this directory.
> > -DEFAULT_TESTCASES = testcases_default_gen
> > +TEST_FILES := $(selfdir)/kvm/runner
> 
> Downside of this one is that if someone is not planning to use runner they
> will still get this directory copied.
> 
> Should we instead add 'rsync -u' in 'testcases' rule below which can copy
> runner only when it is updated or not present? This would be similar to
> original patch but instead of 'cp' it will be 'rsync' command. This will
> also copy runner when someone is explicitly using it.

Hmm, but someone could be using the runner without (re)generating the default
testcases.  We could add a "runner" target, but that seems like overkill.

I'm good with either option, but I think my vote would be have the runner get
copied on install, i.e. just do the "TEST_FILES := $(selfdir)/kvm/runner" above.
But again, either way works for me.

> I think we should also include 'tests' directory which we are using for
> custom testcases and they also .

Heh, they also what?

> >  EXTRA_CLEAN += $(GEN_HDRS) \
> >                $(LIBKVM_OBJS) \
> >                $(SPLIT_TEST_GEN_OBJ) \
> >                $(TEST_DEP_FILES) \
> >                $(TEST_GEN_OBJ) \
> > -              $(OUTPUT)/$(DEFAULT_TESTCASES) \
> >                cscope.*
> >  
> >  $(LIBKVM_C_OBJ): $(OUTPUT)/%.o: %.c $(GEN_HDRS)
> > @@ -386,21 +382,18 @@ cscope:
> >         cscope -b
> >  
> >  # Generate runner testcases in DEFAULT_TESTCASES directory.
> > -# $(OUTPUT) is either CWD or specified in the make command.
> > -tests_install: list_progs = $(patsubst $(OUTPUT)/%,%,$(TEST_GEN_PROGS))
> > -tests_install:
> > -       $(foreach tc, $(TEST_PROGS), \
> > -               $(shell mkdir -p $(OUTPUT)/$(DEFAULT_TESTCASES)/$(patsubst %.sh,%,$(tc))))
> > -       $(foreach tc, $(TEST_PROGS), \
> > -               $(shell echo $(tc) > $(patsubst %.sh,$(OUTPUT)/$(DEFAULT_TESTCASES)/%/default.test,$(tc))))
> > +DEFAULT_TESTCASES = default_testcases
> > +
> > +testcases: list_progs = $(notdir $(patsubst $(OUTPUT)/%,%,$(TEST_GEN_PROGS)))
> > +testcases: install
> > +       $(foreach tc, $(notdir $(TEST_PROGS)), \
> > +               $(shell mkdir -p $(INSTALL_PATH)/$(DEFAULT_TESTCASES)/$(notdir $(patsubst %.sh,%,$(tc)))))
> > +       $(foreach tc, $(notdir $(TEST_PROGS)), \
> > +               $(shell echo $(tc) > $(patsubst %.sh,$(INSTALL_PATH)/$(DEFAULT_TESTCASES)/%/default.test,$(tc))))
> >  
> >         $(foreach tc, $(list_progs), \
> > -               $(shell mkdir -p $(OUTPUT)/$(DEFAULT_TESTCASES)/$(tc)))
> > +               $(shell mkdir -p $(INSTALL_PATH)/$(DEFAULT_TESTCASES)/$(tc)))
> >         $(foreach tc, $(list_progs), \
> > -               $(shell echo $(tc) > $(patsubst %,$(OUTPUT)/$(DEFAULT_TESTCASES)/%/default.test,$(tc))))
> > -
> > -       @if [ ! -d $(OUTPUT)/runner ]; then             \
> > -               cp -r $(selfdir)/kvm/runner $(OUTPUT);  \
> > -       fi
> > +               $(shell echo $(tc) > $(patsubst %,$(INSTALL_PATH)/$(DEFAULT_TESTCASES)/%/default.test,$(tc))))
> 
> All looks good here.
> 
> Should I send a next version of this series with the changes you have
> suggested or you wanna take the diff you created and apply before merge?

Go ahead and send the next version.  Given the number of tweaks, it'd be helpful
to be closer to WYSIWYG.  My changes are at:

  https://github.com/sean-jc/linux slf/runner

if you want to use that as a reference and/or grab changes directly.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.