From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 8FACE1FF0B2 for ; Fri, 25 Sep 2026 14:27:54 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 27FDC216F1; Fri, 25 Sep 2026 14:27:27 +0200 (CEST) From: Fiona Ebner To: pve-devel@lists.proxmox.com Subject: [PATCH qemu-server 4/7] snapshot: add module for snapshot-related functionality Date: Fri, 25 Sep 2026 14:26:28 +0200 Message-ID: <20260925122646.139215-5-f.ebner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260925122646.139215-1-f.ebner@proxmox.com> References: <20260925122646.139215-1-f.ebner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1790339210089 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.501 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: TJK7OEINVXQIZQKVMHH6C77OWMM6QL46 X-Message-ID-Hash: TJK7OEINVXQIZQKVMHH6C77OWMM6QL46 X-MailFrom: f.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Initially, the 'Snapshot' module contains the functions for creating and removing a snapshot from a volume. Drops two more calls from QemuConfig back to QemuServer, making progress towards getting rid of that cyclic dependency. This was the only usage of the qmp_cmd() helper in QemuServer, so remove the import. Signed-off-by: Fiona Ebner --- src/PVE/QemuConfig.pm | 5 +- src/PVE/QemuServer.pm | 150 +----------------------------- src/PVE/QemuServer/Makefile | 1 + src/PVE/QemuServer/Snapshot.pm | 162 +++++++++++++++++++++++++++++++++ src/test/snapshot-test.pm | 7 +- 5 files changed, 170 insertions(+), 155 deletions(-) create mode 100644 src/PVE/QemuServer/Snapshot.pm diff --git a/src/PVE/QemuConfig.pm b/src/PVE/QemuConfig.pm index 39e88366..c84f10f8 100644 --- a/src/PVE/QemuConfig.pm +++ b/src/PVE/QemuConfig.pm @@ -18,6 +18,7 @@ use PVE::QemuServer::Monitor qw(mon_cmd); use PVE::QemuServer; use PVE::QemuServer::Machine; use PVE::QemuServer::Memory qw(get_current_memory); +use PVE::QemuServer::Snapshot; use PVE::RESTEnvironment qw(log_warn); use PVE::Storage; use PVE::Tools; @@ -401,7 +402,7 @@ sub __snapshot_create_vol_snapshot { print "snapshotting '$device' ($drive->{file})\n"; - PVE::QemuServer::qemu_volume_snapshot($vmid, $device, $storecfg, $drive, $snapname); + PVE::QemuServer::Snapshot::qemu_volume_snapshot($vmid, $device, $storecfg, $drive, $snapname); } sub __snapshot_delete_remove_drive { @@ -458,7 +459,7 @@ sub __snapshot_delete_vol_snapshot { ); } - PVE::QemuServer::qemu_volume_snapshot_delete( + PVE::QemuServer::Snapshot::qemu_volume_snapshot_delete( $vmid, $storecfg, $drive, $snapname, $attached_deviceid, $running, ); diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm index e76a6ec4..322ad7fa 100644 --- a/src/PVE/QemuServer.pm +++ b/src/PVE/QemuServer.pm @@ -83,7 +83,7 @@ use PVE::QemuServer::DriveDevice qw(print_drivedevice_full scsihw_infos); use PVE::QemuServer::Machine; use PVE::QemuServer::Memory qw(get_current_memory); use PVE::QemuServer::MetaInfo; -use PVE::QemuServer::Monitor qw(mon_cmd qmp_cmd vm_qmp_peer); +use PVE::QemuServer::Monitor qw(mon_cmd vm_qmp_peer); use PVE::QemuServer::Network; use PVE::QemuServer::OVMF; use PVE::QemuServer::PCI qw(print_pci_addr print_pcie_addr parse_hostpci get_pci_bridges); @@ -4332,134 +4332,6 @@ sub qemu_cpu_hotplug { } } -sub qemu_volume_snapshot { - my ($vmid, $deviceid, $storecfg, $drive, $snap) = @_; - - my $volid = $drive->{file}; - my $running = PVE::QemuServer::Helpers::vm_running_locally($vmid); - - my $do_snapshots_type = do_snapshots_type($storecfg, $drive, $deviceid, $running); - - if ($do_snapshots_type eq 'internal') { - print "internal qemu snapshot\n"; - my $qmp_peer = PVE::QemuServer::Drive::drive_qmp_peer($storecfg, $vmid, $drive); - qmp_cmd($qmp_peer, 'blockdev-snapshot-internal-sync', device => $deviceid, name => $snap); - } elsif ($do_snapshots_type eq 'external') { - my $machine_version = PVE::QemuServer::Machine::get_current_qemu_machine($vmid); - if (!PVE::QemuServer::Machine::is_machine_version_at_least($machine_version, 10, 0)) { - die "storage for '$volid' is configured for snapshots as a volume chain - this requires" - . " QEMU machine version >= 10.0. See" - . " https://pve.proxmox.com/wiki/QEMU_Machine_Version_Upgrade\n"; - } - 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 $qmp_peer = PVE::QemuServer::Drive::drive_qmp_peer($storecfg, $vmid, $drive); - PVE::QemuServer::VolumeChain::blockdev_external_snapshot( - $storecfg, $qmp_peer, $machine_version, $deviceid, $drive, $snap, $parent_snap, - ); - } elsif ($do_snapshots_type eq 'storage') { - PVE::Storage::volume_snapshot($storecfg, $volid, $snap); - } -} - -sub qemu_volume_snapshot_delete { - my ($vmid, $storecfg, $drive, $snap, $attached_deviceid, $running) = @_; - - my $volid = $drive->{file}; - - my $do_snapshots_type = do_snapshots_type($storecfg, $drive, $attached_deviceid, $running); - - if ($do_snapshots_type eq 'internal') { - my $qmp_peer = PVE::QemuServer::Drive::drive_qmp_peer($storecfg, $vmid, $drive); - qmp_cmd( - $qmp_peer, - 'blockdev-snapshot-delete-internal-sync', - device => $attached_deviceid, - name => $snap, - ); - } elsif ($do_snapshots_type eq 'external') { - my $machine_version = PVE::QemuServer::Machine::get_current_qemu_machine($vmid); - if (!PVE::QemuServer::Machine::is_machine_version_at_least($machine_version, 10, 0)) { - die "storage for '$volid' is configured for snapshots as a volume chain - this requires" - . " QEMU machine version >= 10.0. See" - . " https://pve.proxmox.com/wiki/QEMU_Machine_Version_Upgrade\n"; - } - - print "delete qemu external snapshot\n"; - - my $path = PVE::Storage::path($storecfg, $volid); - my $snapshots = PVE::Storage::volume_snapshot_info($storecfg, $volid); - - die "could not find snapshot '$snap' for volume '$volid'\n" - if !defined($snapshots->{$snap}); - - my $parentsnap = $snapshots->{$snap}->{parent}; - my $childsnap = $snapshots->{$snap}->{child}; - - my $qmp_peer = PVE::QemuServer::Drive::drive_qmp_peer($storecfg, $vmid, $drive); - - # if we delete the first snasphot, we commit because the first snapshot original base image, it should be big. - # improve-me: if firstsnap > child : commit, if firstsnap < child do a stream. - if (!$parentsnap) { - print "delete first snapshot $snap\n"; - - my $snap_size = $snapshots->{$snap}->{'virtual-size'}; - my $child_size = $snapshots->{$childsnap}->{'virtual-size'}; - if (defined($child_size) && defined($snap_size) && $child_size > $snap_size) { - print - "resize '$snap' ($snap_size bytes) to match '$childsnap' ($child_size bytes)\n"; - PVE::Storage::volume_resize($storecfg, $volid, $child_size, $running, $snap); - } - - PVE::QemuServer::VolumeChain::blockdev_commit( - $storecfg, - $qmp_peer, - $machine_version, - $attached_deviceid, - $drive, - $childsnap, - $snap, - ); - - PVE::Storage::rename_snapshot($storecfg, $volid, $snap, $childsnap); - - PVE::QemuServer::VolumeChain::blockdev_replace( - $storecfg, - $qmp_peer, - $machine_version, - $attached_deviceid, - $drive, - $snap, - $childsnap, - $snapshots->{$childsnap}->{child}, - ); - } else { - #intermediate snapshot, we always stream the snapshot to child snapshot - print "stream intermediate snapshot $snap to $childsnap\n"; - PVE::QemuServer::VolumeChain::blockdev_stream( - $storecfg, - $qmp_peer, - $machine_version, - $attached_deviceid, - $drive, - $snap, - $parentsnap, - $childsnap, - ); - } - } elsif ($do_snapshots_type eq 'storage') { - PVE::Storage::volume_snapshot_delete( - $storecfg, - $volid, - $snap, - $attached_deviceid ? 1 : undef, - ); - } -} - sub foreach_volid { my ($conf, $func, @param) = @_; @@ -7810,26 +7682,6 @@ sub restore_tar_archive { warn $@ if $@; } -sub do_snapshots_type { - my ($storecfg, $drive, $deviceid, $running) = @_; - - #we use storage snapshot if vm is not running or if disk is unused; - return 'storage' if !$running || !$deviceid; - - if ( - $deviceid eq 'drive-tpmstate0' - && !PVE::QemuServer::Drive::drive_uses_qsd_fuse($storecfg, $drive) - ) { - return 'storage'; - } - - if (my $method = PVE::Storage::volume_qemu_snapshot_method($storecfg, $drive->{file})) { - return 'internal' if $method eq 'qemu'; - return 'external' if $method eq 'mixed'; - } - return 'storage'; -} - =head3 template_create($vmid, $conf [, $disk]) Converts all used disk volumes for the VM with the identifier C<$vmid> and diff --git a/src/PVE/QemuServer/Makefile b/src/PVE/QemuServer/Makefile index 060fac23..61ed1a4e 100644 --- a/src/PVE/QemuServer/Makefile +++ b/src/PVE/QemuServer/Makefile @@ -27,6 +27,7 @@ SOURCES=Agent.pm \ QSD.pm \ RNG.pm \ RunState.pm \ + Snapshot.pm \ StateFile.pm \ USB.pm \ Virtiofs.pm \ diff --git a/src/PVE/QemuServer/Snapshot.pm b/src/PVE/QemuServer/Snapshot.pm new file mode 100644 index 00000000..a0d4c239 --- /dev/null +++ b/src/PVE/QemuServer/Snapshot.pm @@ -0,0 +1,162 @@ +package PVE::QemuServer::Snapshot; + +use strict; +use warnings; + +use PVE::Storage; + +use PVE::QemuServer::Drive; +use PVE::QemuServer::Helpers; +use PVE::QemuServer::Machine; +use PVE::QemuServer::Monitor qw(qmp_cmd); +use PVE::QemuServer::VolumeChain; + +sub do_snapshots_type { + my ($storecfg, $drive, $deviceid, $running) = @_; + + #we use storage snapshot if vm is not running or if disk is unused; + return 'storage' if !$running || !$deviceid; + + if ( + $deviceid eq 'drive-tpmstate0' + && !PVE::QemuServer::Drive::drive_uses_qsd_fuse($storecfg, $drive) + ) { + return 'storage'; + } + + if (my $method = PVE::Storage::volume_qemu_snapshot_method($storecfg, $drive->{file})) { + return 'internal' if $method eq 'qemu'; + return 'external' if $method eq 'mixed'; + } + return 'storage'; +} + +sub qemu_volume_snapshot { + my ($vmid, $deviceid, $storecfg, $drive, $snap) = @_; + + my $volid = $drive->{file}; + my $running = PVE::QemuServer::Helpers::vm_running_locally($vmid); + + my $do_snapshots_type = do_snapshots_type($storecfg, $drive, $deviceid, $running); + + if ($do_snapshots_type eq 'internal') { + print "internal qemu snapshot\n"; + my $qmp_peer = PVE::QemuServer::Drive::drive_qmp_peer($storecfg, $vmid, $drive); + qmp_cmd($qmp_peer, 'blockdev-snapshot-internal-sync', device => $deviceid, name => $snap); + } elsif ($do_snapshots_type eq 'external') { + my $machine_version = PVE::QemuServer::Machine::get_current_qemu_machine($vmid); + if (!PVE::QemuServer::Machine::is_machine_version_at_least($machine_version, 10, 0)) { + die "storage for '$volid' is configured for snapshots as a volume chain - this requires" + . " QEMU machine version >= 10.0. See" + . " https://pve.proxmox.com/wiki/QEMU_Machine_Version_Upgrade\n"; + } + 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 $qmp_peer = PVE::QemuServer::Drive::drive_qmp_peer($storecfg, $vmid, $drive); + PVE::QemuServer::VolumeChain::blockdev_external_snapshot( + $storecfg, $qmp_peer, $machine_version, $deviceid, $drive, $snap, $parent_snap, + ); + } elsif ($do_snapshots_type eq 'storage') { + PVE::Storage::volume_snapshot($storecfg, $volid, $snap); + } +} + +sub qemu_volume_snapshot_delete { + my ($vmid, $storecfg, $drive, $snap, $attached_deviceid, $running) = @_; + + my $volid = $drive->{file}; + + my $do_snapshots_type = do_snapshots_type($storecfg, $drive, $attached_deviceid, $running); + + if ($do_snapshots_type eq 'internal') { + my $qmp_peer = PVE::QemuServer::Drive::drive_qmp_peer($storecfg, $vmid, $drive); + qmp_cmd( + $qmp_peer, + 'blockdev-snapshot-delete-internal-sync', + device => $attached_deviceid, + name => $snap, + ); + } elsif ($do_snapshots_type eq 'external') { + my $machine_version = PVE::QemuServer::Machine::get_current_qemu_machine($vmid); + if (!PVE::QemuServer::Machine::is_machine_version_at_least($machine_version, 10, 0)) { + die "storage for '$volid' is configured for snapshots as a volume chain - this requires" + . " QEMU machine version >= 10.0. See" + . " https://pve.proxmox.com/wiki/QEMU_Machine_Version_Upgrade\n"; + } + + print "delete qemu external snapshot\n"; + + my $path = PVE::Storage::path($storecfg, $volid); + my $snapshots = PVE::Storage::volume_snapshot_info($storecfg, $volid); + + die "could not find snapshot '$snap' for volume '$volid'\n" + if !defined($snapshots->{$snap}); + + my $parentsnap = $snapshots->{$snap}->{parent}; + my $childsnap = $snapshots->{$snap}->{child}; + + my $qmp_peer = PVE::QemuServer::Drive::drive_qmp_peer($storecfg, $vmid, $drive); + + # if we delete the first snasphot, we commit because the first snapshot original base image, it should be big. + # improve-me: if firstsnap > child : commit, if firstsnap < child do a stream. + if (!$parentsnap) { + print "delete first snapshot $snap\n"; + + my $snap_size = $snapshots->{$snap}->{'virtual-size'}; + my $child_size = $snapshots->{$childsnap}->{'virtual-size'}; + if (defined($child_size) && defined($snap_size) && $child_size > $snap_size) { + print + "resize '$snap' ($snap_size bytes) to match '$childsnap' ($child_size bytes)\n"; + PVE::Storage::volume_resize($storecfg, $volid, $child_size, $running, $snap); + } + + PVE::QemuServer::VolumeChain::blockdev_commit( + $storecfg, + $qmp_peer, + $machine_version, + $attached_deviceid, + $drive, + $childsnap, + $snap, + ); + + PVE::Storage::rename_snapshot($storecfg, $volid, $snap, $childsnap); + + PVE::QemuServer::VolumeChain::blockdev_replace( + $storecfg, + $qmp_peer, + $machine_version, + $attached_deviceid, + $drive, + $snap, + $childsnap, + $snapshots->{$childsnap}->{child}, + ); + } else { + #intermediate snapshot, we always stream the snapshot to child snapshot + print "stream intermediate snapshot $snap to $childsnap\n"; + PVE::QemuServer::VolumeChain::blockdev_stream( + $storecfg, + $qmp_peer, + $machine_version, + $attached_deviceid, + $drive, + $snap, + $parentsnap, + $childsnap, + ); + } + } elsif ($do_snapshots_type eq 'storage') { + PVE::Storage::volume_snapshot_delete( + $storecfg, + $volid, + $snap, + $attached_deviceid ? 1 : undef, + ); + } +} + +1; diff --git a/src/test/snapshot-test.pm b/src/test/snapshot-test.pm index 0c5715c5..44e55d6a 100644 --- a/src/test/snapshot-test.pm +++ b/src/test/snapshot-test.pm @@ -404,10 +404,6 @@ sub set_migration_caps { } # ignored # BEGIN redefine PVE::QemuServer methods -sub do_snapshots_type { - return 'storage'; -} - sub vm_start { my ($storecfg, $vmid, $params, $migrate_opts) = @_; @@ -463,6 +459,9 @@ $qemu_config_module->mock('has_feature', \&has_feature); $qemu_config_module->mock('__snapshot_save_vmstate', \&__snapshot_save_vmstate); $qemu_config_module->mock('assert_config_exists_on_node', \&assert_config_exists_on_node); +my $qemu_snapshot_module = Test::MockModule->new('PVE::QemuServer::Snapshot'); +$qemu_snapshot_module->mock('do_snapshots_type' => sub { return 'storage'; }); + # ignore existing replication config my $repl_config_module = Test::MockModule->new('PVE::ReplicationConfig'); $repl_config_module->mock('new' => sub { return bless {}, "PVE::ReplicationConfig" }); -- 2.47.3