[pve-devel] [PATCH storage v4 02/12] storage: replace build-in stat with File::stat

[Alwin Antreich]

On Wed, Apr 22, 2020 at 05:35:05PM +0200, Dietmar Maurer wrote:
> AFAIK this can have ugly side effects ...
Okay, I was not aware of any know side effects.

I took the File::stat, since we use it already in pve-cluster,
qemu-server, pve-common, ... . And a off-list discussion with Thomas and
Fabian G.

If there is a better solution, I am happy to work on it.

> 
> > On April 22, 2020 4:57 PM Alwin Antreich <a.antreich at proxmox.com> wrote:
> > 
> >  
> > to minimize variable declarations. And allow to mock this method in
> > tests instead of the perl build-in stat.
> > 
> > Signed-off-by: Alwin Antreich <a.antreich at proxmox.com>
> > ---
> >  PVE/Diskmanage.pm     |  9 +++++----
> >  PVE/Storage/Plugin.pm | 34 ++++++++++------------------------
> >  2 files changed, 15 insertions(+), 28 deletions(-)
> > 
> > diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
> > index 13e7cd8..cac944d 100644
> > --- a/PVE/Diskmanage.pm
> > +++ b/PVE/Diskmanage.pm
> > @@ -6,6 +6,7 @@ use PVE::ProcFSTools;
> >  use Data::Dumper;
> >  use Cwd qw(abs_path);
> >  use Fcntl ':mode';
> > +use File::stat;
> >  use JSON;
> >  
> >  use PVE::Tools qw(extract_param run_command file_get_contents file_read_firstline dir_glob_regex dir_glob_foreach trim);
> > @@ -673,11 +674,11 @@ sub get_disks {
> >  sub get_partnum {
> >      my ($part_path) = @_;
> >  
> > -    my ($mode, $rdev) = (stat($part_path))[2,6];
> > +    my $st = stat($part_path);
> >  
> > -    next if !$mode || !S_ISBLK($mode) || !$rdev;
> > -    my $major = PVE::Tools::dev_t_major($rdev);
> > -    my $minor = PVE::Tools::dev_t_minor($rdev);
> > +    next if !$st->mode || !S_ISBLK($st->mode) || !$st->rdev;
> > +    my $major = PVE::Tools::dev_t_major($st->rdev);
> > +    my $minor = PVE::Tools::dev_t_minor($st->rdev);
> >      my $partnum_path = "/sys/dev/block/$major:$minor/";
> >  
> >      my $partnum;
> > diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
> > index 4489a77..d2dfad6 100644
> > --- a/PVE/Storage/Plugin.pm
> > +++ b/PVE/Storage/Plugin.pm
> > @@ -7,6 +7,7 @@ use Fcntl ':mode';
> >  use File::chdir;
> >  use File::Path;
> >  use File::Basename;
> > +use File::stat;
> >  use Time::Local qw(timelocal);
> >  
> >  use PVE::Tools qw(run_command);
> > @@ -718,12 +719,10 @@ sub free_image {
> >  sub file_size_info {
> >      my ($filename, $timeout) = @_;
> >  
> > -    my @fs = stat($filename);
> > -    my $mode = $fs[2];
> > -    my $ctime = $fs[10];
> > +    my $st = stat($filename);
> >  
> > -    if (S_ISDIR($mode)) {
> > -	return wantarray ? (0, 'subvol', 0, undef, $ctime) : 1;
> > +    if (S_ISDIR($st->mode)) {
> > +	return wantarray ? (0, 'subvol', 0, undef, $st->ctime) : 1;
> >      }
> >  
> >      my $json = '';
> > @@ -741,7 +740,7 @@ sub file_size_info {
> >  
> >      my ($size, $format, $used, $parent) = $info->@{qw(virtual-size format actual-size backing-filename)};
> >  
> > -    return wantarray ? ($size, $format, $used, $parent, $ctime) : $size;
> > +    return wantarray ? ($size, $format, $used, $parent, $st->ctime) : $size;
> >  }
> >  
> >  sub volume_size_info {
> > @@ -918,22 +917,9 @@ my $get_subdir_files = sub {
> >  
> >      foreach my $fn (<$path/*>) {
> >  
> > -	my ($dev,
> > -	    $ino,
> > -	    $mode,
> > -	    $nlink,
> > -	    $uid,
> > -	    $gid,
> > -	    $rdev,
> > -	    $size,
> > -	    $atime,
> > -	    $mtime,
> > -	    $ctime,
> > -	    $blksize,
> > -	    $blocks
> > -	) = stat($fn);
> > -
> > -	next if S_ISDIR($mode);
> > +	my $st = stat($fn);
> > +
> > +	next if S_ISDIR($st->mode);
> >  
> >  	my $info;
> >  
> > @@ -972,8 +958,8 @@ my $get_subdir_files = sub {
> >  	    };
> >  	}
> >  
> > -	$info->{size} = $size;
> > -	$info->{ctime} //= $ctime;
> > +	$info->{size} = $st->size;
> > +	$info->{ctime} //= $st->ctime;
> >  
> >  	push @$res, $info;
> >      }
> > -- 
> > 2.20.1
> > 
> > 
> > _______________________________________________
> > pve-devel mailing list
> > pve-devel at pve.proxmox.com
> > https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel