Re: [pve-devel] [PATCH docs v4 0/6] feature #1027 virtio-9p/virtio-fs

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

thanks for working on this! it's a long-standing feature request and
implementing it will make quite a few people happy. also sorry for not
getting back at you in v2/3 already. there's some high level stuff that
I'll reply with here, and then some more concrete feedback on individual
patches.

there is some overlap (well, not so much atm, but I think there should
be more ;)) with Dominik's hardware map, so it might make sense to
coordinate. 

On April 25, 2023 12:21 pm, Markus Frank wrote:
> pve-docs:
> 
> Markus Frank (1):
>   added shared filesystem doc for virtio-fs & virtio-9p
> 
>  qm.adoc | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> 
> pve-access-control:
> 
> v3:
>  * replaced /dirs with /map/dirs/<dirid>
> 
> v2:
>  * admin gives access via an ACL (/dirs/<dirid>)
> 
> Markus Frank (1):
>   added acls for Shared Files Directories
> 
>  src/PVE/API2/Directory.pm | 68 +++++++++++++++++++++++++++++++++++++++
>  src/PVE/AccessControl.pm  | 16 +++++++++
>  src/PVE/RPCEnvironment.pm | 12 ++++++-
>  3 files changed, 95 insertions(+), 1 deletion(-)
>  create mode 100644 src/PVE/API2/Directory.pm
> 
> 
> pve-manager:
> 
> v4:
>  * moved extract_dir_path from qemu-server to DirConfig in pve-manager
> 
> v2:
>  * admins define dirs on the host that are eligibly for mounting into 
>   guests (<dirid>: /path/tp/share)

I think pve-manager is the wrong place for this.

the module hierarchy is

pve-manager - qemu-server/pve-container - pve-guest-common - pve-storage - pve-access-control - pve-common

(things to the right can use things to the left, but not the other way round)

there are only two places where we have intentional cycles:
pve-firewall <-> qemu-server/pve-container
ha-manager <-> qemu-server/pve-container

(and the doc generator, but that can be worked around and is not at
runtime ;))

so something like this, which needs to be used by both qemu-server and,
in the future, pve-container needs to go into one of the following:
- pve-guest-common
- pve-storage
- pve-common

common is out, as that sits below pve-cluster and pve-access-control.
that leaves either pve-guest-common or pve-storage.

you must not introduce code in pve-manager (like PVE::DirConfig) that is
needed in qemu-server.

besides this question of moving, in my mind, the following would be much
nicer:
- define dir entities cluster-wide (like storages)
-- id
-- optional default host path
-- ?
- allow limiting and overriding per node (so that dir "foo" can be
  backed by a path "/X" on one node, path "/Y" on another, and not be
  available at all on a third node)
- the only downside is editing requires a cluster-wide lock, but that is
  something that is not done frequently so it doesn't really hurt.

ideally this would follow the same scheme as the hardware map, since it
has quite similar semantics. I am not sure if we want to put it into the
hardware map, but it might be an option as well (it is kinda of passing
through a host dir, like passing through an USB or PCI device after
all). having it in the hardware map would possibly allow a common set of
helpers (like for finding out whether a given dir is available on a
node, and getting the correct config, or for ACL checks).

> Markus Frank (3):
>   added Config for Shared Filesystem Directories
>   added Shared Files tab in Node Settings
>   added options to add virtio-9p & virtio-fs Shared Filesystems to qemu
>     config
> 
>  PVE/API2/DirConfig.pm                | 129 +++++++++++++++++++
>  PVE/API2/Makefile                    |   1 +
>  PVE/API2/Nodes.pm                    |   6 +
>  PVE/DirConfig.pm                     | 155 +++++++++++++++++++++++
>  PVE/Makefile                         |   1 +
>  www/manager6/Makefile                |   2 +
>  www/manager6/Utils.js                |   1 +
>  www/manager6/data/PermPathStore.js   |   3 +
>  www/manager6/node/Config.js          |  12 ++
>  www/manager6/node/SharedFiles.js     | 177 +++++++++++++++++++++++++++
>  www/manager6/qemu/HardwareView.js    |  19 +++
>  www/manager6/qemu/SharedFilesEdit.js | 101 +++++++++++++++
>  12 files changed, 607 insertions(+)
>  create mode 100644 PVE/API2/DirConfig.pm
>  create mode 100644 PVE/DirConfig.pm
>  create mode 100644 www/manager6/node/SharedFiles.js
>  create mode 100644 www/manager6/qemu/SharedFilesEdit.js
> 
> 
> qemu-server:
> 
> v4:
>  * moved extract_dir_path from qemu-server to DirConfig
> 
> v3:
>  * created own socket and get file descriptor for virtiofsd
>  so there is no race between starting virtiofsd & qemu
>  * added TODO to replace virtiofsd with rust implementation in bookworm
>  (I packaged the rust implementation for bookworm & the C implementation
>  in qemu will be removed in qemu 8.0)
> 
> v2:
>  * replaced sharedfiles_fmt path in qemu-server with dirid:
>  * user can use the dirid to specify the directory without requiring root access
> 
> Markus Frank (1):
>   feature #1027: virtio-9p & virtio-fs support
> 
>  PVE/API2/Qemu.pm  |  19 +++++++
>  PVE/QemuServer.pm | 141 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 160 insertions(+)
> 
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
>