[kvm-unit-tests v2 0/2] README: Markdown fixes and improvements

[kvm-unit-tests v2 0/2] README: Markdown fixes and improvements

[Wainer dos Santos Moschetta]

This series does fix issues with formatting of the README's
markdown. On patch 02 is added a new section that describes
the test configuration file.

Changes v1 -> v2:
 - Keep markdown minimal while still fixing the formatting issues [drjones]
 - Sent new content on separate patch (added patch 02) [drjones]

v1: https://lore.kernel.org/kvm/20200116212054.4041-1-wainersm@redhat.com

Wainer dos Santos Moschetta (2):
  README: Fix markdown formatting
  README: Add intro about the configuration file

 README.md | 83 +++++++++++++++++++++++++++++++++----------------------
 1 file changed, 50 insertions(+), 33 deletions(-)

-- 
2.23.0

[kvm-unit-tests v2 1/2] README: Fix markdown formatting

[Wainer dos Santos Moschetta]

There are formatting issues that prevent README.md
from being rendered correctly in a browser. This
patch fixes the following categories of issues:

- blocks which aren't indented correctly;
- texts wrapped in <> which need escape, or
  be replaced with another thing.

Also some inline commands are marked with ``.

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
---
 README.md | 72 ++++++++++++++++++++++++++++++-------------------------
 1 file changed, 39 insertions(+), 33 deletions(-)

diff --git a/README.md b/README.md
index 1a9a4ab..367c92a 100644
--- a/README.md
+++ b/README.md
@@ -13,12 +13,11 @@ To create the test images do:
     ./configure
     make
 
-in this directory. Test images are created in ./<ARCH>/*.flat
+in this directory. Test images are created in ./ARCH/\*.flat
 
 ## Standalone tests
 
-The tests can be built as standalone
-To create and use standalone tests do:
+The tests can be built as standalone. To create and use standalone tests do:
 
     ./configure
     make standalone
@@ -26,7 +25,7 @@ To create and use standalone tests do:
     (go to somewhere)
     ./some-test
 
-'make install' will install all tests in PREFIX/share/kvm-unit-tests/tests,
+`make install` will install all tests in PREFIX/share/kvm-unit-tests/tests,
 each as a standalone test.
 
 
@@ -42,39 +41,40 @@ or:
 
 to run them all.
 
-To select a specific qemu binary, specify the QEMU=<path>
+By default the runner script searches for a suitable qemu binary in the system.
+To select a specific qemu binary though, specify the QEMU=path/to/binary
 environment variable:
 
     QEMU=/tmp/qemu/x86_64-softmmu/qemu-system-x86_64 ./x86-run ./x86/msr.flat
 
 To select an accelerator, for example "kvm" or "tcg", specify the
-ACCEL=<name> environment variable:
+ACCEL=name environment variable:
 
     ACCEL=kvm ./x86-run ./x86/msr.flat
 
 # Unit test inputs
 
-Unit tests use QEMU's '-append <args...>' parameter for command line
+Unit tests use QEMU's '-append args...' parameter for command line
 inputs, i.e. all args will be available as argv strings in main().
 Additionally a file of the form
 
-KEY=VAL
-KEY2=VAL
-...
+    KEY=VAL
+    KEY2=VAL
+    ...
 
-may be passed with '-initrd <file>' to become the unit test's environ,
-which can then be accessed in the usual ways, e.g. VAL = getenv("KEY")
-Any key=val strings can be passed, but some have reserved meanings in
+may be passed with '-initrd file' to become the unit test's environ,
+which can then be accessed in the usual ways, e.g. VAL = getenv("KEY").
+ Any key=val strings can be passed, but some have reserved meanings in
 the framework. The list of reserved environment variables is below
 
- QEMU_ACCEL            ... either kvm or tcg
- QEMU_VERSION_STRING   ... string of the form `qemu -h | head -1`
- KERNEL_VERSION_STRING ... string of the form `uname -r`
+    QEMU_ACCEL                   either kvm or tcg
+    QEMU_VERSION_STRING          string of the form `qemu -h | head -1`
+    KERNEL_VERSION_STRING        string of the form `uname -r`
 
 Additionally these self-explanatory variables are reserved
 
- QEMU_MAJOR, QEMU_MINOR, QEMU_MICRO, KERNEL_VERSION, KERNEL_PATCHLEVEL,
- KERNEL_SUBLEVEL, KERNEL_EXTRAVERSION
+    QEMU_MAJOR, QEMU_MINOR, QEMU_MICRO, KERNEL_VERSION, KERNEL_PATCHLEVEL,
+    KERNEL_SUBLEVEL, KERNEL_EXTRAVERSION
 
 # Guarding unsafe tests
 
@@ -85,42 +85,48 @@ host. kvm-unit-tests provides two ways to handle tests like those.
     unittests.cfg file. When a unit test is in the nodefault group
     it is only run when invoked
 
-    a) independently, arch-run arch/test
-    b) by specifying any other non-nodefault group it is in,
-       groups = nodefault,mygroup : ./run_tests.sh -g mygroup
-    c) by specifying all tests should be run, ./run_tests.sh -a
+     a) independently, `ARCH-run ARCH/test`
+
+     b) by specifying any other non-nodefault group it is in,
+        groups = nodefault,mygroup : `./run_tests.sh -g mygroup`
+
+     c) by specifying all tests should be run, `./run_tests.sh -a`
 
  2) Making the test conditional on errata in the code,
+    ```
     if (ERRATA(abcdef012345)) {
         do_unsafe_test();
     }
+    ```
 
     With the errata condition the unsafe unit test is only run
     when
 
-    a) the ERRATA_abcdef012345 environ variable is provided and 'y'
-    b) the ERRATA_FORCE environ variable is provided and 'y'
-    c) by specifying all tests should be run, ./run_tests.sh -a
+    a) the ERRATA_abcdef012345 environment variable is provided and 'y'
+
+    b) the ERRATA_FORCE environment variable is provided and 'y'
+
+    c) by specifying all tests should be run, `./run_tests.sh -a`
        (The -a switch ensures the ERRATA_FORCE is provided and set
         to 'y'.)
 
-The errata.txt file provides a mapping of the commits needed by errata
+The ./errata.txt file provides a mapping of the commits needed by errata
 conditionals to their respective minimum kernel versions. By default,
 when the user does not provide an environ, then an environ generated
-from the errata.txt file and the host's kernel version is provided to
+from the ./errata.txt file and the host's kernel version is provided to
 all unit tests.
 
 # Contributing
 
 ## Directory structure
 
-    .:				configure script, top-level Makefile, and run_tests.sh
-    ./scripts:		helper scripts for building and running tests
-    ./lib:			general architecture neutral services for the tests
-    ./lib/<ARCH>:	architecture dependent services for the tests
-    ./<ARCH>:		the sources of the tests and the created objects/images
+    .:                  configure script, top-level Makefile, and run_tests.sh
+    ./scripts:          helper scripts for building and running tests
+    ./lib:              general architecture neutral services for the tests
+    ./lib/<ARCH>:       architecture dependent services for the tests
+    ./<ARCH>:           the sources of the tests and the created objects/images
 
-See <ARCH>/README for architecture specific documentation.
+See ./ARCH/README for architecture specific documentation.
 
 ## Style
 
-- 
2.23.0

[kvm-unit-tests v2 2/2] README: Add intro about the configuration file

[Wainer dos Santos Moschetta]

The 'Guarding unsafe tests' section mention the unittests.cfg
file which was never introduced before. In this change
it was added a section with a few words about the tests
configuration file (unittests.cfg).

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
---
 README.md | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/README.md b/README.md
index 367c92a..763759e 100644
--- a/README.md
+++ b/README.md
@@ -52,6 +52,17 @@ ACCEL=name environment variable:
 
     ACCEL=kvm ./x86-run ./x86/msr.flat
 
+# Tests configuration file
+
+The test case may need specific runtime configurations, for
+example, extra QEMU parameters and time to execute limited, the
+runner script reads those information from a configuration file found
+at ./ARCH/unittests.cfg.
+
+The configuration file also contain the groups (if any) each test belong
+to. So that a given group can be executed by specifying its name in the
+runner's -g option.
+
 # Unit test inputs
 
 Unit tests use QEMU's '-append args...' parameter for command line
-- 
2.23.0

Re: [kvm-unit-tests v2 1/2] README: Fix markdown formatting

[Andrew Jones]

On Mon, Jan 20, 2020 at 04:43:09PM -0300, Wainer dos Santos Moschetta wrote:
> There are formatting issues that prevent README.md
> from being rendered correctly in a browser. This
> patch fixes the following categories of issues:
> 
> - blocks which aren't indented correctly;
> - texts wrapped in <> which need escape, or
>   be replaced with another thing.
> 
> Also some inline commands are marked with ``.
> 
> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> ---
>  README.md | 72 ++++++++++++++++++++++++++++++-------------------------
>  1 file changed, 39 insertions(+), 33 deletions(-)

Thanks

Reviewed-by: Andrew Jones <drjones@redhat.com>

But, I still have a few comments below.

> 
> diff --git a/README.md b/README.md
> index 1a9a4ab..367c92a 100644
> --- a/README.md
> +++ b/README.md
> @@ -13,12 +13,11 @@ To create the test images do:
>      ./configure
>      make
>  
> -in this directory. Test images are created in ./<ARCH>/*.flat
> +in this directory. Test images are created in ./ARCH/\*.flat
>  
>  ## Standalone tests
>  
> -The tests can be built as standalone
> -To create and use standalone tests do:
> +The tests can be built as standalone. To create and use standalone tests do:

I'd prefer we use two spaces after the period for the sentence punctuation,
like many kernel documents have.

>  
>      ./configure
>      make standalone
> @@ -26,7 +25,7 @@ To create and use standalone tests do:
>      (go to somewhere)
>      ./some-test
>  
> -'make install' will install all tests in PREFIX/share/kvm-unit-tests/tests,
> +`make install` will install all tests in PREFIX/share/kvm-unit-tests/tests,
>  each as a standalone test.
>  
>  
> @@ -42,39 +41,40 @@ or:
>  
>  to run them all.
>  
> -To select a specific qemu binary, specify the QEMU=<path>
> +By default the runner script searches for a suitable qemu binary in the system.
> +To select a specific qemu binary though, specify the QEMU=path/to/binary
>  environment variable:
>  
>      QEMU=/tmp/qemu/x86_64-softmmu/qemu-system-x86_64 ./x86-run ./x86/msr.flat
>  
>  To select an accelerator, for example "kvm" or "tcg", specify the
> -ACCEL=<name> environment variable:
> +ACCEL=name environment variable:
>  
>      ACCEL=kvm ./x86-run ./x86/msr.flat
>  
>  # Unit test inputs
>  
> -Unit tests use QEMU's '-append <args...>' parameter for command line
> +Unit tests use QEMU's '-append args...' parameter for command line
>  inputs, i.e. all args will be available as argv strings in main().
>  Additionally a file of the form
>  
> -KEY=VAL
> -KEY2=VAL
> -...
> +    KEY=VAL
> +    KEY2=VAL
> +    ...
>  
> -may be passed with '-initrd <file>' to become the unit test's environ,
> -which can then be accessed in the usual ways, e.g. VAL = getenv("KEY")
> -Any key=val strings can be passed, but some have reserved meanings in
> +may be passed with '-initrd file' to become the unit test's environ,
> +which can then be accessed in the usual ways, e.g. VAL = getenv("KEY").
> + Any key=val strings can be passed, but some have reserved meanings in

Looks like an unintentional space added before the 'Any'. Actually maybe
it's time to reformat all paragraphs. I'd like each line to have a length
of +/- 76 characters.

>  the framework. The list of reserved environment variables is below
>  
> - QEMU_ACCEL            ... either kvm or tcg
> - QEMU_VERSION_STRING   ... string of the form `qemu -h | head -1`
> - KERNEL_VERSION_STRING ... string of the form `uname -r`
> +    QEMU_ACCEL                   either kvm or tcg
> +    QEMU_VERSION_STRING          string of the form `qemu -h | head -1`
> +    KERNEL_VERSION_STRING        string of the form `uname -r`
>  
>  Additionally these self-explanatory variables are reserved
>  
> - QEMU_MAJOR, QEMU_MINOR, QEMU_MICRO, KERNEL_VERSION, KERNEL_PATCHLEVEL,
> - KERNEL_SUBLEVEL, KERNEL_EXTRAVERSION
> +    QEMU_MAJOR, QEMU_MINOR, QEMU_MICRO, KERNEL_VERSION, KERNEL_PATCHLEVEL,
> +    KERNEL_SUBLEVEL, KERNEL_EXTRAVERSION
>  
>  # Guarding unsafe tests
>  
> @@ -85,42 +85,48 @@ host. kvm-unit-tests provides two ways to handle tests like those.
>      unittests.cfg file. When a unit test is in the nodefault group
>      it is only run when invoked
>  
> -    a) independently, arch-run arch/test
> -    b) by specifying any other non-nodefault group it is in,
> -       groups = nodefault,mygroup : ./run_tests.sh -g mygroup
> -    c) by specifying all tests should be run, ./run_tests.sh -a
> +     a) independently, `ARCH-run ARCH/test`
> +
> +     b) by specifying any other non-nodefault group it is in,
> +        groups = nodefault,mygroup : `./run_tests.sh -g mygroup`
> +
> +     c) by specifying all tests should be run, `./run_tests.sh -a`
>  
>   2) Making the test conditional on errata in the code,
> +    ```
>      if (ERRATA(abcdef012345)) {
>          do_unsafe_test();
>      }
> +    ```
>  
>      With the errata condition the unsafe unit test is only run
>      when
>  
> -    a) the ERRATA_abcdef012345 environ variable is provided and 'y'
> -    b) the ERRATA_FORCE environ variable is provided and 'y'
> -    c) by specifying all tests should be run, ./run_tests.sh -a
> +    a) the ERRATA_abcdef012345 environment variable is provided and 'y'
> +
> +    b) the ERRATA_FORCE environment variable is provided and 'y'
> +
> +    c) by specifying all tests should be run, `./run_tests.sh -a`
>         (The -a switch ensures the ERRATA_FORCE is provided and set
>          to 'y'.)
>  
> -The errata.txt file provides a mapping of the commits needed by errata
> +The ./errata.txt file provides a mapping of the commits needed by errata
>  conditionals to their respective minimum kernel versions. By default,
>  when the user does not provide an environ, then an environ generated
> -from the errata.txt file and the host's kernel version is provided to
> +from the ./errata.txt file and the host's kernel version is provided to
>  all unit tests.
>  
>  # Contributing
>  
>  ## Directory structure
>  
> -    .:				configure script, top-level Makefile, and run_tests.sh
> -    ./scripts:		helper scripts for building and running tests
> -    ./lib:			general architecture neutral services for the tests
> -    ./lib/<ARCH>:	architecture dependent services for the tests
> -    ./<ARCH>:		the sources of the tests and the created objects/images
> +    .:                  configure script, top-level Makefile, and run_tests.sh
> +    ./scripts:          helper scripts for building and running tests
> +    ./lib:              general architecture neutral services for the tests
> +    ./lib/<ARCH>:       architecture dependent services for the tests
> +    ./<ARCH>:           the sources of the tests and the created objects/images
>  
> -See <ARCH>/README for architecture specific documentation.
> +See ./ARCH/README for architecture specific documentation.
>  
>  ## Style
>  
> -- 
> 2.23.0
>

Besides the space before 'Any' this patch is fine, which is why I gave the
r-b. I can do a reformatting patch on top of this myself for my other
comments. However if you're going to respin this, then please consider
reformatting the line lengths and the sentence punctuation. Also please
changing occurrences of "qemu" and "kvm" to "QEMU" and "KVM" when they are
being used as names, rather than parts of paths. E.g.

 The KVM test suite is in kvm-unit-tests.
 We can run QEMU with qemu-system-x86_64.

Thanks,
drew

Re: [kvm-unit-tests v2 2/2] README: Add intro about the configuration file

[Andrew Jones]

On Mon, Jan 20, 2020 at 04:43:10PM -0300, Wainer dos Santos Moschetta wrote:
> The 'Guarding unsafe tests' section mention the unittests.cfg
> file which was never introduced before. In this change
> it was added a section with a few words about the tests
> configuration file (unittests.cfg).
> 
> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> ---
>  README.md | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/README.md b/README.md
> index 367c92a..763759e 100644
> --- a/README.md
> +++ b/README.md
> @@ -52,6 +52,17 @@ ACCEL=name environment variable:
>  
>      ACCEL=kvm ./x86-run ./x86/msr.flat
>  
> +# Tests configuration file
> +
> +The test case may need specific runtime configurations, for

s/The test case/Test cases/

> +example, extra QEMU parameters and time to execute limited, the

s,and time to execute limited,and/or timeouts,

> +runner script reads those information from a configuration file found

s/those/that/

> +at ./ARCH/unittests.cfg.
> +
> +The configuration file also contain the groups (if any) each test belong

s/contain/contains/
s/belong/belongs/

> +to. So that a given group can be executed by specifying its name in the

s/So that a/A/
s/can/may/

> +runner's -g option.
> +
>  # Unit test inputs
>  
>  Unit tests use QEMU's '-append args...' parameter for command line
> -- 
> 2.23.0
> 

Thanks,
drew

Re: [kvm-unit-tests v2 1/2] README: Fix markdown formatting

[Paolo Bonzini]

On 21/01/20 10:18, Andrew Jones wrote:
> Besides the space before 'Any' this patch is fine, which is why I gave the
> r-b. I can do a reformatting patch on top of this myself for my other
> comments. However if you're going to respin this, then please consider
> reformatting the line lengths and the sentence punctuation. Also please
> changing occurrences of "qemu" and "kvm" to "QEMU" and "KVM" when they are
> being used as names, rather than parts of paths. E.g.
> 
>  The KVM test suite is in kvm-unit-tests.
>  We can run QEMU with qemu-system-x86_64.

Applied the suggested tweaks and queued, thanks to both of you!

Paolo