* [pve-devel] [PATCH container] fix regression breaking container restore from PBS
@ 2025-11-17 11:30 Filip Schauer
2025-11-17 11:58 ` Christian Ebner
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Filip Schauer @ 2025-11-17 11:30 UTC (permalink / raw)
To: pve-devel
This fixes a regression caused by an abs_filesystem_path call introduced
with OCI image support. When trying to restore a container from a
Proxmox Backup Server, this tried to resolve a path on a PBS datastore
as a local file system path. This is fixed by only calling
abs_filesystem_path on path-based storages.
Fixes: 2aed26d320ae ("add support for OCI images as container
templates")
Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
src/PVE/API2/LXC.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index cffa17a..f74f19d 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -544,7 +544,7 @@ __PACKAGE__->register_method({
my $rootdir = PVE::LXC::mount_all($vmid, $storage_cfg, $conf, 1);
my $archivepath = '-';
$archivepath = PVE::Storage::abs_filesystem_path($storage_cfg, $archive)
- if ($archive ne '-');
+ if ($archive ne '-' && $storage_cfg->{path});
$bwlimit = PVE::Storage::get_bandwidth_limit(
'restore', [keys %used_storages], $bwlimit,
);
--
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] 7+ messages in thread* Re: [pve-devel] [PATCH container] fix regression breaking container restore from PBS
2025-11-17 11:30 [pve-devel] [PATCH container] fix regression breaking container restore from PBS Filip Schauer
@ 2025-11-17 11:58 ` Christian Ebner
2025-11-17 12:22 ` Filip Schauer
2025-11-17 13:54 ` [pve-devel] superseded: " Filip Schauer
2 siblings, 0 replies; 7+ messages in thread
From: Christian Ebner @ 2025-11-17 11:58 UTC (permalink / raw)
To: Proxmox VE development discussion, Filip Schauer
On 11/17/25 12:31 PM, Filip Schauer wrote:
> This fixes a regression caused by an abs_filesystem_path call introduced
> with OCI image support. When trying to restore a container from a
> Proxmox Backup Server, this tried to resolve a path on a PBS datastore
> as a local file system path. This is fixed by only calling
> abs_filesystem_path on path-based storages.
>
> Fixes: 2aed26d320ae ("add support for OCI images as container
> templates")
>
> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
Changes LGTM and fix the PBS restore, while not affecting tar based backups.
`path` is defined as storage config property for all storages supporting
content type `backup`.
Reviewed-by: Christian Ebner <c.ebner@proxmox.com>
Tested-by: Christian Ebner <c.ebner@proxmox.com>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [pve-devel] [PATCH container] fix regression breaking container restore from PBS
2025-11-17 11:30 [pve-devel] [PATCH container] fix regression breaking container restore from PBS Filip Schauer
2025-11-17 11:58 ` Christian Ebner
@ 2025-11-17 12:22 ` Filip Schauer
2025-11-17 13:54 ` Fabian Grünbichler
2025-11-17 13:54 ` [pve-devel] superseded: " Filip Schauer
2 siblings, 1 reply; 7+ messages in thread
From: Filip Schauer @ 2025-11-17 12:22 UTC (permalink / raw)
To: pve-devel
On 17/11/2025 12:31, Filip Schauer wrote:
> This fixes a regression caused by an abs_filesystem_path call introduced
> with OCI image support. When trying to restore a container from a
> Proxmox Backup Server, this tried to resolve a path on a PBS datastore
> as a local file system path. This is fixed by only calling
> abs_filesystem_path on path-based storages.
>
> Fixes: 2aed26d320ae ("add support for OCI images as container
> templates")
>
> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
> ---
> src/PVE/API2/LXC.pm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
> index cffa17a..f74f19d 100644
> --- a/src/PVE/API2/LXC.pm
> +++ b/src/PVE/API2/LXC.pm
> @@ -544,7 +544,7 @@ __PACKAGE__->register_method({
> my $rootdir = PVE::LXC::mount_all($vmid, $storage_cfg, $conf, 1);
> my $archivepath = '-';
> $archivepath = PVE::Storage::abs_filesystem_path($storage_cfg, $archive)
> - if ($archive ne '-');
> + if ($archive ne '-' && $storage_cfg->{path});
> $bwlimit = PVE::Storage::get_bandwidth_limit(
> 'restore', [keys %used_storages], $bwlimit,
> );
Unfortunatelly I only just noticed that this seems to break OCI images
for me.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [pve-devel] [PATCH container] fix regression breaking container restore from PBS
2025-11-17 12:22 ` Filip Schauer
@ 2025-11-17 13:54 ` Fabian Grünbichler
2025-11-17 18:40 ` Thomas Lamprecht
0 siblings, 1 reply; 7+ messages in thread
From: Fabian Grünbichler @ 2025-11-17 13:54 UTC (permalink / raw)
To: Proxmox VE development discussion
On November 17, 2025 1:22 pm, Filip Schauer wrote:
> On 17/11/2025 12:31, Filip Schauer wrote:
>> This fixes a regression caused by an abs_filesystem_path call introduced
>> with OCI image support. When trying to restore a container from a
>> Proxmox Backup Server, this tried to resolve a path on a PBS datastore
>> as a local file system path. This is fixed by only calling
>> abs_filesystem_path on path-based storages.
>>
>> Fixes: 2aed26d320ae ("add support for OCI images as container
>> templates")
>>
>> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
>> ---
>> src/PVE/API2/LXC.pm | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
>> index cffa17a..f74f19d 100644
>> --- a/src/PVE/API2/LXC.pm
>> +++ b/src/PVE/API2/LXC.pm
>> @@ -544,7 +544,7 @@ __PACKAGE__->register_method({
>> my $rootdir = PVE::LXC::mount_all($vmid, $storage_cfg, $conf, 1);
>> my $archivepath = '-';
>> $archivepath = PVE::Storage::abs_filesystem_path($storage_cfg, $archive)
>> - if ($archive ne '-');
>> + if ($archive ne '-' && $storage_cfg->{path});
>> $bwlimit = PVE::Storage::get_bandwidth_limit(
>> 'restore', [keys %used_storages], $bwlimit,
>> );
>
> Unfortunatelly I only just noticed that this seems to break OCI images
> for me.
$storage_cfg is not the section of the storage, but the full storage.cfg
..
but I think this whole section should be revamped..
we should have
PVE::LXC::archive_is_oci_format($storage_cfg, $archive)
and then we can just inline that and have a combined
PVE::LXC::restore_oci_archive($storage_cfg, $archive, $conf)
that combines the two current helpers (extract_oci_config and
merge_oci_conf_into_pct_conf) which are both only called once..
and we end up with
if ($restore && $archive ne '-') {
print "restoring '$archive' now..\n";
} elsif (PVE::LXC::archive_is_oci_format($storage_cfg, $archive) {
print "Detected OCI archive\n";
PVE::LXC::restore_oci_archive($storage_cfg, $archive, $conf);
} else {
# Not an OCI image, so restore it as an LXC image instead
PVE::LXC::Create::restore_archive(
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [pve-devel] [PATCH container] fix regression breaking container restore from PBS
2025-11-17 13:54 ` Fabian Grünbichler
@ 2025-11-17 18:40 ` Thomas Lamprecht
2025-11-18 9:07 ` Fabian Grünbichler
0 siblings, 1 reply; 7+ messages in thread
From: Thomas Lamprecht @ 2025-11-17 18:40 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Grünbichler
Am 17.11.25 um 14:53 schrieb Fabian Grünbichler:
> we should have
>
> PVE::LXC::archive_is_oci_format($storage_cfg, $archive)
>
> and then we can just inline that and have a combined
>
> PVE::LXC::restore_oci_archive($storage_cfg, $archive, $conf)
>
> that combines the two current helpers (extract_oci_config and
> merge_oci_conf_into_pct_conf) which are both only called once..
Ack.
> and we end up with
>
> if ($restore && $archive ne '-') {
> print "restoring '$archive' now..\n";
> } elsif (PVE::LXC::archive_is_oci_format($storage_cfg, $archive) {
> print "Detected OCI archive\n";
> PVE::LXC::restore_oci_archive($storage_cfg, $archive, $conf);
> } else {
> # Not an OCI image, so restore it as an LXC image instead
> PVE::LXC::Create::restore_archive(
>
This if-elsif-else chain is wrong though, i.e., the first one needs to be separate,
and can move in the else branch. I ended up with:
if (
!$restore # OCI image format is solely supported for fresh creation.
&& PVE::LXC::Create::archive_is_oci_format($storage_cfg, $archive)
) {
print "Detected OCI archive\n";
PVE::LXC::Create::restore_oci_archive(
$storage_cfg, $archive, $rootdir, $conf,
);
} else {
print "restoring '$archive' now..\n" if $restore && $archive ne '-';
....
I'd appreciate another look though!
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [pve-devel] [PATCH container] fix regression breaking container restore from PBS
2025-11-17 18:40 ` Thomas Lamprecht
@ 2025-11-18 9:07 ` Fabian Grünbichler
0 siblings, 0 replies; 7+ messages in thread
From: Fabian Grünbichler @ 2025-11-18 9:07 UTC (permalink / raw)
To: Proxmox VE development discussion, Thomas Lamprecht
On November 17, 2025 7:40 pm, Thomas Lamprecht wrote:
> Am 17.11.25 um 14:53 schrieb Fabian Grünbichler:
>> we should have
>>
>> PVE::LXC::archive_is_oci_format($storage_cfg, $archive)
>>
>> and then we can just inline that and have a combined
>>
>> PVE::LXC::restore_oci_archive($storage_cfg, $archive, $conf)
>>
>> that combines the two current helpers (extract_oci_config and
>> merge_oci_conf_into_pct_conf) which are both only called once..
>
> Ack.
>
>> and we end up with
>>
>> if ($restore && $archive ne '-') {
>> print "restoring '$archive' now..\n";
>> } elsif (PVE::LXC::archive_is_oci_format($storage_cfg, $archive) {
>> print "Detected OCI archive\n";
>> PVE::LXC::restore_oci_archive($storage_cfg, $archive, $conf);
>> } else {
>> # Not an OCI image, so restore it as an LXC image instead
>> PVE::LXC::Create::restore_archive(
>>
>
> This if-elsif-else chain is wrong though, i.e., the first one needs to be separate,
> and can move in the else branch. I ended up with:
>
> if (
> !$restore # OCI image format is solely supported for fresh creation.
> && PVE::LXC::Create::archive_is_oci_format($storage_cfg, $archive)
> ) {
> print "Detected OCI archive\n";
> PVE::LXC::Create::restore_oci_archive(
> $storage_cfg, $archive, $rootdir, $conf,
> );
> } else {
> print "restoring '$archive' now..\n" if $restore && $archive ne '-';
> ....
>
> I'd appreciate another look though!
Yes, Filip also noted that off-list yesterday, sorry!
changes look good to me.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pve-devel] superseded: [PATCH container] fix regression breaking container restore from PBS
2025-11-17 11:30 [pve-devel] [PATCH container] fix regression breaking container restore from PBS Filip Schauer
2025-11-17 11:58 ` Christian Ebner
2025-11-17 12:22 ` Filip Schauer
@ 2025-11-17 13:54 ` Filip Schauer
2 siblings, 0 replies; 7+ messages in thread
From: Filip Schauer @ 2025-11-17 13:54 UTC (permalink / raw)
To: pve-devel
On 17/11/2025 12:31, Filip Schauer wrote:
> This fixes a regression caused by an abs_filesystem_path call introduced
> with OCI image support. When trying to restore a container from a
> Proxmox Backup Server, this tried to resolve a path on a PBS datastore
> as a local file system path. This is fixed by only calling
> abs_filesystem_path on path-based storages.
>
> Fixes: 2aed26d320ae ("add support for OCI images as container
> templates")
>
> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
> ---
> src/PVE/API2/LXC.pm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
> index cffa17a..f74f19d 100644
> --- a/src/PVE/API2/LXC.pm
> +++ b/src/PVE/API2/LXC.pm
> @@ -544,7 +544,7 @@ __PACKAGE__->register_method({
> my $rootdir = PVE::LXC::mount_all($vmid, $storage_cfg, $conf, 1);
> my $archivepath = '-';
> $archivepath = PVE::Storage::abs_filesystem_path($storage_cfg, $archive)
> - if ($archive ne '-');
> + if ($archive ne '-' && $storage_cfg->{path});
> $bwlimit = PVE::Storage::get_bandwidth_limit(
> 'restore', [keys %used_storages], $bwlimit,
> );
Superseded by:
https://lore.proxmox.com/pve-devel/20251117135205.157724-1-f.schauer@proxmox.com/
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-11-18 9:07 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-17 11:30 [pve-devel] [PATCH container] fix regression breaking container restore from PBS Filip Schauer
2025-11-17 11:58 ` Christian Ebner
2025-11-17 12:22 ` Filip Schauer
2025-11-17 13:54 ` Fabian Grünbichler
2025-11-17 18:40 ` Thomas Lamprecht
2025-11-18 9:07 ` Fabian Grünbichler
2025-11-17 13:54 ` [pve-devel] superseded: " Filip Schauer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox