README: Markdown fixes and improvements

PATCH In review Wainer dos Santos Moschetta 3 participants 6 messages Jan 20, 2020 – Jan 21, 2020 KVM

Share & tools

Wainer dos Santos Moschetta

author

2020-01-20 19:43 UTC #0 lore raw

v2 cover README: Markdown fixes and improvements

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/[email protected]

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(-)

Signature · 1 line

2.23.0

Wainer dos Santos Moschetta

author

2020-01-20 19:43 UTC #1 lore raw

PATCH v2 1/2 README: Fix markdown formatting

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 <[email protected]>

---
 README.md | 72 ++++++++++++++++++++++++++++++-------------------------
 1 file changed, 39 insertions(+), 33 deletions(-)

README.md +39 ~~-33~~

index 1a9a4ab..367c92a 100644
@@ -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

Signature · 1 line

2.23.0

Andrew Jones

Reviewed-by on v2 1/2

2020-01-21 09:18 UTC #2 lore raw

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

Quoted 8 lines · 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.

Quoted 6 lines

Also some inline commands are marked with ``.
Signed-off-by Wainer dos Santos Moschetta <[email protected]>
---
 README.md | 72 ++++++++++++++++++++++++++++++-------------------------
 1 file changed, 39 insertions(+), 33 deletions(-)

Thanks

Reviewed-by Andrew Jones <[email protected]>

But, I still have a few comments below.

Quoted 7 lines

README.md +0 -0

index 1a9a4ab..367c92a 100644
@@ -13,12 +13,11 @@ To create the test images do:
     ./configure

README.md To create the test images do: · line 20

Quoted 10 lines

README.md +2 -3

     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.

Quoted 39 lines

README.md +5 -7

     ./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
-...

README.md or: · line 67

Quoted 10 lines

README.md +6 -3

+    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.

Quoted 85 lines

README.md +26 ~~-20~~

 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

Signature · 1 line

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

Paolo Bonzini

maintainer

2020-01-21 12:20 UTC #3 lore raw

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

Quoted 9 lines · Andrew Jones

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

Wainer dos Santos Moschetta

author

2020-01-20 19:43 UTC #4 lore raw

PATCH v2 2/2 README: Add intro about the configuration file

In reply to 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 <[email protected]>

---
 README.md | 11 +++++++++++
 1 file changed, 11 insertions(+)

README.md +11 -0

index 367c92a..763759e 100644
@@ -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

Signature · 1 line

2.23.0

Andrew Jones

2020-01-21 09:22 UTC #5 lore raw

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

Quoted 15 lines · 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 <[email protected]>
---
 README.md | 11 +++++++++++
 1 file changed, 11 insertions(+)
README.md +0 -0
index 367c92a..763759e 100644

README.md ACCEL=name environment variable: · line 57

Quoted 6 lines

README.md +3 -0

@@ -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/

README.md ACCEL=name environment variable: · line 58

Quoted 1 line

README.md +1 -0

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

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

README.md ACCEL=name environment variable: · line 59

Quoted 1 line

README.md +1 -0

+runner script reads those information from a configuration file found

s/those/that/

README.md ACCEL=name environment variable: · line 62

Quoted 3 lines

README.md +3 -0

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

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

README.md ACCEL=name environment variable: · line 63

Quoted 1 line

README.md +1 -0

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

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

Quoted 8 lines

README.md +2 -0

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

Signature · 1 line

2.23.0

Thanks,
drew