[pve-devel] [PATCH docs 1/3] Add man-page and notes about custom CPU models

[Aaron Lauterer]

a few small nits and some suggestions on the phrasing inline

On 6/18/20 11:05 AM, Stefan Reiter wrote:
> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
> ---
> 
> Requires qemu-server with patch 2 applied.
> 
>   Makefile                      |  1 +
>   cpu-models.conf.adoc          | 82 +++++++++++++++++++++++++++++++++++
>   gen-cpu-models.conf.5-opts.pl | 14 ++++++
>   qm.adoc                       | 11 +++++
>   4 files changed, 108 insertions(+)
>   create mode 100644 cpu-models.conf.adoc
>   create mode 100755 gen-cpu-models.conf.5-opts.pl
> 
> diff --git a/Makefile b/Makefile
> index ca56e96..98b6b44 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -53,6 +53,7 @@ GEN_SCRIPTS=					\
>   	gen-pct-network-opts.pl			\
>   	gen-pct-mountpoint-opts.pl		\
>   	gen-qm.conf.5-opts.pl			\
> +	gen-cpu-models.conf.5-opts.pl 		\
>   	gen-qm-cloud-init-opts.pl		\
>   	gen-vzdump.conf.5-opts.pl		\
>   	gen-pve-firewall-cluster-opts.pl	\
> diff --git a/cpu-models.conf.adoc b/cpu-models.conf.adoc
> new file mode 100644
> index 0000000..02f1834
> --- /dev/null
> +++ b/cpu-models.conf.adoc
> @@ -0,0 +1,82 @@
> +ifdef::manvolnum[]
> +cpu-models.conf(5)
> +==================
> +:pve-toplevel:
> +
> +NAME
> +----
> +
> +cpu-models.conf - Custom CPU model configuration file
> +
> +
> +SYNOPSIS
> +--------
> +
> +'/etc/pve/virtual-guest/cpu-models.conf'
> +
> +
> +DESCRIPTION
> +-----------
> +endif::manvolnum[]
> +
> +ifndef::manvolnum[]
> +Custom CPU Model Configuration
> +==============================
> +endif::manvolnum[]
> +ifdef::wiki[]
> +:pve-toplevel:
> +:title: Manual: cpu-models.conf
> +endif::wiki[]
> +
> +The `/etc/pve/virtual-guest/cpu-models.conf` file stores custom CPU
> +models, which can be used by VMs to get access to advanced CPU
> +features (e.g. custom CPU flags).
> +
> +
> +File Format
> +-----------
> +
> +CPU models each have their own section in the file, beginning with
> +the header:
> +
> + cpu-model: <name>
> +
> +Note that <name> does not include the 'custom-' prefix, which is
> +required in VM configs to denote custom CPU models.
> +
> +E.g. when <name> is 'example', a VM can use the model as
> +'custom-example'.

Try to avoid abbreviations like `E.g.` and use `For example`.

For that sentence in general I would suggest to rewrite it a bit to 
something like:

"For example, if the <name> is 'foobar', the CPU for a VM needs to be 
configured as 'custom-foobar'."


> +
> +Each section can specify options indented by one TAB character
> +or multiple spaces and seperated from the value with a space, e.g.:

s/seperated/separated/

I would like to split up that long sentence a bit.

What about the following? I hope I don't change the meaning:

"Each section can specify options. They are indented by either one TAB 
character or multiple spaces. The option and its value is separated by 
one space, for example:"

> +
> + reported-model qemu64
> +
> +See below for all available options.
> +
> +Blank lines in the file are ignored, and lines starting with a `#`
> +character are treated as comments and are also ignored.

Maybe split it up into shorter sentences as well:

s/, and lines/. Lines/
> +
> +
> +Options
> +-------
> +
> +include::cpu-models.conf.5-opts.adoc[]
> +
> +
> +Example File
> +------------
> +
> +----
> +cpu-model: avx
> +    flags +avx;+avx2
> +    phys-bits host
> +    hidden 0
> +    hv-vendor-id proxmox
> +    reported-model kvm64
> +----
> +
> +
> +ifdef::manvolnum[]
> +include::pve-copyright.adoc[]
> +endif::manvolnum[]
> diff --git a/gen-cpu-models.conf.5-opts.pl b/gen-cpu-models.conf.5-opts.pl
> new file mode 100755
> index 0000000..7c35d69
> --- /dev/null
> +++ b/gen-cpu-models.conf.5-opts.pl
> @@ -0,0 +1,14 @@
> +#!/usr/bin/perl
> +
> +use lib '.';
> +use strict;
> +use warnings;
> +use PVE::RESTHandler;
> +use PVE::QemuServer::CPUConfig;
> +
> +my $prop = PVE::QemuServer::CPUConfig::add_cpu_json_properties({});
> +
> +# cputype is given as section header and explained seperately
> +delete $prop->{cputype};
> +
> +print PVE::RESTHandler::dump_properties($prop);
> diff --git a/qm.adoc b/qm.adoc
> index cd6641f..f94140f 100644
> --- a/qm.adoc
> +++ b/qm.adoc
> @@ -352,6 +352,17 @@ the kvm64 default. If you don’t care about live migration or have a homogeneou
>   cluster where all nodes have the same CPU, set the CPU type to host, as in
>   theory this will give your guests maximum performance.
>   
> +Custom CPU Types
> +^^^^^^^^^^^^^^^^
> +
> +You can also specify custom CPU types with a configurable set of features. These

s/also //

> +are specified in the configuration file `/etc/pve/virtual-guest/cpu-models.conf`
> +by an administrator. See `man cpu-models.conf` for format details.
> +
> +Once specified, custom types can easily be selected via the GUI by any user with
> +`Sys.Audit` permissions on `/nodes` or via the CLI, though the latter (as well
> +as the API) requires the name to be prefixed with 'custom-'.

Hmm, what about:

"Specified custom types can be selected by any user with the `Sys.Audit` 
permissions on `/nodes`. When configuring a custom CPU type via the CLI 
or API the name needs to be prefixes with 'custom-'."

> +
>   Meltdown / Spectre related CPU flags
>   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   
>