KVM Archive on lore.kernel.org
 help / color / Atom feed
From: Andrew Jones <drjones@redhat.com>
To: Alexandru Elisei <alexandru.elisei@arm.com>
Cc: kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu, pbonzini@redhat.com
Subject: Re: [kvm-unit-tests RFC PATCH 0/1] configure: arm: Replace --vmm with --target
Date: Tue, 20 Apr 2021 18:51:01 +0200
Message-ID: <20210420165101.irbx2upgqbazkvlt@gator.home> (raw)
In-Reply-To: <20210420161338.70914-1-alexandru.elisei@arm.com>

Hi Alex,

On Tue, Apr 20, 2021 at 05:13:37PM +0100, Alexandru Elisei wrote:
> This is an RFC because it's not exactly clear to me that this is the best
> approach. I'm also open to using a different name for the new option, maybe
> something like --platform if it makes more sense.

I like 'target'.

> 
> I see two use cases for the patch:
> 
> 1. Using different files when compiling kvm-unit-tests to run as an EFI app
> as opposed to a KVM guest (described in the commit message).
> 
> 2. This is speculation on my part, but I can see extending
> arm/unittests.cfg with a "target" test option which can be used to decide
> which tests need to be run based on the configure --target value. For
> example, migration tests don't make much sense on kvmtool, which doesn't
> have migration support. Similarly, the micro-bench test doesn't make much
> sense (to me, at least) as an EFI app. Of course, this is only useful if
> there are automated scripts to run the tests under kvmtool or EFI, which
> doesn't look likely at the moment, so I left it out of the commit message.

Sounds like a good idea. unittests.cfg could get a new option 'targets'
where a list of targets is given. If targets is not present, then the
test assumes it's for all targets. Might be nice to also accept !<target>
syntax. E.g.

# builds/runs for all targets
[mytest]
file = mytest.flat

# builds/runs for given targets
[mytest2]
file = mytest2.flat
targets = qemu,kvmtool

# builds/runs for all targets except disabled targets
[mytest3]
file = mytest3.flat
targets = !kvmtool

And it wouldn't bother me to have special logic for kvmtool's lack of
migration put directly in scripts/runtime.bash

diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 132389c7dd59..0d5cb51df4f4 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -132,7 +132,7 @@ function run()
     }
 
     cmdline=$(get_cmdline $kernel)
-    if grep -qw "migration" <<<$groups ; then
+    if grep -qw "migration" <<<$groups && [ "$TARGET" != "kvmtool" ]; then
         cmdline="MIGRATION=yes $cmdline"
     fi
     if [ "$verbose" = "yes" ]; then

> 
> Using --vmm will trigger a warning. I was thinking about removing it entirely in
> a about a year's time, but that's not set in stone. Note that qemu users
> (probably the vast majority of people) will not be affected by this change as
> long as they weren't setting --vmm explicitely to its default value of "qemu".
>

While we'd risk automated configure+build tools, like git{hub,lab} CI,
failing, I think the risk is pretty low right now that anybody is using
the option. Also, we might as well make them change sooner than later by
failing configure. IOW, I'd just do s/vmm/target/g to rename it now. If
we are concerned about the disruption, then I'd just make vmm an alias
for target and not bother deprecating it ever.

Thanks,
drew


  parent reply index

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-20 16:13 Alexandru Elisei
2021-04-20 16:13 ` [kvm-unit-tests RFC PATCH 1/1] " Alexandru Elisei
2021-04-20 16:51 ` Andrew Jones [this message]
2021-04-22 15:17   ` [kvm-unit-tests RFC PATCH 0/1] " Alexandru Elisei
2021-04-22 15:57     ` Andrew Jones
2021-04-23 15:43       ` Alexandru Elisei
2021-04-26  8:59         ` Andrew Jones
2021-04-26 14:11           ` Alexandru Elisei

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=20210420165101.irbx2upgqbazkvlt@gator.home \
    --to=drjones@redhat.com \
    --cc=alexandru.elisei@arm.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=pbonzini@redhat.com \
    /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

KVM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvm/0 kvm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvm kvm/ https://lore.kernel.org/kvm \
		kvm@vger.kernel.org
	public-inbox-index kvm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.kvm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git