mbox series

[v3,00/11] 9pfs: add tests using local fs driver

Message ID cover.1601655308.git.qemu_oss@crudebyte.com
Headers show
Series 9pfs: add tests using local fs driver | expand

Message

Christian Schoenebeck Oct. 2, 2020, 4:15 p.m. UTC
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'.
It consists of 3 parts:

1. libqos patches 1 and 2 remove a limitation of the qtest/libqos subsystem:
   support for more than one device using the same (official) QEMU device
   name.

2. Patches 3 to 6 enhance debugging issues with the qtest framework.

3. Patches 7 to 11 actually introduce 9pfs 'local' test cases using the qtest
   framework. These 'local' tests are adding a test directory 'qtest-9p-local'
   inside the current working directory (using get_current_dir()), which is
   typically the build directory, before running the tests. That test
   directory is automatically recreated next time the test suite is run again,
   to ensure the 9pfs 'local' tests always run consistently on a clean test
   directory. The test directory is used by the 'local' driver as root of its
   export path. So it will add/read/write/delete real files and directories
   inside that test directory.

v2->v3:

  * concat_path() is now just a wrapper for g_build_filename() (patch 8).

  * Dropped function strpr(), using g_strdup_printf() instead (patch 8, 9).

  * Fixed incorrect size for array allocation in split() function
    (patch 11).

v1->v2:

  * The libqos debugging features are now turned on by command line argument
    '--verbose' instead of using environment variables (patches 4, 5, 6).

  * The new 9pfs 'local' tests no longer depend on patches 1 and 2 by no
    longer using a libqos multi-device approach, but rather modifying the
    final QEMU command line for each test instead; see discussion of v1
    for reason (patches 7 to 11).

  * Use GCC_FMT_ATTR on helper function strpr() (patch 8).

  * Updated commit log comments.

Christian Schoenebeck (11):
  libqos/qgraph: add qemu_name to QOSGraphNode
  libqos/qgraph: add qos_node_create_driver_named()
  libqos/qgraph: add qos_dump_graph()
  tests/qtest/qos-test: dump qos graph if verbose
  tests/qtest/qos-test: dump environment variables if verbose
  tests/qtest/qos-test: dump QEMU command if verbose
  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/qgraph.c          | 108 ++++++++++++++-
 tests/qtest/libqos/qgraph.h          |  36 +++++
 tests/qtest/libqos/qgraph_internal.h |   1 +
 tests/qtest/libqos/virtio-9p.c       |  90 ++++++++++++
 tests/qtest/libqos/virtio-9p.h       |  10 ++
 tests/qtest/qos-test.c               |  15 +-
 tests/qtest/virtio-9p-test.c         | 197 ++++++++++++++++++++++++---
 7 files changed, 432 insertions(+), 25 deletions(-)

Comments

Christian Schoenebeck Oct. 8, 2020, 1:11 p.m. UTC | #1
On Donnerstag, 8. Oktober 2020 14:36:11 CEST Paolo Bonzini wrote:
> On 02/10/20 18:15, Christian Schoenebeck wrote:
> > If qtests are run in verbose mode (i.e. if --verbose CL argument
> > was provided) then print the assembled qemu command line for each
> > test.
> > 
> > Instead of using g_test_message() rather use printf() in combination
> > with g_test_verbose(), to avoid g_test_message() cluttering the
> > output.
> > 
> > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > ---
> > 
> >  tests/qtest/qos-test.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/tests/qtest/qos-test.c b/tests/qtest/qos-test.c
> > index fe240b32a7..b9f0942386 100644
> > --- a/tests/qtest/qos-test.c
> > +++ b/tests/qtest/qos-test.c
> > @@ -89,6 +89,9 @@ static void qos_set_machines_devices_available(void)
> > 
> >  static void restart_qemu_or_continue(char *path)
> >  {
> > 
> > +    if (g_test_verbose()) {
> > +        printf("Run QEMU with: '%s'\n", path);
> > +    }
> > 
> >      /* compares the current command line with the
> >      
> >       * one previously executed: if they are the same,
> >       * don't restart QEMU, if they differ, stop previous
> 
> This need not even be made verbose I think.

I actually find that handy in this way. By default I prefer the current output 
where I just see the name of the test currently running. I only need to see 
the actual QEMU command if there is some kind of problem to fix.

Best regards,
Christian Schoenebeck
Paolo Bonzini Oct. 8, 2020, 1:52 p.m. UTC | #2
On 08/10/20 15:42, Christian Schoenebeck wrote:
>>
>> I'm okay I guess with using g_test_message on 2.62 or newer, and
>> assuming people don't use --tap --verbose on older versions.
> Simpler solution: just appending '#' character in front of each printf() line, 
> that would be both fine for TAP and regular output:
> http://testanything.org/tap-specification.html#diagnostics

I'm not sure how it would be simpler than a

#if !GLIB_CHECK_VERSION(2, 62, 0)
#define qemu_test_message printf
#else
#define qemu_test_message g_test_message
#endif

but you choose.

Paolo