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 C07859230D for ; Tue, 27 Dec 2022 12:13:21 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id EF2991A0B5 for ; Tue, 27 Dec 2022 12:12:50 +0100 (CET) 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) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Tue, 27 Dec 2022 12:12:49 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id D331D43DED for ; Tue, 27 Dec 2022 12:12:48 +0100 (CET) Date: Tue, 27 Dec 2022 12:12:47 +0100 From: Wolfgang Bumiller To: Markus Frank Cc: pve-devel@lists.proxmox.com Message-ID: <20221227111247.tn63tebf4jwwtv3d@casey.proxmox.com> References: <20221223131007.130310-1-m.frank@proxmox.com> <20221223131007.130310-7-m.frank@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20221223131007.130310-7-m.frank@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.219 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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [qemu.pm, qemuserver.pm] Subject: Re: [pve-devel] [PATCH qemu-server v2 6/6] 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: Tue, 27 Dec 2022 11:13:21 -0000 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 > --- > 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) = @_; >