[pve-devel] [PATCH qemu-server v3 19/34] backup: keep track of block-node size for fleecing

[Fabian Grünbichler]

On November 7, 2024 5:51 pm, Fiona Ebner wrote:
> For fleecing, the size needs to match exactly what QEMU sees. In
> particular, EFI disks might be attached with a 'size=' option, meaning
> that size can be different from the volume's size. Commit 36377acf
> ("backup: disk info: also keep track of size") introduced size
> tracking and it was used for fleecing since then, but the accurate
> size information needs to be queried via QMP.
> 
> Should also help with the following issue reported in the community
> forum:
> https://forum.proxmox.com/threads/152202
> 
> Signed-off-by: Fiona Ebner <f.ebner at proxmox.com>
> ---
> 
> Changes in v3:
> * only use query-block QMP command after the VM is enforced running
> 
>  PVE/VZDump/QemuServer.pm | 37 ++++++++++++++++++++++++++++++++-----
>  1 file changed, 32 insertions(+), 5 deletions(-)
> 
> diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
> index c46e607c..1ebafe6d 100644
> --- a/PVE/VZDump/QemuServer.pm
> +++ b/PVE/VZDump/QemuServer.pm
> @@ -551,7 +551,7 @@ my sub allocate_fleecing_images {
>  		my $name = "vm-$vmid-fleece-$n";
>  		$name .= ".$format" if $scfg->{path};
>  
> -		my $size = PVE::Tools::convert_size($di->{size}, 'b' => 'kb');
> +		my $size = PVE::Tools::convert_size($di->{'block-node-size'}, 'b' => 'kb');
>  
>  		$di->{'fleece-volid'} = PVE::Storage::vdisk_alloc(
>  		    $self->{storecfg}, $fleecing_storeid, $vmid, $format, $name, $size);
> @@ -600,7 +600,7 @@ my sub attach_fleecing_images {
>  	    my $drive = "file=$path,if=none,id=$devid,format=$format,discard=unmap";
>  	    # Specify size explicitly, to make it work if storage backend rounded up size for
>  	    # fleecing image when allocating.
> -	    $drive .= ",size=$di->{size}" if $format eq 'raw';
> +	    $drive .= ",size=$di->{'block-node-size'}" if $format eq 'raw';
>  	    $drive =~ s/\\/\\\\/g;
>  	    my $ret = PVE::QemuServer::Monitor::hmp_cmd($vmid, "drive_add auto \"$drive\"", 60);
>  	    die "attaching fleecing image $volid failed - $ret\n" if $ret !~ m/OK/s;
> @@ -609,7 +609,7 @@ my sub attach_fleecing_images {
>  }
>  
>  my sub check_and_prepare_fleecing {
> -    my ($self, $vmid, $fleecing_opts, $disks, $is_template, $qemu_support) = @_;
> +    my ($self, $task, $vmid, $fleecing_opts, $disks, $is_template, $qemu_support) = @_;

$disks here is $task->{disks} (see below)

>  
>      # Even if the VM was started specifically for fleecing, it's possible that the VM is resumed and
>      # then starts doing IO. For VMs that are not resumed the fleecing images will just stay empty,
> @@ -626,6 +626,8 @@ my sub check_and_prepare_fleecing {
>      }
>  
>      if ($use_fleecing) {
> +	$self->query_block_node_sizes($vmid, $task);

query_block_node_sizes only uses $task->{disks}

> +
>  	my ($default_format, $valid_formats) = PVE::Storage::storage_default_format(
>  	    $self->{storecfg}, $fleecing_opts->{storage});
>  	my $format = scalar(grep { $_ eq 'qcow2' } $valid_formats->@*) ? 'qcow2' : 'raw';
> @@ -721,7 +723,7 @@ sub archive_pbs {
>  	my $is_template = PVE::QemuConfig->is_template($self->{vmlist}->{$vmid});
>  
>  	$task->{'use-fleecing'} = check_and_prepare_fleecing(
> -	    $self, $vmid, $opts->{fleecing}, $task->{disks}, $is_template, $qemu_support);
> +	    $self, $task, $vmid, $opts->{fleecing}, $task->{disks}, $is_template, $qemu_support);
>  
>  	my $fs_frozen = $self->qga_fs_freeze($task, $vmid);
>  
> @@ -905,7 +907,7 @@ sub archive_vma {
>  	$attach_tpmstate_drive->($self, $task, $vmid);
>  
>  	$task->{'use-fleecing'} = check_and_prepare_fleecing(
> -	    $self, $vmid, $opts->{fleecing}, $task->{disks}, $is_template, $qemu_support);
> +	    $self, $task, $vmid, $opts->{fleecing}, $task->{disks}, $is_template, $qemu_support);

here we can see $task->{disks} being passed in

>  
>  	my $outfh;
>  	if ($opts->{stdout}) {
> @@ -1042,6 +1044,31 @@ sub qga_fs_thaw {
>      $self->logerr($@) if $@;
>  }
>  
> +# The size for fleecing images needs to be exactly the same size as QEMU sees. E.g. EFI disk can bex
> +# attached with a smaller size then the underyling image on the storage.
> +sub query_block_node_sizes {
> +    my ($self, $vmid, $task) = @_;
> +
> +    my $block_info = mon_cmd($vmid, "query-block");
> +    $block_info = { map { $_->{device} => $_ } $block_info->@* };
> +
> +    for my $diskinfo ($task->{disks}->@*) {

only usage of $task

so we don't actually need to add $task as parameter to the two existing
subs, but can just modify this here to take $task->{disks} directly? or
did I overlook something?

if we do have to keep $task as parameter, it should come before $vmid in
the argument list, to be consistent with the rest..

other than that, consider this patch

Reviewed-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>

> +	my $drive_key = $diskinfo->{virtdev};
> +	$drive_key .= "-backup" if $drive_key eq 'tpmstate0';
> +	my $block_node_size =
> +	    eval { $block_info->{"drive-$drive_key"}->{inserted}->{image}->{'virtual-size'}; };
> +	if (!$block_node_size) {
> +	    $self->loginfo(
> +		"could not determine block node size of drive '$drive_key' - using fallback");
> +	    $block_node_size = $diskinfo->{size}
> +		or die "could not determine size of drive '$drive_key'\n";
> +	}
> +	$diskinfo->{'block-node-size'} = $block_node_size;
> +    }
> +
> +    return;
> +}
> +
>  # we need a running QEMU/KVM process for backup, starts a paused (prelaunch)
>  # one if VM isn't already running
>  sub enforce_vm_running_for_backup {
> -- 
> 2.39.5
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
>