From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
by lore.proxmox.com (Postfix) with ESMTPS id BE6EE1FF195
for <inbox@lore.proxmox.com>; Tue, 13 May 2025 12:47:52 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id C987E1ED09;
Tue, 13 May 2025 12:48:12 +0200 (CEST)
Date: Tue, 13 May 2025 12:48:07 +0200 (CEST)
From: =?UTF-8?Q?Fabian_Gr=C3=BCnbichler?= <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Message-ID: <1379246505.14356.1747133287907@webmail.proxmox.com>
In-Reply-To: <mailman.30.1745322744.394.pve-devel@lists.proxmox.com>
References: <20250422115141.808427-1-alexandre.derumier@groupe-cyllene.com>
<mailman.30.1745322744.394.pve-devel@lists.proxmox.com>
MIME-Version: 1.0
X-Priority: 3
Importance: Normal
X-Mailer: Open-Xchange Mailer v7.10.6-Rev75
X-Originating-Client: open-xchange-appsuite
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.044 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
information. [qemuserver.pm, qemuconfig.pm, drive.pm]
Subject: Re: [pve-devel] [PATCH qemu-server 14/14] qcow2: add external
snapshot support
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
> Alexandre Derumier via pve-devel <pve-devel@lists.proxmox.com> hat am 22.04.2025 13:51 CEST geschrieben:
> Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
> ---
> PVE/QemuConfig.pm | 4 +-
> PVE/QemuServer.pm | 237 +++++++++++++++++++++++++++++++++++-----
> PVE/QemuServer/Drive.pm | 39 ++++---
> 3 files changed, 239 insertions(+), 41 deletions(-)
>
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 5cce7094..aff430df 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
[..]
> @@ -4543,13 +4716,36 @@ sub qemu_volume_snapshot_delete {
> });
> }
>
> + my $do_snapshots_with_qemu = do_snapshots_with_qemu($storecfg, $volid, $attached_deviceid);
> +
> if ($attached_deviceid && do_snapshots_with_qemu($storecfg, $volid, $attached_deviceid)) {
> - mon_cmd(
> - $vmid,
> - 'blockdev-snapshot-delete-internal-sync',
> - device => $attached_deviceid,
> - name => $snap,
> - );
> + if ($do_snapshots_with_qemu == 2) {
> + mon_cmd(
> + $vmid,
> + 'blockdev-snapshot-delete-internal-sync',
> + device => $attached_deviceid,
> + name => $snap,
> + );
> + } elsif ($do_snapshots_with_qemu == 3) {
> + print "delete qemu external snapshot\n";
> +
> + my $path = PVE::Storage::path($storecfg, $volid);
> + my $snapshots = PVE::Storage::volume_snapshot_info($storecfg, $volid);
> + my $parentsnap = $snapshots->{$snap}->{parent};
> + my $childsnap = $snapshots->{$snap}->{child};
> +
> + # 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";
> + blockdev_commit($storecfg, $vmid, $attached_deviceid, $drive, $childsnap, $snap);
> + blockdev_rename($storecfg, $vmid, $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";
> + blockdev_stream($storecfg, $vmid, $attached_deviceid, $drive, $snap, $parentsnap, $childsnap);
> + }
> + }
something here seems also broken, I did the following:
start VM
add disk on extsnap storage
create snapshot test
create snapshot test2
output of "qm listsnapshot 106":
qm listsnapshot 106
`-> test 2025-05-13 11:59:11 no-description
`-> test2 2025-05-13 12:09:35 no-description
`-> current You are here!
outout of "lvs extsnap"
LV VG Attr LSize Pool Origin Data% Meta% Move Log Cpy%Sync Convert
snap-test-vm-106-disk-0.qcow2 extsnap -wi-a----- 36.00g
snap-test2-vm-106-disk-0.qcow2 extsnap -wi-a----- 36.00g
vm-106-disk-0.qcow2 extsnap -wi-a----- 36.00g
output of perl -e 'use PVE::Storage; use Data::Dumper; $Data::Dumper::Sortkeys = 1; print Dumper(PVE::Storage::volume_snapshot_info(PVE::Storage::config(), "extsnap:vm-106-disk-0.qcow2"));'
$VAR1 = {
'current' => {
'ext' => 1,
'file' => '/dev/extsnap/vm-106-disk-0.qcow2',
'order' => 0,
'parent' => 'test2',
'volid' => 'extsnap:vm-106-disk-0.qcow2',
'volname' => 'vm-106-disk-0.qcow2'
},
'test' => {
'child' => 'test2',
'ext' => 1,
'file' => '/dev/extsnap/snap-test-vm-106-disk-0.qcow2',
'order' => 2,
'volid' => 'extsnap:snap-test-vm-106-disk-0.qcow2',
'volname' => 'snap-test-vm-106-disk-0.qcow2'
},
'test2' => {
'child' => 'current',
'ext' => 1,
'file' => '/dev/extsnap/snap-test2-vm-106-disk-0.qcow2',
'order' => 1,
'parent' => 'test',
'volid' => 'extsnap:snap-test2-vm-106-disk-0.qcow2',
'volname' => 'snap-test2-vm-106-disk-0.qcow2'
}
};
removed snapshot test2 while VM is running:
delete qemu external snapshot
stream intermediate snapshot test2 to current
stream-drive-scsi1: transferred 309.0 MiB of 32.0 GiB (0.94%) in 0s
stream-drive-scsi1: stream-job finished
delete old /dev/extsnap/snap-test2-vm-106-disk-0.qcow2
TASK ERROR: error deleting snapshot test2
from journal:
May 13 12:36:29 pve74test01 pvedaemon[68741]: <root@pam> starting task UPID:pve74test01:00013D6E:0010F721:682320AD:qmdelsnapshot:106:root@pam:
May 13 12:36:29 pve74test01 pvedaemon[81262]: <root@pam> delete snapshot VM 106: test2
May 13 12:36:31 pve74test01 pvedaemon[81262]: VM 106 qmp command failed - VM 106 qmp command 'blockdev-del' failed - Failed to find node with node-name='e-VgFpR5u0cSSgGcquQm4semoCyWK'
May 13 12:36:31 pve74test01 pvedaemon[81262]: VM 106 qmp command failed - VM 106 qmp command 'blockdev-del' failed - Failed to find node with node-name='f-VgFpR5u0cSSgGcquQm4semoCyWK'
May 13 12:36:31 pve74test01 pvedaemon[81262]: error deleting snapshot test2
May 13 12:36:31 pve74test01 pvedaemon[68741]: <root@pam> end task UPID:pve74test01:00013D6E:0010F721:682320AD:qmdelsnapshot:106:root@pam: error deleting snapshot test2
starting over, VM not running, all traces of snapshots and old volumes removed.
add new disk on extsnap storage
create snapshot test
create snapshot test2
remove snapshot test:
commit: merge content of /dev/extsnap/snap-test2-vm-106-disk-0.qcow2 into /dev/extsnap/snap-test-vm-106-disk-0.qcow2
Image committed.
delete snap-test2-vm-106-disk-0.qcow2
TASK ERROR: error delete old snapshot volume snap-test2-vm-106-disk-0.qcow2: unable to parse lvm volume name 'snap-test2-vm-106-disk-0.qcow2'
starting over, VM not running, all traces of snapshots and old volumes removed.
add new disk on extsnap storage
create snapshot test
create snapshot test2
remove snapshot test2:
rebase: merge diff content between /dev/extsnap/snap-test-vm-106-disk-0.qcow2 and /dev/extsnap/vm-106-disk-0.qcow2 into /dev/extsnap/vm-106-disk-0.qcow2
error delete old snapshot volume snap-test2-vm-106-disk-0.qcow2
what??
it seems to me you didn't really test the version you sent w.r.t. basic snapshot actions?
> } else {
> PVE::Storage::volume_snapshot_delete(
> $storecfg, $volid, $snap, $attached_deviceid ? 1 : undef);
> @@ -7778,27 +7974,16 @@ sub foreach_storage_used_by_vm {
> }
> }
>
> -my $qemu_snap_storage = {
> - rbd => 1,
> -};
> sub do_snapshots_with_qemu {
> my ($storecfg, $volid, $deviceid) = @_;
>
> - return if $deviceid =~ m/tpmstate0/;
> + return if $deviceid && $deviceid =~ m/tpmstate0/;
>
> - my $storage_name = PVE::Storage::parse_volume_id($volid);
> - my $scfg = $storecfg->{ids}->{$storage_name};
> - die "could not find storage '$storage_name'\n" if !defined($scfg);
> + my $snapshot_type = PVE::Storage::volume_has_feature($storecfg, 'snapshot', $volid);
>
> - if ($qemu_snap_storage->{$scfg->{type}} && !$scfg->{krbd}){
> - return 1;
> - }
> + return $snapshot_type if $snapshot_type == 2 || $snapshot_type == 3;
>
> - if ($volid =~ m/\.(qcow2|qed)$/){
> - return 1;
> - }
> -
> - return;
> + return undef;
> }
>
> sub qga_check_running {
> diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
> index 0737034d..93903a59 100644
> --- a/PVE/QemuServer/Drive.pm
> +++ b/PVE/QemuServer/Drive.pm
> @@ -27,6 +27,9 @@ print_drive
> print_drive_throttle_group
> generate_drive_blockdev
> generate_throttle_group
> +generate_blockdev_throttle
> +generate_format_blockdev
> +generate_file_blockdev
> );
>
> our $QEMU_FORMAT_RE = qr/raw|qcow|qcow2|qed|vmdk|cloop/;
> @@ -1074,6 +1077,8 @@ sub print_drive_throttle_group {
> sub generate_file_blockdev {
> my ($storecfg, $drive, $snap, $nodename) = @_;
>
> + $snap = undef if $snap && $snap eq 'current';
> +
> my $volid = $drive->{file};
> my $driveid = get_drive_id($drive);
>
> @@ -1209,6 +1214,8 @@ sub generate_file_blockdev {
> sub generate_format_blockdev {
> my ($storecfg, $drive, $file, $snap, $nodename) = @_;
>
> + $snap = undef if $snap && $snap eq 'current';
> +
> my $volid = $drive->{file};
> #nbd don't support format blockdev, return the fileblockdev
> return $file if $volid =~ /^nbd:/;
> @@ -1280,6 +1287,15 @@ sub generate_backing_chain_blockdev {
> return $chain_blockdev;
> }
>
> +sub generate_blockdev_throttle {
> + my ($drive, $blockdev_file) = @_;
> +
> + my $drive_id = get_drive_id($drive);
> + #this is the topfilter entry point, use $drive-drive_id as nodename
> + my $blockdev_throttle = { driver => "throttle", 'node-name' => "drive-$drive_id", 'throttle-group' => "throttle-drive-$drive_id", 'file' => $blockdev_file };
> + return $blockdev_throttle;
> +}
> +
> sub generate_drive_blockdev {
> my ($storecfg, $drive, $live_restore_name) = @_;
>
> @@ -1303,22 +1319,19 @@ sub generate_drive_blockdev {
> #pflash0 don't support throttle-filter
> return $blockdev_format if $drive_id eq 'pflash0';
>
> - my $blockdev_live_restore = undef;
> - if ($live_restore_name) {
> - die "$drive_id: Proxmox Backup Server backed drive cannot auto-detect the format\n"
> - if !$drive->{format};
> + return generate_blockdev_throttle($drive, $blockdev_format) if !$live_restore_name;
>
> - $blockdev_live_restore = { 'node-name' => "liverestore-drive-$drive_id",
> - backing => $live_restore_name,
> - 'auto-remove' => 'on', format => "alloc-track",
> - file => $blockdev_format };
> - }
> + die "$drive_id: Proxmox Backup Server backed drive cannot auto-detect the format\n"
> + if !$drive->{format};
>
> - #this is the topfilter entry point, use $drive-drive_id as nodename
> - my $blockdev_throttle = { driver => "throttle", 'node-name' => "drive-$drive_id", 'throttle-group' => "throttle-drive-$drive_id" };
> #put liverestore filter between throttle && format filter
> - $blockdev_throttle->{file} = $live_restore_name ? $blockdev_live_restore : $blockdev_format;
> - return $blockdev_throttle,
> + my $blockdev_live_restore = { 'node-name' => "liverestore-drive-$drive_id",
> + backing => $live_restore_name,
> + 'auto-remove' => 'on', format => "alloc-track",
> + file => $blockdev_format };
> +
> + return generate_blockdev_throttle($drive, $blockdev_live_restore);
> +
> }
>
> sub encode_base62 {
> --
> 2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel