* [pve-devel] [PATCH container v3 1/8] api: create: move delayed_mp_param variable closer to usage
2026-01-23 14:34 [pve-devel] [PATCH-SERIES container/manager v3 0/8] api: restore: allow keeping not backed-up volumes Fiona Ebner
@ 2026-01-23 14:34 ` Fiona Ebner
2026-01-23 14:34 ` [pve-devel] [PATCH container v3 2/8] api: create: reduce scope for $mp_param variable Fiona Ebner
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Fiona Ebner @ 2026-01-23 14:34 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
New in v3.
src/PVE/API2/LXC.pm | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 3d74f71..204e1c8 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -384,7 +384,6 @@ __PACKAGE__->register_method({
if !$storage_only_mode && !defined($mp_param->{rootfs});
# check storage access, activate storage
- my $delayed_mp_param = {};
PVE::LXC::Config->foreach_volume(
$mp_param,
sub {
@@ -465,6 +464,8 @@ __PACKAGE__->register_method({
}
}
}
+
+ my $delayed_mp_param = {};
if ($storage_only_mode) {
if ($restore) {
if (!defined($orig_mp_param)) {
--
2.47.3
_______________________________________________
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 container v3 2/8] api: create: reduce scope for $mp_param variable
2026-01-23 14:34 [pve-devel] [PATCH-SERIES container/manager v3 0/8] api: restore: allow keeping not backed-up volumes Fiona Ebner
2026-01-23 14:34 ` [pve-devel] [PATCH container v3 1/8] api: create: move delayed_mp_param variable closer to usage Fiona Ebner
@ 2026-01-23 14:34 ` Fiona Ebner
2026-01-23 14:34 ` [pve-devel] [PATCH container v3 3/8] api: create: factor out create_ct_determine_mp_param helper Fiona Ebner
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Fiona Ebner @ 2026-01-23 14:34 UTC (permalink / raw)
To: pve-devel
Introduce an $api_mp_param variable for better separation to prepare
for factoring out a helper.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
New in v3.
src/PVE/API2/LXC.pm | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 204e1c8..4370c57 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -360,16 +360,16 @@ __PACKAGE__->register_method({
my $is_root = $authuser eq 'root@pam';
my $no_disk_param = {};
- my $mp_param = {};
+ my $api_mp_param = {};
my $storage_only_mode = 1;
foreach my $opt (keys %$param) {
my $value = $param->{$opt};
if ($opt eq 'rootfs' || $opt =~ m/^mp\d+$/) {
# allow to use simple numbers (add default storage in that case)
if ($value =~ m/^\d+(\.\d+)?$/) {
- $mp_param->{$opt} = "$storage:$value";
+ $api_mp_param->{$opt} = "$storage:$value";
} else {
- $mp_param->{$opt} = $value;
+ $api_mp_param->{$opt} = $value;
}
$storage_only_mode = 0;
} elsif ($opt =~ m/^unused\d+$/) {
@@ -381,11 +381,11 @@ __PACKAGE__->register_method({
}
die "mount points configured, but 'rootfs' not set - aborting\n"
- if !$storage_only_mode && !defined($mp_param->{rootfs});
+ if !$storage_only_mode && !defined($api_mp_param->{rootfs});
# check storage access, activate storage
PVE::LXC::Config->foreach_volume(
- $mp_param,
+ $api_mp_param,
sub {
my ($ms, $mountpoint) = @_;
@@ -403,7 +403,7 @@ __PACKAGE__->register_method({
);
# check/activate default storage
- &$check_and_activate_storage($storage) if !defined($mp_param->{rootfs});
+ &$check_and_activate_storage($storage) if !defined($api_mp_param->{rootfs});
PVE::LXC::Config->update_pct_config($vmid, $conf, 0, $no_disk_param);
@@ -465,6 +465,7 @@ __PACKAGE__->register_method({
}
}
+ my $mp_param;
my $delayed_mp_param = {};
if ($storage_only_mode) {
if ($restore) {
@@ -521,8 +522,11 @@ __PACKAGE__->register_method({
},
);
} else {
+ $mp_param = $api_mp_param;
$mp_param->{rootfs} = "$storage:4"; # defaults to 4GB
}
+ } else {
+ $mp_param = $api_mp_param;
}
# up until here we did not modify the container, besides the lock
--
2.47.3
_______________________________________________
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 container v3 3/8] api: create: factor out create_ct_determine_mp_param helper
2026-01-23 14:34 [pve-devel] [PATCH-SERIES container/manager v3 0/8] api: restore: allow keeping not backed-up volumes Fiona Ebner
2026-01-23 14:34 ` [pve-devel] [PATCH container v3 1/8] api: create: move delayed_mp_param variable closer to usage Fiona Ebner
2026-01-23 14:34 ` [pve-devel] [PATCH container v3 2/8] api: create: reduce scope for $mp_param variable Fiona Ebner
@ 2026-01-23 14:34 ` Fiona Ebner
2026-01-23 14:34 ` [pve-devel] [PATCH container v3 4/8] run make tidy Fiona Ebner
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Fiona Ebner @ 2026-01-23 14:34 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
New in v3.
Avoided running make tidy here to make it easier to review.
The next patch can be squashed into this when applying for that.
src/PVE/API2/LXC.pm | 157 ++++++++++++++++++++++++++------------------
1 file changed, 94 insertions(+), 63 deletions(-)
diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 4370c57..ede8f62 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -135,6 +135,89 @@ __PACKAGE__->register_method({
},
});
+my sub create_ct_determine_mp_param {
+ my (
+ $storage_cfg,
+ $vmid,
+ $archive,
+ $api_mp_param,
+ $orig_mp_param,
+ $restore,
+ $storage,
+ $storage_only_mode,
+ $is_root,
+ ) = @_;
+
+ my $mp_param;
+ my $delayed_mp_param = {};
+
+ if (!$storage_only_mode) {
+ $mp_param = $api_mp_param;
+ return ($mp_param, $delayed_mp_param);
+ }
+
+ if (!$restore) {
+ $mp_param = $api_mp_param;
+ $mp_param->{rootfs} = "$storage:4"; # defaults to 4GB
+ return ($mp_param, $delayed_mp_param);
+ }
+
+ if (!defined($orig_mp_param)) {
+ print "recovering backed-up configuration from '$archive'\n";
+ (undef, $orig_mp_param) =
+ PVE::LXC::Create::recover_config($storage_cfg, $archive, $vmid);
+ }
+ $mp_param = $orig_mp_param;
+ die
+ "rootfs configuration could not be recovered, please check and specify manually!\n"
+ if !defined($mp_param->{rootfs});
+ PVE::LXC::Config->foreach_volume(
+ $mp_param,
+ sub {
+ my ($ms, $mountpoint) = @_;
+ my $type = $mountpoint->{type};
+ if ($type eq 'volume') {
+ die
+ "unable to detect disk size - please specify $ms (size)\n"
+ if !defined($mountpoint->{size});
+ my $disksize = $mountpoint->{size} / (1024 * 1024 * 1024); # create_disks expects GB as unit size
+ delete $mountpoint->{size};
+ $mountpoint->{volume} = "$storage:$disksize";
+ $mp_param->{$ms} = PVE::LXC::Config->print_ct_mountpoint(
+ $mountpoint,
+ $ms eq 'rootfs',
+ );
+ } else {
+ my $type = $mountpoint->{type};
+ die
+ "restoring rootfs to $type mount is only possible by specifying -rootfs manually!\n"
+ if ($ms eq 'rootfs');
+ die
+ "restoring '$ms' to $type mount is only possible for root\n"
+ if !$is_root;
+
+ if ($mountpoint->{backup}) {
+ warn "WARNING - unsupported configuration!\n";
+ warn
+ "backup was enabled for $type mount point $ms ('$mountpoint->{mp}')\n";
+ warn
+ "mount point configuration will be restored after archive extraction!\n";
+ warn
+ "contained files will be restored to wrong directory!\n";
+ }
+ delete $mp_param->{$ms}; # actually delay bind/dev mps
+ $delayed_mp_param->{$ms} =
+ PVE::LXC::Config->print_ct_mountpoint(
+ $mountpoint,
+ $ms eq 'rootfs',
+ );
+ }
+ },
+ );
+
+ return ($mp_param, $delayed_mp_param);
+}
+
__PACKAGE__->register_method({
name => 'create_vm',
path => '',
@@ -465,69 +548,17 @@ __PACKAGE__->register_method({
}
}
- my $mp_param;
- my $delayed_mp_param = {};
- if ($storage_only_mode) {
- if ($restore) {
- if (!defined($orig_mp_param)) {
- print "recovering backed-up configuration from '$archive'\n";
- (undef, $orig_mp_param) =
- PVE::LXC::Create::recover_config($storage_cfg, $archive, $vmid);
- }
- $mp_param = $orig_mp_param;
- die
- "rootfs configuration could not be recovered, please check and specify manually!\n"
- if !defined($mp_param->{rootfs});
- PVE::LXC::Config->foreach_volume(
- $mp_param,
- sub {
- my ($ms, $mountpoint) = @_;
- my $type = $mountpoint->{type};
- if ($type eq 'volume') {
- die
- "unable to detect disk size - please specify $ms (size)\n"
- if !defined($mountpoint->{size});
- my $disksize = $mountpoint->{size} / (1024 * 1024 * 1024); # create_disks expects GB as unit size
- delete $mountpoint->{size};
- $mountpoint->{volume} = "$storage:$disksize";
- $mp_param->{$ms} = PVE::LXC::Config->print_ct_mountpoint(
- $mountpoint,
- $ms eq 'rootfs',
- );
- } else {
- my $type = $mountpoint->{type};
- die
- "restoring rootfs to $type mount is only possible by specifying -rootfs manually!\n"
- if ($ms eq 'rootfs');
- die
- "restoring '$ms' to $type mount is only possible for root\n"
- if !$is_root;
-
- if ($mountpoint->{backup}) {
- warn "WARNING - unsupported configuration!\n";
- warn
- "backup was enabled for $type mount point $ms ('$mountpoint->{mp}')\n";
- warn
- "mount point configuration will be restored after archive extraction!\n";
- warn
- "contained files will be restored to wrong directory!\n";
- }
- delete $mp_param->{$ms}; # actually delay bind/dev mps
- $delayed_mp_param->{$ms} =
- PVE::LXC::Config->print_ct_mountpoint(
- $mountpoint,
- $ms eq 'rootfs',
- );
- }
- },
- );
- } else {
- $mp_param = $api_mp_param;
- $mp_param->{rootfs} = "$storage:4"; # defaults to 4GB
- }
- } else {
- $mp_param = $api_mp_param;
- }
+ my ($mp_param, $delayed_mp_param) = create_ct_determine_mp_param(
+ $storage_cfg,
+ $vmid,
+ $archive,
+ $api_mp_param,
+ $orig_mp_param,
+ $restore,
+ $storage,
+ $storage_only_mode,
+ $is_root,
+ );
# up until here we did not modify the container, besides the lock
$destroy_config_on_error = 1;
--
2.47.3
_______________________________________________
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 container v3 4/8] run make tidy
2026-01-23 14:34 [pve-devel] [PATCH-SERIES container/manager v3 0/8] api: restore: allow keeping not backed-up volumes Fiona Ebner
` (2 preceding siblings ...)
2026-01-23 14:34 ` [pve-devel] [PATCH container v3 3/8] api: create: factor out create_ct_determine_mp_param helper Fiona Ebner
@ 2026-01-23 14:34 ` Fiona Ebner
2026-01-23 14:34 ` [pve-devel] [PATCH container v3 5/8] api: create: create_ct_determine_mp_param: improve code style Fiona Ebner
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Fiona Ebner @ 2026-01-23 14:34 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
New in v3.
Done separately for readability of the previous commit, can be
squashed into that.
src/PVE/API2/LXC.pm | 29 ++++++++++-------------------
1 file changed, 10 insertions(+), 19 deletions(-)
diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index ede8f62..8623d63 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -164,12 +164,10 @@ my sub create_ct_determine_mp_param {
if (!defined($orig_mp_param)) {
print "recovering backed-up configuration from '$archive'\n";
- (undef, $orig_mp_param) =
- PVE::LXC::Create::recover_config($storage_cfg, $archive, $vmid);
+ (undef, $orig_mp_param) = PVE::LXC::Create::recover_config($storage_cfg, $archive, $vmid);
}
$mp_param = $orig_mp_param;
- die
- "rootfs configuration could not be recovered, please check and specify manually!\n"
+ die "rootfs configuration could not be recovered, please check and specify manually!\n"
if !defined($mp_param->{rootfs});
PVE::LXC::Config->foreach_volume(
$mp_param,
@@ -177,40 +175,33 @@ my sub create_ct_determine_mp_param {
my ($ms, $mountpoint) = @_;
my $type = $mountpoint->{type};
if ($type eq 'volume') {
- die
- "unable to detect disk size - please specify $ms (size)\n"
+ die "unable to detect disk size - please specify $ms (size)\n"
if !defined($mountpoint->{size});
my $disksize = $mountpoint->{size} / (1024 * 1024 * 1024); # create_disks expects GB as unit size
delete $mountpoint->{size};
$mountpoint->{volume} = "$storage:$disksize";
$mp_param->{$ms} = PVE::LXC::Config->print_ct_mountpoint(
- $mountpoint,
- $ms eq 'rootfs',
+ $mountpoint, $ms eq 'rootfs',
);
} else {
my $type = $mountpoint->{type};
die
"restoring rootfs to $type mount is only possible by specifying -rootfs manually!\n"
if ($ms eq 'rootfs');
- die
- "restoring '$ms' to $type mount is only possible for root\n"
+ die "restoring '$ms' to $type mount is only possible for root\n"
if !$is_root;
if ($mountpoint->{backup}) {
warn "WARNING - unsupported configuration!\n";
- warn
- "backup was enabled for $type mount point $ms ('$mountpoint->{mp}')\n";
+ warn "backup was enabled for $type mount point $ms ('$mountpoint->{mp}')\n";
warn
"mount point configuration will be restored after archive extraction!\n";
- warn
- "contained files will be restored to wrong directory!\n";
+ warn "contained files will be restored to wrong directory!\n";
}
delete $mp_param->{$ms}; # actually delay bind/dev mps
- $delayed_mp_param->{$ms} =
- PVE::LXC::Config->print_ct_mountpoint(
- $mountpoint,
- $ms eq 'rootfs',
- );
+ $delayed_mp_param->{$ms} = PVE::LXC::Config->print_ct_mountpoint(
+ $mountpoint, $ms eq 'rootfs',
+ );
}
},
);
--
2.47.3
_______________________________________________
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 container v3 5/8] api: create: create_ct_determine_mp_param: improve code style
2026-01-23 14:34 [pve-devel] [PATCH-SERIES container/manager v3 0/8] api: restore: allow keeping not backed-up volumes Fiona Ebner
` (3 preceding siblings ...)
2026-01-23 14:34 ` [pve-devel] [PATCH container v3 4/8] run make tidy Fiona Ebner
@ 2026-01-23 14:34 ` Fiona Ebner
2026-01-23 14:34 ` [pve-devel] [PATCH container v3 6/8] api: create: get rid of $storage_only_mode variable Fiona Ebner
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Fiona Ebner @ 2026-01-23 14:34 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
New in v3.
src/PVE/API2/LXC.pm | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 8623d63..f7d5de4 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -177,7 +177,8 @@ my sub create_ct_determine_mp_param {
if ($type eq 'volume') {
die "unable to detect disk size - please specify $ms (size)\n"
if !defined($mountpoint->{size});
- my $disksize = $mountpoint->{size} / (1024 * 1024 * 1024); # create_disks expects GB as unit size
+ # create_disks expects GB as unit size
+ my $disksize = $mountpoint->{size} / (1024 * 1024 * 1024);
delete $mountpoint->{size};
$mountpoint->{volume} = "$storage:$disksize";
$mp_param->{$ms} = PVE::LXC::Config->print_ct_mountpoint(
@@ -185,9 +186,10 @@ my sub create_ct_determine_mp_param {
);
} else {
my $type = $mountpoint->{type};
- die
- "restoring rootfs to $type mount is only possible by specifying -rootfs manually!\n"
- if ($ms eq 'rootfs');
+ if ($ms eq 'rootfs') {
+ die "restoring rootfs to $type mount is only possible by specifying -rootfs"
+ . " manually!\n";
+ }
die "restoring '$ms' to $type mount is only possible for root\n"
if !$is_root;
--
2.47.3
_______________________________________________
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 container v3 6/8] api: create: get rid of $storage_only_mode variable
2026-01-23 14:34 [pve-devel] [PATCH-SERIES container/manager v3 0/8] api: restore: allow keeping not backed-up volumes Fiona Ebner
` (4 preceding siblings ...)
2026-01-23 14:34 ` [pve-devel] [PATCH container v3 5/8] api: create: create_ct_determine_mp_param: improve code style Fiona Ebner
@ 2026-01-23 14:34 ` Fiona Ebner
2026-01-23 14:34 ` [pve-devel] [PATCH container v3 7/8] api: restore: allow keeping not backed-up volumes Fiona Ebner
2026-01-23 14:34 ` [pve-devel] [PATCH manager v3 8/8] ui: restore: enable safeguarding of mount point volumes by default Fiona Ebner
7 siblings, 0 replies; 9+ messages in thread
From: Fiona Ebner @ 2026-01-23 14:34 UTC (permalink / raw)
To: pve-devel
The name is rather vague. The variable is set if and only if there are
no mp params specified via the API, so explicitly check for that
instead. This also makes it visible that in the non-restore case, the
result of create_ct_determine_mp_param() does not depend upon
$api_mp_param.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
New in v3.
src/PVE/API2/LXC.pm | 19 ++++---------------
1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index f7d5de4..fc278a7 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -137,28 +137,20 @@ __PACKAGE__->register_method({
my sub create_ct_determine_mp_param {
my (
- $storage_cfg,
- $vmid,
- $archive,
- $api_mp_param,
- $orig_mp_param,
- $restore,
- $storage,
- $storage_only_mode,
+ $storage_cfg, $vmid, $archive, $api_mp_param, $orig_mp_param, $restore, $storage,
$is_root,
) = @_;
my $mp_param;
my $delayed_mp_param = {};
- if (!$storage_only_mode) {
+ if (scalar(keys $api_mp_param->%*)) {
$mp_param = $api_mp_param;
return ($mp_param, $delayed_mp_param);
}
if (!$restore) {
- $mp_param = $api_mp_param;
- $mp_param->{rootfs} = "$storage:4"; # defaults to 4GB
+ $mp_param = { rootfs => "$storage:4" }; # defaults to 4GB
return ($mp_param, $delayed_mp_param);
}
@@ -437,7 +429,6 @@ __PACKAGE__->register_method({
my $no_disk_param = {};
my $api_mp_param = {};
- my $storage_only_mode = 1;
foreach my $opt (keys %$param) {
my $value = $param->{$opt};
if ($opt eq 'rootfs' || $opt =~ m/^mp\d+$/) {
@@ -447,7 +438,6 @@ __PACKAGE__->register_method({
} else {
$api_mp_param->{$opt} = $value;
}
- $storage_only_mode = 0;
} elsif ($opt =~ m/^unused\d+$/) {
warn "ignoring '$opt', cannot create/restore with unused volume\n";
delete $param->{$opt};
@@ -457,7 +447,7 @@ __PACKAGE__->register_method({
}
die "mount points configured, but 'rootfs' not set - aborting\n"
- if !$storage_only_mode && !defined($api_mp_param->{rootfs});
+ if scalar(keys $api_mp_param->%*) && !defined($api_mp_param->{rootfs});
# check storage access, activate storage
PVE::LXC::Config->foreach_volume(
@@ -549,7 +539,6 @@ __PACKAGE__->register_method({
$orig_mp_param,
$restore,
$storage,
- $storage_only_mode,
$is_root,
);
--
2.47.3
_______________________________________________
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 container v3 7/8] api: restore: allow keeping not backed-up volumes
2026-01-23 14:34 [pve-devel] [PATCH-SERIES container/manager v3 0/8] api: restore: allow keeping not backed-up volumes Fiona Ebner
` (5 preceding siblings ...)
2026-01-23 14:34 ` [pve-devel] [PATCH container v3 6/8] api: create: get rid of $storage_only_mode variable Fiona Ebner
@ 2026-01-23 14:34 ` Fiona Ebner
2026-01-23 14:34 ` [pve-devel] [PATCH manager v3 8/8] ui: restore: enable safeguarding of mount point volumes by default Fiona Ebner
7 siblings, 0 replies; 9+ messages in thread
From: Fiona Ebner @ 2026-01-23 14:34 UTC (permalink / raw)
To: pve-devel
Same rationale as in pve-manager commit 5f855ccf ("ui: restore:
improve warning for restoring container with same ID"): it's
surprising to (new) users that all owned mount point volumes are
erased upon container restore, even those that are not currently
selected for backup. This is different from VM restore, where volumes
attached at drives not present in the backup will be kept around as
unused volumes.
Many users got tripped up by this over the years (e.g. [0][1][2]).
While the warning added by pve-manager commit 5f855ccf helps, fact is
that there are still new reports about lost data and thus very bad UX,
because of this behavior.
This patch adds an option to bring the behavior more in line with VM
restore. A container backup does not contain the detailed information
about which mount point volumes were included, so rely on the 'backup'
flag to determine which ones were included and which were not. Note
this is a bit more careful than VM restore, which only looks whether a
volume with the same key is included in the backup and does not also
consider the current 'backup' flag.
Remove snapshots from the kept volumes, there are no snapshots after
restore.
Note that this does not change the fact that mount point volumes
(according to the configuration contained in the backup) will be
allocated and thus more space is required in scenarios where some
volumes are kept.
The long term plan is to allow selecting actions for volumes
individually.
[0]: https://bugzilla.proxmox.com/show_bug.cgi?id=3783
[1]: https://forum.proxmox.com/threads/109707/post-745415
[2]: https://forum.proxmox.com/threads/111760/post-482045
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
Changes in v3:
* Rebase on master and then on refactorings.
* Don't preserve all when API mp/rootfs parameters are given, and
rather also check volumes present in the backup.
* Require 'restore' parameter to be present when
'restore-safeguard-mp-volumes' is specified.
* Add POD for restore_keep_non_backup_volumes()
* Slightly improve log messages.
src/PVE/API2/LXC.pm | 163 ++++++++++++++++++++++++++++++++++++++------
1 file changed, 144 insertions(+), 19 deletions(-)
diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index fc278a7..05704cc 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -15,6 +15,7 @@ use PVE::Firewall;
use PVE::GuestHelpers;
use PVE::INotify;
use PVE::JSONSchema qw(get_standard_option);
+use PVE::RESTEnvironment qw(log_warn);
use PVE::RESTHandler;
use PVE::RPCEnvironment;
use PVE::RRD;
@@ -69,6 +70,76 @@ my $check_storage_access_migrate = sub {
if !$scfg->{content}->{rootdir};
};
+=head3 restore_keep_non_backup_volumes
+
+ my $kept_volumes = restore_keep_non_backup_volumes(
+ $storage_cfg, $old_conf, $volumes_in_backup,
+ );
+
+Modifies C<$old_conf> dropping the config keys for volumes that are kept and returns an array
+reference with those volumes. The C<$volumes_in_backup> parameter is a hash reference with an entry
+for each mount point present in the backup.
+
+=cut
+
+my sub restore_keep_non_backup_volumes {
+ my ($storecfg, $old_conf, $volumes_in_backup) = @_;
+
+ my $kept_volumes = []; # an array to preserve the order
+ my $kept_volumes_hash = {}; # hash to simplify check for presence
+
+ PVE::LXC::Config->foreach_volume_full(
+ $old_conf,
+ { include_unused => 1 },
+ sub {
+ my ($ms, $mountpoint) = @_;
+
+ return if $mountpoint->{type} ne 'volume';
+
+ my ($keep, $reason);
+ # keep if either not previously backed up or not currently marked for backup
+ if (!$volumes_in_backup->{$ms}) {
+ ($keep, $reason) = (1, "not previously backed up");
+ } elsif (!PVE::LXC::Config->mountpoint_backup_enabled($ms, $mountpoint)) {
+ ($keep, $reason) = (1, "not currently backed up");
+ }
+
+ if ($keep) {
+ my $volid = $mountpoint->{volume};
+ $kept_volumes_hash->{$volid} = 1;
+ push $kept_volumes->@*, $volid;
+
+ delete $old_conf->{$ms};
+
+ my $description = "'$ms' ($volid";
+ $description .= ",mp=$mountpoint->{mp}" if $mountpoint->{mp};
+ $description .= ")";
+ print "NOTE: keeping $description as unused - $reason\n";
+ }
+ },
+ );
+
+ # after the restore, there are no snapshots anymore
+ for my $snapname (keys $old_conf->{snapshots}->%*) {
+ PVE::LXC::Config->foreach_volume(
+ $old_conf->{snapshots}->{$snapname},
+ sub {
+ my ($ms, $mountpoint) = @_;
+
+ my $volid = $mountpoint->{volume};
+
+ return if !$kept_volumes_hash->{$volid};
+
+ eval { PVE::Storage::volume_snapshot_delete($storecfg, $volid, $snapname); };
+ log_warn("unable to remove snapshot '$snapname' from kept volume '$volid' - $@")
+ if $@;
+ },
+ );
+ }
+
+ return $kept_volumes;
+}
+
__PACKAGE__->register_method({
subclass => "PVE::API2::LXC::Config",
path => '{vmid}/config',
@@ -135,6 +206,27 @@ __PACKAGE__->register_method({
},
});
+my sub create_ct_get_volumes_in_backup {
+ my ($orig_mp_param) = @_;
+
+ my $volumes_in_backup = {};
+
+ PVE::LXC::Config->foreach_volume(
+ $orig_mp_param,
+ sub {
+ my ($ms, $mountpoint) = @_;
+ my $type = $mountpoint->{type};
+ if ($type eq 'volume') {
+ if (PVE::LXC::Config->mountpoint_backup_enabled($ms, $mountpoint)) {
+ $volumes_in_backup->{$ms} = 1;
+ }
+ }
+ },
+ );
+
+ return $volumes_in_backup;
+}
+
my sub create_ct_determine_mp_param {
my (
$storage_cfg, $vmid, $archive, $api_mp_param, $orig_mp_param, $restore, $storage,
@@ -144,20 +236,24 @@ my sub create_ct_determine_mp_param {
my $mp_param;
my $delayed_mp_param = {};
- if (scalar(keys $api_mp_param->%*)) {
- $mp_param = $api_mp_param;
- return ($mp_param, $delayed_mp_param);
- }
-
if (!$restore) {
- $mp_param = { rootfs => "$storage:4" }; # defaults to 4GB
- return ($mp_param, $delayed_mp_param);
+ # default is 4 GiB rootfs on the specified storage
+ $mp_param = scalar(keys $api_mp_param->%*) ? $api_mp_param : { rootfs => "$storage:4" };
+ return ($mp_param, $delayed_mp_param, {});
}
if (!defined($orig_mp_param)) {
print "recovering backed-up configuration from '$archive'\n";
(undef, $orig_mp_param) = PVE::LXC::Create::recover_config($storage_cfg, $archive, $vmid);
}
+
+ my $volumes_in_backup = create_ct_get_volumes_in_backup($orig_mp_param);
+
+ if (scalar(keys $api_mp_param->%*)) {
+ $mp_param = $api_mp_param;
+ return ($mp_param, $delayed_mp_param, $volumes_in_backup);
+ }
+
$mp_param = $orig_mp_param;
die "rootfs configuration could not be recovered, please check and specify manually!\n"
if !defined($mp_param->{rootfs});
@@ -200,7 +296,7 @@ my sub create_ct_determine_mp_param {
},
);
- return ($mp_param, $delayed_mp_param);
+ return ($mp_param, $delayed_mp_param, $volumes_in_backup);
}
__PACKAGE__->register_method({
@@ -298,6 +394,16 @@ __PACKAGE__->register_method({
default => 0,
description => "Add the CT as a HA resource after it was created.",
},
+ 'restore-safeguard-mp-volumes' => {
+ optional => 1,
+ type => 'boolean',
+ requires => 'restore',
+ default => 0,
+ description =>
+ "Preserve mount point volumes that are not included in the backup or do not"
+ . " currently have the 'backup' flag set, keeping them as 'unused' entries in"
+ . " the configuration.",
+ },
}),
},
returns => {
@@ -317,6 +423,8 @@ __PACKAGE__->register_method({
my $bwlimit = extract_param($param, 'bwlimit');
my $start_after_create = extract_param($param, 'start');
my $ha_managed = extract_param($param, 'ha-managed');
+ my $restore_safeguard_mp_volumes =
+ extract_param($param, 'restore-safeguard-mp-volumes');
my $basecfg_fn = PVE::LXC::Config->config_file($vmid);
my $same_container_exists = -f $basecfg_fn;
@@ -487,6 +595,7 @@ __PACKAGE__->register_method({
my $was_template;
my $vollist = [];
+ my $kept_volumes = [];
eval {
my $orig_mp_param; # only used if $restore
if ($restore) {
@@ -531,16 +640,17 @@ __PACKAGE__->register_method({
}
}
- my ($mp_param, $delayed_mp_param) = create_ct_determine_mp_param(
- $storage_cfg,
- $vmid,
- $archive,
- $api_mp_param,
- $orig_mp_param,
- $restore,
- $storage,
- $is_root,
- );
+ my ($mp_param, $delayed_mp_param, $volumes_in_backup) =
+ create_ct_determine_mp_param(
+ $storage_cfg,
+ $vmid,
+ $archive,
+ $api_mp_param,
+ $orig_mp_param,
+ $restore,
+ $storage,
+ $is_root,
+ );
# up until here we did not modify the container, besides the lock
$destroy_config_on_error = 1;
@@ -549,7 +659,12 @@ __PACKAGE__->register_method({
# we always have the 'create' lock so check for more than 1 entry
if (scalar(keys %$old_conf) > 1) {
- # destroy old container volumes
+ # destroy old container volumes - keep not backed-up ones if requested
+ if ($restore_safeguard_mp_volumes) {
+ $kept_volumes = restore_keep_non_backup_volumes(
+ $storage_cfg, $old_conf, $volumes_in_backup,
+ );
+ }
PVE::LXC::destroy_lxc_container(
$storage_cfg,
$vmid,
@@ -619,6 +734,13 @@ __PACKAGE__->register_method({
foreach my $mp (keys %$delayed_mp_param) {
$conf->{$mp} = $delayed_mp_param->{$mp};
}
+
+ # register kept volumes as unused
+ for my $volid ($kept_volumes->@*) {
+ eval { PVE::LXC::Config->add_unused_volume($conf, $volid); };
+ log_warn("orphaned volume '$volid' - $@") if $@;
+ }
+
# If the template flag was set, we try to convert again to template after restore
if ($was_template) {
print STDERR "Convert restored container to template...\n";
@@ -632,6 +754,9 @@ __PACKAGE__->register_method({
warn $@ if $@;
PVE::LXC::destroy_disks($storage_cfg, $vollist);
if ($destroy_config_on_error) {
+ log_warn("orphaned volumes: " . join(',', $kept_volumes->@*))
+ if scalar($kept_volumes->@*) > 0;
+
eval { PVE::LXC::Config->destroy_config($vmid) };
warn $@ if $@;
--
2.47.3
_______________________________________________
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 manager v3 8/8] ui: restore: enable safeguarding of mount point volumes by default
2026-01-23 14:34 [pve-devel] [PATCH-SERIES container/manager v3 0/8] api: restore: allow keeping not backed-up volumes Fiona Ebner
` (6 preceding siblings ...)
2026-01-23 14:34 ` [pve-devel] [PATCH container v3 7/8] api: restore: allow keeping not backed-up volumes Fiona Ebner
@ 2026-01-23 14:34 ` Fiona Ebner
7 siblings, 0 replies; 9+ messages in thread
From: Fiona Ebner @ 2026-01-23 14:34 UTC (permalink / raw)
To: pve-devel
Same rationale as in pve-manager commit 5f855ccf ("ui: restore:
improve warning for restoring container with same ID"): it's
surprising to (new) users that all owned mount point volumes are
erased upon container restore, even those that are not currently
selected for backup. This is different from VM restore, where volumes
attached at drives not present in the backup will be kept around as
unused volumes.
Opt-in by default to the new option to change this behavior.
Remove the special message printed regarding removal of mount point
volumes to avoid confusion/complexity. With the checkbox for
safeguarding being present, attention is already directed towards this
behavior. And since the checkbox is enabled by default, one needs to
explicitly opt-out for not backed-up mount points. For backed-up mount
points, it should be clear that they are overwritten by the restore,
since the confirm dialog already states "This will permanently erase
current CT data".
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
Changes in v3:
* Slightly improve tooltip.
NOTE: dependency bump for pve-container is needed!
NOTE: better viewed as a word-based diff
www/manager6/window/Restore.js | 98 ++++++++++++++++++++--------------
1 file changed, 58 insertions(+), 40 deletions(-)
diff --git a/www/manager6/window/Restore.js b/www/manager6/window/Restore.js
index e8d8de4e..841a5db8 100644
--- a/www/manager6/window/Restore.js
+++ b/www/manager6/window/Restore.js
@@ -50,6 +50,9 @@ Ext.define('PVE.window.Restore', {
if (values.storage) {
params.storage = values.storage;
}
+ if (values['restore-safeguard-mp-volumes']) {
+ params['restore-safeguard-mp-volumes'] = 1;
+ }
['bwlimit', 'cores', 'name', 'memory', 'sockets'].forEach((opt) => {
if ((values[opt] ?? '') !== '') {
@@ -98,7 +101,6 @@ Ext.define('PVE.window.Restore', {
if (view.vmid) {
if (view.vmtype === 'lxc') {
confirmMsg += `. ${gettext('This will permanently erase current CT data.')}`;
- confirmMsg += `<br>${gettext('Mount point volumes are also erased.')}`;
} else {
confirmMsg += `. ${gettext('This will permanently erase current VM data.')}`;
}
@@ -256,47 +258,63 @@ Ext.define('PVE.window.Restore', {
];
if (me.vmtype === 'lxc') {
- items.push({
- xtype: 'radiogroup',
- fieldLabel: gettext('Privilege Level'),
- reference: 'noVNCScalingGroup',
- height: '15px', // renders faster with value assigned
- layout: {
- type: 'hbox',
- algin: 'stretch',
+ items.push(
+ {
+ xtype: 'radiogroup',
+ fieldLabel: gettext('Privilege Level'),
+ reference: 'noVNCScalingGroup',
+ height: '15px', // renders faster with value assigned
+ layout: {
+ type: 'hbox',
+ algin: 'stretch',
+ },
+ autoEl: {
+ tag: 'div',
+ 'data-qtip': gettext(
+ 'Choose if you want to keep or override the privilege level of the restored Container.',
+ ),
+ },
+ items: [
+ {
+ xtype: 'radiofield',
+ name: 'unprivileged',
+ inputValue: 'keep',
+ boxLabel: gettext('From Backup'),
+ flex: 1,
+ checked: true,
+ },
+ {
+ xtype: 'radiofield',
+ name: 'unprivileged',
+ inputValue: '1',
+ boxLabel: gettext('Unprivileged'),
+ flex: 1,
+ },
+ {
+ xtype: 'radiofield',
+ name: 'unprivileged',
+ inputValue: '0',
+ boxLabel: gettext('Privileged'),
+ flex: 1,
+ //margin: '0 0 0 10',
+ },
+ ],
},
- autoEl: {
- tag: 'div',
- 'data-qtip': gettext(
- 'Choose if you want to keep or override the privilege level of the restored Container.',
- ),
+ {
+ xtype: 'proxmoxcheckbox',
+ name: 'restore-safeguard-mp-volumes',
+ itemId: 'restoreSafeguardMpVolumes',
+ fieldLabel: gettext('Safeguard Mount Point Volumes'),
+ flex: 1,
+ checked: true,
+ autoEl: {
+ tag: 'div',
+ 'data-qtip': gettext(
+ "Preserve mount point volumes that are not included in the backup as 'Unused Disk' entries.",
+ ),
+ },
},
- items: [
- {
- xtype: 'radiofield',
- name: 'unprivileged',
- inputValue: 'keep',
- boxLabel: gettext('From Backup'),
- flex: 1,
- checked: true,
- },
- {
- xtype: 'radiofield',
- name: 'unprivileged',
- inputValue: '1',
- boxLabel: gettext('Unprivileged'),
- flex: 1,
- },
- {
- xtype: 'radiofield',
- name: 'unprivileged',
- inputValue: '0',
- boxLabel: gettext('Privileged'),
- flex: 1,
- //margin: '0 0 0 10',
- },
- ],
- });
+ );
} else if (me.vmtype === 'qemu') {
items.push(
{
--
2.47.3
_______________________________________________
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