From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 2471791F9A for ; Mon, 10 Oct 2022 14:18:02 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id EEEFF38F4 for ; Mon, 10 Oct 2022 14:17:31 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Mon, 10 Oct 2022 14:17:30 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 5F28944909 for ; Mon, 10 Oct 2022 14:17:30 +0200 (CEST) Date: Mon, 10 Oct 2022 14:17:22 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox VE development discussion References: <20221007142922.165009-1-m.frank@proxmox.com> In-Reply-To: <20221007142922.165009-1-m.frank@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.16.0 (https://github.com/astroidmail/astroid) Message-Id: <1665403751.7pco5sktqu.astroid@nora.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.143 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH qemu-server] feature #1027: virtio-9p & virtio-fs support X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 10 Oct 2022 12:18:02 -0000 On October 7, 2022 4:29 pm, Markus Frank wrote: > adds support for sharing directorys with a guest vm. >=20 > virtio-9p can be simply started with qemu. > virtio-fs needs virtiofsd to be started before qemu. >=20 > Signed-off-by: Markus Frank > --- > I chose MAX_SHAREDFILES to be 10, because I think it is more than enough. famous last words ;) >=20 > PVE/QemuServer.pm | 113 ++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 113 insertions(+) >=20 > diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm > index 4e85dd0..580133b 100644 > --- a/PVE/QemuServer.pm > +++ b/PVE/QemuServer.pm > @@ -272,6 +272,31 @@ my $rng_fmt =3D { > }, > }; > =20 > +my $sharedfiles_fmt =3D { > + type =3D> { > + type =3D> 'string', > + default_key =3D> 1, > + enum =3D> ['virtio-9p', 'virtio-fs'], > + description =3D> "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 =3D> "virtio-sharedfiles-type", > + optional =3D> 1, > + }, > + path =3D> { > + type =3D> 'string', > + description =3D> "path you want to share with the guest VM", > + format_description =3D> "virtio-sharedfiles-path", > + optional =3D> 1, this should have more restrictions than being an arbitrary string. ideally we could take this opportunity and implement some sort of=20 "defined host source paths" feature that we can re-use for bind mounts=20 as well: - admin defines dirs on the host that are eligibly for mounting into=20 guests - admin gives access via an ACL - user can then use this object as bind mount/shared dir source without=20 requiring root access (and not allow arbitrary paths here at all?) > + }, > + tag =3D> { > + type =3D> 'string', > + description =3D> "tag name for mounting in the guest VM", > + format_description =3D> "virtio-sharedfiles-tag", > + optional =3D> 1, > + }, > +}; > + > my $meta_info_fmt =3D { > 'ctime' =3D> { > type =3D> 'integer', > @@ -826,6 +851,7 @@ while (my ($k, $v) =3D each %$confdesc) { > =20 > my $MAX_USB_DEVICES =3D 5; > my $MAX_NETS =3D 32; > +my $MAX_SHAREDFILES =3D 10; > my $MAX_SERIAL_PORTS =3D 4; > my $MAX_PARALLEL_PORTS =3D 3; > my $MAX_NUMA =3D 8; > @@ -968,6 +994,12 @@ my $netdesc =3D { > description =3D> "Specify network devices.", > }; > =20 > +my $sharedfilesdesc =3D { > + optional =3D> 1, > + type =3D> 'string', format =3D> $sharedfiles_fmt, > + description =3D> "share files between host and guest", > +}; > + > PVE::JSONSchema::register_standard_option("pve-qm-net", $netdesc); > =20 > my $ipconfig_fmt =3D { > @@ -1029,6 +1061,10 @@ for (my $i =3D 0; $i < $MAX_NETS; $i++) { > $confdesc_cloudinit->{"ipconfig$i"} =3D $ipconfigdesc; > } > =20 > +for (my $i =3D 0; $i < $MAX_SHAREDFILES; $i++) { > + $confdesc->{"sharedfiles$i"} =3D $sharedfilesdesc; > +} > + > foreach my $key (keys %$confdesc_cloudinit) { > $confdesc->{$key} =3D $confdesc_cloudinit->{$key}; > } > @@ -1933,6 +1969,16 @@ sub parse_net { > return $res; > } > =20 > +sub parse_sharedfiles { > + my ($value) =3D @_; > + > + return if !$value; > + my $res =3D eval { parse_property_string($sharedfiles_fmt, $value) }= ; > + > + warn $@ if $@; > + return $res; > +} > + > # ipconfigX ip=3Dcidr,gw=3Dip,ip6=3Dcidr,gw6=3Dip > sub parse_ipconfig { > my ($data) =3D @_; > @@ -4022,6 +4068,45 @@ sub config_to_command { > push @$devices, '-device', $netdevicefull; > } > =20 > + my $onevirtiofs =3D 0; > + for (my $i =3D 0; $i < $MAX_SHAREDFILES; $i++) { > + my $sharedfilesstr =3D "sharedfiles$i"; > + > + next if !$conf->{$sharedfilesstr}; > + my $sharedfiles =3D parse_sharedfiles($conf->{$sharedfilesstr}); > + next if !$sharedfiles; > + > + die $sharedfilesstr.' needs a type (virtio-9p or virtio-fs)' if !$share= dfiles->{type}; > + die $sharedfilesstr.' needs a path to a directory to share' if !$shared= files->{path}; should check whether path exists and is a directory > + 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"; > + } > + > + mkdir $sharedfiles->{path}; > + > + if ($sharedfiles->{type} eq 'virtio-9p') { > + push @$devices, '-fsdev', 'local,security_model=3Dpassthrough,id=3D= fsdev'.$i > + .',path=3D'.$sharedfiles->{path}; not sure about the encoding here on the qemu side, but this right now=20 allows injecting ',' followed by other fsdev options, so likely we want=20 to escape/encode and have more checks on the path. > + push @$devices, '-device', 'virtio-9p-pci,id=3Dfs'.$i.',fsdev=3Dfsd= ev'.$i > + .',mount_tag=3D'.$sharedfiles->{tag}; > + } > + if ($sharedfiles->{type} eq 'virtio-fs') { > + push @$devices, '-chardev', 'socket,id=3Dvirtfs'.$i > + .',path=3D/var/run/virtiofsd/vm'.$vmid.'-fs'.$i; > + push @$devices, '-device', 'vhost-user-fs-pci,queue-size=3D1024,cha= rdev=3Dvirtfs'.$i > + .',tag=3D'.$sharedfiles->{tag}; > + $onevirtiofs =3D 1; > + } > + } > + > + if ($onevirtiofs) { > + push @$devices, '-object', 'memory-backend-file,id=3Dmem,' > + .'size=3D'.$conf->{memory}.'M,mem-path=3D/dev/shm,share=3Don'; > + push @$devices, '-numa', 'node,memdev=3Dmem'; > + } > + > if ($conf->{ivshmem}) { > my $ivshmem =3D parse_property_string($ivshmem_fmt, $conf->{ivshmem}); > =20 > @@ -4107,6 +4192,22 @@ sub config_to_command { > return wantarray ? ($cmd, $vollist, $spice_port) : $cmd; > } > =20 > +sub start_virtiofs { > + my ($vmid, $path, $fsid) =3D @_; > + # 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 =3D fork(); > + if ($pid =3D=3D 0) { > + run_command('/usr/lib/kvm/virtiofsd --daemonize --socket-path=3D/var/ru= n/virtiofsd/' > + .'vm'.$vmid.'-fs'.$fsid.' -o source=3D'.$path.' -o cache=3Dalways')= ; run_command like that is pretty much always wrong ;) please use run_command(['usr/lib/kvm/virtiofsd', '--daemonize', ...]) and extract stuff into variables as needed if it gets too long/.. > + POSIX::_exit(0); > + } elsif (!defined($pid)) { > + die "could not fork to start virtiofsd"; > + } > +} > + > sub check_rng_source { > my ($source) =3D @_; > =20 > @@ -5497,6 +5598,18 @@ sub vm_start_nolock { > my ($cmd, $vollist, $spice_port) =3D config_to_command($storecfg, $v= mid, > $conf, $defaults, $forcemachine, $forcecpu, $params->{'pbs-backing'}); > =20 > + for (my $i =3D 0; $i < $MAX_SHAREDFILES; $i++) { > + my $sharedfilesstr =3D "sharedfiles$i"; > + > + next if !$conf->{$sharedfilesstr}; > + my $sharedfiles =3D parse_sharedfiles($conf->{$sharedfilesstr}); > + next if !$sharedfiles; > + > + if ($sharedfiles && $sharedfiles->{type} eq 'virtio-fs' && !$conf->{num= a}) { > + start_virtiofs($vmid, $sharedfiles->{path}, $i); > + } > + } > + > my $migration_ip; > my $get_migration_ip =3D sub { > my ($nodename) =3D @_; > --=20 > 2.30.2 >=20 >=20 >=20 > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel >=20 >=20 >=20