Re: [pve-devel] [PATCH qemu-server 5/5] blockdev-stream/-commit: make backing file relative

[Fiona Ebner <f.ebner@proxmox.com>]

Am 29.07.25 um 9:39 AM schrieb Fabian Grünbichler:
> to avoid the resulting qcow2 file referencing its backing file via an absolute
> path, which makes renaming the base of the storage impossible.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>

Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Tested-by: Fiona Ebner <f.ebner@proxmox.com>

with some minor suggestions below. Thank you for tackling this!

> ---
>  src/PVE/QemuServer/Blockdev.pm | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/src/PVE/QemuServer/Blockdev.pm b/src/PVE/QemuServer/Blockdev.pm
> index 6e105571..71d5ef69 100644
> --- a/src/PVE/QemuServer/Blockdev.pm
> +++ b/src/PVE/QemuServer/Blockdev.pm
> @@ -5,6 +5,7 @@ use warnings;
>  
>  use Digest::SHA;
>  use Fcntl qw(S_ISBLK S_ISCHR);
> +use File::Basename;

Nit: could use qw(basename dirname) to make the imports explicit

>  use File::stat;
>  use JSON;
>  
> @@ -954,12 +955,19 @@ sub blockdev_replace {
>          $parent_fmt_blockdev->{backing} = $target_fmt_blockdev->{'node-name'};
>          mon_cmd($vmid, 'blockdev-reopen', options => [$parent_fmt_blockdev]);
>  
> +        my $backing_file = $target_file_blockdev->{filename};
> +        my $backing_dir = dirname($backing_file);
> +        if ($backing_dir eq dirname($parent_file_blockdev->{filename})) {
> +            # make backing file relative if in same directory
> +            $backing_file = basename($backing_file);
> +        }

Nit: while only used twice, could still become a helper to avoid making
the functions here longer.

> +
>          #change backing-file in qcow2 metadatas
>          mon_cmd(
>              $vmid, 'change-backing-file',
>              device => $deviceid,
>              'image-node-name' => $parent_fmt_blockdev->{'node-name'},
> -            'backing-file' => $target_file_blockdev->{filename},
> +            'backing-file' => $backing_file,
>          );
>      }
>  
> @@ -1069,11 +1077,18 @@ sub blockdev_stream {
>          { 'snapshot-name' => $snap },
>      );
>  
> +    my $backing_file = $parent_file_blockdev->{filename};
> +    my $backing_dir = dirname($backing_file);
> +    if ($backing_dir eq dirname($target_file_blockdev->{filename})) {
> +        # make backing file relative if in same directory
> +        $backing_file = basename($backing_file);
> +    }
> +
>      my $job_id = "stream-$deviceid";
>      my $jobs = {};
>      my $options = { 'job-id' => $job_id, device => $target_fmt_blockdev->{'node-name'} };
>      $options->{'base-node'} = $parent_fmt_blockdev->{'node-name'};
> -    $options->{'backing-file'} = $parent_file_blockdev->{filename};
> +    $options->{'backing-file'} = $backing_file;
>  
>      mon_cmd($vmid, 'block-stream', %$options);
>      $jobs->{$job_id} = {};



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