Re: [pve-devel] [PATCH storage 12/26] plugin: support new vtypes in activate_storage checks

Wolfgang Bumiller
2025-08-08T13:15:47Z

On Wed, Jul 30, 2025 at 10:36:59AM +0200, Fabian Grünbichler wrote:
> On July 29, 2025 1:15 pm, Wolfgang Bumiller wrote:
> > Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> > ---
> > src/PVE/Storage/Plugin.pm | 20 ++++++++++++++++++--
> > 1 file changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
> > index 98788d1..047b2fc 100644
> > --- a/src/PVE/Storage/Plugin.pm
> > +++ b/src/PVE/Storage/Plugin.pm
> > @@ -1882,15 +1882,31 @@ sub activate_storage {
> >
> > # check that content dirs are pairwise inequal
> > my $resolved_subdirs = {};
> > - for my $vtype (sort keys $scfg->{content}->%*) {
> > + my %done;
> > + my $check = sub {
> > + my ($vtype) = @_;
> > +
> > + return if $done{$vtype};
> > + $done{$vtype} = 1;
> > +
> > my $subdir = $class->get_subdir($scfg, $vtype);
> > my $abs_subdir = abs_path($subdir);
> > - next if !defined($abs_subdir);
> > + return if !defined($abs_subdir);
> >
> > 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;
> > + };
> > + for my $vtype (sort keys $scfg->{content}->%*) {
> > + if ($vtype eq 'images') {
>
> shouldn't we also check for 'images' as well? we don't want that to
> overlap with vm-vol or ct-vol!

Yes.

>
> > + $check->('vm-vol');
> > + $check->('ct-vol');
> > + } elsif ($vtype eq 'rootdir') {
>
> same here, since this was actually mapped to the dir for 'images' in
> practice?

The reason this change even exists was because 'images' and 'rootdir' by
*default* are the same. But also, rootdir had no effect, so after some
thought, maybe we should just warn users when they try to modify the
'rootdir' directory?

As in:
for my $vtype (sort keys $scfg->{content}->%*) {
if ($vtype eq 'rootdir') {
warn "content-dir for 'rootdir' has no effect, set dirs for 'ct-vol' instead\n";
next;
}

$check->($vtype);

if ($vtype eq 'images') {
$check->('vm-vol');
$check->('ct-vol');
}
}

>
> > + $check->('ct-vol');
> > + } else {
> > + $check->($vtype);
> > + }
> > }
> > }
> > }
> > --
> > 2.47.2