all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Hannes Laimer <h.laimer@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Fiona Ebner <f.ebner@proxmox.com>
Subject: Re: [pve-devel] [PATCH qemu-server] snapshot create/delete: die early for snapshot-as-volume-chain for pre-10.0 machine version
Date: Thu, 7 Aug 2025 14:04:23 +0200	[thread overview]
Message-ID: <6e3ffc1e-ccfc-4ec6-93f7-03bef79c8b78@proxmox.com> (raw)
In-Reply-To: <20250807104832.51784-1-f.ebner@proxmox.com>

Could reproduce the problem described in [0], patch works as advertised,
so consider this

Tested-by: Hannes Laimer <h.laimer@proxmox.com>

On 07.08.25 12:48, Fiona Ebner wrote:
> As reported in the community forum [0], a running VM with pre-10.0
> machine version using a storage with snapshot-as-volume-chain will run
> into issues when creating a snapshot. Similarly deleting the snapshot
> of such a VM would fail. Having '-blockdev' is a hard requirement for
> the implementation of the snapshot-as-volume-chain feature for running
> VMs, so die and suggest upgrading the machine version.
> 
> [0]: https://forum.proxmox.com/threads/lvm-thick-with-iscsi-pve-9-0-3.169319/
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>   src/PVE/QemuServer.pm | 15 +++++++++++++--
>   1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
> index d42e2115..e30b27cb 100644
> --- a/src/PVE/QemuServer.pm
> +++ b/src/PVE/QemuServer.pm
> @@ -4447,12 +4447,17 @@ sub qemu_volume_snapshot {
>           print "internal qemu snapshot\n";
>           mon_cmd($vmid, '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 $machine_version = PVE::QemuServer::Machine::get_current_qemu_machine($vmid);
>           PVE::QemuServer::Blockdev::blockdev_external_snapshot(
>               $storecfg, $vmid, $machine_version, $deviceid, $drive, $snap, $parent_snap,
>           );
> @@ -4489,6 +4494,13 @@ sub qemu_volume_snapshot_delete {
>               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);
> @@ -4499,7 +4511,6 @@ sub qemu_volume_snapshot_delete {
>   
>           my $parentsnap = $snapshots->{$snap}->{parent};
>           my $childsnap = $snapshots->{$snap}->{child};
> -        my $machine_version = PVE::QemuServer::Machine::get_current_qemu_machine($vmid);
>   
>           # 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.



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


  reply	other threads:[~2025-08-07 12:02 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-07 10:47 Fiona Ebner
2025-08-07 12:04 ` Hannes Laimer [this message]
2025-08-07 12:32 ` [pve-devel] applied: " Thomas Lamprecht

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=6e3ffc1e-ccfc-4ec6-93f7-03bef79c8b78@proxmox.com \
    --to=h.laimer@proxmox.com \
    --cc=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.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal