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.antre...@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.antre...@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@pve.proxmox.com
> > https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
