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

[DERUMIER, Alexandre]

> > 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.