qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Christian Schoenebeck <qemu_oss@crudebyte.com>
To: qemu-devel@nongnu.org
Cc: Greg Kurz <groug@kaod.org>,
	Peter Maydell <peter.maydell@linaro.org>,
	Thomas Huth <thuth@redhat.com>
Subject: Re: [PATCH for-5.2] tests/9pfs: Mark "local" tests as "slow"
Date: Mon, 23 Nov 2020 19:08:18 +0100	[thread overview]
Message-ID: <1652137.SzlE5fuBRn@silver> (raw)
In-Reply-To: <160615331159.1339160.8458952247626802787.stgit@bahia.lan>

On Montag, 23. November 2020 18:41:51 CET Greg Kurz wrote:
> The "local" tests can fail on some automated build systems as
> reported here:
> 
> https://lists.nongnu.org/archive/html/qemu-devel/2020-11/msg05510.html
> 
> This will need to be investigated and addressed later. Let's go for a
> workaround in the meantime : mark the "local" tests as "slow" so that
> they aren't executed with a simple "make check" like in the case above.
> 
> Reported-by: Cole Robinson <crobinso@redhat.com>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
> 
> Unless there's a strong argument against merging this, I'll post
> a PR with this fix for RC3.
> ---
>  tests/qtest/virtio-9p-test.c |   35 +++++++++++++++++++++--------------
>  1 file changed, 21 insertions(+), 14 deletions(-)
> 
> diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
> index 21e340fa5f43..dad37ace9772 100644
> --- a/tests/qtest/virtio-9p-test.c
> +++ b/tests/qtest/virtio-9p-test.c
> @@ -1454,20 +1454,27 @@ static void register_virtio_9p_test(void)
>      qos_add_test("synth/readdir/split_128", "virtio-9p",
>                   fs_readdir_split_128,  &opts);
> 
> -
> -    /* 9pfs test cases using the 'local' filesystem driver */
> -    opts.before = assign_9p_local_driver;
> -    qos_add_test("local/config", "virtio-9p", pci_config,  &opts);
> -    qos_add_test("local/create_dir", "virtio-9p", fs_create_dir, &opts);
> -    qos_add_test("local/unlinkat_dir", "virtio-9p", fs_unlinkat_dir,
> &opts); -    qos_add_test("local/create_file", "virtio-9p", fs_create_file,
> &opts); -    qos_add_test("local/unlinkat_file", "virtio-9p",
> fs_unlinkat_file, &opts); -    qos_add_test("local/symlink_file",
> "virtio-9p", fs_symlink_file, &opts); -   
> qos_add_test("local/unlinkat_symlink", "virtio-9p", fs_unlinkat_symlink, - 
>                &opts);
> -    qos_add_test("local/hardlink_file", "virtio-9p", fs_hardlink_file,
> &opts); -    qos_add_test("local/unlinkat_hardlink", "virtio-9p",
> fs_unlinkat_hardlink, -                 &opts);
> +    if (g_test_slow()) {

If you don't mind, I would suggest simply using

if (!g_test_slow()) {
	return;
}

here to keep the diff noise low.

> +        /* 9pfs test cases using the 'local' filesystem driver */
> +        /*
> +         * XXX: Until we are sure that these tests can run everywhere,
> +         * keep them as "slow" so that they aren't run with "make check"
> +         */
> +        opts.before = assign_9p_local_driver;
> +        qos_add_test("local/config", "virtio-9p", pci_config,  &opts);
> +        qos_add_test("local/create_dir", "virtio-9p", fs_create_dir,
> &opts); +        qos_add_test("local/unlinkat_dir", "virtio-9p",
> fs_unlinkat_dir, &opts); +        qos_add_test("local/create_file",
> "virtio-9p", fs_create_file, &opts); +       
> qos_add_test("local/unlinkat_file", "virtio-9p", fs_unlinkat_file, +       
>              &opts);
> +        qos_add_test("local/symlink_file", "virtio-9p", fs_symlink_file,
> &opts); +        qos_add_test("local/unlinkat_symlink", "virtio-9p",
> fs_unlinkat_symlink, +                     &opts);
> +        qos_add_test("local/hardlink_file", "virtio-9p", fs_hardlink_file,
> +                     &opts);
> +        qos_add_test("local/unlinkat_hardlink", "virtio-9p",
> +                     fs_unlinkat_hardlink, &opts);
> +    }
>  }
> 
>  libqos_init(register_virtio_9p_test);

I agree that this is okay for now to avoid inconveniences with nearby 5.2 
release, but do we agree that this is going to be reverted when 6.0 
development phase starts?

In 6.0 I would rather handle this by introducing a previously mentioned 
'loglevel' option for 9p, and asking for the required log data if the local 
tests fail for somebody. I can take care about the loglevel option when 6.0 
phase starts.

Best regards,
Christian Schoenebeck




  reply	other threads:[~2020-11-23 18:09 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-23 17:41 [PATCH for-5.2] tests/9pfs: Mark "local" tests as "slow" Greg Kurz
2020-11-23 18:08 ` Christian Schoenebeck [this message]
2020-11-23 19:35   ` Christian Schoenebeck
2020-11-24  7:23     ` Greg Kurz
2020-11-24  7:20   ` Greg Kurz

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=1652137.SzlE5fuBRn@silver \
    --to=qemu_oss@crudebyte.com \
    --cc=groug@kaod.org \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).