[pve-devel] [PATCH qemu-server 1/3] api: clone: add missing sort to hash

[pve-devel] [PATCH qemu-server 1/3] api: clone: add missing sort to hash

[Alexander Zeidler]

When cloning was repeatedly attempted, the error message indicated a
different unsupported volume each time. The hash is now sorted to always
mention the same volume as long as it has not been fixed.

Signed-off-by: Alexander Zeidler <a.zeidler@proxmox.com>
---
 PVE/API2/Qemu.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 53ff7092..fd08fae0 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -3874,7 +3874,7 @@ __PACKAGE__->register_method({
 	    my $fullclone = {};
 	    my $vollist = [];
 
-	    foreach my $opt (keys %$oldconf) {
+	    foreach my $opt (sort keys %$oldconf) {
 		my $value = $oldconf->{$opt};
 
 		# do not copy snapshot related info
-- 
2.39.5



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

[pve-devel] [PATCH qemu-server 2/3] api: clone: extend error message by volume ID

[Alexander Zeidler]

So far, the error message only contained the name of the disk
(tpmstate0, efidisk0, ...), which can also lead to the assumption that a
specific disk type is the problem. Now the volume ID is primarily
listed.

Example:
 # qm clone 101 102 --full --snapname foo

Before:
> Full clone feature is not supported for drive 'tpmstate0'

After:
> Full clone feature is not supported for 'local-zfs:base-100-disk-2/vm-101-disk-2' (tpmstate0)

Signed-off-by: Alexander Zeidler <a.zeidler@proxmox.com>
---
 PVE/API2/Qemu.pm | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index fd08fae0..661cf1e9 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -3899,18 +3899,19 @@ __PACKAGE__->register_method({
 		    if (PVE::QemuServer::drive_is_cdrom($drive, 1)) {
 			$newconf->{$opt} = $value; # simply copy configuration
 		    } else {
+			my $volid = $drive->{file};
 			if ($full || PVE::QemuServer::drive_is_cloudinit($drive)) {
-			    die "Full clone feature is not supported for drive '$opt'\n"
-				if !PVE::Storage::volume_has_feature($storecfg, 'copy', $drive->{file}, $snapname, $running);
+			    die "Full clone feature is not supported for '$volid' ($opt)\n"
+				if !PVE::Storage::volume_has_feature($storecfg, 'copy', $volid, $snapname, $running);
 			    $fullclone->{$opt} = 1;
 			} else {
 			    # not full means clone instead of copy
-			    die "Linked clone feature is not supported for drive '$opt'\n"
-				if !PVE::Storage::volume_has_feature($storecfg, 'clone', $drive->{file}, $snapname, $running);
+			    die "Linked clone feature is not supported for '$volid' ($opt)\n"
+				if !PVE::Storage::volume_has_feature($storecfg, 'clone', $volid, $snapname, $running);
 			}
 			$drives->{$opt} = $drive;
 			next if PVE::QemuServer::drive_is_cloudinit($drive);
-			push @$vollist, $drive->{file};
+			push @$vollist, $volid;
 		    }
 		} else {
 		    # copy everything else
-- 
2.39.5



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

[pve-devel] [PATCH qemu-server 3/3] api: clone: mention "snapshot" in the error message if specified

[Alexander Zeidler]

as it may be the only cause of the clone incompatibility

Example:
 # qm clone 101 102 --full --snapname foo

Before:
> Full clone feature is not supported for 'local-zfs:base-100-disk-2/vm-101-disk-2' (tpmstate0)

After:
> Full clone feature is not supported for a snapshot of 'local-zfs:base-100-disk-2/vm-101-disk-2' (tpmstate0)

Signed-off-by: Alexander Zeidler <a.zeidler@proxmox.com>
---
 PVE/API2/Qemu.pm | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 661cf1e9..df1776a6 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -3900,13 +3900,16 @@ __PACKAGE__->register_method({
 			$newconf->{$opt} = $value; # simply copy configuration
 		    } else {
 			my $volid = $drive->{file};
+			my $msg = "clone feature is not supported for";
+			$msg .= " a snapshot of" if $snapname;
+			$msg .= " '$volid' ($opt)";
 			if ($full || PVE::QemuServer::drive_is_cloudinit($drive)) {
-			    die "Full clone feature is not supported for '$volid' ($opt)\n"
+			    die "Full $msg\n"
 				if !PVE::Storage::volume_has_feature($storecfg, 'copy', $volid, $snapname, $running);
 			    $fullclone->{$opt} = 1;
 			} else {
 			    # not full means clone instead of copy
-			    die "Linked clone feature is not supported for '$volid' ($opt)\n"
+			    die "Linked $msg\n"
 				if !PVE::Storage::volume_has_feature($storecfg, 'clone', $volid, $snapname, $running);
 			}
 			$drives->{$opt} = $drive;
-- 
2.39.5



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

[pve-devel] applied-series: [PATCH qemu-server 1/3] api: clone: add missing sort to hash

[Fiona Ebner]

Am 04.12.24 um 11:30 schrieb Alexander Zeidler:
> When cloning was repeatedly attempted, the error message indicated a
> different unsupported volume each time. The hash is now sorted to always
> mention the same volume as long as it has not been fixed.
> 
> Signed-off-by: Alexander Zeidler <a.zeidler@proxmox.com>
> ---
>  PVE/API2/Qemu.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 53ff7092..fd08fae0 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -3874,7 +3874,7 @@ __PACKAGE__->register_method({
>  	    my $fullclone = {};
>  	    my $vollist = [];
>  
> -	    foreach my $opt (keys %$oldconf) {
> +	    foreach my $opt (sort keys %$oldconf) {
>  		my $value = $oldconf->{$opt};
>  
>  		# do not copy snapshot related info

applied series, thanks! Squashed in a change to replace "foreach" with
"for" here while we're already touching the line.


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