From: Fiona Ebner <f.ebner@proxmox.com>
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 [thread overview]
Message-ID: <20260925122646.139215-5-f.ebner@proxmox.com> (raw)
In-Reply-To: <20260925122646.139215-1-f.ebner@proxmox.com>
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 <f.ebner@proxmox.com>
---
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
next prev parent reply other threads:[~2026-09-25 12:27 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-25 12:26 [PATCH-SERIES qemu-server 0/7] fix #8071: volume snapshot delete: use current drive information to fix SAVC handling Fiona Ebner
2026-09-25 12:26 ` [PATCH qemu-server 1/7] volume snapshot (delete): avoid using deprecated check_running() helper Fiona Ebner
2026-09-25 12:26 ` [PATCH qemu-server 2/7] volume snapshot delete: move getting attached device ID to caller Fiona Ebner
2026-09-25 12:26 ` [PATCH qemu-server 3/7] fix #8071: volume snapshot delete: use current drive information to fix SAVC handling Fiona Ebner
2026-09-25 12:26 ` Fiona Ebner [this message]
2026-09-25 12:26 ` [PATCH qemu-server 5/7] snapshot: use v5.36 and subroutine signatures Fiona Ebner
2026-09-25 12:26 ` [PATCH qemu-server 6/7] volume chain: blockdev delete: remove superfluous parse_volume_id() call Fiona Ebner
2026-09-25 12:26 ` [PATCH qemu-server 7/7] volume chain: blockdev delete: pass volume ID instead of drive and mark private Fiona Ebner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260925122646.139215-5-f.ebner@proxmox.com \
--to=f.ebner@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.