Re: [pve-devel] [PATCH qemu-server 1/2] migration: avoid crash with heavy IO on local VM disk

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

one nit below, otherwise very nice to have this!

On May 3, 2024 10:34 am, Fiona Ebner wrote:
> There is a possibility that the drive-mirror job is not yet done when
> the migration wants to inactivate the source's blockdrives:
> 
>> bdrv_co_write_req_prepare: Assertion `!(bs->open_flags & BDRV_O_INACTIVE)' failed.
> 
> This can be prevented by using the 'write-blocking' copy mode (also
> called active mode) for the mirror. However, with active mode, the
> guest write speed is limited by the synchronous writes to the mirror
> target. For this reason, a way to start out in the faster 'background'
> mode and later switch to active mode was introduced in QEMU 8.2.
> 
> The switch is done once the mirror job for all drives is ready to be
> completed to reduce the time spent where guest IO is limited.
> 
> Reported rarely, but steadily over the years:
> https://forum.proxmox.com/threads/78954/post-353651
> https://forum.proxmox.com/threads/78954/post-380015
> https://forum.proxmox.com/threads/100020/post-431660
> https://forum.proxmox.com/threads/111831/post-482425
> https://forum.proxmox.com/threads/111831/post-499807
> https://forum.proxmox.com/threads/137849/
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  PVE/QemuMigrate.pm                    |  9 ++++++
>  PVE/QemuServer.pm                     | 41 +++++++++++++++++++++++++++
>  test/MigrationTest/QemuMigrateMock.pm |  3 ++
>  3 files changed, 53 insertions(+)
> 
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index 8d9b35ae..649cfec4 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -1142,6 +1142,15 @@ sub phase2 {
>  	    $self->log('info', "$drive: start migration to $nbd_uri");
>  	    PVE::QemuServer::qemu_drive_mirror($vmid, $drive, $nbd_uri, $vmid, undef, $self->{storage_migration_jobs}, 'skip', undef, $bwlimit, $bitmap);
>  	}
> +
> +	my $kvm_version = PVE::QemuServer::kvm_user_version();

wouldn't this need to check the *running* qemu binary for the migrated
VM, not the *installed* qemu binary on the system?

> +	if (min_version($kvm_version, 8, 2)) {
> +	    $self->log('info', "switching mirror jobs to actively synced mode");
> +	    PVE::QemuServer::qemu_drive_mirror_switch_to_active_mode(
> +		$vmid,
> +		$self->{storage_migration_jobs},
> +	    );
> +	}
>      }
>  
>      $self->log('info', "starting online/live migration on $migrate_uri");
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 82e7d6a6..5bc313e2 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -8118,6 +8118,47 @@ sub qemu_blockjobs_cancel {
>      }
>  }
>  
> +# Callers should version guard this (only available with a binary >= QEMU 8.2)
> +sub qemu_drive_mirror_switch_to_active_mode {
> +    my ($vmid, $jobs) = @_;
> +
> +    my $switching = {};
> +
> +    for my $job (sort keys $jobs->%*) {
> +	print "$job: switching to actively synced mode\n";
> +
> +	eval {
> +	    mon_cmd(
> +		$vmid,
> +		"block-job-change",
> +		id => $job,
> +		type => 'mirror',
> +		'copy-mode' => 'write-blocking',
> +	    );
> +	    $switching->{$job} = 1;
> +	};
> +	die "could not switch mirror job $job to active mode - $@\n" if $@;
> +    }
> +
> +    while (1) {
> +	my $stats = mon_cmd($vmid, "query-block-jobs");
> +
> +	my $running_jobs = {};
> +	$running_jobs->{$_->{device}} = $_ for $stats->@*;
> +
> +	for my $job (sort keys $switching->%*) {
> +	    if ($running_jobs->{$job}->{'actively-synced'}) {
> +		print "$job: successfully switched to actively synced mode\n";
> +		delete $switching->{$job};
> +	    }
> +	}
> +
> +	last if scalar(keys $switching->%*) == 0;
> +
> +	sleep 1;
> +    }
> +}
> +
>  # Check for bug #4525: drive-mirror will open the target drive with the same aio setting as the
>  # source, but some storages have problems with io_uring, sometimes even leading to crashes.
>  my sub clone_disk_check_io_uring {
> diff --git a/test/MigrationTest/QemuMigrateMock.pm b/test/MigrationTest/QemuMigrateMock.pm
> index 1efabe24..2b9e91b3 100644
> --- a/test/MigrationTest/QemuMigrateMock.pm
> +++ b/test/MigrationTest/QemuMigrateMock.pm
> @@ -152,6 +152,9 @@ $MigrationTest::Shared::qemu_server_module->mock(
>  	}
>  	return;
>      },
> +    qemu_drive_mirror_switch_to_active_mode => sub {
> +	return;
> +    },
>      set_migration_caps => sub {
>  	return;
>      },
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 


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