all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [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] 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

* 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

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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal