yesBad/pve-storage
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

activate storage: ensure content directories are created before checking them

Browse Source 
checking the content dirs for clashes via abs_path must be done after
the logic for creating them ran, as abs_path is working on actual
filesystem level, so it will return undf if the directory does not
exist, in which case we then set a hash entry for "undef", and the
next for loop round then resolved again to "undef", resulting in a
false-positive of the check.

Avoid the dangerous "return if" stanzas and reverse them to an actual
if block, which is much safer to adapt. Then move the check for
duplicate content-dir usage after that.

best viewed with white space change ignored: git show -w

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
This commit is contained in:
Thomas Lamprecht
2023-06-09 13:26:06 +02:00
parent dcc761c628
commit 8e623a2930
1 changed files with 31 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

53
src/PVE/Storage/Plugin.pm
View File

@ -1372,31 +1372,40 @@ sub activate_storage {
warn "${storeid}: 'mkdir' option is deprecated. Use 'create-base-path' or 'create-subdirs' instead.\n"
if defined($scfg->{mkdir});
# check that content dirs are pairwise inequal
my $resolved_subdirs = {};
if (defined($scfg->{content})) {
foreach my $vtype (keys $scfg->{content}->%*) {
my $abs_subdir = abs_path($class->get_subdir($scfg, $vtype));
die "storage '$storeid' uses directory $abs_subdir for multiple content types\n"
if defined($resolved_subdirs->{$abs_subdir});
$resolved_subdirs->{$abs_subdir} = 1;
}
}
return if defined($scfg->{'create-subdirs'}) && !$scfg->{'create-subdirs'};
# FIXME The mkdir option is deprecated. Remove with PVE 9?
return if defined($scfg->{mkdir}) && !$scfg->{mkdir};
if (defined($scfg->{content})) {
foreach my $vtype (keys %$vtype_subdirs) {
# OpenVZMigrate uses backup (dump) dir
if (defined($scfg->{content}->{$vtype}) ||
($vtype eq 'backup' && defined($scfg->{content}->{'rootdir'}))) {
my $subdir = $class->get_subdir($scfg, $vtype);
mkpath $subdir if $subdir ne $path;
# (opt-out) create content dirs and check validity
if (
(!defined($scfg->{'create-subdirs'}) || $scfg->{'create-subdirs'})
# FIXME The mkdir option is deprecated. Remove with PVE 9?
|| (!defined($scfg->{mkdir}) || $scfg->{mkdir})
) {
for my $vtype (sort keys %$vtype_subdirs) {
# OpenVZMigrate uses backup (dump) dir
if (
defined($scfg->{content}->{$vtype})
|| ($vtype eq 'backup' && defined($scfg->{content}->{'rootdir'}))
) {
my $subdir = $class->get_subdir($scfg, $vtype);
mkpath $subdir if $subdir ne $path;
}
}
}
# check that content dirs are pairwise inequal
my $resolved_subdirs = {};
for my $vtype (sort keys $scfg->{content}->%*) {
my $subdir = $class->get_subdir($scfg, $vtype);
my $abs_subdir = abs_path($subdir);
if (!defined($abs_subdir)) {
warn "could not get absolute path for '$subdir' - $!\n";
next;
}
die "storage '$storeid' uses directory $abs_subdir for multiple content types\n"
if defined($abs_subdir) && defined($resolved_subdirs->{$abs_subdir});
$resolved_subdirs->{$abs_subdir} = 1;
}
}
}