[pve-devel] [PATCH pve-storage 09/13] storage: add volume_support_qemu_snapshot

[Wolfgang Bumiller]

On Tue, Jul 15, 2025 at 03:38:57PM +0000, DERUMIER, Alexandre via pve-devel wrote:
> From: "DERUMIER, Alexandre" <alexandre.derumier at groupe-cyllene.com>
> To: "w.bumiller at proxmox.com" <w.bumiller at proxmox.com>
> CC: "pve-devel at lists.proxmox.com" <pve-devel at lists.proxmox.com>,
>   "t.lamprecht at proxmox.com" <t.lamprecht at proxmox.com>,
>  "f.gruenbichler at proxmox.com" <f.gruenbichler at proxmox.com>
> Subject: Re: [pve-devel] [PATCH pve-storage 09/13] storage: add
>  volume_support_qemu_snapshot
> Date: Tue, 15 Jul 2025 15:38:57 +0000
> Message-ID: <82c18a632e730b7c18a93aacd9f4d623d56061be.camel at groupe-cyllene.com>
> 
> >>Yeah I tried some quick tests and it seems to be a bit tricky. Or
> >>maybe
> >>I just missed something.
> 
> Just have done fast tests, I think I have found a way. 
> (I'll do more test tomorrow to see if everything is ok in guest)
> 
> 
> 
> sub qemu_volume_snapshot {
>     my ($vmid, $deviceid, $storecfg, $drive, $snap) = @_;
> .....
> 
>     } elsif ($do_snapshots_type eq 'external') {
>         my $storeid = (PVE::Storage::parse_volume_id($volid))[0];
>         my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
>         print "external qemu snapshot\n";
>         my $snapshots = PVE::Storage::volume_snapshot_info($storecfg,
> $volid);
>         my $parent_snap = $snapshots->{'current'}->{parent};
>         my $machine_version =
> PVE::QemuServer::Machine::get_current_qemu_machine($vmid);
> 
> #no rename here anymore
> 
>             PVE::QemuServer::Blockdev::blockdev_external_snapshot(
>                 $storecfg, $vmid, $machine_version, $deviceid, $drive,
> $snap, $parent_snap
>             );
> ...
> }
> 
> 
> sub blockdev_external_snapshot {
>     my ($storecfg, $vmid, $machine_version, $deviceid, $drive, $snap,
> $parent_snap) = @_;
> 
>     print "Creating a new current volume with $snap as backing snap\n";
> 
>     my $volid = $drive->{file};
> 
>     #rename current to snap && preallocate add a new current file with
> reference to snapbacking-file, we can remove the $running param
> 
>     my $running = undef
>     PVE::Storage::volume_snapshot($storecfg, $volid, $snap, undef);
> 
>     #call the blockdev_rename, only to reopen internal blockdev current
> active image to snap volname
> 
>     my $skip_volume_rename=1;
>     #reopen current to snap    
>      PVE::QemuServer::Blockdev::blockdev_rename(
>             $storecfg,
>             $vmid,
>             $machine_version,
>             $deviceid,
>             $drive,   
>             'current',
>             $snap,
>             $parent_snap,
>             $skip_volume_rename
>      );
> 
>     ###the rest is the same than before
> 
>     #be sure to add drive in write mode
>     delete($drive->{ro});
> 
>     my $new_file_blockdev = generate_file_blockdev($storecfg, $drive);
>     my $new_fmt_blockdev = generate_format_blockdev($storecfg, $drive,
> $new_file_blockdev);
> 
>     my $snap_file_blockdev = generate_file_blockdev($storecfg, $drive,
> $snap);
>     my $snap_fmt_blockdev = generate_format_blockdev(
>         $storecfg,
>         $drive,
>         $snap_file_blockdev,
>         { 'snapshot-name' => $snap },
>     );
> 
> 
>     #backing need to be forced to undef in blockdev, to avoid reopen of
> backing-file on blockdev-add
>     $new_fmt_blockdev->{backing} = undef;
> 
>     mon_cmd($vmid, 'blockdev-add', %$new_fmt_blockdev);
> 
>     mon_cmd(
>         $vmid, 'blockdev-snapshot',
>         node => $snap_fmt_blockdev->{'node-name'},
>         overlay => $new_fmt_blockdev->{'node-name'},
>     );
> }

seems to work - at least on my lvm tests