All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Vipin Sharma <vipinsh@google.com>
Cc: kvm@vger.kernel.org, kvmarm@lists.linux.dev,
	kvm-riscv@lists.infradead.org,  pbonzini@redhat.com,
	borntraeger@linux.ibm.com, frankja@linux.ibm.com,
	 imbrenda@linux.ibm.com, anup@brainfault.org,
	atish.patra@linux.dev,  zhaotianrui@loongson.cn,
	maobibo@loongson.cn, chenhuacai@kernel.org,  maz@kernel.org,
	oliver.upton@linux.dev, ajones@ventanamicro.com
Subject: Re: [PATCH v4 8/9] KVM: selftests: Add rule to generate default tests for KVM selftests runner
Date: Mon, 24 Aug 2026 14:21:34 -0700	[thread overview]
Message-ID: <aoy13na7ceOOeh0K@google.com> (raw)
In-Reply-To: <20260820220340.GC618052.vipinsh@google.com>

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.

WARNING: multiple messages have this Message-ID (diff)
From: Sean Christopherson <seanjc@google.com>
To: Vipin Sharma <vipinsh@google.com>
Cc: kvm@vger.kernel.org, kvmarm@lists.linux.dev,
	kvm-riscv@lists.infradead.org,  pbonzini@redhat.com,
	borntraeger@linux.ibm.com, frankja@linux.ibm.com,
	 imbrenda@linux.ibm.com, anup@brainfault.org,
	atish.patra@linux.dev,  zhaotianrui@loongson.cn,
	maobibo@loongson.cn, chenhuacai@kernel.org,  maz@kernel.org,
	oliver.upton@linux.dev, ajones@ventanamicro.com
Subject: Re: [PATCH v4 8/9] KVM: selftests: Add rule to generate default tests for KVM selftests runner
Date: Mon, 24 Aug 2026 14:21:34 -0700	[thread overview]
Message-ID: <aoy13na7ceOOeh0K@google.com> (raw)
In-Reply-To: <20260820220340.GC618052.vipinsh@google.com>

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.

-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv

  reply	other threads:[~2026-08-24 21:21 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-31 19:41 [PATCH v4 0/9] KVM: selftests: Create KVM selftests runner Vipin Sharma
2026-03-31 19:41 ` Vipin Sharma
2026-03-31 19:41 ` [PATCH v4 1/9] KVM: selftest: Create KVM selftest runner Vipin Sharma
2026-03-31 19:41   ` Vipin Sharma
2026-06-11  1:17   ` Ackerley Tng
2026-06-11  1:17     ` Ackerley Tng
2026-06-11 19:08     ` Sean Christopherson
2026-06-11 19:08       ` Sean Christopherson
2026-06-11 20:16       ` Ackerley Tng
2026-06-11 20:16         ` Ackerley Tng
2026-06-12  1:07         ` Sean Christopherson
2026-06-12  1:07           ` Sean Christopherson
2026-06-12 19:45           ` Ackerley Tng
2026-06-12 19:45             ` Ackerley Tng
2026-06-12 23:56             ` Sean Christopherson
2026-06-12 23:56               ` Sean Christopherson
2026-06-16 16:15               ` Ackerley Tng
2026-06-16 16:15                 ` Ackerley Tng
2026-06-16 18:29                 ` Vipin Sharma
2026-06-16 18:29                   ` Vipin Sharma
2026-06-16 21:43     ` Vipin Sharma
2026-06-16 21:43       ` Vipin Sharma
2026-06-16 23:04       ` Ackerley Tng
2026-06-16 23:04         ` Ackerley Tng
2026-03-31 19:41 ` [PATCH v4 2/9] KVM: selftests: Provide executables path option to the " Vipin Sharma
2026-03-31 19:41   ` Vipin Sharma
2026-03-31 19:41 ` [PATCH v4 3/9] KVM: selftests: Add timeout option in selftests runner Vipin Sharma
2026-03-31 19:41   ` Vipin Sharma
2026-07-29 19:39   ` Sean Christopherson
2026-07-29 19:39     ` Sean Christopherson
2026-08-20 23:34     ` Vipin Sharma
2026-08-20 23:34       ` Vipin Sharma
2026-03-31 19:41 ` [PATCH v4 4/9] KVM: selftests: Add option to save selftest runner output to a directory Vipin Sharma
2026-03-31 19:41   ` Vipin Sharma
2026-03-31 19:41 ` [PATCH v4 5/9] KVM: selftests: Run tests concurrently in KVM selftests runner Vipin Sharma
2026-03-31 19:41   ` Vipin Sharma
2026-03-31 19:41 ` [PATCH v4 6/9] KVM: selftests: Add various print flags to KVM selftest runner Vipin Sharma
2026-03-31 19:41   ` Vipin Sharma
2026-07-29 19:14   ` Sean Christopherson
2026-07-29 19:14     ` Sean Christopherson
2026-08-20 23:37     ` Vipin Sharma
2026-08-20 23:37       ` Vipin Sharma
2026-03-31 19:42 ` [PATCH v4 7/9] KVM: selftests: Print sticky KVM selftests runner status at bottom Vipin Sharma
2026-03-31 19:42   ` Vipin Sharma
2026-03-31 19:42 ` [PATCH v4 8/9] KVM: selftests: Add rule to generate default tests for KVM selftests runner Vipin Sharma
2026-03-31 19:42   ` Vipin Sharma
2026-07-29 19:07   ` Sean Christopherson
2026-07-29 19:07     ` Sean Christopherson
2026-08-20 23:29     ` Vipin Sharma
2026-08-20 23:29       ` Vipin Sharma
2026-08-24 21:21       ` Sean Christopherson [this message]
2026-08-24 21:21         ` Sean Christopherson
2026-03-31 19:42 ` [PATCH v4 9/9] KVM: selftests: Provide README.rst " Vipin Sharma
2026-03-31 19:42   ` Vipin Sharma
2026-07-29 19:09   ` Sean Christopherson
2026-07-29 19:09     ` Sean Christopherson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aoy13na7ceOOeh0K@google.com \
    --to=seanjc@google.com \
    --cc=ajones@ventanamicro.com \
    --cc=anup@brainfault.org \
    --cc=atish.patra@linux.dev \
    --cc=borntraeger@linux.ibm.com \
    --cc=chenhuacai@kernel.org \
    --cc=frankja@linux.ibm.com \
    --cc=imbrenda@linux.ibm.com \
    --cc=kvm-riscv@lists.infradead.org \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.linux.dev \
    --cc=maobibo@loongson.cn \
    --cc=maz@kernel.org \
    --cc=oliver.upton@linux.dev \
    --cc=pbonzini@redhat.com \
    --cc=vipinsh@google.com \
    --cc=zhaotianrui@loongson.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.