Re: [LTP] Possible fix for LTP Issue #1219

[Petr Vorel <pvorel@suse.cz>]

Hi Coey,

> I created an issue against LTP: https://github.com/linux-test-project/ltp/issues/1219. ‘pevik’ suggested that I send a patch here. I’ll admit that I’m uncertain what form would be preferred, but here’s what I’ll share:
> [[PATCH]]

FYI commit message like this is not much helpful (usually the reason for the
change is important to describe, you don't put your Signed-off-by:), but we can
fix it before merge.

> diff --git a/testcases/kernel/kvm/kvm_pagefault01.c b/testcases/kernel/kvm/kvm_pagefault01.c
> index 16b3137c0..649bf429a 100644
> --- a/testcases/kernel/kvm/kvm_pagefault01.c
> +++ b/testcases/kernel/kvm/kvm_pagefault01.c
> @@ -214,6 +214,10 @@ static struct tst_test test = {
>         .setup = setup,
>         .cleanup = tst_kvm_cleanup,
>         .needs_root = 1,
> +       .needs_drivers = (const char *const []) {
> +               "kvm",
> +               NULL
> +       },
>         .supported_archs = (const char *const []) {
>                 "x86_64",
>                 NULL
> diff --git a/testcases/kernel/kvm/kvm_svm01.c b/testcases/kernel/kvm/kvm_svm01.c
> index 32d15526b..f81602567 100644
> --- a/testcases/kernel/kvm/kvm_svm01.c
> +++ b/testcases/kernel/kvm/kvm_svm01.c
> @@ -108,6 +108,10 @@ static struct tst_test test = {
>         .test_all = tst_kvm_run,
>         .setup = tst_kvm_setup,
>         .cleanup = tst_kvm_cleanup,
> +       .needs_drivers = (const char *const []) {
> +               "kvm",
> +               NULL
> +       },
>         .supported_archs = (const char *const []) {
>                 "x86_64",
>                 "x86",
> diff --git a/testcases/kernel/kvm/kvm_svm02.c b/testcases/kernel/kvm/kvm_svm02.c
> index 6914fdcba..701f2731d 100644
> --- a/testcases/kernel/kvm/kvm_svm02.c
> +++ b/testcases/kernel/kvm/kvm_svm02.c
> @@ -129,6 +129,10 @@ static struct tst_test test = {
>         .test_all = tst_kvm_run,
>         .setup = tst_kvm_setup,
>         .cleanup = tst_kvm_cleanup,
> +       .needs_drivers = (const char *const []) {
> +               "kvm",
> +               NULL
> +       },
>         .supported_archs = (const char *const []) {
>                 "x86_64",
>                 "x86",
> diff --git a/testcases/kernel/kvm/kvm_svm03.c b/testcases/kernel/kvm/kvm_svm03.c
> index 87164d013..87f9887d8 100644
> --- a/testcases/kernel/kvm/kvm_svm03.c
> +++ b/testcases/kernel/kvm/kvm_svm03.c
> @@ -88,6 +88,9 @@ static void *vm_thread(void *arg)

> static void setup(void)
> {
> +       /* Run the common 'tst_kvm_setup()' first. */
> +       tst_kvm_setup();
IMHO "kvm" in .needs_drivers is enough, but I might be wrong.
@Martin?

> +
>         struct sigaction sa = { .sa_handler = sighandler };
>         pthread_mutexattr_t attr;

> @@ -155,6 +158,10 @@ static struct tst_test test = {
>         .setup = setup,
>         .cleanup = cleanup,
>         .min_cpus = 2,
> +       .needs_drivers = (const char *const []) {
> +               "kvm",
> +               NULL
> +       },
>         .supported_archs = (const char *const []) {
>                 "x86_64",
>                 "x86",
> diff --git a/testcases/kernel/kvm/kvm_svm04.c b/testcases/kernel/kvm/kvm_svm04.c
> index e69f0d4be..d8d3bdd96 100644
> --- a/testcases/kernel/kvm/kvm_svm04.c
> +++ b/testcases/kernel/kvm/kvm_svm04.c
> @@ -297,6 +297,10 @@ static struct tst_test test = {
>         .test_all = tst_kvm_run,
>         .setup = tst_kvm_setup,
>         .cleanup = tst_kvm_cleanup,
> +       .needs_drivers = (const char *const []) {
> +               "kvm",
> +               NULL
> +       },
>         .supported_archs = (const char *const []) {
>                 "x86_64",
>                 "x86",
> diff --git a/testcases/kernel/kvm/lib_host.c b/testcases/kernel/kvm/lib_host.c
> index 8e3d6094e..17215c23b 100644
> --- a/testcases/kernel/kvm/lib_host.c
> +++ b/testcases/kernel/kvm/lib_host.c
> @@ -323,7 +323,14 @@ void tst_kvm_clear_guest_signal(struct tst_kvm_instance *inst)

> void tst_kvm_setup(void)
> {
> -
> +       /* Do a quick check that the 'kvm' module is actually loaded by
> +          checking for '/dev/kvm'. If that device file is not present, then
> +          the module is likely not loaded in which case we should just CONF
> +          out.
> +       */
> +       if (access("/dev/kvm", F_OK) != 0) {
> +                tst_brk(TCONF, "The test requires 'kvm' device, which is not loaded.");
> +       }
Therefore I would remove this.

Kind regards,
Petr

> }

> void tst_kvm_run(void)
> [[/PATCH]]

> I’ll admit that this possibly contains parts that you may not want, but it includes the parts of the issue that I raised.

> Coey Minear

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp