| Message ID | 160615331159.1339160.8458952247626802787.stgit@bahia.lan |
|---|---|
| State | New |
| Headers | show |
| Series | [for-5.2] tests/9pfs: Mark "local" tests as "slow" | expand |
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
On Montag, 23. November 2020 19:08:18 CET Christian Schoenebeck wrote: > 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 BTW, if I had to shoot in the dark, then my guess would be that system where mkdir() failed with ENOTSUPP, does not support xattrs. If that's the root cause, then another option would be skipping the 'local' 9p tests if the host system lacks xattr support, instead of disabling these tests for all people by default. Best regards, Christian Schoenebeck
On Mon, 23 Nov 2020 19:08:18 +0100 Christian Schoenebeck <qemu_oss@crudebyte.com> wrote: > 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. > Sure, I'll do that. > > + /* 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 > >
On Mon, 23 Nov 2020 20:35:58 +0100 Christian Schoenebeck <qemu_oss@crudebyte.com> wrote: > On Montag, 23. November 2020 19:08:18 CET Christian Schoenebeck wrote: > > 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 > > BTW, if I had to shoot in the dark, then my guess would be that system where > mkdir() failed with ENOTSUPP, does not support xattrs. > > If that's the root cause, then another option would be skipping the 'local' 9p > tests if the host system lacks xattr support, instead of disabling these tests > for all people by default. > Maybe but it's a bit too late now. RC3 is later today. I really must post something that fixes the "make check" breakage this morning. > Best regards, > Christian Schoenebeck > >
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()) { + /* 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);
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(-)