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

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

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


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel