[pve-devel] [PATCH pve-storage 2/5] qcow2: add external snapshot support

[Fabian Grünbichler]

> DERUMIER, Alexandre <alexandre.derumier at groupe-cyllene.com> hat am 19.05.2025 14:08 CEST geschrieben:
> 
>  
> > > sub free_image {
> >      my ($class, $storeid, $scfg, $volname, $isBase, $format) = @_;
> >  
> > @@ -980,6 +994,51 @@ sub free_image {
> >  # TODO taken from PVE/QemuServer/Drive.pm, avoiding duplication
> > would be nice
> >  my @checked_qemu_img_formats = qw(raw qcow qcow2 qed vmdk cloop);
> >  
> > +sub qemu_img_create {
> 
> >>should live in some helper module..
> 
> I have sent a separate patch serie today. 
> > 
> 
> > @@ -1506,7 +1648,40 @@ sub status {
> >  sub volume_snapshot_info {
> >      my ($class, $scfg, $storeid, $volname) = @_;
> >  
> > -    die "volume_snapshot_info is not implemented for $class";
> > +    my $path = $class->filesystem_path($scfg, $volname);
> > +    my ($vtype, $name, $vmid, $basename, $basevmid, $isBase,
> > $format) = $class->parse_volname($volname);
> > +
> > +    my $backing_chain = 1;
> 
> >>shouldn't this depend on $snapext ?
> 
> I'm also using it in lvm plugin. Do you think we need to add the
> snapext option in lvm too ?
> 
> > +    }
> > +    my $info = {};
> > +    my $order = 0;
> > +    for my $snap (@$snapshots) {
> 
> >>this doesn't work for internal snapshots, as then qemu-img info just
> >>returns a single object.. or if we pass --backingchain also in that
> >>case, then the code below still doesn't correctly handle it..
> 
> I didn't plan to use it for internal snapshots, as currently it's only
> use in the replication code (and we can't replcate internal snapshot)
> 
> Do you see an usecase ? I think we don't you have any user cli command
> to list snapshots in differents plugins currently ?
> if later we add replication on external snapshots, we shouldn't return
> internal snapshot list in this case.

a bit torn on this - while it's true it's currently only used for replication
(and now external snapshots), nothing about this says that's the case..
e.g., it would also be interesting for checking config versus volume snapshot
tree consistency?