From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <f.ebner@proxmox.com>
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 07FA196412
 for <pve-devel@lists.proxmox.com>; Mon, 15 Apr 2024 15:54:44 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id DD5D4B15F
 for <pve-devel@lists.proxmox.com>; Mon, 15 Apr 2024 15:54:43 +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) server-digest SHA256)
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS
 for <pve-devel@lists.proxmox.com>; Mon, 15 Apr 2024 15:54:42 +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 A6FEA44AAD
 for <pve-devel@lists.proxmox.com>; Mon, 15 Apr 2024 15:54:42 +0200 (CEST)
Message-ID: <c3af7312-6e9d-48f6-9641-67e3efc81efa@proxmox.com>
Date: Mon, 15 Apr 2024 15:54:41 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
 Filip Schauer <f.schauer@proxmox.com>
References: <20240415131702.94922-1-f.schauer@proxmox.com>
 <20240415131702.94922-2-f.schauer@proxmox.com>
Content-Language: en-US
From: Fiona Ebner <f.ebner@proxmox.com>
In-Reply-To: <20240415131702.94922-2-f.schauer@proxmox.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL -0.071 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 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 common v2 1/2] add get_device_stat helper
 subroutine
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Mon, 15 Apr 2024 13:54:44 -0000

Am 15.04.24 um 15:17 schrieb Filip Schauer:
> The get_device_stat subroutine gets a device path, validates it, and
> returns the file mode and the device identifier.
> 
> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
> ---
>  src/PVE/Tools.pm | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
> index 766c809..d1b53e5 100644
> --- a/src/PVE/Tools.pm
> +++ b/src/PVE/Tools.pm
> @@ -7,7 +7,8 @@ use Date::Format qw(time2str);
>  use Digest::MD5;
>  use Digest::SHA;
>  use Encode;
> -use Fcntl qw(:DEFAULT :flock);
> +use Errno qw(ENOENT);

We already have "use POSIX" with various error codes, so should be added
there.

> +use Fcntl qw(:DEFAULT :flock :mode);
>  use File::Basename;
>  use File::Path qw(make_path);
>  use Filesys::Df (); # don't overwrite our df()
> @@ -1853,6 +1854,21 @@ sub dev_t_minor($) {
>      return (($dev_t >> 12) & 0xfff00) | ($dev_t & 0xff);
>  }
>  
> +sub get_device_stat {

It's not the whole stat, so the name is a bit misleading. Tools.pm is
already very big and mixes different things, so not super happy about
adding new things to it. If it would return the whole stat, it could
still be fine IMHO, because it's more likely to be reusable later.
Alternatively, we can keep this code in pve-container.

> +    my ($path) = @_;
> +
> +    die "Path is not defined\n" if !defined($path);
> +
> +    my ($mode, $rdev) = (stat($path))[2, 6];
> +    die "Device $path does not exist\n" if $! == ENOENT;
> +    die "Error accessing device $path\n"
> +	if (!defined($mode) || !defined($rdev));
> +    die "$path is not a device\n"
> +	if (!S_ISBLK($mode) && !S_ISCHR($mode));

Style nit: useless parentheses, post-if can fit on the same line.

> +
> +    return ($mode, $rdev);
> +}
> +
>  # Given an array of array refs [ \[a b c], \[a b b], \[e b a] ]
>  # Returns the intersection of elements as a single array [a b]
>  sub array_intersect {