[pve-devel] [PATCH qemu-server] migrate: silence 'Use of uninitialized value' warning

[pve-devel] [PATCH qemu-server] migrate: silence 'Use of uninitialized value' warning

[Dominik Csapak]

If no vfio device is present during migration, and the transferred
(main) memory did not change between loop cycles, we get a warning:

 Use of uninitialized value $last_vfio_transferred in string ne

To silence that, check if the transferred vfio value is defined before,
and always write a defined value to $last_vfio_transferred.

This was noticed by a forum user:
https://forum.proxmox.com/threads/166161/

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/QemuMigrate.pm | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index b7bf2aa3..f72a9d33 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -1321,9 +1321,12 @@ sub phase2 {
 	    last;
 	}
 
-	if ($memstat->{transferred} ne $last_mem_transferred ||
-	    $stat->{vfio}->{transferred} ne $last_vfio_transferred
-	) {
+	if (
+	    $memstat->{transferred} ne $last_mem_transferred
+	    || (defined($stat->{vfio}->{transferred})
+		&& $stat->{vfio}->{transferred} ne $last_vfio_transferred)
+	)
+	{
 	    my $trans = $memstat->{transferred} || 0;
 	    my $vfio_transferred = $stat->{vfio}->{transferred} || 0;
 	    my $rem = $memstat->{remaining} || 0;
@@ -1389,7 +1392,7 @@ sub phase2 {
 	}
 
 	$last_mem_transferred = $memstat->{transferred};
-	$last_vfio_transferred = $stat->{vfio}->{transferred};
+	$last_vfio_transferred = $stat->{vfio}->{transferred} // 0;
     }
 
     if ($self->{storage_migration}) {
-- 
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] migrate: code cleanup: factor out variables for transferred memory and vfio

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

I'll apply together with this follow-up if somebody gives me a T-b/R-b

 PVE/QemuMigrate.pm | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index f72a9d33..5a54f173 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -1286,6 +1286,9 @@ sub phase2 {
 
 	my $memstat = $stat->{ram};
 
+	my $mem_transferred = $memstat->{transferred} || 0;
+	my $vfio_transferred = $stat->{vfio}->{transferred} || 0;
+
 	if ($status eq 'completed') {
 	    my $delay = time() - $start;
 	    if ($delay > 0) {
@@ -1294,11 +1297,9 @@ sub phase2 {
 		my $downtime = $stat->{downtime} || 0;
 		$self->log('info', "average migration speed: $avg_speed/s - downtime $downtime ms");
 	    }
-	    my $trans = $memstat->{transferred} || 0;
-	    my $vfio_transferred = $stat->{vfio}->{transferred} || 0;
 
-	    if ($trans > 0 || $vfio_transferred > 0) {
-		my $transferred_h = render_bytes($trans, 1);
+	    if ($mem_transferred > 0 || $vfio_transferred > 0) {
+		my $transferred_h = render_bytes($mem_transferred, 1);
 		my $summary = "transferred $transferred_h VM-state";
 
 		if ($vfio_transferred > 0) {
@@ -1321,14 +1322,9 @@ sub phase2 {
 	    last;
 	}
 
-	if (
-	    $memstat->{transferred} ne $last_mem_transferred
-	    || (defined($stat->{vfio}->{transferred})
-		&& $stat->{vfio}->{transferred} ne $last_vfio_transferred)
-	)
-	{
-	    my $trans = $memstat->{transferred} || 0;
-	    my $vfio_transferred = $stat->{vfio}->{transferred} || 0;
+	if ($mem_transferred ne $last_mem_transferred
+	    || $vfio_transferred ne $last_vfio_transferred
+	) {
 	    my $rem = $memstat->{remaining} || 0;
 	    my $total = $memstat->{total} || 0;
 	    my $speed = ($memstat->{'pages-per-second'} // 0) * ($memstat->{'page-size'} // 0);
@@ -1341,7 +1337,7 @@ sub phase2 {
 	    my $should_log = $usleep > 100_000 ? 1 : ($i % 10) == 0;
 
 	    my $total_h = render_bytes($total, 1);
-	    my $transferred_h = render_bytes($trans, 1);
+	    my $transferred_h = render_bytes($mem_transferred, 1);
 	    my $speed_h = render_bytes($speed, 1);
 
 	    my $progress = "transferred $transferred_h of $total_h VM-state, ${speed_h}/s";
@@ -1391,8 +1387,8 @@ sub phase2 {
 	    }
 	}
 
-	$last_mem_transferred = $memstat->{transferred};
-	$last_vfio_transferred = $stat->{vfio}->{transferred} // 0;
+	$last_mem_transferred = $mem_transferred;
+	$last_vfio_transferred = $vfio_transferred;
     }
 
     if ($self->{storage_migration}) {
-- 
2.39.5



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

Re: [pve-devel] [PATCH qemu-server] migrate: code cleanup: factor out variables for transferred memory and vfio

[Dominik Csapak]

Changes look good to me, tested a few migrations, saw no issues, consider this:

Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Dominik Csapak <d.csapak@proxmox.com>

On 5/20/25 15:14, Fiona Ebner wrote:
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
> 
> I'll apply together with this follow-up if somebody gives me a T-b/R-b
> 
>   PVE/QemuMigrate.pm | 26 +++++++++++---------------
>   1 file changed, 11 insertions(+), 15 deletions(-)
> 
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index f72a9d33..5a54f173 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -1286,6 +1286,9 @@ sub phase2 {
>   
>   	my $memstat = $stat->{ram};
>   
> +	my $mem_transferred = $memstat->{transferred} || 0;
> +	my $vfio_transferred = $stat->{vfio}->{transferred} || 0;
> +
>   	if ($status eq 'completed') {
>   	    my $delay = time() - $start;
>   	    if ($delay > 0) {
> @@ -1294,11 +1297,9 @@ sub phase2 {
>   		my $downtime = $stat->{downtime} || 0;
>   		$self->log('info', "average migration speed: $avg_speed/s - downtime $downtime ms");
>   	    }
> -	    my $trans = $memstat->{transferred} || 0;
> -	    my $vfio_transferred = $stat->{vfio}->{transferred} || 0;
>   
> -	    if ($trans > 0 || $vfio_transferred > 0) {
> -		my $transferred_h = render_bytes($trans, 1);
> +	    if ($mem_transferred > 0 || $vfio_transferred > 0) {
> +		my $transferred_h = render_bytes($mem_transferred, 1);
>   		my $summary = "transferred $transferred_h VM-state";
>   
>   		if ($vfio_transferred > 0) {
> @@ -1321,14 +1322,9 @@ sub phase2 {
>   	    last;
>   	}
>   
> -	if (
> -	    $memstat->{transferred} ne $last_mem_transferred
> -	    || (defined($stat->{vfio}->{transferred})
> -		&& $stat->{vfio}->{transferred} ne $last_vfio_transferred)
> -	)
> -	{
> -	    my $trans = $memstat->{transferred} || 0;
> -	    my $vfio_transferred = $stat->{vfio}->{transferred} || 0;
> +	if ($mem_transferred ne $last_mem_transferred
> +	    || $vfio_transferred ne $last_vfio_transferred
> +	) {
>   	    my $rem = $memstat->{remaining} || 0;
>   	    my $total = $memstat->{total} || 0;
>   	    my $speed = ($memstat->{'pages-per-second'} // 0) * ($memstat->{'page-size'} // 0);
> @@ -1341,7 +1337,7 @@ sub phase2 {
>   	    my $should_log = $usleep > 100_000 ? 1 : ($i % 10) == 0;
>   
>   	    my $total_h = render_bytes($total, 1);
> -	    my $transferred_h = render_bytes($trans, 1);
> +	    my $transferred_h = render_bytes($mem_transferred, 1);
>   	    my $speed_h = render_bytes($speed, 1);
>   
>   	    my $progress = "transferred $transferred_h of $total_h VM-state, ${speed_h}/s";
> @@ -1391,8 +1387,8 @@ sub phase2 {
>   	    }
>   	}
>   
> -	$last_mem_transferred = $memstat->{transferred};
> -	$last_vfio_transferred = $stat->{vfio}->{transferred} // 0;
> +	$last_mem_transferred = $mem_transferred;
> +	$last_vfio_transferred = $vfio_transferred;
>       }
>   
>       if ($self->{storage_migration}) {



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

[pve-devel] applied: [PATCH qemu-server] migrate: silence 'Use of uninitialized value' warning

[Fiona Ebner]

On Mon, 19 May 2025 16:43:57 +0200, Dominik Csapak wrote:
> If no vfio device is present during migration, and the transferred
> (main) memory did not change between loop cycles, we get a warning:
> 
>  Use of uninitialized value $last_vfio_transferred in string ne
> 
> To silence that, check if the transferred vfio value is defined before,
> and always write a defined value to $last_vfio_transferred.
> 
> [...]

Applied together with my follow-up, thanks!

[1/1] migrate: silence 'Use of uninitialized value' warning
      commit: 7d1ec25bc176969ccc86ecc2a0109fc59ec812f0

[1/1] migrate: code cleanup: factor out variables for transferred memory and vfio
      commit: c7609527716a111f4a6147ea68fbd327dd16eb87


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