From nobody Fri Mar 29 15:56:15 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 216.205.24.74 as permitted sender) client-ip=216.205.24.74; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-74.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 216.205.24.74 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1585094309; cv=none; d=zohomail.com; s=zohoarc; b=jykqkCUIMSjhCzuicjMtBg9mW2KQIy8aWuAmDX0lEcvWt+u6C44D1R0ac0F/6o7DJoKBTmzxRcAc0czLg52NlaHyaGzoz9kdW+QiqMp8KfVib24QBpe75k6bY6ceB60FcfmrRMqvfy/ik5kTxeltHOHMdG9fkbwnp15Ufkrq9Ts= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1585094309; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Sender:Subject:To; bh=z6AB5kLsxcfbqdGiSHAinW3l2sySXl6elaIlLqZevtQ=; b=ibtj6Gu7cd1lbZfCUVzkW+XOjv+MALq6ywvyld+vTmBzu4HfOIK0Hn/x/mqAvfuWKcj+4I0bXaLUDnU8BZArO2JOe/9R9CeiWtDQ4ypmc8w/UrkR/fuzUr61uspxj1ruRfVjGXoNB51zZ/roLgg59a8HbSxkrW+/rLUkI60v6Qw= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 216.205.24.74 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) header.from= Return-Path: Received: from us-smtp-delivery-74.mimecast.com (us-smtp-delivery-74.mimecast.com [216.205.24.74]) by mx.zohomail.com with SMTPS id 15850943097547.542946220480303; Tue, 24 Mar 2020 16:58:29 -0700 (PDT) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-15-3o_IeVaTMxqW8mJmninMPw-1; Tue, 24 Mar 2020 19:58:26 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 29D591851C2C; Tue, 24 Mar 2020 23:58:19 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9A70BBBBCE; Tue, 24 Mar 2020 23:58:16 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id F064B1809565; Tue, 24 Mar 2020 23:58:10 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 02ONw8fs011795 for ; Tue, 24 Mar 2020 19:58:08 -0400 Received: by smtp.corp.redhat.com (Postfix) id 6AB1E94B41; Tue, 24 Mar 2020 23:58:08 +0000 (UTC) Received: from localhost (unknown [10.36.110.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1DA2C6EFB4; Tue, 24 Mar 2020 23:58:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1585094308; h=from:from:sender:sender: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:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=z6AB5kLsxcfbqdGiSHAinW3l2sySXl6elaIlLqZevtQ=; b=Dxn1sYgsdGVuWUI3Thsq60ZTwe/GtdORPMkSQ5C/hGybzq7/m/dxnlVdgZ2f+EUwyBn1Qr zJ3S+yYvcSrQ5OTAgLDNU62BdAZeq1ZLbre4/Lk/kYbv9sFf3bTmCAJaanMcpW6CSp61PE t5cfO8k8dlo92MFZcEPFFwKYsVrf/qg= X-MC-Unique: 3o_IeVaTMxqW8mJmninMPw-1 From: marcandre.lureau@redhat.com To: libvir-list@redhat.com Subject: [libvirt PATCH] util: keep the pidfile locked Date: Wed, 25 Mar 2020 00:58:00 +0100 Message-Id: <20200324235800.1880165-1-marcandre.lureau@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-loop: libvir-list@redhat.com Cc: mprivozn@redhat.com, =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) Content-Type: text/plain; charset="utf-8" From: Marc-Andr=C3=A9 Lureau Unfortunately, advisory record locking lose the lock if any fd refering to the file is closed. There doesn't seem to be a way to preserve the lock atomically. We could eventually retake the lock if low pidfilefd is required. This fixes processes being leaked, as they are not killed in virPidFileForceCleanupPath() if the lock can be taken. Here also, we may consider this is not good enough, as a process may leak by simply closing the pidfilefd. Fixes commit d146105f1e4a9e0ab179f0b78c070ea38b9d5334 ("virCommand: Actually acquire pidfile instead of just writing it") Signed-off-by: Marc-Andr=C3=A9 Lureau Reviewed-by: Michal Privoznik --- src/util/vircommand.c | 12 ++---------- tests/commanddata/test4.log | 2 +- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 77078d09fb..b84fb40948 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -797,8 +797,7 @@ virExec(virCommandPtr cmd) virProcessSetMaxCoreSize(0, cmd->maxCore) < 0) goto fork_error; if (cmd->pidfile) { - VIR_AUTOCLOSE pidfilefd =3D -1; - int newpidfilefd =3D -1; + int pidfilefd =3D -1; char c; =20 pidfilefd =3D virPidFileAcquirePath(cmd->pidfile, false, getpid()); @@ -818,14 +817,7 @@ virExec(virCommandPtr cmd) VIR_FORCE_CLOSE(pipesync[0]); VIR_FORCE_CLOSE(pipesync[1]); =20 - /* This is here only to move the pidfilefd - * to the lowest possible number. */ - if ((newpidfilefd =3D dup(pidfilefd)) < 0) { - virReportSystemError(errno, "%s", _("Unable to dup FD")); - goto fork_error; - } - - /* newpidfilefd is intentionally leaked. */ + /* pidfilefd is intentionally leaked. */ } =20 if (cmd->hook) { diff --git a/tests/commanddata/test4.log b/tests/commanddata/test4.log index 5820f28307..24a37a7e96 100644 --- a/tests/commanddata/test4.log +++ b/tests/commanddata/test4.log @@ -9,7 +9,7 @@ ENV:USER=3Dtest FD:0 FD:1 FD:2 -FD:3 +FD:5 DAEMON:yes CWD:/ UMASK:0022 --=20 2.26.0.rc2.42.g98cedd0233