public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH storage/qemu-server 0/5] avoid absolute qcow2 references
@ 2025-07-29  7:38 Fabian Grünbichler
  2025-07-29  7:38 ` [pve-devel] [PATCH storage 1/5] plugin: fix typo in rebase log message Fabian Grünbichler
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Fabian Grünbichler @ 2025-07-29  7:38 UTC (permalink / raw)
  To: pve-devel

we don't want qcow2 files to reference their backing chains via
absolute paths, as that makes renaming the base dir or VG of the storage
impossible. in most places, qemu already allows simply passing a
filename as backing-file reference, which will be interpreted as a
reference relative to the backed image.

I haven't found any further code paths that trigger absolute references,
but I might have missed some. the full backing chain should show
relative backing-file members when queried via

qemu-img info --output json --format qcow2 --backing-chain /path/to/main/image.qcow2

such, as:

        "full-backing-filename": "/var/lib/extsnap/images/210/snap-test2-vm-210-disk-0.qcow2",
        "backing-filename": "snap-test2-vm-210-disk-0.qcow2",

note that full-backing-filename will always contain the resolved,
absolute path and that is okay. we could warn about both members
containing full paths in `volume_snapshot_info`.

for existing "broken" images, an "unsafe" rebase with

qemu-img rebase -u -f qcow2 -F qcow2 -b <relative backing file path> <absolute backed filed path>

should just rewrite the qcow2 header to replace the backing file
reference - this should of course not be run while the image is being
written by other process or QEMU.

pve-storage:

Fabian Grünbichler (4):
  plugin: fix typo in rebase log message
  lvm plugin: fix typo in rebase log message
  plugin: use relative path for qcow2 rebase command
  lvm plugin: use relative path for qcow2 rebase command

 src/PVE/Storage/LVMPlugin.pm | 5 +++--
 src/PVE/Storage/Plugin.pm    | 6 +++---
 2 files changed, 6 insertions(+), 5 deletions(-)

qemu-server:

Fabian Grünbichler (1):
  blockdev-stream/-commit: make backing file relative

 src/PVE/QemuServer/Blockdev.pm | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

-- 
2.39.5



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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [pve-devel] [PATCH storage 1/5] plugin: fix typo in rebase log message
  2025-07-29  7:38 [pve-devel] [PATCH storage/qemu-server 0/5] avoid absolute qcow2 references Fabian Grünbichler
@ 2025-07-29  7:38 ` Fabian Grünbichler
  2025-07-29  8:09   ` Fiona Ebner
  2025-07-29  7:38 ` [pve-devel] [PATCH storage 2/5] lvm " Fabian Grünbichler
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Fabian Grünbichler @ 2025-07-29  7:38 UTC (permalink / raw)
  To: pve-devel

the format here is of course qcow2, and the actual command was correct, but the
log message was not.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/PVE/Storage/Plugin.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 2bd05bd..99e0e76 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -1437,7 +1437,7 @@ sub volume_snapshot_delete {
             my $parentpath = $snapshots->{$parentsnap}->{file};
             print
                 "$volname: deleting snapshot '$snap' by rebasing '$childsnap' on top of '$parentsnap'\n";
-            print "running 'qemu-img rebase -b $parentpath -F qcow -f qcow2 $childpath'\n";
+            print "running 'qemu-img rebase -b $parentpath -F qcow2 -f qcow2 $childpath'\n";
             $cmd = [
                 '/usr/bin/qemu-img',
                 'rebase',
-- 
2.39.5



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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [pve-devel] [PATCH storage 2/5] lvm plugin: fix typo in rebase log message
  2025-07-29  7:38 [pve-devel] [PATCH storage/qemu-server 0/5] avoid absolute qcow2 references Fabian Grünbichler
  2025-07-29  7:38 ` [pve-devel] [PATCH storage 1/5] plugin: fix typo in rebase log message Fabian Grünbichler
@ 2025-07-29  7:38 ` Fabian Grünbichler
  2025-07-29  8:09   ` Fiona Ebner
  2025-07-29  7:38 ` [pve-devel] [PATCH storage 3/5] plugin: use relative path for qcow2 rebase command Fabian Grünbichler
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Fabian Grünbichler @ 2025-07-29  7:38 UTC (permalink / raw)
  To: pve-devel

this was copied over from Plugin.pm

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/PVE/Storage/LVMPlugin.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index c1f5474..db1be98 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -1120,7 +1120,7 @@ sub volume_snapshot_delete {
         my $parentpath = $snapshots->{$parentsnap}->{file};
         print
             "$volname: deleting snapshot '$snap' by rebasing '$childsnap' on top of '$parentsnap'\n";
-        print "running 'qemu-img rebase -b $parentpath -F qcow -f qcow2 $childpath'\n";
+        print "running 'qemu-img rebase -b $parentpath -F qcow2 -f qcow2 $childpath'\n";
         $cmd = [
             '/usr/bin/qemu-img',
             'rebase',
-- 
2.39.5



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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [pve-devel] [PATCH storage 3/5] plugin: use relative path for qcow2 rebase command
  2025-07-29  7:38 [pve-devel] [PATCH storage/qemu-server 0/5] avoid absolute qcow2 references Fabian Grünbichler
  2025-07-29  7:38 ` [pve-devel] [PATCH storage 1/5] plugin: fix typo in rebase log message Fabian Grünbichler
  2025-07-29  7:38 ` [pve-devel] [PATCH storage 2/5] lvm " Fabian Grünbichler
@ 2025-07-29  7:38 ` Fabian Grünbichler
  2025-07-29  9:01   ` Fiona Ebner
  2025-07-29  7:38 ` [pve-devel] [PATCH storage 4/5] lvm " Fabian Grünbichler
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Fabian Grünbichler @ 2025-07-29  7:38 UTC (permalink / raw)
  To: pve-devel

otherwise the resulting qcow2 file will contain an absolute path, which makes
changing the backing path of the directory storage impossible.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/PVE/Storage/Plugin.pm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 99e0e76..48cda30 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -1434,15 +1434,15 @@ sub volume_snapshot_delete {
 
         } else {
             #we rebase the child image on the parent as new backing image
-            my $parentpath = $snapshots->{$parentsnap}->{file};
             print
                 "$volname: deleting snapshot '$snap' by rebasing '$childsnap' on top of '$parentsnap'\n";
-            print "running 'qemu-img rebase -b $parentpath -F qcow2 -f qcow2 $childpath'\n";
+            my $rel_parent_path = get_snap_name($class, $volname, $parentsnap);
+            print "running 'qemu-img rebase -b $rel_parent_path -F qcow2 -f qcow2 $childpath'\n";
             $cmd = [
                 '/usr/bin/qemu-img',
                 'rebase',
                 '-b',
-                $parentpath,
+                $rel_parent_path,
                 '-F',
                 'qcow2',
                 '-f',
-- 
2.39.5



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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [pve-devel] [PATCH storage 4/5] lvm plugin: use relative path for qcow2 rebase command
  2025-07-29  7:38 [pve-devel] [PATCH storage/qemu-server 0/5] avoid absolute qcow2 references Fabian Grünbichler
                   ` (2 preceding siblings ...)
  2025-07-29  7:38 ` [pve-devel] [PATCH storage 3/5] plugin: use relative path for qcow2 rebase command Fabian Grünbichler
@ 2025-07-29  7:38 ` Fabian Grünbichler
  2025-07-29  9:04   ` Fiona Ebner
  2025-07-29  7:38 ` [pve-devel] [PATCH qemu-server 5/5] blockdev-stream/-commit: make backing file relative Fabian Grünbichler
  2025-07-29 12:02 ` [pve-devel] superseded: [PATCH storage/qemu-server 0/5] avoid absolute qcow2 references Fabian Grünbichler
  5 siblings, 1 reply; 12+ messages in thread
From: Fabian Grünbichler @ 2025-07-29  7:38 UTC (permalink / raw)
  To: pve-devel

otherwise the resulting qcow2 file will contain an absolute path, which makes
renaming the backing VG of the storage impossible.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/PVE/Storage/LVMPlugin.pm | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index db1be98..9a04db6 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -1120,12 +1120,13 @@ sub volume_snapshot_delete {
         my $parentpath = $snapshots->{$parentsnap}->{file};
         print
             "$volname: deleting snapshot '$snap' by rebasing '$childsnap' on top of '$parentsnap'\n";
-        print "running 'qemu-img rebase -b $parentpath -F qcow2 -f qcow2 $childpath'\n";
+        my $rel_parent_path = get_snap_name($class, $volname, $parentsnap);
+        print "running 'qemu-img rebase -b $rel_parent_path -F qcow2 -f qcow2 $childpath'\n";
         $cmd = [
             '/usr/bin/qemu-img',
             'rebase',
             '-b',
-            $parentpath,
+            $rel_parent_path,
             '-F',
             'qcow2',
             '-f',
-- 
2.39.5



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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [pve-devel] [PATCH qemu-server 5/5] blockdev-stream/-commit: make backing file relative
  2025-07-29  7:38 [pve-devel] [PATCH storage/qemu-server 0/5] avoid absolute qcow2 references Fabian Grünbichler
                   ` (3 preceding siblings ...)
  2025-07-29  7:38 ` [pve-devel] [PATCH storage 4/5] lvm " Fabian Grünbichler
@ 2025-07-29  7:38 ` Fabian Grünbichler
  2025-07-29  9:17   ` Fiona Ebner
  2025-07-29 12:02 ` [pve-devel] superseded: [PATCH storage/qemu-server 0/5] avoid absolute qcow2 references Fabian Grünbichler
  5 siblings, 1 reply; 12+ messages in thread
From: Fabian Grünbichler @ 2025-07-29  7:38 UTC (permalink / raw)
  To: pve-devel

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>
---
 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;
 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);
+        }
+
         #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} = {};
-- 
2.39.5



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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [pve-devel] [PATCH storage 1/5] plugin: fix typo in rebase log message
  2025-07-29  7:38 ` [pve-devel] [PATCH storage 1/5] plugin: fix typo in rebase log message Fabian Grünbichler
@ 2025-07-29  8:09   ` Fiona Ebner
  0 siblings, 0 replies; 12+ messages in thread
From: Fiona Ebner @ 2025-07-29  8:09 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fabian Grünbichler

Am 29.07.25 um 9:38 AM schrieb Fabian Grünbichler:
> the format here is of course qcow2, and the actual command was correct, but the
> log message was not.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>

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

> ---
>  src/PVE/Storage/Plugin.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
> index 2bd05bd..99e0e76 100644
> --- a/src/PVE/Storage/Plugin.pm
> +++ b/src/PVE/Storage/Plugin.pm
> @@ -1437,7 +1437,7 @@ sub volume_snapshot_delete {
>              my $parentpath = $snapshots->{$parentsnap}->{file};
>              print
>                  "$volname: deleting snapshot '$snap' by rebasing '$childsnap' on top of '$parentsnap'\n";
> -            print "running 'qemu-img rebase -b $parentpath -F qcow -f qcow2 $childpath'\n";
> +            print "running 'qemu-img rebase -b $parentpath -F qcow2 -f qcow2 $childpath'\n";
>              $cmd = [
>                  '/usr/bin/qemu-img',
>                  'rebase',

But I'd prefer to join() the command array and print it directly to
avoid such issues altogether.


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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [pve-devel] [PATCH storage 2/5] lvm plugin: fix typo in rebase log message
  2025-07-29  7:38 ` [pve-devel] [PATCH storage 2/5] lvm " Fabian Grünbichler
@ 2025-07-29  8:09   ` Fiona Ebner
  0 siblings, 0 replies; 12+ messages in thread
From: Fiona Ebner @ 2025-07-29  8:09 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fabian Grünbichler

Am 29.07.25 um 9:38 AM schrieb Fabian Grünbichler:
> this was copied over from Plugin.pm
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>

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

> ---
>  src/PVE/Storage/LVMPlugin.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
> index c1f5474..db1be98 100644
> --- a/src/PVE/Storage/LVMPlugin.pm
> +++ b/src/PVE/Storage/LVMPlugin.pm
> @@ -1120,7 +1120,7 @@ sub volume_snapshot_delete {
>          my $parentpath = $snapshots->{$parentsnap}->{file};
>          print
>              "$volname: deleting snapshot '$snap' by rebasing '$childsnap' on top of '$parentsnap'\n";
> -        print "running 'qemu-img rebase -b $parentpath -F qcow -f qcow2 $childpath'\n";
> +        print "running 'qemu-img rebase -b $parentpath -F qcow2 -f qcow2 $childpath'\n";
>          $cmd = [
>              '/usr/bin/qemu-img',
>              'rebase',

But I'd prefer to join() the command array and print it directly to
avoid such issues altogether.


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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [pve-devel] [PATCH storage 3/5] plugin: use relative path for qcow2 rebase command
  2025-07-29  7:38 ` [pve-devel] [PATCH storage 3/5] plugin: use relative path for qcow2 rebase command Fabian Grünbichler
@ 2025-07-29  9:01   ` Fiona Ebner
  0 siblings, 0 replies; 12+ messages in thread
From: Fiona Ebner @ 2025-07-29  9:01 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fabian Grünbichler

Am 29.07.25 um 9:39 AM schrieb Fabian Grünbichler:
> otherwise the resulting qcow2 file will contain an absolute path, which makes
> changing the backing path of the directory 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>

> ---
>  src/PVE/Storage/Plugin.pm | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
> index 99e0e76..48cda30 100644
> --- a/src/PVE/Storage/Plugin.pm
> +++ b/src/PVE/Storage/Plugin.pm
> @@ -1434,15 +1434,15 @@ sub volume_snapshot_delete {
>  
>          } else {
>              #we rebase the child image on the parent as new backing image
> -            my $parentpath = $snapshots->{$parentsnap}->{file};
>              print
>                  "$volname: deleting snapshot '$snap' by rebasing '$childsnap' on top of '$parentsnap'\n";
> -            print "running 'qemu-img rebase -b $parentpath -F qcow2 -f qcow2 $childpath'\n";
> +            my $rel_parent_path = get_snap_name($class, $volname, $parentsnap);
> +            print "running 'qemu-img rebase -b $rel_parent_path -F qcow2 -f qcow2 $childpath'\n";
>              $cmd = [
>                  '/usr/bin/qemu-img',
>                  'rebase',
>                  '-b',
> -                $parentpath,
> +                $rel_parent_path,
>                  '-F',
>                  'qcow2',
>                  '-f',



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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [pve-devel] [PATCH storage 4/5] lvm plugin: use relative path for qcow2 rebase command
  2025-07-29  7:38 ` [pve-devel] [PATCH storage 4/5] lvm " Fabian Grünbichler
@ 2025-07-29  9:04   ` Fiona Ebner
  0 siblings, 0 replies; 12+ messages in thread
From: Fiona Ebner @ 2025-07-29  9:04 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fabian Grünbichler

Am 29.07.25 um 9:39 AM schrieb Fabian Grünbichler:
> otherwise the resulting qcow2 file will contain an absolute path, which makes
> renaming the backing VG of the storage impossible.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>

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

with a nit below:

> ---
>  src/PVE/Storage/LVMPlugin.pm | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
> index db1be98..9a04db6 100644
> --- a/src/PVE/Storage/LVMPlugin.pm
> +++ b/src/PVE/Storage/LVMPlugin.pm
> @@ -1120,12 +1120,13 @@ sub volume_snapshot_delete {
>          my $parentpath = $snapshots->{$parentsnap}->{file};

This variable is unused now and could be removed like in the previous patch.

>          print
>              "$volname: deleting snapshot '$snap' by rebasing '$childsnap' on top of '$parentsnap'\n";
> -        print "running 'qemu-img rebase -b $parentpath -F qcow2 -f qcow2 $childpath'\n";
> +        my $rel_parent_path = get_snap_name($class, $volname, $parentsnap);
> +        print "running 'qemu-img rebase -b $rel_parent_path -F qcow2 -f qcow2 $childpath'\n";
>          $cmd = [
>              '/usr/bin/qemu-img',
>              'rebase',
>              '-b',
> -            $parentpath,
> +            $rel_parent_path,
>              '-F',
>              'qcow2',
>              '-f',



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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [pve-devel] [PATCH qemu-server 5/5] blockdev-stream/-commit: make backing file relative
  2025-07-29  7:38 ` [pve-devel] [PATCH qemu-server 5/5] blockdev-stream/-commit: make backing file relative Fabian Grünbichler
@ 2025-07-29  9:17   ` Fiona Ebner
  0 siblings, 0 replies; 12+ messages in thread
From: Fiona Ebner @ 2025-07-29  9:17 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fabian Grünbichler

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [pve-devel] superseded: [PATCH storage/qemu-server 0/5] avoid absolute qcow2 references
  2025-07-29  7:38 [pve-devel] [PATCH storage/qemu-server 0/5] avoid absolute qcow2 references Fabian Grünbichler
                   ` (4 preceding siblings ...)
  2025-07-29  7:38 ` [pve-devel] [PATCH qemu-server 5/5] blockdev-stream/-commit: make backing file relative Fabian Grünbichler
@ 2025-07-29 12:02 ` Fabian Grünbichler
  5 siblings, 0 replies; 12+ messages in thread
From: Fabian Grünbichler @ 2025-07-29 12:02 UTC (permalink / raw)
  To: Proxmox VE development discussion

by https://lore.proxmox.com/pve-devel/20250729115320.579286-1-f.gruenbichler@proxmox.com/T/#t

On July 29, 2025 9:38 am, Fabian Grünbichler wrote:
> we don't want qcow2 files to reference their backing chains via
> absolute paths, as that makes renaming the base dir or VG of the storage
> impossible. in most places, qemu already allows simply passing a
> filename as backing-file reference, which will be interpreted as a
> reference relative to the backed image.
> 
> I haven't found any further code paths that trigger absolute references,
> but I might have missed some. the full backing chain should show
> relative backing-file members when queried via
> 
> qemu-img info --output json --format qcow2 --backing-chain /path/to/main/image.qcow2
> 
> such, as:
> 
>         "full-backing-filename": "/var/lib/extsnap/images/210/snap-test2-vm-210-disk-0.qcow2",
>         "backing-filename": "snap-test2-vm-210-disk-0.qcow2",
> 
> note that full-backing-filename will always contain the resolved,
> absolute path and that is okay. we could warn about both members
> containing full paths in `volume_snapshot_info`.
> 
> for existing "broken" images, an "unsafe" rebase with
> 
> qemu-img rebase -u -f qcow2 -F qcow2 -b <relative backing file path> <absolute backed filed path>
> 
> should just rewrite the qcow2 header to replace the backing file
> reference - this should of course not be run while the image is being
> written by other process or QEMU.
> 
> pve-storage:
> 
> Fabian Grünbichler (4):
>   plugin: fix typo in rebase log message
>   lvm plugin: fix typo in rebase log message
>   plugin: use relative path for qcow2 rebase command
>   lvm plugin: use relative path for qcow2 rebase command
> 
>  src/PVE/Storage/LVMPlugin.pm | 5 +++--
>  src/PVE/Storage/Plugin.pm    | 6 +++---
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> qemu-server:
> 
> Fabian Grünbichler (1):
>   blockdev-stream/-commit: make backing file relative
> 
>  src/PVE/QemuServer/Blockdev.pm | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> -- 
> 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-07-29 12:01 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-07-29  7:38 [pve-devel] [PATCH storage/qemu-server 0/5] avoid absolute qcow2 references Fabian Grünbichler
2025-07-29  7:38 ` [pve-devel] [PATCH storage 1/5] plugin: fix typo in rebase log message Fabian Grünbichler
2025-07-29  8:09   ` Fiona Ebner
2025-07-29  7:38 ` [pve-devel] [PATCH storage 2/5] lvm " Fabian Grünbichler
2025-07-29  8:09   ` Fiona Ebner
2025-07-29  7:38 ` [pve-devel] [PATCH storage 3/5] plugin: use relative path for qcow2 rebase command Fabian Grünbichler
2025-07-29  9:01   ` Fiona Ebner
2025-07-29  7:38 ` [pve-devel] [PATCH storage 4/5] lvm " Fabian Grünbichler
2025-07-29  9:04   ` Fiona Ebner
2025-07-29  7:38 ` [pve-devel] [PATCH qemu-server 5/5] blockdev-stream/-commit: make backing file relative Fabian Grünbichler
2025-07-29  9:17   ` Fiona Ebner
2025-07-29 12:02 ` [pve-devel] superseded: [PATCH storage/qemu-server 0/5] avoid absolute qcow2 references Fabian Grünbichler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal