public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH qemu-server/manager/access-control/docs v2 0/6] feature #1027 virtio-9p/virtio-fs
@ 2022-12-23 13:10 Markus Frank
  2022-12-23 13:10 ` [pve-devel] [PATCH docs v2 1/6] added shared filesystem doc for virtio-fs & virtio-9p Markus Frank
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Markus Frank @ 2022-12-23 13:10 UTC (permalink / raw)
  To: pve-devel

v2:
replaced sharedfiles_fmt path in qemu-server with dirid:
- admin defines dirs on the host that are eligibly for mounting into 
  guests (<dirid>: /path/tp/share)
- admin gives access via an ACL (/dirs/<dirid>)
- user can then use the dirid to specify the directory
  without requiring root access


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:

Markus Frank (1):
  added acls for Shared Files Directories

 src/PVE/AccessControl.pm  |  2 ++
 src/PVE/RPCEnvironment.pm | 12 +++++++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)



pve-manager:

Markus Frank (3):
  added Config for Shared Files Directories
  added Shared Files tab in Node Settings
  added options to add virtio-9p & virtio-fs fileshare to qemu config

 PVE/API2/DirConfig.pm                | 129 +++++++++++++++++++
 PVE/API2/Makefile                    |   1 +
 PVE/API2/Nodes.pm                    |   6 +
 PVE/DirConfig.pm                     | 139 +++++++++++++++++++++
 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, 591 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:

Markus Frank (1):
  feature #1027: virtio-9p & virtio-fs support

 PVE/API2/Qemu.pm  |  20 ++++++-
 PVE/QemuServer.pm | 135 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 154 insertions(+), 1 deletion(-)

-- 
2.30.2





^ permalink raw reply	[flat|nested] 9+ messages in thread

* [pve-devel] [PATCH docs v2 1/6] added shared filesystem doc for virtio-fs & virtio-9p
  2022-12-23 13:10 [pve-devel] [PATCH qemu-server/manager/access-control/docs v2 0/6] feature #1027 virtio-9p/virtio-fs Markus Frank
@ 2022-12-23 13:10 ` Markus Frank
  2022-12-23 13:10 ` [pve-devel] [PATCH access-control v2 2/6] added acls for Shared Filesystem Directories Markus Frank
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Markus Frank @ 2022-12-23 13:10 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
 qm.adoc | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/qm.adoc b/qm.adoc
index 45ec17f..9a687f8 100644
--- a/qm.adoc
+++ b/qm.adoc
@@ -932,6 +932,67 @@ 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_sharedfiles]]
+Shared Filesystems
+~~~~~~~~~~~~~~~~~~
+
+add directories for Shared Filesystems
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+To add a directory either add a directory to the "Shared Files" list in the node
+config in the WebUI or add it to the /etc/pve/nodes/<nodename>/dirs config file
+like this:
+
+----
+<dirid>: /path/to/share
+----
+
+9pfs (virtio-9p)
+^^^^^^^^^^^^^^^^
+
+QEMU's 9pfs uses the Plan 9 Filesystem Protocol to share a directory on the host
+with a guest VM.
+
+To share a directory with 9p, run the following command:
+
+----
+qm set <vmid> -sharedfiles0 virtio-9p,dirid=<dirid>,tag=<mount tag>
+----
+
+To mount QEMU's 9pfs in a guest VM with the Linux kernel 9p driver, run the
+following command:
+
+----
+mount -t 9p -o trans=virtio,version=9p2000.L <mount tag> <mount point>
+----
+
+https://www.linux-kvm.org/page/9p_virtio
+
+https://wiki.qemu.org/Documentation/9psetup
+
+virtio-fs
+^^^^^^^^^
+
+Virtio-fs is a shared file system, that enables sharing between host and
+guest VM while taking advantage of the locality of virtual machines and the
+hypervisor to get a higher throughput than 9p.
+Numa must be disabled to use virtio-fs.
+
+To share a directory with virtio-fs, run the following command:
+
+----
+qm set <vmid> -sharedfiles0 virtio-fs,dirid=<dirid>,tag=<mount tag>
+----
+
+To mount virtio-fs in a guest VM with the Linux kernel virtiofs driver, run the
+following command:
+
+----
+mount -t virtiofs <mount tag> <mount point>
+----
+
+https://virtio-fs.gitlab.io/howto-qemu.html
+
 [[qm_bootorder]]
 Device Boot Order
 ~~~~~~~~~~~~~~~~~
-- 
2.30.2





^ permalink raw reply	[flat|nested] 9+ messages in thread

* [pve-devel] [PATCH access-control v2 2/6] added acls for Shared Filesystem Directories
  2022-12-23 13:10 [pve-devel] [PATCH qemu-server/manager/access-control/docs v2 0/6] feature #1027 virtio-9p/virtio-fs Markus Frank
  2022-12-23 13:10 ` [pve-devel] [PATCH docs v2 1/6] added shared filesystem doc for virtio-fs & virtio-9p Markus Frank
@ 2022-12-23 13:10 ` Markus Frank
  2023-01-16 15:45   ` Thomas Lamprecht
  2022-12-23 13:10 ` [pve-devel] [PATCH manager v2 3/6] added Config " Markus Frank
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Markus Frank @ 2022-12-23 13:10 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
 src/PVE/AccessControl.pm  |  2 ++
 src/PVE/RPCEnvironment.pm | 12 +++++++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
index a95d072..742304c 100644
--- a/src/PVE/AccessControl.pm
+++ b/src/PVE/AccessControl.pm
@@ -1221,6 +1221,8 @@ sub check_path {
 	|/storage/[[:alnum:]\.\-\_]+
 	|/vms
 	|/vms/[1-9][0-9]{2,}
+	|/dirs
+	|/dirs/[[:alnum:]\.\-\_]+
     )$!xs;
 }
 
diff --git a/src/PVE/RPCEnvironment.pm b/src/PVE/RPCEnvironment.pm
index 0ee2346..f8bbc56 100644
--- a/src/PVE/RPCEnvironment.pm
+++ b/src/PVE/RPCEnvironment.pm
@@ -187,10 +187,11 @@ sub compute_api_permission {
 	nodes => qr/Sys\.|Permissions\.Modify/,
 	sdn => qr/SDN\.|Permissions\.Modify/,
 	dc => qr/Sys\.Audit|SDN\./,
+	dirs => qr/Sys\.|Permissions\.Modify/,
     };
     map { $res->{$_} = {} } keys %$priv_re_map;
 
-    my $required_paths = ['/', '/nodes', '/access/groups', '/vms', '/storage', '/sdn'];
+    my $required_paths = ['/', '/nodes', '/access/groups', '/vms', '/storage', '/sdn', '/dirs'];
 
     my $checked_paths = {};
     foreach my $path (@$required_paths, keys %{$usercfg->{acl}}) {
@@ -240,6 +241,7 @@ sub get_effective_permissions {
 	'/sdn' => 1,
 	'/storage' => 1,
 	'/vms' => 1,
+	'/dirs' => 1,
     };
 
     my $cfg = $self->{user_cfg};
@@ -355,6 +357,14 @@ sub check_vm_perm {
     return $self->check_full($user, "/vms/$vmid", $privs, $any, $noerr);
 };
 
+sub check_dir_perm {
+    my ($self, $user, $dirid, $privs, $any, $noerr) = @_;
+
+    my $cfg = $self->{user_cfg};
+
+    return $self->check_full($user, "/dirs/$dirid", $privs, $any, $noerr);
+};
+
 sub is_group_member {
     my ($self, $group, $user) = @_;
 
-- 
2.30.2





^ permalink raw reply	[flat|nested] 9+ messages in thread

* [pve-devel] [PATCH manager v2 3/6] added Config for Shared Filesystem Directories
  2022-12-23 13:10 [pve-devel] [PATCH qemu-server/manager/access-control/docs v2 0/6] feature #1027 virtio-9p/virtio-fs Markus Frank
  2022-12-23 13:10 ` [pve-devel] [PATCH docs v2 1/6] added shared filesystem doc for virtio-fs & virtio-9p Markus Frank
  2022-12-23 13:10 ` [pve-devel] [PATCH access-control v2 2/6] added acls for Shared Filesystem Directories Markus Frank
@ 2022-12-23 13:10 ` Markus Frank
  2022-12-23 13:10 ` [pve-devel] [PATCH qemu-server v2 4/6] added Shared Files tab in Node Settings Markus Frank
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Markus Frank @ 2022-12-23 13:10 UTC (permalink / raw)
  To: pve-devel

and made an API Endpoint for getting, adding and removing
directories to the config.

Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
 PVE/API2/DirConfig.pm | 129 +++++++++++++++++++++++++++++++++++++++
 PVE/API2/Makefile     |   1 +
 PVE/API2/Nodes.pm     |   6 ++
 PVE/DirConfig.pm      | 139 ++++++++++++++++++++++++++++++++++++++++++
 PVE/Makefile          |   1 +
 5 files changed, 276 insertions(+)
 create mode 100644 PVE/API2/DirConfig.pm
 create mode 100644 PVE/DirConfig.pm

diff --git a/PVE/API2/DirConfig.pm b/PVE/API2/DirConfig.pm
new file mode 100644
index 00000000..f551f2d3
--- /dev/null
+++ b/PVE/API2/DirConfig.pm
@@ -0,0 +1,129 @@
+package PVE::API2::DirConfig;
+
+use strict;
+use warnings;
+
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::DirConfig;
+use PVE::Tools qw(extract_param);
+
+use base qw(PVE::RESTHandler);
+
+__PACKAGE__->register_method({
+    name => 'get_config',
+    path => '',
+    method => 'GET',
+    description => "Get Directories for Host Directory Sharing.",
+    permissions => {
+	check => ['perm', '/', [ 'Sys.Audit' ]],
+    },
+    proxyto => 'node',
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	},
+    },
+    returns => {
+	type => 'array',
+	items => {
+	    type => 'object',
+	    properties => {
+		dirid => {
+		    type => 'string',
+		    description => 'Directory ID',
+		},
+		path => {
+		    type => 'string',
+		    description => 'Host Directory Path',
+		},
+	    },
+	},
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $config = PVE::DirConfig::load_config($param->{node});
+	delete $config->{description};
+	my $result = [];
+	foreach my $key (keys %{$config}) {
+	    push @$result, {
+		dirid => $key,
+		path => $config->{$key},
+	    };
+	}
+
+	return $result;
+    }
+});
+
+__PACKAGE__->register_method({
+    name => 'add_dir',
+    path => '',
+    method => 'POST',
+    description => "Add Directories for Host Directory Sharing.",
+    permissions => {
+	check => ['perm', '/', [ 'Sys.Modify' ]],
+    },
+    protected => 1,
+    proxyto => 'node',
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    dirid => {
+		type => 'string',
+		pattern => '[a-zA-Z0-9\-]+',
+	    },
+	    path => {
+		type => 'string',
+		maxLength => 4096,
+		format => 'pve-storage-path',
+	    },
+	},
+    },
+    returns => { type => "null" },
+    code => sub {
+	my ($param) = @_;
+	my $node = extract_param($param, 'node');
+	my $dirid = extract_param($param, 'dirid');
+	my $path = extract_param($param, 'path');
+	PVE::DirConfig::add_dir_config($node, $dirid, $path);
+	return undef;
+    },
+});
+
+
+__PACKAGE__->register_method({
+    name => 'del_dir',
+    path => '',
+    method => 'DELETE',
+    description => "Remove Directory from Host Directory Sharing.",
+    permissions => {
+	check => ['perm', '/', [ 'Sys.Modify' ]],
+    },
+    protected => 1,
+    proxyto => 'node',
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    dirid => {
+		type => 'string',
+		pattern => '[a-zA-Z0-9\-]+',
+	    },
+	},
+    },
+    returns => { type => "null" },
+    code => sub {
+	my ($param) = @_;
+
+	my $node = extract_param($param, 'node');
+	my $dirid = extract_param($param, 'dirid');
+	PVE::DirConfig::del_dir_config($node, $dirid);
+	return undef;
+    },
+});
+
+
+1;
diff --git a/PVE/API2/Makefile b/PVE/API2/Makefile
index 5c08ebe0..1b96223c 100644
--- a/PVE/API2/Makefile
+++ b/PVE/API2/Makefile
@@ -12,6 +12,7 @@ PERLSOURCE = 			\
 	Ceph.pm			\
 	Certificates.pm		\
 	Cluster.pm		\
+	DirConfig.pm		\
 	HAConfig.pm		\
 	Hardware.pm		\
 	Network.pm		\
diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index 150828e1..c899ebbd 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -48,6 +48,7 @@ use PVE::API2::LXC::Status;
 use PVE::API2::LXC;
 use PVE::API2::Network;
 use PVE::API2::NodeConfig;
+use PVE::API2::DirConfig;
 use PVE::API2::Qemu::CPU;
 use PVE::API2::Qemu;
 use PVE::API2::Replication;
@@ -199,6 +200,11 @@ __PACKAGE__->register_method ({
     path => 'config',
 });
 
+__PACKAGE__->register_method ({
+    subclass => "PVE::API2::DirConfig",
+    path => 'dirs',
+});
+
 if ($have_sdn) {
     __PACKAGE__->register_method ({
 	subclass => "PVE::API2::Network::SDN::Zones::Status",
diff --git a/PVE/DirConfig.pm b/PVE/DirConfig.pm
new file mode 100644
index 00000000..26e7803e
--- /dev/null
+++ b/PVE/DirConfig.pm
@@ -0,0 +1,139 @@
+package PVE::DirConfig;
+
+use strict;
+use warnings;
+
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::Tools qw(file_get_contents file_set_contents lock_file);
+
+my $dir_config_lock = '/var/lock/dirs.lock';
+
+sub config_file {
+    my ($node) = @_;
+
+    return "/etc/pve/nodes/${node}/dirs";
+}
+
+sub load_config {
+    my ($node) = @_;
+
+    my $filename = config_file($node);
+    my $raw = eval { PVE::Tools::file_get_contents($filename); };
+    return {} if !$raw;
+
+    return parse_file_config($raw, $filename);
+}
+
+sub write_config {
+    my ($node, $conf) = @_;
+
+    my $filename = config_file($node);
+
+    my $raw = write_file_config($conf);
+
+    PVE::Tools::file_set_contents($filename, $raw);
+}
+
+sub lock_config {
+    my ($node, $realcode, @param) = @_;
+
+    # make sure configuration file is up-to-date
+    my $code = sub {
+	PVE::Cluster::cfs_update();
+	$realcode->(@_);
+    };
+
+    my $res = lock_file($dir_config_lock, 10, $code, @param);
+
+    die $@ if $@;
+
+    return $res;
+}
+
+my $descr = " Description for Shared Files Directory Config.\n"
+    ." Add Directories with:\n dirid: /path/to/share";
+
+my $dir_desc = {
+    path => {
+	type => 'string',
+	format_description => 'path',
+	description => 'path of Directory ID',
+	default_key => 1,
+    },
+};
+
+my $conf_schema = {
+    type => 'object',
+    properties => {},
+};
+
+sub parse_file_config : prototype($$) {
+    my ($content, $filename) = @_;
+    return undef if !defined($content);
+
+    my $conf = PVE::JSONSchema::parse_config($conf_schema, $filename, $content, 'description');
+
+    return $conf;
+}
+
+sub parse_dir_config {
+    my ($data) = @_;
+
+    return PVE::JSONSchema::parse_property_string($dir_desc, $data);
+}
+
+sub print_dir_config {
+    my ($data) = @_;
+
+    return PVE::JSONSchema::print_property_string($data, $dir_desc);
+}
+
+sub add_dir_config {
+    my ($node, $dirid, $path) = @_;
+    my $code = sub {
+	my $conf = load_config($node);
+	if (! -e $path) {
+	    mkdir $path;
+	}
+	if ((-e $path) && (! -d $path)) {
+	    die "Path $path exists but is not a directory\n"
+	}
+	$conf->{$dirid} = $path;
+	write_config($node, $conf);
+    };
+    lock_config($node, $code);
+    die $@ if $@;
+}
+
+sub del_dir_config {
+    my ($node, $dirid) = @_;
+    my $code = sub {
+	my $conf = load_config($node);
+	delete $conf->{$dirid};
+	write_config($node, $conf);
+    };
+    lock_config($node, $code);
+    die $@ if $@;
+}
+
+sub write_file_config {
+    my ($conf) = @_;
+
+    my $raw = '';
+    # add description as comment to top of file
+    foreach my $cl (split(/\n/, $descr)) {
+	$raw .= '#' . $cl . "\n";
+    }
+
+    for my $key (sort keys %$conf) {
+	next if ($key eq 'description');
+	my $value = $conf->{$key};
+	die "detected invalid newline inside property '$key'\n"
+	    if $value =~ m/\n/;
+	$raw .= "$key: $value\n";
+    }
+
+    return $raw;
+}
+
+1;
diff --git a/PVE/Makefile b/PVE/Makefile
index 48b85d33..c11066bd 100644
--- a/PVE/Makefile
+++ b/PVE/Makefile
@@ -9,6 +9,7 @@ PERLSOURCE = 			\
 	AutoBalloon.pm		\
 	CertCache.pm		\
 	CertHelpers.pm		\
+	DirConfig.pm		\
 	ExtMetric.pm		\
 	HTTPServer.pm		\
 	Jobs.pm			\
-- 
2.30.2





^ permalink raw reply	[flat|nested] 9+ messages in thread

* [pve-devel] [PATCH qemu-server v2 4/6] added Shared Files tab in Node Settings
  2022-12-23 13:10 [pve-devel] [PATCH qemu-server/manager/access-control/docs v2 0/6] feature #1027 virtio-9p/virtio-fs Markus Frank
                   ` (2 preceding siblings ...)
  2022-12-23 13:10 ` [pve-devel] [PATCH manager v2 3/6] added Config " Markus Frank
@ 2022-12-23 13:10 ` Markus Frank
  2022-12-23 13:10 ` [pve-devel] [PATCH qemu-server v2 5/6] added options to add virtio-9p & virtio-fs Shared Filesystems to qemu config Markus Frank
  2022-12-23 13:10 ` [pve-devel] [PATCH qemu-server v2 6/6] feature #1027: virtio-9p & virtio-fs support Markus Frank
  5 siblings, 0 replies; 9+ messages in thread
From: Markus Frank @ 2022-12-23 13:10 UTC (permalink / raw)
  To: pve-devel

to add/remove/show directories that are available for shared
filesystems.

and added /dir path to PermPathStore.

Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
 www/manager6/Makefile              |   1 +
 www/manager6/data/PermPathStore.js |   3 +
 www/manager6/node/Config.js        |  12 ++
 www/manager6/node/SharedFiles.js   | 177 +++++++++++++++++++++++++++++
 4 files changed, 193 insertions(+)
 create mode 100644 www/manager6/node/SharedFiles.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 9786337b..7146fab1 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -195,6 +195,7 @@ JSSRC= 							\
 	node/CmdMenu.js					\
 	node/Config.js					\
 	node/Directory.js				\
+	node/SharedFiles.js				\
 	node/LVM.js					\
 	node/LVMThin.js					\
 	node/StatusView.js				\
diff --git a/www/manager6/data/PermPathStore.js b/www/manager6/data/PermPathStore.js
index cf702c03..3ac2e6fb 100644
--- a/www/manager6/data/PermPathStore.js
+++ b/www/manager6/data/PermPathStore.js
@@ -13,6 +13,7 @@ Ext.define('PVE.data.PermPathStore', {
 	{ 'value': '/sdn/zones' },
 	{ 'value': '/storage' },
 	{ 'value': '/vms' },
+	{ 'value': '/dirs' },
     ],
 
     constructor: function(config) {
@@ -39,6 +40,8 @@ Ext.define('PVE.data.PermPathStore', {
 		    break;
 		case 'pool': path = '/pool/' + record.get('pool');
 		    break;
+		case 'dirs': path = '/dirs/' + record.get('dirs');
+		    break;
 	    }
 	    if (path !== undefined && !donePaths[path]) {
 		me.add({ value: path });
diff --git a/www/manager6/node/Config.js b/www/manager6/node/Config.js
index 7e5b1112..ed2f0fcb 100644
--- a/www/manager6/node/Config.js
+++ b/www/manager6/node/Config.js
@@ -407,6 +407,18 @@ Ext.define('PVE.node.Config', {
 		});
 	}
 
+	if (caps.nodes['Sys.Modify']) {
+	    me.items.push(
+		{
+		    xtype: 'pveSharedFilesList',
+		    title: gettext('Shared Files'),
+		    iconCls: 'fa fa-folder',
+		    onlineHelp: 'qm_sharedfiles',
+		    itemId: 'sharedFiles',
+		},
+	    );
+	}
+
 	me.items.push(
 	    {
 		title: gettext('Task History'),
diff --git a/www/manager6/node/SharedFiles.js b/www/manager6/node/SharedFiles.js
new file mode 100644
index 00000000..9fa7797e
--- /dev/null
+++ b/www/manager6/node/SharedFiles.js
@@ -0,0 +1,177 @@
+Ext.define('PVE.node.CreateSharedFiles', {
+    extend: 'Proxmox.window.Edit',
+    xtype: 'pveCreateSharedFiles',
+
+    subject: "Shared Directories",
+
+    onlineHelp: 'qm_sharedfiles',
+
+    initComponent: function() {
+        var me = this;
+
+	if (!me.nodename) {
+	    throw "no node name specified";
+	}
+
+	me.isCreate = true;
+
+        Ext.applyIf(me, {
+	    url: "/nodes/" + me.nodename + "/dirs",
+	    method: 'POST',
+	    items: [
+		{
+		    xtype: 'proxmoxtextfield',
+		    name: 'dirid',
+		    fieldLabel: gettext('Directory ID'),
+		    allowBlank: false,
+		},
+		{
+		    xtype: 'proxmoxtextfield',
+		    name: 'path',
+		    fieldLabel: gettext('Directory Path'),
+		    allowBlank: false,
+		},
+            ],
+        });
+
+        me.callParent();
+    },
+});
+
+Ext.define('PVE.node.SharedFilesList', {
+    extend: 'Ext.grid.Panel',
+    xtype: 'pveSharedFilesList',
+
+    viewModel: {
+	data: {
+	    path: '',
+	},
+	formulas: {
+	    dirid: (get) => get('dirid'),
+	},
+    },
+
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	removeDirectory: function() {
+	    let me = this;
+	    let vm = me.getViewModel();
+	    let view = me.getView();
+
+	    const dirid = vm.get('dirid');
+	    console.log(dirid);
+	    if (!view.nodename) {
+		throw "no node name specified";
+	    }
+
+	    if (!dirid) {
+		throw "no directory name specified";
+	    }
+	    Ext.create('Proxmox.window.SafeDestroy', {
+		url: `/nodes/${view.nodename}/dirs`,
+		item: { id: dirid },
+		params: { dirid: dirid },
+		taskName: 'sharedfilesremove',
+		autoShow: true,
+		listeners: {
+		    destroy: () => view.reload(),
+		},
+	    }).show();
+	},
+    },
+
+    stateful: true,
+    stateId: 'grid-node-directory',
+    columns: [
+	{
+	    text: gettext('Directory ID'),
+	    dataIndex: 'dirid',
+	    flex: 1,
+	},
+	{
+	    text: gettext('Directory Path'),
+	    dataIndex: 'path',
+	    flex: 1,
+	},
+    ],
+
+    rootVisible: false,
+    useArrows: true,
+
+    tbar: [
+	{
+	    text: gettext('Reload'),
+	    iconCls: 'fa fa-refresh',
+	    handler: function() {
+		this.up('panel').reload();
+	    },
+	},
+	{
+	    text: `${gettext('Create')}: ${gettext('Directory')}`,
+	    handler: function() {
+		let view = this.up('panel');
+		Ext.create('PVE.node.CreateSharedFiles', {
+		    nodename: view.nodename,
+		    listeners: {
+			destroy: () => view.reload(),
+		    },
+		    autoShow: true,
+		});
+	    },
+	},
+	{
+	    text: gettext('Remove'),
+	    itemId: 'removeDir',
+	    handler: 'removeDirectory',
+	    disabled: true,
+	    bind: {
+		disabled: '{!dirid}',
+	    },
+	},
+    ],
+
+    reload: function() {
+	let me = this;
+	me.store.load();
+	me.store.sort();
+    },
+
+    listeners: {
+	activate: function() {
+	    this.reload();
+	},
+	selectionchange: function(model, selected) {
+	    let me = this;
+	    let vm = me.getViewModel();
+
+	    vm.set('dirid', selected[0]?.data.dirid || '');
+	},
+    },
+
+    initComponent: function() {
+        let me = this;
+
+	me.nodename = me.pveSelNode.data.node;
+	if (!me.nodename) {
+	    throw "no node name specified";
+	}
+
+	Ext.apply(me, {
+	    store: {
+		fields: ['dirid', 'path'],
+		proxy: {
+		    type: 'proxmox',
+		    url: `/api2/json/nodes/${me.nodename}/dirs`,
+		},
+		sorters: 'dirid',
+	    },
+	});
+
+	me.callParent();
+
+	Proxmox.Utils.monStoreErrors(me, me.getStore(), true);
+	me.reload();
+    },
+});
+
-- 
2.30.2





^ permalink raw reply	[flat|nested] 9+ messages in thread

* [pve-devel] [PATCH qemu-server v2 5/6] added options to add virtio-9p & virtio-fs Shared Filesystems to qemu config
  2022-12-23 13:10 [pve-devel] [PATCH qemu-server/manager/access-control/docs v2 0/6] feature #1027 virtio-9p/virtio-fs Markus Frank
                   ` (3 preceding siblings ...)
  2022-12-23 13:10 ` [pve-devel] [PATCH qemu-server v2 4/6] added Shared Files tab in Node Settings Markus Frank
@ 2022-12-23 13:10 ` Markus Frank
  2022-12-23 13:10 ` [pve-devel] [PATCH qemu-server v2 6/6] feature #1027: virtio-9p & virtio-fs support Markus Frank
  5 siblings, 0 replies; 9+ messages in thread
From: Markus Frank @ 2022-12-23 13:10 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
 www/manager6/Makefile                |   1 +
 www/manager6/Utils.js                |   1 +
 www/manager6/qemu/HardwareView.js    |  19 +++++
 www/manager6/qemu/SharedfilesEdit.js | 101 +++++++++++++++++++++++++++
 4 files changed, 122 insertions(+)
 create mode 100644 www/manager6/qemu/SharedfilesEdit.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 7146fab1..cb035734 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -237,6 +237,7 @@ JSSRC= 							\
 	qemu/QemuBiosEdit.js				\
 	qemu/RNGEdit.js					\
 	qemu/SSHKey.js					\
+	qemu/SharedfilesEdit.js				\
 	qemu/ScsiHwEdit.js				\
 	qemu/SerialEdit.js				\
 	qemu/Smbios1Edit.js				\
diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 8c118fa2..5ac830a3 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1579,6 +1579,7 @@ Ext.define('PVE.Utils', {
 	serial: 4,
 	rng: 1,
 	tpmstate: 1,
+	sharedfiles: 10,
     },
 
     // we can have usb6 and up only for specific machine/ostypes
diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
index af35a980..10091aee 100644
--- a/www/manager6/qemu/HardwareView.js
+++ b/www/manager6/qemu/HardwareView.js
@@ -309,6 +309,16 @@ Ext.define('PVE.qemu.HardwareView', {
 	    never_delete: !caps.nodes['Sys.Console'],
 	    header: gettext("VirtIO RNG"),
 	};
+	for (let i = 0; i < PVE.Utils.hardware_counts.sharedfiles; i++) {
+	    let confid = "sharedfiles" + i.toString();
+	    rows[confid] = {
+		group: 50,
+		order: i,
+		iconCls: 'folder',
+		editor: 'PVE.qemu.SharedfilesEdit',
+		header: gettext('Shared FS') + ' (' + confid +')',
+	    };
+	}
 
 	var sorterFn = function(rec1, rec2) {
 	    var v1 = rec1.data.key;
@@ -582,6 +592,7 @@ Ext.define('PVE.qemu.HardwareView', {
 	    const noVMConfigDiskPerm = !caps.vms['VM.Config.Disk'];
 	    const noVMConfigCDROMPerm = !caps.vms['VM.Config.CDROM'];
 	    const noVMConfigCloudinitPerm = !caps.vms['VM.Config.Cloudinit'];
+	    const noVMConfigOptionsPerm = !caps.vms['VM.Config.Options'];
 
 	    me.down('#addUsb').setDisabled(noSysConsolePerm || isAtUsbLimit());
 	    me.down('#addPci').setDisabled(noSysConsolePerm || isAtLimit('hostpci'));
@@ -591,6 +602,7 @@ Ext.define('PVE.qemu.HardwareView', {
 	    me.down('#addRng').setDisabled(noSysConsolePerm || isAtLimit('rng'));
 	    efidisk_menuitem.setDisabled(noVMConfigDiskPerm || isAtLimit('efidisk'));
 	    me.down('#addTpmState').setDisabled(noSysConsolePerm || isAtLimit('tpmstate'));
+	    me.down('#addFileshare').setDisabled(noVMConfigOptionsPerm || isAtLimit('sharedfiles'));
 	    me.down('#addCloudinitDrive').setDisabled(noVMConfigCDROMPerm || noVMConfigCloudinitPerm || hasCloudInit);
 
 	    if (!rec) {
@@ -735,6 +747,13 @@ Ext.define('PVE.qemu.HardwareView', {
 				disabled: !caps.nodes['Sys.Console'],
 				handler: editorFactory('RNGEdit'),
 			    },
+			    {
+				text: gettext("Shared Filesystem"),
+				itemId: 'addFileshare',
+				iconCls: 'fa fa-folder',
+				disabled: !caps.nodes['Sys.Console'],
+				handler: editorFactory('SharedfilesEdit'),
+			    },
 			],
 		    }),
 		},
diff --git a/www/manager6/qemu/SharedfilesEdit.js b/www/manager6/qemu/SharedfilesEdit.js
new file mode 100644
index 00000000..8ebcef6c
--- /dev/null
+++ b/www/manager6/qemu/SharedfilesEdit.js
@@ -0,0 +1,101 @@
+Ext.define('PVE.qemu.SharedfilesInputPanel', {
+    extend: 'Proxmox.panel.InputPanel',
+    xtype: 'pveSharedfilesInputPanel',
+    onlineHelp: 'qm_sharedfiles',
+
+    insideWizard: false,
+
+    onGetValues: function(values) {
+	var me = this;
+	var confid = me.confid;
+	var params = {};
+	params[confid] = PVE.Parser.printPropertyString(values, 'type');
+	return params;
+    },
+
+    setSharedfiles: function(confid, data) {
+	var me = this;
+	me.confid = confid;
+	me.sharedfiles = data;
+	me.setValues(me.sharedfiles);
+    },
+    items: [
+	{
+	    name: 'type',
+	    xtype: 'proxmoxKVComboBox',
+	    fieldLabel: gettext('Shared FS Type'),
+	    comboItems: [['virtio-9p', 'virtio-9p'], ['virtio-fs', 'virtio-fs']],
+	    allowBlank: false,
+	},
+	{
+	    xtype: 'proxmoxtextfield',
+	    emptyText: 'dirid',
+	    fieldLabel: gettext('Directory ID'),
+	    name: 'dirid',
+	    allowBlank: false,
+	},
+	{
+	    xtype: 'proxmoxtextfield',
+	    emptyText: 'tag name',
+	    fieldLabel: gettext('tag'),
+	    name: 'tag',
+	    allowBlank: false,
+	},
+    ],
+    initComponent: function() {
+	var me = this;
+
+	me.sharedfiles = {};
+	me.confid = 'sharedfiles0';
+	me.callParent();
+    },
+});
+
+Ext.define('PVE.qemu.SharedfilesEdit', {
+    extend: 'Proxmox.window.Edit',
+
+    subject: gettext('Filesystem Passthrough'),
+
+    initComponent: function() {
+	var me = this;
+
+	me.isCreate = !me.confid;
+
+	var ipanel = Ext.create('PVE.qemu.SharedfilesInputPanel', {
+	    confid: me.confid,
+	    isCreate: me.isCreate,
+	});
+
+	Ext.applyIf(me, {
+	    items: ipanel,
+	});
+
+	me.callParent();
+
+	me.load({
+	    success: function(response) {
+	        me.conf = response.result.data;
+	        var i, confid;
+	        if (!me.isCreate) {
+		    var value = me.conf[me.confid];
+		    var sharedfiles = PVE.Parser.parsePropertyString(value, "type");
+		    if (!sharedfiles) {
+			Ext.Msg.alert(gettext('Error'), 'Unable to parse sharedfiles options');
+			me.close();
+			return;
+		    }
+		    ipanel.setSharedfiles(me.confid, sharedfiles);
+		} else {
+		    for (i = 0; i < PVE.Utils.hardware_counts.sharedfiles; i++) {
+		        confid = 'sharedfiles' + i.toString();
+		        if (!Ext.isDefined(me.conf[confid])) {
+			    me.confid = confid;
+			    break;
+			}
+		    }
+		    ipanel.setSharedfiles(me.confid, {});
+		}
+	    },
+	});
+    },
+});
-- 
2.30.2





^ permalink raw reply	[flat|nested] 9+ messages in thread

* [pve-devel] [PATCH qemu-server v2 6/6] feature #1027: virtio-9p & virtio-fs support
  2022-12-23 13:10 [pve-devel] [PATCH qemu-server/manager/access-control/docs v2 0/6] feature #1027 virtio-9p/virtio-fs Markus Frank
                   ` (4 preceding siblings ...)
  2022-12-23 13:10 ` [pve-devel] [PATCH qemu-server v2 5/6] added options to add virtio-9p & virtio-fs Shared Filesystems to qemu config Markus Frank
@ 2022-12-23 13:10 ` Markus Frank
  2022-12-27 11:12   ` Wolfgang Bumiller
  5 siblings, 1 reply; 9+ messages in thread
From: Markus Frank @ 2022-12-23 13:10 UTC (permalink / raw)
  To: pve-devel

adds support for sharing directorys with a guest vm

virtio-9p can be simply started with qemu.
virtio-fs needs virtiofsd to be started before qemu.

Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
 PVE/API2/Qemu.pm  |  20 ++++++-
 PVE/QemuServer.pm | 135 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 154 insertions(+), 1 deletion(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index badfc37..404778d 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -639,6 +639,8 @@ my $check_vm_modify_config_perm = sub {
 	    # the user needs Disk and PowerMgmt privileges to change the vmstate
 	    # also needs privileges on the storage, that will be checked later
 	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Disk', 'VM.PowerMgmt' ]);
+	} elsif ($opt =~ m/^sharedfiles\d$/) {
+	    # needs $param->{$opt} so checkout $check_vm_dir_perm
 	} else {
 	    # catches hostpci\d+, args, lock, etc.
 	    # new options will be checked here
@@ -649,6 +651,20 @@ my $check_vm_modify_config_perm = sub {
     return 1;
 };
 
+my $check_vm_dir_perm = sub {
+    my ($rpcenv, $authuser, $param) = @_;
+
+    return 1 if $authuser eq 'root@pam';
+
+    foreach my $opt (keys %{$param}) {
+	if ($opt =~ m/^sharedfiles\d$/) {
+	    my $sharedfiles = PVE::QemuServer::parse_sharedfiles($param->{$opt});
+	    $rpcenv->check_dir_perm($authuser, $sharedfiles->{dirid}, ['VM.Config.Options']);
+	}
+    }
+    return 1;
+};
+
 __PACKAGE__->register_method({
     name => 'vmlist',
     path => '',
@@ -875,7 +891,7 @@ __PACKAGE__->register_method({
 	    &$check_storage_access($rpcenv, $authuser, $storecfg, $vmid, $param, $storage);
 
 	    &$check_vm_modify_config_perm($rpcenv, $authuser, $vmid, $pool, [ keys %$param]);
-
+	    &$check_vm_dir_perm($rpcenv, $authuser, $param);
 	    &$check_vm_create_serial_perm($rpcenv, $authuser, $vmid, $pool, $param);
 	    &$check_vm_create_usb_perm($rpcenv, $authuser, $vmid, $pool, $param);
 
@@ -1576,6 +1592,8 @@ my $update_vm_api  = sub {
 
     &$check_vm_modify_config_perm($rpcenv, $authuser, $vmid, undef, [keys %$param]);
 
+    &$check_vm_dir_perm($rpcenv, $authuser, $param);
+
     &$check_storage_access($rpcenv, $authuser, $storecfg, $vmid, $param);
 
     my $updatefn =  sub {
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index a746b3d..bde514f 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -44,6 +44,7 @@ use PVE::SysFSTools;
 use PVE::Systemd;
 use PVE::Tools qw(run_command file_read_firstline file_get_contents dir_glob_foreach get_host_arch $IPV6RE);
 
+
 use PVE::QMPClient;
 use PVE::QemuConfig;
 use PVE::QemuServer::Helpers qw(min_version config_aware_timeout windows_version);
@@ -56,6 +57,7 @@ use PVE::QemuServer::Memory;
 use PVE::QemuServer::Monitor qw(mon_cmd);
 use PVE::QemuServer::PCI qw(print_pci_addr print_pcie_addr print_pcie_root_port parse_hostpci);
 use PVE::QemuServer::USB qw(parse_usb_device);
+use PVE::DirConfig;
 
 my $have_sdn;
 eval {
@@ -267,6 +269,31 @@ my $rng_fmt = {
     },
 };
 
+my $sharedfiles_fmt = {
+    type => {
+	type => 'string',
+	default_key => 1,
+	enum => ['virtio-9p', 'virtio-fs'],
+	description => "sharedfiles via"
+	    ." virtio-9p (https://www.linux-kvm.org/page/9p_virtio)"
+	    ." or virtio-fs (https://virtio-fs.gitlab.io/howto-qemu.html)",
+	format_description => "virtio-sharedfiles-type",
+	optional => 1,
+    },
+    dirid => {
+	type => 'string',
+	description => "dirid of directory you want to share with the guest VM",
+	format_description => "virtio-sharedfiles-dirid",
+	optional => 1,
+    },
+    tag => {
+	type => 'string',
+	description => "tag name for mounting in the guest VM",
+	format_description => "virtio-sharedfiles-tag",
+	optional => 1,
+    },
+};
+
 my $meta_info_fmt = {
     'ctime' => {
 	type => 'integer',
@@ -828,6 +855,7 @@ while (my ($k, $v) = each %$confdesc) {
 
 my $MAX_USB_DEVICES = 14;
 my $MAX_NETS = 32;
+my $MAX_SHAREDFILES = 10;
 my $MAX_SERIAL_PORTS = 4;
 my $MAX_PARALLEL_PORTS = 3;
 my $MAX_NUMA = 8;
@@ -970,6 +998,12 @@ my $netdesc = {
     description => "Specify network devices.",
 };
 
+my $sharedfilesdesc = {
+    optional => 1,
+    type => 'string', format => $sharedfiles_fmt,
+    description => "share files between host and guest",
+};
+
 PVE::JSONSchema::register_standard_option("pve-qm-net", $netdesc);
 
 my $ipconfig_fmt = {
@@ -1031,6 +1065,10 @@ for (my $i = 0; $i < $MAX_NETS; $i++)  {
     $confdesc_cloudinit->{"ipconfig$i"} = $ipconfigdesc;
 }
 
+for (my $i = 0; $i < $MAX_SHAREDFILES; $i++)  {
+    $confdesc->{"sharedfiles$i"} = $sharedfilesdesc;
+}
+
 foreach my $key (keys %$confdesc_cloudinit) {
     $confdesc->{$key} = $confdesc_cloudinit->{$key};
 }
@@ -1964,6 +2002,16 @@ sub parse_net {
     return $res;
 }
 
+sub parse_sharedfiles {
+    my ($value) = @_;
+
+    return if !$value;
+    my $res = eval { parse_property_string($sharedfiles_fmt, $value) };
+
+    warn $@ if $@;
+    return $res;
+}
+
 # ipconfigX ip=cidr,gw=ip,ip6=cidr,gw6=ip
 sub parse_ipconfig {
     my ($data) = @_;
@@ -4073,6 +4121,44 @@ sub config_to_command {
 	push @$devices, '-device', $netdevicefull;
     }
 
+    my $onevirtiofs = 0;
+    for (my $i = 0; $i < $MAX_SHAREDFILES; $i++) {
+	my $sharedfilesstr = "sharedfiles$i";
+
+	next if !$conf->{$sharedfilesstr};
+	my $sharedfiles = parse_sharedfiles($conf->{$sharedfilesstr});
+	next if !$sharedfiles;
+
+	die $sharedfilesstr.' needs a type (virtio-9p or virtio-fs)' if !$sharedfiles->{type};
+	die $sharedfilesstr.' needs a dirid of a directory to share' if !$sharedfiles->{dirid};
+	die $sharedfilesstr.' needs a mount tag' if !$sharedfiles->{tag};
+
+	if ($sharedfiles->{type} eq 'virtio-fs' && $conf->{numa}) {
+	    die "disable numa to use virtio-fs or use virtio-9p instead";
+	}
+
+	my $path = extract_dir_path($nodename, $sharedfiles->{dirid});
+
+	if ($sharedfiles->{type} eq 'virtio-9p') {
+	    push @$devices, '-fsdev', "local,security_model=passthrough,id=fsdev$i"
+		.",path=$path";
+	    push @$devices, '-device', "virtio-9p-pci,id=fs9p$i,fsdev=fsdev$i"
+		.",mount_tag=$sharedfiles->{tag}";
+	}
+	if ($sharedfiles->{type} eq 'virtio-fs') {
+	    push @$devices, '-chardev', "socket,id=virtfs$i,path=/var/run/virtiofsd/vm$vmid-fs$i";
+	    push @$devices, '-device', 'vhost-user-fs-pci,queue-size=1024'
+		.",chardev=virtfs$i,tag=$sharedfiles->{tag}";
+	    $onevirtiofs = 1;
+	}
+    }
+
+    if ($onevirtiofs) {
+	push @$devices, '-object', 'memory-backend-file,id=mem'
+	    .",size=$conf->{memory}M,mem-path=/dev/shm,share=on";
+	push @$devices, '-numa', 'node,memdev=mem';
+    }
+
     if ($conf->{ivshmem}) {
 	my $ivshmem = parse_property_string($ivshmem_fmt, $conf->{ivshmem});
 
@@ -4158,6 +4244,40 @@ sub config_to_command {
     return wantarray ? ($cmd, $vollist, $spice_port) : $cmd;
 }
 
+sub extract_dir_path {
+    my ($nodename, $dirid) = @_;
+    my $dir_config = PVE::DirConfig::load_config($nodename);
+    my $path = $dir_config->{$dirid};
+    if (!$path) {
+	die "Directory ID $dirid does not exist\n";
+    }
+    if (! -e $path) {
+	die "Path $path does not exist\n";
+    }
+    if ((-e $path) && (! -d $path)) {
+	die "Path $path exists but is not a directory\n"
+    }
+    return $path;
+}
+
+sub start_virtiofs {
+    my ($vmid, $path, $fsid) = @_;
+    # virtiofsd does not run in background until vhost-user connects
+    # to the socket, so it has to be started in a fork or with a tool
+    # like daemonize
+
+    my $pid = fork();
+    my $socketstr = "--socket-path=/var/run/virtiofsd/vm$vmid-fs$fsid";
+
+    if ($pid == 0) {
+	run_command(['/usr/lib/kvm/virtiofsd', '--daemonize', $socketstr,
+		'-o', "source=$path", '-o', 'cache=always']);
+	POSIX::_exit(0);
+    } elsif (!defined($pid)) {
+        die "could not fork to start virtiofsd";
+    }
+}
+
 sub check_rng_source {
     my ($source) = @_;
 
@@ -5713,6 +5833,21 @@ sub vm_start_nolock {
     my ($cmd, $vollist, $spice_port) = config_to_command($storecfg, $vmid,
 	$conf, $defaults, $forcemachine, $forcecpu, $params->{'pbs-backing'});
 
+    my $nodename = nodename();
+    for (my $i = 0; $i < $MAX_SHAREDFILES; $i++) {
+	my $sharedfilesstr = "sharedfiles$i";
+
+	next if !$conf->{$sharedfilesstr};
+	my $sharedfiles = parse_sharedfiles($conf->{$sharedfilesstr});
+	next if !$sharedfiles;
+
+	my $path = extract_dir_path($nodename, $sharedfiles->{dirid});
+
+	if ($sharedfiles && $sharedfiles->{type} eq 'virtio-fs' && !$conf->{numa}) {
+	    start_virtiofs($vmid, $path, $i);
+	}
+    }
+
     my $migration_ip;
     my $get_migration_ip = sub {
 	my ($nodename) = @_;
-- 
2.30.2





^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [pve-devel] [PATCH qemu-server v2 6/6] feature #1027: virtio-9p & virtio-fs support
  2022-12-23 13:10 ` [pve-devel] [PATCH qemu-server v2 6/6] feature #1027: virtio-9p & virtio-fs support Markus Frank
@ 2022-12-27 11:12   ` Wolfgang Bumiller
  0 siblings, 0 replies; 9+ messages in thread
From: Wolfgang Bumiller @ 2022-12-27 11:12 UTC (permalink / raw)
  To: Markus Frank; +Cc: pve-devel

Without going too much into this series itself, I've been using
virtiofsd manually myself lately and immediately ran into an issue that
we also need to address, see below:

On Fri, Dec 23, 2022 at 02:10:07PM +0100, Markus Frank wrote:
> adds support for sharing directorys with a guest vm
> 
> virtio-9p can be simply started with qemu.
> virtio-fs needs virtiofsd to be started before qemu.
> 
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> ---
>  PVE/API2/Qemu.pm  |  20 ++++++-
>  PVE/QemuServer.pm | 135 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 154 insertions(+), 1 deletion(-)
> 
> @@ -4158,6 +4244,40 @@ sub config_to_command {
>      return wantarray ? ($cmd, $vollist, $spice_port) : $cmd;
>  }
>  
> +sub extract_dir_path {
> +    my ($nodename, $dirid) = @_;
> +    my $dir_config = PVE::DirConfig::load_config($nodename);
> +    my $path = $dir_config->{$dirid};
> +    if (!$path) {
> +	die "Directory ID $dirid does not exist\n";
> +    }
> +    if (! -e $path) {
> +	die "Path $path does not exist\n";
> +    }
> +    if ((-e $path) && (! -d $path)) {
> +	die "Path $path exists but is not a directory\n"
> +    }
> +    return $path;
> +}
> +
> +sub start_virtiofs {
> +    my ($vmid, $path, $fsid) = @_;
> +    # virtiofsd does not run in background until vhost-user connects
> +    # to the socket, so it has to be started in a fork or with a tool
> +    # like daemonize

This is not really useful. The code now basically daemonizes it twice
and we have a race between virtiofsd opening the socket and qemu
connecting to it, which will cause qemu to fail and virtiofsd to linger.

Instead of using `--socket-path we should setup the socket ourselves and
pass `--fd` to virtiofsd, and not pass `--daemonize` because the point
where it happens is completely useless.

One *hacky* alternative would be to wait for its "Waiting for ..."
message.
Alternatively we could take this upstream and perhaps end up patching
it to daemonize between listen() and accept() as that would be a lot
more useful. If we do that, we should also consider a few more changes,
like an option to pass an already `accept()`ed fd, or an option to allow
multiple connections (so that each accept() will just fork off a new
instance - not necessarily useful for PVE, but useful for regular VMs,
iff done in a way that one can write a systemd service unit for shares
where virtiofsd is *correctly* daemonizing for it...)

> +
> +    my $pid = fork();
> +    my $socketstr = "--socket-path=/var/run/virtiofsd/vm$vmid-fs$fsid";
> +
> +    if ($pid == 0) {
> +	run_command(['/usr/lib/kvm/virtiofsd', '--daemonize', $socketstr,
> +		'-o', "source=$path", '-o', 'cache=always']);
> +	POSIX::_exit(0);
> +    } elsif (!defined($pid)) {
> +        die "could not fork to start virtiofsd";
> +    }
> +}
> +
>  sub check_rng_source {
>      my ($source) = @_;
>  




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [pve-devel] [PATCH access-control v2 2/6] added acls for Shared Filesystem Directories
  2022-12-23 13:10 ` [pve-devel] [PATCH access-control v2 2/6] added acls for Shared Filesystem Directories Markus Frank
@ 2023-01-16 15:45   ` Thomas Lamprecht
  0 siblings, 0 replies; 9+ messages in thread
From: Thomas Lamprecht @ 2023-01-16 15:45 UTC (permalink / raw)
  To: Proxmox VE development discussion, Markus Frank

Am 23/12/2022 um 14:10 schrieb Markus Frank:
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> ---
>  src/PVE/AccessControl.pm  |  2 ++
>  src/PVE/RPCEnvironment.pm | 12 +++++++++++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
> index a95d072..742304c 100644
> --- a/src/PVE/AccessControl.pm
> +++ b/src/PVE/AccessControl.pm
> @@ -1221,6 +1221,8 @@ sub check_path {
>  	|/storage/[[:alnum:]\.\-\_]+
>  	|/vms
>  	|/vms/[1-9][0-9]{2,}
> +	|/dirs
> +	|/dirs/[[:alnum:]\.\-\_]+

I do not like this too much, iff we expose this at the ACL level I'd rather like to
use a /map/<type>/<id> path, as we need that for Dominik's HW (PCI(e)) mappings anyway,
and I think we could reuse such a mapping ACL object path for even more things (e.g., VMID
(allocation) ranges, CPU cores (for cpu task set/pinning), ...

Besides that, note that our access model normally adds privileges based of the top-level
ACL object path, with the fitting roles - e.g., here that could be Dirs.Audit, Dirs.Modify
Dirs.Use – but with above it will then naturally something like Map.Audit, Map.Modify,
Map.Use.




^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-01-16 15:46 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-23 13:10 [pve-devel] [PATCH qemu-server/manager/access-control/docs v2 0/6] feature #1027 virtio-9p/virtio-fs Markus Frank
2022-12-23 13:10 ` [pve-devel] [PATCH docs v2 1/6] added shared filesystem doc for virtio-fs & virtio-9p Markus Frank
2022-12-23 13:10 ` [pve-devel] [PATCH access-control v2 2/6] added acls for Shared Filesystem Directories Markus Frank
2023-01-16 15:45   ` Thomas Lamprecht
2022-12-23 13:10 ` [pve-devel] [PATCH manager v2 3/6] added Config " Markus Frank
2022-12-23 13:10 ` [pve-devel] [PATCH qemu-server v2 4/6] added Shared Files tab in Node Settings Markus Frank
2022-12-23 13:10 ` [pve-devel] [PATCH qemu-server v2 5/6] added options to add virtio-9p & virtio-fs Shared Filesystems to qemu config Markus Frank
2022-12-23 13:10 ` [pve-devel] [PATCH qemu-server v2 6/6] feature #1027: virtio-9p & virtio-fs support Markus Frank
2022-12-27 11:12   ` Wolfgang Bumiller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal