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.eb...@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.gruenbich...@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@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
