* [pve-devel] [PATCH storage v2 1/5] plugin: fix typo in rebase log message
2025-07-29 11:53 [pve-devel] [PATCH storage/qemu-server v2 0/5] avoid absolute qcow2 references Fabian Grünbichler
@ 2025-07-29 11:53 ` Fabian Grünbichler
2025-07-29 12:04 ` Fiona Ebner
2025-07-29 11:53 ` [pve-devel] [PATCH storage v2 2/5] lvm " Fabian Grünbichler
` (4 subsequent siblings)
5 siblings, 1 reply; 9+ messages in thread
From: Fabian Grünbichler @ 2025-07-29 11:53 UTC (permalink / raw)
To: pve-devel
by directly printing the to-be-executed command, instead of copying it which is
error-prone.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
Notes:
v2: join command instead of fixing manually copied message
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..de41bfe 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -1437,7 +1437,6 @@ 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";
$cmd = [
'/usr/bin/qemu-img',
'rebase',
@@ -1449,6 +1448,7 @@ sub volume_snapshot_delete {
'qcow2',
$childpath,
];
+ print "running '", join(' ', $cmd->@*), "'\n";
eval { run_command($cmd) };
if ($@) {
#in case of abort, the state of the snap is still clean, just a little bit bigger
--
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] 9+ messages in thread
* Re: [pve-devel] [PATCH storage v2 1/5] plugin: fix typo in rebase log message
2025-07-29 11:53 ` [pve-devel] [PATCH storage v2 1/5] plugin: fix typo in rebase log message Fabian Grünbichler
@ 2025-07-29 12:04 ` Fiona Ebner
2025-07-29 12:15 ` Fabian Grünbichler
0 siblings, 1 reply; 9+ messages in thread
From: Fiona Ebner @ 2025-07-29 12:04 UTC (permalink / raw)
To: Fabian Grünbichler, pve-devel
Am 29.07.25 um 1:53 PM schrieb Fabian Grünbichler:
> by directly printing the to-be-executed command, instead of copying it which is
> error-prone.
>
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>
> Notes:
> v2: join command instead of fixing manually copied message
>
> 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..de41bfe 100644
> --- a/src/PVE/Storage/Plugin.pm
> +++ b/src/PVE/Storage/Plugin.pm
> @@ -1437,7 +1437,6 @@ 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";
> $cmd = [
> '/usr/bin/qemu-img',
> 'rebase',
> @@ -1449,6 +1448,7 @@ sub volume_snapshot_delete {
> 'qcow2',
> $childpath,
> ];
> + print "running '", join(' ', $cmd->@*), "'\n";
Style nit: we usually do concatenation with dot, was using commas for
passing a list to print intentional? Same for the next patch.
> eval { run_command($cmd) };
> if ($@) {
> #in case of abort, the state of the snap is still clean, just a little bit bigger
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH storage v2 1/5] plugin: fix typo in rebase log message
2025-07-29 12:04 ` Fiona Ebner
@ 2025-07-29 12:15 ` Fabian Grünbichler
0 siblings, 0 replies; 9+ messages in thread
From: Fabian Grünbichler @ 2025-07-29 12:15 UTC (permalink / raw)
To: Fiona Ebner, pve-devel
On July 29, 2025 2:04 pm, Fiona Ebner wrote:
> Am 29.07.25 um 1:53 PM schrieb Fabian Grünbichler:
>> by directly printing the to-be-executed command, instead of copying it which is
>> error-prone.
>>
>> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
>> Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
>> ---
>>
>> Notes:
>> v2: join command instead of fixing manually copied message
>>
>> 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..de41bfe 100644
>> --- a/src/PVE/Storage/Plugin.pm
>> +++ b/src/PVE/Storage/Plugin.pm
>> @@ -1437,7 +1437,6 @@ 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";
>> $cmd = [
>> '/usr/bin/qemu-img',
>> 'rebase',
>> @@ -1449,6 +1448,7 @@ sub volume_snapshot_delete {
>> 'qcow2',
>> $childpath,
>> ];
>> + print "running '", join(' ', $cmd->@*), "'\n";
>
> Style nit: we usually do concatenation with dot, was using commas for
> passing a list to print intentional? Same for the next patch.
force of habit, no real reason - feel free to fixup on applying :)
>> eval { run_command($cmd) };
>> if ($@) {
>> #in case of abort, the state of the snap is still clean, just a little bit bigger
>
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pve-devel] [PATCH storage v2 2/5] lvm plugin: fix typo in rebase log message
2025-07-29 11:53 [pve-devel] [PATCH storage/qemu-server v2 0/5] avoid absolute qcow2 references Fabian Grünbichler
2025-07-29 11:53 ` [pve-devel] [PATCH storage v2 1/5] plugin: fix typo in rebase log message Fabian Grünbichler
@ 2025-07-29 11:53 ` Fabian Grünbichler
2025-07-29 11:53 ` [pve-devel] [PATCH storage v2 3/5] plugin: use relative path for qcow2 rebase command Fabian Grünbichler
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Fabian Grünbichler @ 2025-07-29 11:53 UTC (permalink / raw)
To: pve-devel
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..5a84e82 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -1120,7 +1120,6 @@ 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";
$cmd = [
'/usr/bin/qemu-img',
'rebase',
@@ -1132,6 +1131,7 @@ sub volume_snapshot_delete {
'qcow2',
$childpath,
];
+ print "running '", join(' ', $cmd->@*), "'\n";
eval { run_command($cmd) };
if ($@) {
#in case of abort, the state of the snap is still clean, just a little bit bigger
--
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] 9+ messages in thread
* [pve-devel] [PATCH storage v2 3/5] plugin: use relative path for qcow2 rebase command
2025-07-29 11:53 [pve-devel] [PATCH storage/qemu-server v2 0/5] avoid absolute qcow2 references Fabian Grünbichler
2025-07-29 11:53 ` [pve-devel] [PATCH storage v2 1/5] plugin: fix typo in rebase log message Fabian Grünbichler
2025-07-29 11:53 ` [pve-devel] [PATCH storage v2 2/5] lvm " Fabian Grünbichler
@ 2025-07-29 11:53 ` Fabian Grünbichler
2025-07-29 11:53 ` [pve-devel] [PATCH storage v2 4/5] lvm " Fabian Grünbichler
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Fabian Grünbichler @ 2025-07-29 11:53 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>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Tested-by: Fiona Ebner <f.ebner@proxmox.com>
---
src/PVE/Storage/Plugin.pm | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index de41bfe..f54a38c 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -1434,14 +1434,14 @@ 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";
+ my $rel_parent_path = get_snap_name($class, $volname, $parentsnap);
$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] 9+ messages in thread
* [pve-devel] [PATCH storage v2 4/5] lvm plugin: use relative path for qcow2 rebase command
2025-07-29 11:53 [pve-devel] [PATCH storage/qemu-server v2 0/5] avoid absolute qcow2 references Fabian Grünbichler
` (2 preceding siblings ...)
2025-07-29 11:53 ` [pve-devel] [PATCH storage v2 3/5] plugin: use relative path for qcow2 rebase command Fabian Grünbichler
@ 2025-07-29 11:53 ` Fabian Grünbichler
2025-07-29 11:53 ` [pve-devel] [PATCH qemu-server v2 5/5] blockdev-stream/-commit: make backing file relative Fabian Grünbichler
2025-07-29 12:51 ` [pve-devel] applied-series: [PATCH storage/qemu-server v2 0/5] avoid absolute qcow2 references Fiona Ebner
5 siblings, 0 replies; 9+ messages in thread
From: Fabian Grünbichler @ 2025-07-29 11:53 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>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
Notes:
v2: drop unused variable
src/PVE/Storage/LVMPlugin.pm | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index 5a84e82..46c8d8e 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -1117,14 +1117,14 @@ 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";
+ my $rel_parent_path = get_snap_name($class, $volname, $parentsnap);
$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] 9+ messages in thread
* [pve-devel] [PATCH qemu-server v2 5/5] blockdev-stream/-commit: make backing file relative
2025-07-29 11:53 [pve-devel] [PATCH storage/qemu-server v2 0/5] avoid absolute qcow2 references Fabian Grünbichler
` (3 preceding siblings ...)
2025-07-29 11:53 ` [pve-devel] [PATCH storage v2 4/5] lvm " Fabian Grünbichler
@ 2025-07-29 11:53 ` Fabian Grünbichler
2025-07-29 12:51 ` [pve-devel] applied-series: [PATCH storage/qemu-server v2 0/5] avoid absolute qcow2 references Fiona Ebner
5 siblings, 0 replies; 9+ messages in thread
From: Fabian Grünbichler @ 2025-07-29 11:53 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>
---
Notes:
v2: move logic into its own helper
src/PVE/QemuServer/Blockdev.pm | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/src/PVE/QemuServer/Blockdev.pm b/src/PVE/QemuServer/Blockdev.pm
index 6e105571..08bc48dd 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 qw(basename dirname);
use File::stat;
use JSON;
@@ -885,6 +886,20 @@ sub blockdev_delete {
PVE::Storage::volume_snapshot_delete($storecfg, $volid, $snap, 1);
}
+my sub blockdev_relative_backing_file {
+ my ($backing, $backed) = @_;
+
+ my $backing_file = $backing->{filename};
+ my $backed_file = $backed->{filename};
+
+ if (dirname($backing_file) eq dirname($backed_file)) {
+ # make backing file relative if in same directory
+ return basename($backing_file);
+ }
+
+ return $backing_file;
+}
+
sub blockdev_replace {
my (
$storecfg,
@@ -954,12 +969,15 @@ sub blockdev_replace {
$parent_fmt_blockdev->{backing} = $target_fmt_blockdev->{'node-name'};
mon_cmd($vmid, 'blockdev-reopen', options => [$parent_fmt_blockdev]);
+ my $backing_file =
+ blockdev_relative_backing_file($target_file_blockdev, $parent_file_blockdev);
+
#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 +1087,13 @@ sub blockdev_stream {
{ 'snapshot-name' => $snap },
);
+ my $backing_file = blockdev_relative_backing_file($parent_file_blockdev, $target_file_blockdev);
+
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] 9+ messages in thread
* [pve-devel] applied-series: [PATCH storage/qemu-server v2 0/5] avoid absolute qcow2 references
2025-07-29 11:53 [pve-devel] [PATCH storage/qemu-server v2 0/5] avoid absolute qcow2 references Fabian Grünbichler
` (4 preceding siblings ...)
2025-07-29 11:53 ` [pve-devel] [PATCH qemu-server v2 5/5] blockdev-stream/-commit: make backing file relative Fabian Grünbichler
@ 2025-07-29 12:51 ` Fiona Ebner
5 siblings, 0 replies; 9+ messages in thread
From: Fiona Ebner @ 2025-07-29 12:51 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Grünbichler
Am 29.07.25 um 1:53 PM schrieb Fabian Grünbichler:
> 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.
Applied series, with the discussed fix-up squashed in, thanks!
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 9+ messages in thread