From patchwork Fri Mar 7 01:53:12 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Chamberlain X-Patchwork-Id: 14005675 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 825982F32 for ; Fri, 7 Mar 2025 01:53:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.137.202.133 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741312398; cv=none; b=XeBXLh68AvI+BI3H8iUqBYpSMM7+UWARsjFYcFpbIKJ0mm+hMCRhY35aUI7nafu5tWrAURahCaEUXt5rr4J5FziYB3sKPzGMhJIf/BenPMdjP52BFlRJdmT4AGrZN6m2MScfcNNw4c4/fcTzd4WBKkLcSY4qEZ6qnogAeGLtGCY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741312398; c=relaxed/simple; bh=HO8/G+HZxR4pUXHTGgkwGwxSGAPBvNO+Zuo+1KDtcbg=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version; b=phPGjHr0Wvrc7EcTV50Hx3ahOdnKlgrCyMhX2kduISrj6nV6SqLKFjExGKgP96TnOBgL/LsASEpuINVihjPvpwEVDXkwIwfGLnClWNm8jF16S5AhQ0S9ImuLI5hZpf18574K3csf+1ACa/vBiFokPNtkqbDznMe3Q1X9SwXzTI4= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=kernel.org; spf=none smtp.mailfrom=infradead.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b=zWUUKOwA; arc=none smtp.client-ip=198.137.202.133 Authentication-Results: smtp.subspace.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=kernel.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="zWUUKOwA" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Sender:Content-Transfer-Encoding: MIME-Version:Message-ID:Date:Subject:Cc:To:From:Reply-To:Content-Type: Content-ID:Content-Description:In-Reply-To:References; bh=stRumLyZ2SvYnhxsytqczGChwl2G8AG51IUoKF0T5Ds=; b=zWUUKOwAcE7FsMG/MfImuMp+AN n0qTgd7ZAQ/bi8qsaJy4QbKnIJ+jOAkbcrDDR4IjZQvNH21G2VHCfvRNDnhVpgG1zsaFCJw7JWNGK Fyo0doU/To0vS5pU3APh/E1hVO4t5g7t4Zm/v8pWjP/8XBMi2gV6oBuIz4hF++MSEZgnpL7SNd4Mf BYqhfz7MdE2040h1TGoZFDDoDBYHqOtQMlTQ6WVKTDRUreFHMab9afqYgQaRLNgo2uehgLpCP4qBk XcWZueTUXI4LzN+Q/O+qsghVURKVQIVsSjr1fsamRzNkfZMaN0AAAkKjPoIlUIQJIQ41J5mns7rNQ ZqNAo+Rg==; Received: from mcgrof by bombadil.infradead.org with local (Exim 4.98 #2 (Red Hat Linux)) id 1tqMtl-0000000CqzS-2JzK; Fri, 07 Mar 2025 01:53:13 +0000 From: Luis Chamberlain To: kdevops@lists.linux.dev Cc: joel.granados@kernel.org, Luis Chamberlain , Swarna Prabhu Subject: [PATCH] scripts/libvirt_pool.sh: fix regression with pool heuristics Date: Thu, 6 Mar 2025 17:53:12 -0800 Message-ID: <20250307015312.3063619-1-mcgrof@kernel.org> X-Mailer: git-send-email 2.48.1 Precedence: bulk X-Mailing-List: kdevops@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Sender: Luis Chamberlain Heavy users of kdevops will often want to dedicate an entire partition so that it can be used for all guests of *all users*. This also allows us to share *all guestfs images* so we only have to download a guestfs image once for custom images like Debian Trixie. For instance: sudo virsh pool-dumpxml xfs1 | grep path /xfs1 If we go back in history we can see this used to work, let's try with the first repo's tree's history, we *used* to get: mcgrof@big-box /xfs1/mcgrof/debug-pools/kdevops (git::main)$ make mrproper; make defconfig-kernel-hacking > /dev/null 2>&1 grep CONFIG_KDEVOPS_STORAGE_POOL_PATH .config CONFIG_KDEVOPS_STORAGE_POOL_PATH="/xfs1" Now we get: mcgrof@big-box /xfs1/mcgrof/debug-pools/kdevops (git::main)$ make mrproper; make defconfig-kernel-hacking > /dev/null 2>&1 grep CONFIG_KDEVOPS_STORAGE_POOL_PATH .config CONFIG_KDEVOPS_STORAGE_POOL_PATH="/xfs1/mcgrof/debug-pools/kdevops/default So now we lie, because the heuristic to see if we can use sudo for virsh pool stuff is broken, so we try to create a new pool every single time the user uses kdevops. I bisected this down to commit bc731be7f131 ("scripts: Adjust heuristic to see if current user can sudo"), so the pool heuristic has been broken since January 13th. That also means we were forcing down each new CI in place the download of a new debian trixie image. There's a few issues with this new heuristic: 1) CAN_SUDO=get_can_sudo Well that's just a string, not an execution so we never even ran this new routine. 2) It's a good thing we never ran it anyway because the routine calls exit 3) The sudo heuristic was broken as it assumed the -eq would capture previous routine's exit call but, $(foo) it actually just captures the output. So it could never have worked. Fix the heuristics. Even though a first reaction may be to see if we can move away from these shell heuristics with ansible, these helpers are used by scripts we use to set default variables inside Kconfig strings and bools so ansible can't be used unless we're comfortable with the idea of calling ansible as part of a local playbook to do some local work. Maybe that is not crazy idea after all. Cc: Swarna Prabhu Cc: Joel Granados Signed-off-by: Luis Chamberlain --- I already applied the patch and pushe dit as its a fix I've been dying to have for a while now, I just had no time to bisect until now. Posting to the list for posterity. scripts/libvirt_pool.sh | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/scripts/libvirt_pool.sh b/scripts/libvirt_pool.sh index ffae92817ca8..ce25fdd6e552 100644 --- a/scripts/libvirt_pool.sh +++ b/scripts/libvirt_pool.sh @@ -13,12 +13,10 @@ get_can_sudo() # 2. With sudo it has two messages: one for paswordless sudo and # one passwordfull sudo. But we ignore the distinction as both # of these mean that can_sudo is "y". - if [[ $(sudo -nv 2>&1 | grep 'may not' > /dev/null) -eq 0 ]]; then + if sudo -nv 2>&1 | grep -q 'may not'; then echo "n" - exit fi echo "y" - exit } get_pool_vars() @@ -30,7 +28,7 @@ get_pool_vars() fi fi - CAN_SUDO=get_can_sudo + CAN_SUDO="$(get_can_sudo)" if [[ "$USES_QEMU_USER_SESSION" != "y" ]]; then REQ_SUDO="sudo"