Re: [pve-devel] [PATCH common v2 1/2] add get_device_stat helper subroutine

[Fiona Ebner <f.ebner@proxmox.com>]

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 {