Re: [pve-devel] [PATCH docs v8 3/7] added shared filesystem doc for virtio-fs

[Fiona Ebner <f.ebner@proxmox.com>]

IMHO the wording/word order with "shared filesystem doc" could be
improved. It's the doc/section for the shared filesystem virtio-fs.

Am 08.11.23 um 09:52 schrieb Markus Frank:
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> ---
>  qm.adoc | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 82 insertions(+), 2 deletions(-)
> 
> diff --git a/qm.adoc b/qm.adoc
> index c4f1024..571c42e 100644
> --- a/qm.adoc
> +++ b/qm.adoc
> @@ -996,6 +996,85 @@ recommended to always use a limiter to avoid guests using too many host
>  resources. If desired, a value of '0' for `max_bytes` can be used to disable
>  all limits.
>  
> +[[qm_virtiofs]]
> +Virtio-fs
> +~~~~~~~~~
> +
> +Virtio-fs is a shared file system, that enables sharing a directory between

I think the comma should not be here

> +host and guest VM while taking advantage of the locality of virtual machines

Maybe split the sentence, e.g. "guest VM. It takes advantage"

> +and the hypervisor to get a higher throughput than 9p.

I'd say "the 9p remote file system protocol" so that people hearing it
for the first time will have an idea too.

> +
> +To use virtio-fs, the https://gitlab.com/virtio-fs/virtiofsd[virtiofsd] daemon
> +needs to run in the background.
> +In {pve} this process starts immediately before the start of QEMU.

I think there should be a comma after {pve}. My thesis advisor taught me
the following trick that works in many cases: "If you can read it as a
stand-alone sentence, there should be a comma." For example, the "there
should be a comma" can be read as a stand-alone sentence ;)

Nit: since it's the same paragraph, there should be no newline before
this line.

> +
> +Linux VMs with kernel >=5.4 support this feature by default.
> +
> +There is a guide available on how to utilize virtio-fs in Windows VMs.
> +https://github.com/virtio-win/kvm-guest-drivers-windows/wiki/Virtiofs:-Shared-file-system
> +
> +Known limitations

L should be capitalized for title-case

> +^^^^^^^^^^^^^^^^^
> +
> +* Virtiofsd crashing means no recovery until VM is fully stopped
> +and restarted.
> +* Virtiofsd not responding may result in NFS-like hanging access in the VM.
> +* Memory hotplug does not work in combination with virtio-fs.

Out of interest: is this being worked on upstream, a fundamental design
limitation or a limitation on our side?

> +* Windows cannot understand ACLs. Therefore, disable it for Windows VMs,
> +otherwise the virtio-fs device will not be visible within the VMs.

If there is no corresponding warning for this in qemu-server yet, please
add one

> +
> +Add mapping for Shared Directories

M should be capitalized for title-case

> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +To add a mapping, either use the API directly with pvesh as described in the

Since it's a CLI command s/pvesh/`pvesh`/

I'd be inclined to repeat, i.e. "a mapping for a shared directory,"

> +xref:resource_mapping[Resource Mapping] section,

Style nit: no newline here

> +or add the mapping to the configuration file `/etc/pve/mapping/dir.cfg`:

I'd rather not suggest this. Manual editing will cause much more
problems, because there are no checks.

> +
> +----
> +some-dir-id
> +    map node=node1,path=/mnt/share/,submounts=1
> +    map node=node2,path=/mnt/share/,
> +    xattr 1
> +    acl 1
> +----
> +
> +Set `submounts` to `1` when multiple file systems are mounted in a
> +shared directory.

I suppose "and you want to make all available in the guest."

> +
> +Add virtio-fs to VM

to a VM

> +^^^^^^^^^^^^^^^^^^^
> +
> +To share a directory using virtio-fs, you need to specify the directory ID
> +(dirid) that has been configured in the Resource Mapping.

Specify it where? Maybe start out with mentioning the configuration
option, i.e. virtiofs<N> and then what parameters it takes?

"resource mapping" should not be capitalized

> +Additionally, you can set the `cache` option to either `always`, `never`,

Style nit: no newline before this line, it's the same paragraph

> +or `auto`, depending on your requirements.
> +If you want virtio-fs to honor the `O_DIRECT` flag, you can set the
> +`direct-io` parameter to `1`.

Please mention briefly what the implications for the different cache
settings and the direct-io flag are and what the defaults are.

> +Additionally it possible to overwrite the default mapping settings

"Additionally, it is"

> +for xattr & acl by setting then to either `1` or `0`.

`xattr` and `acl`
s/then/them/

> +
> +The `acl` parameter automatically implies `xattr`, that is, it makes no
> +difference whether you set xattr to `0` if acl is set to `1`.

This should be mentioned in the description of the parameter in the
schema too. Also, please use `xattr` and `acl` again.

> +
> +----
> +qm set <vmid> -virtiofs0 dirid=<dirid>,cache=always,direct-io=1
> +qm set <vmid> -virtiofs1 <dirid>,cache=never,xattr=1
> +qm set <vmid> -virtiofs2 <dirid>,acl=1
> +----
> +
> +To mount virtio-fs in a guest VM with the Linux kernel virtio-fs driver, run the
> +following command:

"command inside the guest:" to reduce the number of users trying it on
the host

> +
> +The dirid associated with the path on the current node is also used as the
> +mount tag (name used to mount the device on the guest).

Style nit: Early newline, "mount" fits the 80 character limit