From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTP id CE4485A005E for ; Thu, 9 Feb 2023 09:52:04 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1675932723; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=UiRm/ZX0scEsINYOcBts/JQDn7V9qZs1o9t4bu/b3s4=; b=Rhwe58KBCKQ9smWdZuchwUSFz6fhhYYBwH3wUoUagft/t56OAf5wku3E70x7z7Pk5O1pUG dRwmKHf2v2soVIpweeFtQ0r4EJdda8NnuPCRyCfu3uIN8ZCqYKT1aYxaHofhy7SqBiUuDS Jn+hm0xEFHisCqFCybvCOEo9vX8UhOE= Received: from mail-ej1-f71.google.com (mail-ej1-f71.google.com [209.85.218.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-630-1XrIp5RVN-SXKthOW3IYTA-1; Thu, 09 Feb 2023 03:52:02 -0500 X-MC-Unique: 1XrIp5RVN-SXKthOW3IYTA-1 Received: by mail-ej1-f71.google.com with SMTP id qk8-20020a1709077f8800b008af3fb43869so493028ejc.4 for ; Thu, 09 Feb 2023 00:52:02 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=UiRm/ZX0scEsINYOcBts/JQDn7V9qZs1o9t4bu/b3s4=; b=odgrBHD6f3Jo3I3SSk3pWkMA6NuRHINyarVB0GW5BTgljLddp9lxwM2GZP7UmFul6f 51hsUL9qk4d4gEb6jt10EtJqo49W9Q/Lc+swY26Cnloct3fMbztlY/4WSQ3CqDOIY0BN oHHGpgS6J1gnqH+4bvzArHZcN1RDJZR3hw2tavRSMx28n+EH09ffRnEM20mWQelLxrHW P6UlQ9+HMLx3Z4DAeGMzC03fvhAkvXOuojbld4iwKLHPd7nOcFhWGGDPPJtnoa9gku42 ImwvxybJPAZWCLPlCEMpeMSTTttm5eSjkeKqmyPGDHOxhqWPvVlClApq46u4cw0H89eq ak3g== X-Gm-Message-State: AO0yUKWugJwVOULJvBV/gSRWm7o+aMvMuxOFz8poQYZSfqy2GFytktlW KgwsvBcfy1hm+I1zUppk4qPn/qDppAk20MnenN/mzjbj0GObcU8xkkOBZBwjqBmW5WaCjGJZxjc RHQcG06q2mXNn X-Received: by 2002:a50:9b0d:0:b0:4ab:ec2:3cd1 with SMTP id o13-20020a509b0d000000b004ab0ec23cd1mr4489642edi.25.1675932721013; Thu, 09 Feb 2023 00:52:01 -0800 (PST) X-Google-Smtp-Source: AK7set/orS0J1y3FhnRv9YGj6oWNHAaXYbVN6K4JqmSv2JC1zYc3eJXJeSeHvE3nLx47X1p6Cs7QVA== X-Received: by 2002:a50:9b0d:0:b0:4ab:ec2:3cd1 with SMTP id o13-20020a509b0d000000b004ab0ec23cd1mr4489634edi.25.1675932720803; Thu, 09 Feb 2023 00:52:00 -0800 (PST) Received: from [10.43.2.39] (nat-pool-brq-t.redhat.com. [213.175.37.10]) by smtp.gmail.com with ESMTPSA id p19-20020a50cd93000000b004ab102c90e3sm440228edi.93.2023.02.09.00.52.00 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 09 Feb 2023 00:52:00 -0800 (PST) Message-ID: <41d2c66b-cb07-6e4a-4dd9-615ce46d5497@redhat.com> Date: Thu, 9 Feb 2023 09:52:00 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 Subject: Re: [libvirt PATCH] qemu: allow passt to self-daemonize To: Laine Stump , libvir-list@redhat.com References: <20230208231310.1728051-1-laine@redhat.com> From: =?UTF-8?B?TWljaGFsIFByw612b3puw61r?= In-Reply-To: <20230208231310.1728051-1-laine@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-MailFrom: mprivozn@redhat.com X-Mailman-Rule-Hits: nonmember-moderation X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation Message-ID-Hash: SY3SEKUHQB6OYO2CJHV5ZIP4HOZFXIWI X-Message-ID-Hash: SY3SEKUHQB6OYO2CJHV5ZIP4HOZFXIWI X-Mailman-Approved-At: Thu, 09 Feb 2023 10:29:07 +0100 CC: passt-dev@passt.top X-Mailman-Version: 3.3.3 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 2/9/23 00:13, Laine Stump wrote: > I initially had the passt process being started in an identical > fashion to the slirp-helper - libvirt was daemonizing the new process > and recording its pid in a pidfile. The problem with this is that, > since it is daemonized immediately, any startup error in passt happens > after the daemonization, and thus isn't seen by libvirt - libvirt > believes that the process has started successfully and continues on > its merry way. The result was that sometimes a guest would be started, > but there would be no passt process for qemu to use for network > traffic. > > Instead, we should be starting passt in the same manner we start > dnsmasq - we just exec it as normal (along with a request that passt > create the pidfile, which is just another option on the passt > commandline) and wait for the child process to exit; passt then has a > chance to parse its commandline and complete all the setup prior to > daemonizing itself; if it encounters an error and exits with a non-0 > code, libvirt will see the code and know about the failure. We can > then grab the output from stderr, log that so the "user" has some idea > of what went wrong, and then fail the guest startup. > > Signed-off-by: Laine Stump > --- > src/qemu/qemu_passt.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c > index 0f09bf3db8..f640a69c00 100644 > --- a/src/qemu/qemu_passt.c > +++ b/src/qemu/qemu_passt.c > @@ -141,24 +141,23 @@ qemuPasstStart(virDomainObj *vm, > g_autofree char *passtSocketName = qemuPasstCreateSocketPath(vm, net); > g_autoptr(virCommand) cmd = NULL; > g_autofree char *pidfile = qemuPasstCreatePidFilename(vm, net); > + g_autofree char *errbuf = NULL; > char macaddr[VIR_MAC_STRING_BUFLEN]; > size_t i; > pid_t pid = (pid_t) -1; > int exitstatus = 0; > int cmdret = 0; > - VIR_AUTOCLOSE errfd = -1; > > cmd = virCommandNew(PASST); > > virCommandClearCaps(cmd); > - virCommandSetPidFile(cmd, pidfile); > - virCommandSetErrorFD(cmd, &errfd); > - virCommandDaemonize(cmd); > + virCommandSetErrorBuffer(cmd, &errbuf); > > virCommandAddArgList(cmd, > "--one-off", > "--socket", passtSocketName, > "--mac-addr", virMacAddrFormat(&net->mac, macaddr), > + "--pid", pidfile, The only problem with this approach is that our virPidFile*() functions rely on locking the very first byte. And when reading the pidfile, we try to lock the file and if we succeeded it means the file wasn't locked which means the process holding the lock died and thus the pid in the pidfile is stale. Now, I don't see passt locking the pidfile at all. So effectively, after this patch qemuPasstStop() would do nothing (well, okay, it'll remove the pidfile), qemuPasstSetupCgroup() does nothing, etc. What we usually do in this case, is: we let our code write the pidfile (just like the current code does), but then have a loop that waits a bit for socket to show up. If it doesn't in say 5 seconds we kill the child process (which we know the PID of). You can take inspiration from: qemuDBusStart() or qemuProcessStartManagedPRDaemon(). Michal