[PULL v2 0/5] 9p queue (previous 2020-10-15)

Christian Schoenebeck posted 5 patches 3 years, 5 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/cover.1602943547.git.qemu_oss@crudebyte.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Christian Schoenebeck <qemu_oss@crudebyte.com>, Thomas Huth <thuth@redhat.com>, Laurent Vivier <lvivier@redhat.com>, Greg Kurz <groug@kaod.org>
tests/qtest/libqos/virtio-9p.c | 100 +++++++++++++++++++++
tests/qtest/libqos/virtio-9p.h |  10 +++
tests/qtest/virtio-9p-test.c   | 197 ++++++++++++++++++++++++++++++++++++-----
3 files changed, 286 insertions(+), 21 deletions(-)
[PULL v2 0/5] 9p queue (previous 2020-10-15)
Posted by Christian Schoenebeck 3 years, 5 months ago
The following changes since commit e12ce85b2c79d83a340953291912875c30b3af06:

  Merge remote-tracking branch 'remotes/ehabkost/tags/x86-next-pull-request' into staging (2020-10-16 22:46:28 +0100)

are available in the Git repository at:

  https://github.com/cschoenebeck/qemu.git tags/pull-9p-20201017

for you to fetch changes up to fa4551e3f4416cc8c62086ac430b1ceb4f03eb6b:

  tests/9pfs: add local Tmkdir test (2020-10-17 15:58:39 +0200)

----------------------------------------------------------------
9pfs: add tests using local fs driver

The currently existing 9pfs test cases are all solely using the 9pfs 'synth'
fileystem driver, which is a very simple and purely simulated (in RAM only)
filesystem. There are issues though where the 'synth' fs driver is not
sufficient. For example the following two bugs need test cases running the
9pfs 'local' fs driver:

https://bugs.launchpad.net/qemu/+bug/1336794
https://bugs.launchpad.net/qemu/+bug/1877384

This patch set for that reason introduces 9pfs test cases using the 9pfs
'local' filesystem driver along to the already existing tests on 'synth'.

----------------------------------------------------------------
Christian Schoenebeck (5):
      tests/9pfs: change qtest name prefix to synth
      tests/9pfs: introduce local tests
      tests/9pfs: wipe local 9pfs test directory
      tests/9pfs: add virtio_9p_test_path()
      tests/9pfs: add local Tmkdir test

 tests/qtest/libqos/virtio-9p.c | 100 +++++++++++++++++++++
 tests/qtest/libqos/virtio-9p.h |  10 +++
 tests/qtest/virtio-9p-test.c   | 197 ++++++++++++++++++++++++++++++++++++-----
 3 files changed, 286 insertions(+), 21 deletions(-)

Re: [PULL v2 0/5] 9p queue (previous 2020-10-15)
Posted by Peter Maydell 3 years, 5 months ago
On Sat, 17 Oct 2020 at 15:23, Christian Schoenebeck
<qemu_oss@crudebyte.com> wrote:
>
> The following changes since commit e12ce85b2c79d83a340953291912875c30b3af06:
>
>   Merge remote-tracking branch 'remotes/ehabkost/tags/x86-next-pull-request' into staging (2020-10-16 22:46:28 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/cschoenebeck/qemu.git tags/pull-9p-20201017
>
> for you to fetch changes up to fa4551e3f4416cc8c62086ac430b1ceb4f03eb6b:
>
>   tests/9pfs: add local Tmkdir test (2020-10-17 15:58:39 +0200)
>
> ----------------------------------------------------------------
> 9pfs: add tests using local fs driver
>
> The currently existing 9pfs test cases are all solely using the 9pfs 'synth'
> fileystem driver, which is a very simple and purely simulated (in RAM only)
> filesystem. There are issues though where the 'synth' fs driver is not
> sufficient. For example the following two bugs need test cases running the
> 9pfs 'local' fs driver:
>
> https://bugs.launchpad.net/qemu/+bug/1336794
> https://bugs.launchpad.net/qemu/+bug/1877384
>
> This patch set for that reason introduces 9pfs test cases using the 9pfs
> 'local' filesystem driver along to the already existing tests on 'synth'.

This emits a lot of new warnings during 'make check':

PASS 27 qtest-arm: qos-test
/arm/virt/virtio-mmio/virtio-bus/virtio-9p-device/virtio-9p/virtio-9p-tests/local/config
qemu-system-arm: warning: 9p: degraded performance: a reasonable high
msize should be chosen on client/guest side (chosen msize is <= 8192).
See https://wiki.qemu.org/Documentation/9psetup#msize for details.
PASS 28 qtest-arm: qos-test
/arm/virt/virtio-mmio/virtio-bus/virtio-9p-device/virtio-9p/virtio-9p-tests/local/create_dir

PASS 54 qtest-i386: qos-test
/i386/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/local/config
qemu-system-i386: warning: 9p: degraded performance: a reasonable high
msize should be chosen on client/guest side (chosen msize is <= 8192).
See https://wiki.qemu.org/Documentation/9psetup#msize for details.
PASS 55 qtest-i386: qos-test
/i386/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/local/create_dir

etc.

thanks
-- PMM

Re: [PULL v2 0/5] 9p queue (previous 2020-10-15)
Posted by Christian Schoenebeck 3 years, 5 months ago
On Montag, 19. Oktober 2020 11:52:38 CEST Peter Maydell wrote:
> On Sat, 17 Oct 2020 at 15:23, Christian Schoenebeck
> 
> <qemu_oss@crudebyte.com> wrote:
> > The following changes since commit 
e12ce85b2c79d83a340953291912875c30b3af06:
> >   Merge remote-tracking branch
> >   'remotes/ehabkost/tags/x86-next-pull-request' into staging (2020-10-16
> >   22:46:28 +0100)> 
> > are available in the Git repository at:
> >   https://github.com/cschoenebeck/qemu.git tags/pull-9p-20201017
> > 
> > for you to fetch changes up to fa4551e3f4416cc8c62086ac430b1ceb4f03eb6b:
> >   tests/9pfs: add local Tmkdir test (2020-10-17 15:58:39 +0200)
> > 
> > ----------------------------------------------------------------
> > 9pfs: add tests using local fs driver
> > 
> > The currently existing 9pfs test cases are all solely using the 9pfs
> > 'synth' fileystem driver, which is a very simple and purely simulated (in
> > RAM only) filesystem. There are issues though where the 'synth' fs driver
> > is not sufficient. For example the following two bugs need test cases
> > running the 9pfs 'local' fs driver:
> > 
> > https://bugs.launchpad.net/qemu/+bug/1336794
> > https://bugs.launchpad.net/qemu/+bug/1877384
> > 
> > This patch set for that reason introduces 9pfs test cases using the 9pfs
> > 'local' filesystem driver along to the already existing tests on 'synth'.
> 
> This emits a lot of new warnings during 'make check':
> 
> PASS 27 qtest-arm: qos-test
> /arm/virt/virtio-mmio/virtio-bus/virtio-9p-device/virtio-9p/virtio-9p-tests/
> local/config qemu-system-arm: warning: 9p: degraded performance: a
> reasonable high msize should be chosen on client/guest side (chosen msize
> is <= 8192). See https://wiki.qemu.org/Documentation/9psetup#msize for
> details. PASS 28 qtest-arm: qos-test
> /arm/virt/virtio-mmio/virtio-bus/virtio-9p-device/virtio-9p/virtio-9p-tests/
> local/create_dir
> 
> PASS 54 qtest-i386: qos-test
> /i386/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p
> -tests/local/config qemu-system-i386: warning: 9p: degraded performance: a
> reasonable high msize should be chosen on client/guest side (chosen msize
> is <= 8192). See https://wiki.qemu.org/Documentation/9psetup#msize for
> details. PASS 55 qtest-i386: qos-test
> /i386/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p
> -tests/local/create_dir
> 
> etc.
> 
> thanks
> -- PMM

One warning per test suite run (i.e. per architecture due to 
warn_report_once()), yes. That performance warning is meant for end user 
installations to remind them setting some (reasonable high) value for 9p 
client parameter 'msize' on guest OS side. The warning triggers here because 
the 9p test cases intentionally run with a small 'msize' to guard edge cases.

Would it be Ok for you to merge it with this performance warning for now? I 
can take care of silencing it before 5.2 release. It probably requires to 
introduce a new CL option to suppress performance warnings like these, or by 
finding a way to detect that we're currently just running qtests.

Best regards,
Christian Schoenebeck



Re: [PULL v2 0/5] 9p queue (previous 2020-10-15)
Posted by Peter Maydell 3 years, 5 months ago
On Mon, 19 Oct 2020 at 11:19, Christian Schoenebeck
<qemu_oss@crudebyte.com> wrote:
>
> On Montag, 19. Oktober 2020 11:52:38 CEST Peter Maydell wrote:
> > This emits a lot of new warnings during 'make check':
> >
> > PASS 27 qtest-arm: qos-test
> > /arm/virt/virtio-mmio/virtio-bus/virtio-9p-device/virtio-9p/virtio-9p-tests/
> > local/config qemu-system-arm: warning: 9p: degraded performance: a
> > reasonable high msize should be chosen on client/guest side (chosen msize
> > is <= 8192). See https://wiki.qemu.org/Documentation/9psetup#msize for
> > details. PASS 28 qtest-arm: qos-test
> > /arm/virt/virtio-mmio/virtio-bus/virtio-9p-device/virtio-9p/virtio-9p-tests/
> > local/create_dir
> >
> > PASS 54 qtest-i386: qos-test
> > /i386/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p
> > -tests/local/config qemu-system-i386: warning: 9p: degraded performance: a
> > reasonable high msize should be chosen on client/guest side (chosen msize
> > is <= 8192). See https://wiki.qemu.org/Documentation/9psetup#msize for
> > details. PASS 55 qtest-i386: qos-test
> > /i386/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p
> > -tests/local/create_dir

> One warning per test suite run (i.e. per architecture due to
> warn_report_once()), yes. That performance warning is meant for end user
> installations to remind them setting some (reasonable high) value for 9p
> client parameter 'msize' on guest OS side. The warning triggers here because
> the 9p test cases intentionally run with a small 'msize' to guard edge cases.
>
> Would it be Ok for you to merge it with this performance warning for now? I
> can take care of silencing it before 5.2 release. It probably requires to
> introduce a new CL option to suppress performance warnings like these, or by
> finding a way to detect that we're currently just running qtests.

The usual approach is to suppress this kind of warning by guarding
it with if (qtest_enabled()) { ... }.

I'm generally reluctant to allow new warnings in, because then they
never go away and my scripts build up a long list of "ignore this
particular warning" exceptions.

thanks
-- PMM

Re: [PULL v2 0/5] 9p queue (previous 2020-10-15)
Posted by Christian Schoenebeck 3 years, 5 months ago
On Montag, 19. Oktober 2020 12:22:47 CEST Peter Maydell wrote:
> On Mon, 19 Oct 2020 at 11:19, Christian Schoenebeck
> 
> <qemu_oss@crudebyte.com> wrote:
> > On Montag, 19. Oktober 2020 11:52:38 CEST Peter Maydell wrote:
> > > This emits a lot of new warnings during 'make check':
> > > 
> > > PASS 27 qtest-arm: qos-test
> > > /arm/virt/virtio-mmio/virtio-bus/virtio-9p-device/virtio-9p/virtio-9p-te
> > > sts/ local/config qemu-system-arm: warning: 9p: degraded performance: a
> > > reasonable high msize should be chosen on client/guest side (chosen
> > > msize is <= 8192). See
> > > https://wiki.qemu.org/Documentation/9psetup#msize for details. PASS 28
> > > qtest-arm: qos-test
> > > /arm/virt/virtio-mmio/virtio-bus/virtio-9p-device/virtio-9p/virtio-9p-te
> > > sts/ local/create_dir
> > > 
> > > PASS 54 qtest-i386: qos-test
> > > /i386/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virti
> > > o-9p -tests/local/config qemu-system-i386: warning: 9p: degraded
> > > performance: a reasonable high msize should be chosen on client/guest
> > > side (chosen msize is <= 8192). See
> > > https://wiki.qemu.org/Documentation/9psetup#msize for details. PASS 55
> > > qtest-i386: qos-test
> > > /i386/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virti
> > > o-9p -tests/local/create_dir
> > 
> > One warning per test suite run (i.e. per architecture due to
> > warn_report_once()), yes. That performance warning is meant for end user
> > installations to remind them setting some (reasonable high) value for 9p
> > client parameter 'msize' on guest OS side. The warning triggers here
> > because the 9p test cases intentionally run with a small 'msize' to guard
> > edge cases.
> > 
> > Would it be Ok for you to merge it with this performance warning for now?
> > I
> > can take care of silencing it before 5.2 release. It probably requires to
> > introduce a new CL option to suppress performance warnings like these, or
> > by finding a way to detect that we're currently just running qtests.
> 
> The usual approach is to suppress this kind of warning by guarding
> it with if (qtest_enabled()) { ... }.

Ah, didn't see that one. I do that and resend, thanks!

Best regards,
Christian Schoenebeck