--- Begin Message ---
> +sub get_snap_name {
>>should this be public?
I'll make it private
> +sub get_snap_volname {
>>should this be public?
> +
> +sub parse_snapname {
>>should this be public?
This two methods are used in volume_snapshot_info(), defined in plugin,
and use by lvmplugin too
> DERUMIER, Alexandre hat am 08.07.2025
> 15:42 CEST geschrieben:
>
>
> >>okay, that means we instead need to become more strict with 'snapext'
> >>storages and restrict the volnames there.. maybe to (vm-|base-)-XXX-
> >>*.fmt?
>
> $plugin->parse_volname($volname) don't have $scfg param cu
--- Begin Message ---
>>okay, that means we instead need to become more strict with 'snapext'
>>storages and restrict the volnames there.. maybe to (vm-|base-)-XXX-
>>*.fmt?
$plugin->parse_volname($volname) don't have $scfg param currently,
Do you want to extend it ? (and do change in every plug
--- Begin Message ---
>>Can we please use an actually telling name though? As "ext" is quite
>>often used as term for "extension", and we really win nothing with
>>doing this.
>>
>>Strongly preferring words to be spelled out in full and separated
>>with
>>hyphens, instead of something else or being
Am 08.07.25 um 13:35 schrieb DERUMIER, Alexandre:
>>> okay, that means we instead need to become more strict with 'snapext'
>>> storages and restrict the volnames there.. maybe to (vm-|base-)-XXX-
>>> *.fmt?
>
>>> that means only allowing such names when allocating volumes, and
>>> filtering
>>> w
--- Begin Message ---
>
> I think it'll break parsing of already configured storage without
> snapext option ?
>>I don't think it does?
ah , I have tried with
{ fixed => 1 } only
but it's ok with
{ optional => 1, fixed => 1 }
pvesm set teststorage --snapext 1
update storage failed: can't cha
--- Begin Message ---
>>okay, that means we instead need to become more strict with 'snapext'
>>storages and restrict the volnames there.. maybe to (vm-|base-)-XXX-
>>*.fmt?
>>that means only allowing such names when allocating volumes, and
>>filtering
>>when listing images..
>>
>>since we want to
> DERUMIER, Alexandre hat am 08.07.2025
> 12:04 CEST geschrieben:
>
>
> > +my sub alloc_backed_image {
> > + my ($class, $storeid, $scfg, $volname, $backing_snap) = @_;
> > +
> > + my $path = $class->path($scfg, $volname, $storeid);
> > + my $backing_path = $class->path($scfg, $volna
--- Begin Message ---
> +my sub alloc_backed_image {
> + my ($class, $storeid, $scfg, $volname, $backing_snap) = @_;
> +
> + my $path = $class->path($scfg, $volname, $storeid);
> + my $backing_path = $class->path($scfg, $volname, $storeid,
> $backing_snap);
>>should we use a relative path
> DERUMIER, Alexandre hat am 08.07.2025
> 10:44 CEST geschrieben:
>
>
> > preallocation => { optional => 1 },
> > + snapext => { optional => 1 },
>
> >>needs to be "fixed", as the code doesn't handle mixing internal
> >>and external snapshots on a single storage..
>
> I thin
--- Begin Message ---
> preallocation => { optional => 1 },
> + snapext => { optional => 1 },
>>needs to be "fixed", as the code doesn't handle mixing internal
>>and external snapshots on a single storage..
I think it'll break parsing of already configured storage without
snapext
> DERUMIER, Alexandre hat am 07.07.2025
> 12:18 CEST geschrieben:
> >>for the DirPlugin we need to add another level for the snapshots
> >>on-disk, else we cannot differentiate between weirdly-named image
> >>files and snapshot files..
> >>
> >>so we could add a directory "snapshots" and put a
--- Begin Message ---
>
> or maybe use something else than volume_snapshot_info here, simply
> glob
> all the vm disk && snap files and delete them in random order, as we
> want to delete it anyway.
yes, this is exactly what I meant with tricky ;)
>>if we start deleting snapshots from the "first
> DERUMIER, Alexandre hat am 04.07.2025
> 15:22 CEST geschrieben:
> > + #delete external snapshots
> > + if ($scfg->{snapext}) {
> > + my $snapshots = $class->volume_snapshot_info($scfg,
> > $storeid, $volname);
> > + for my $snapid (
> > + sor
--- Begin Message ---
> + snapext => { optional => 1 },
>>needs to be "fixed", as the code doesn't handle mixing internal
>>and external snapshots on a single storage..
indeed, I'll fix it
>
>
> +my sub alloc_backed_image {
> + my ($class, $storeid, $scfg, $volname, $backing_snap) =
> Alexandre Derumier via pve-devel hat am
> 04.07.2025 08:45 CEST geschrieben:
> add a snapext option to enable the feature
>
> When a snapshot is taken, the current volume is renamed to snap volname
> and a current image is created with the snap volume as backing file
>
> Signed-off-by: Alex
16 matches
Mail list logo