[pve-devel] [PATCH-SERIES pve-common/pmg-api/pve-storage] pbs client: rework namespace usage and minor fixes

[pve-devel] [PATCH-SERIES pve-common/pmg-api/pve-storage] pbs client: rework namespace usage and minor fixes

[Fabian Ebner]

Mostly done in preparation for #3186 (refactor pbs client use in PVE),
to avoid the need to manually set the namespace for all call-sites in
PVE, when it's already present in the storage/PBS config.

pve-common 1/5 and 2/5 and pmg-api 1/1 are improvements touching parts
of the same infrastructure, but not directly related.

The other patches change PBSClient to auto-select the namespace from
its initial configuration if not explicitly overriden with a
namespaced parameter and deprecate namespaced parameters as a whole.

Rationale is that essentially all current users of PBSClient are
configured for one namespace (there is the "status" call, which
doesn't depend on a namespace, but that doesn't contradict the
previous claim). It's less work on the call sites and there's no risk
to forget namespacing a parameter (as happened with pxar_restore in
PMG) if the PBSClient handles it itself.

If the need for handling more than one namespace with a single client
ever arises, we can still add e.g. a set_namespace() function to the
PBSClient.

Also makes it possible to restore a backup from a namespace in PMG,
which currently fails.


Dependency bump for new pve-common is needed for pve-storage and
pmg-api.


pve-common:

Fabian Ebner (5):
  pbs client: delete password: return success for non-existent file
  pbs client: forget snapshot: suppress output
  pbs client: default to configured namespace for non-namespaced
    parameters
  pbs client: deprecate namespaced parameters
  pbs client: backup fs tree: drop namespace parameter

 src/PVE/PBSClient.pm | 34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)


pmg-api:

Fabian Ebner (2):
  api: get group snapshots: take backup-id into account
  api: pbs: don't use namespaced parameters

 src/PMG/API2/PBS/Job.pm | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)


pve-storage:

Fabian Ebner (1):
  api: pbs: file restore: don't use namespaced parameters

 PVE/API2/Storage/FileRestore.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.30.2

[pmg-devel] [PATCH-SERIES pve-common/pmg-api/pve-storage] pbs client: rework namespace usage and minor fixes

[Fabian Ebner]

Mostly done in preparation for #3186 (refactor pbs client use in PVE),
to avoid the need to manually set the namespace for all call-sites in
PVE, when it's already present in the storage/PBS config.

pve-common 1/5 and 2/5 and pmg-api 1/1 are improvements touching parts
of the same infrastructure, but not directly related.

The other patches change PBSClient to auto-select the namespace from
its initial configuration if not explicitly overriden with a
namespaced parameter and deprecate namespaced parameters as a whole.

Rationale is that essentially all current users of PBSClient are
configured for one namespace (there is the "status" call, which
doesn't depend on a namespace, but that doesn't contradict the
previous claim). It's less work on the call sites and there's no risk
to forget namespacing a parameter (as happened with pxar_restore in
PMG) if the PBSClient handles it itself.

If the need for handling more than one namespace with a single client
ever arises, we can still add e.g. a set_namespace() function to the
PBSClient.

Also makes it possible to restore a backup from a namespace in PMG,
which currently fails.


Dependency bump for new pve-common is needed for pve-storage and
pmg-api.


pve-common:

Fabian Ebner (5):
  pbs client: delete password: return success for non-existent file
  pbs client: forget snapshot: suppress output
  pbs client: default to configured namespace for non-namespaced
    parameters
  pbs client: deprecate namespaced parameters
  pbs client: backup fs tree: drop namespace parameter

 src/PVE/PBSClient.pm | 34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)


pmg-api:

Fabian Ebner (2):
  api: get group snapshots: take backup-id into account
  api: pbs: don't use namespaced parameters

 src/PMG/API2/PBS/Job.pm | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)


pve-storage:

Fabian Ebner (1):
  api: pbs: file restore: don't use namespaced parameters

 PVE/API2/Storage/FileRestore.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH pve-common 1/5] pbs client: delete password: return success for non-existent file

[Fabian Ebner]

It's currently possible to add a remote in PMG without password (via
API), but deletion of such a remote would fail here.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/PVE/PBSClient.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/PBSClient.pm b/src/PVE/PBSClient.pm
index 37385d7..f19199c 100644
--- a/src/PVE/PBSClient.pm
+++ b/src/PVE/PBSClient.pm
@@ -77,7 +77,7 @@ sub delete_password {
 
     my $pwfile = password_file_name($self);
 
-    unlink $pwfile or die "deleting password file failed - $!\n";
+    unlink $pwfile or $! == ENOENT or die "deleting password file failed - $!\n";
 };
 
 sub get_password {
-- 
2.30.2

[pmg-devel] [PATCH pve-common 1/5] pbs client: delete password: return success for non-existent file

[Fabian Ebner]

It's currently possible to add a remote in PMG without password (via
API), but deletion of such a remote would fail here.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/PVE/PBSClient.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/PBSClient.pm b/src/PVE/PBSClient.pm
index 37385d7..f19199c 100644
--- a/src/PVE/PBSClient.pm
+++ b/src/PVE/PBSClient.pm
@@ -77,7 +77,7 @@ sub delete_password {
 
     my $pwfile = password_file_name($self);
 
-    unlink $pwfile or die "deleting password file failed - $!\n";
+    unlink $pwfile or $! == ENOENT or die "deleting password file failed - $!\n";
 };
 
 sub get_password {
-- 
2.30.2

[pve-devel] [PATCH pve-common 2/5] pbs client: forget snapshot: suppress output

[Fabian Ebner]

Otherwise, there will be
Result: {
      "data": null
}
when calling via a CLI tool for example. This also makes it consistent
with PVE in preparation to switch to using PBSClient there.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/PVE/PBSClient.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/PBSClient.pm b/src/PVE/PBSClient.pm
index f19199c..4b5b485 100644
--- a/src/PVE/PBSClient.pm
+++ b/src/PVE/PBSClient.pm
@@ -318,7 +318,7 @@ sub forget_snapshot {
 
     (my $namespace, $snapshot) = split_namespaced_parameter($snapshot);
 
-    return run_raw_client_cmd($self, 'forget', ["$snapshot"], namespace => $namespace);
+    return run_client_cmd($self, 'forget', ["$snapshot"], 1, undef, $namespace)
 };
 
 sub prune_group {
-- 
2.30.2

[pmg-devel] [PATCH pve-common 2/5] pbs client: forget snapshot: suppress output

[Fabian Ebner]

Otherwise, there will be
Result: {
      "data": null
}
when calling via a CLI tool for example. This also makes it consistent
with PVE in preparation to switch to using PBSClient there.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/PVE/PBSClient.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/PBSClient.pm b/src/PVE/PBSClient.pm
index f19199c..4b5b485 100644
--- a/src/PVE/PBSClient.pm
+++ b/src/PVE/PBSClient.pm
@@ -318,7 +318,7 @@ sub forget_snapshot {
 
     (my $namespace, $snapshot) = split_namespaced_parameter($snapshot);
 
-    return run_raw_client_cmd($self, 'forget', ["$snapshot"], namespace => $namespace);
+    return run_client_cmd($self, 'forget', ["$snapshot"], 1, undef, $namespace)
 };
 
 sub prune_group {
-- 
2.30.2

[pve-devel] [PATCH pve-common 3/5] pbs client: default to configured namespace for non-namespaced parameters

[Fabian Ebner]

For get_snapshots(), also set the default when no namespaced parameter
is present at all.

This would break any callers that have a namespace in the initial
config and explicitly don't set it for a later call, but the only
such caller is restore_pxar() in PMG, which /should/ be using the
namespace!

In other words, this implicitly fixes the restore_pxar() call in PMG
and avoids the need to extract the namespace from the configuration
(which already is present in the client) on the call site for all
functions that currently take a namespaced parameter.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/PVE/PBSClient.pm | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/src/PVE/PBSClient.pm b/src/PVE/PBSClient.pm
index 4b5b485..dfb8d76 100644
--- a/src/PVE/PBSClient.pm
+++ b/src/PVE/PBSClient.pm
@@ -242,11 +242,11 @@ sub autogen_encryption_key {
     return file_get_contents($encfile);
 };
 
-# Snapshot or group parameters can be either just a string and will then refer to the root
-# namespace, or a tuple of `[namespace, snapshot]`.
-my sub split_namespaced_parameter : prototype($) {
-    my ($snapshot) = @_;
-    return (undef, $snapshot) if !ref($snapshot);
+# Snapshot or group parameters can be either just a string and will then default to the namespace
+# that's part of the initial configuration in new(), or a tuple of `[namespace, snapshot]`.
+my sub split_namespaced_parameter : prototype($$) {
+    my ($self, $snapshot) = @_;
+    return ($self->{scfg}->{namespace}, $snapshot) if !ref($snapshot);
 
     (my $namespace, $snapshot) = @$snapshot;
     return ($namespace, $snapshot);
@@ -258,7 +258,9 @@ sub get_snapshots {
 
     my $namespace;
     if (defined($group)) {
-	($namespace, $group) = split_namespaced_parameter($group);
+	($namespace, $group) = split_namespaced_parameter($self, $group);
+    } else {
+	$namespace = $self->{scfg}->{namespace};
     }
 
     my $param = [];
@@ -296,7 +298,7 @@ sub restore_pxar {
     die "archive name not provided\n" if !defined($pxarname);
     die "restore-target not provided\n" if !defined($target);
 
-    (my $namespace, $snapshot) = split_namespaced_parameter($snapshot);
+    (my $namespace, $snapshot) = split_namespaced_parameter($self, $snapshot);
 
     my $param = [
 	"$snapshot",
@@ -316,7 +318,7 @@ sub forget_snapshot {
 
     die "snapshot not provided\n" if !defined($snapshot);
 
-    (my $namespace, $snapshot) = split_namespaced_parameter($snapshot);
+    (my $namespace, $snapshot) = split_namespaced_parameter($self, $snapshot);
 
     return run_client_cmd($self, 'forget', ["$snapshot"], 1, undef, $namespace)
 };
@@ -326,7 +328,7 @@ sub prune_group {
 
     die "group not provided\n" if !defined($group);
 
-    (my $namespace, $group) = split_namespaced_parameter($group);
+    (my $namespace, $group) = split_namespaced_parameter($self, $group);
 
     # do nothing if no keep options specified for remote
     return [] if scalar(keys %$prune_opts) == 0;
@@ -373,7 +375,7 @@ sub status {
 sub file_restore_list {
     my ($self, $snapshot, $filepath, $base64) = @_;
 
-    (my $namespace, $snapshot) = split_namespaced_parameter($snapshot);
+    (my $namespace, $snapshot) = split_namespaced_parameter($self, $snapshot);
 
     return run_client_cmd(
 	$self,
@@ -409,7 +411,7 @@ sub file_restore_extract_prepare {
 sub file_restore_extract {
     my ($self, $output_file, $snapshot, $filepath, $base64) = @_;
 
-    (my $namespace, $snapshot) = split_namespaced_parameter($snapshot);
+    (my $namespace, $snapshot) = split_namespaced_parameter($self, $snapshot);
 
     my $ret = eval {
 	local $SIG{ALRM} = sub { die "got timeout\n" };
-- 
2.30.2

[pmg-devel] [PATCH pve-common 3/5] pbs client: default to configured namespace for non-namespaced parameters

[Fabian Ebner]

For get_snapshots(), also set the default when no namespaced parameter
is present at all.

This would break any callers that have a namespace in the initial
config and explicitly don't set it for a later call, but the only
such caller is restore_pxar() in PMG, which /should/ be using the
namespace!

In other words, this implicitly fixes the restore_pxar() call in PMG
and avoids the need to extract the namespace from the configuration
(which already is present in the client) on the call site for all
functions that currently take a namespaced parameter.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/PVE/PBSClient.pm | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/src/PVE/PBSClient.pm b/src/PVE/PBSClient.pm
index 4b5b485..dfb8d76 100644
--- a/src/PVE/PBSClient.pm
+++ b/src/PVE/PBSClient.pm
@@ -242,11 +242,11 @@ sub autogen_encryption_key {
     return file_get_contents($encfile);
 };
 
-# Snapshot or group parameters can be either just a string and will then refer to the root
-# namespace, or a tuple of `[namespace, snapshot]`.
-my sub split_namespaced_parameter : prototype($) {
-    my ($snapshot) = @_;
-    return (undef, $snapshot) if !ref($snapshot);
+# Snapshot or group parameters can be either just a string and will then default to the namespace
+# that's part of the initial configuration in new(), or a tuple of `[namespace, snapshot]`.
+my sub split_namespaced_parameter : prototype($$) {
+    my ($self, $snapshot) = @_;
+    return ($self->{scfg}->{namespace}, $snapshot) if !ref($snapshot);
 
     (my $namespace, $snapshot) = @$snapshot;
     return ($namespace, $snapshot);
@@ -258,7 +258,9 @@ sub get_snapshots {
 
     my $namespace;
     if (defined($group)) {
-	($namespace, $group) = split_namespaced_parameter($group);
+	($namespace, $group) = split_namespaced_parameter($self, $group);
+    } else {
+	$namespace = $self->{scfg}->{namespace};
     }
 
     my $param = [];
@@ -296,7 +298,7 @@ sub restore_pxar {
     die "archive name not provided\n" if !defined($pxarname);
     die "restore-target not provided\n" if !defined($target);
 
-    (my $namespace, $snapshot) = split_namespaced_parameter($snapshot);
+    (my $namespace, $snapshot) = split_namespaced_parameter($self, $snapshot);
 
     my $param = [
 	"$snapshot",
@@ -316,7 +318,7 @@ sub forget_snapshot {
 
     die "snapshot not provided\n" if !defined($snapshot);
 
-    (my $namespace, $snapshot) = split_namespaced_parameter($snapshot);
+    (my $namespace, $snapshot) = split_namespaced_parameter($self, $snapshot);
 
     return run_client_cmd($self, 'forget', ["$snapshot"], 1, undef, $namespace)
 };
@@ -326,7 +328,7 @@ sub prune_group {
 
     die "group not provided\n" if !defined($group);
 
-    (my $namespace, $group) = split_namespaced_parameter($group);
+    (my $namespace, $group) = split_namespaced_parameter($self, $group);
 
     # do nothing if no keep options specified for remote
     return [] if scalar(keys %$prune_opts) == 0;
@@ -373,7 +375,7 @@ sub status {
 sub file_restore_list {
     my ($self, $snapshot, $filepath, $base64) = @_;
 
-    (my $namespace, $snapshot) = split_namespaced_parameter($snapshot);
+    (my $namespace, $snapshot) = split_namespaced_parameter($self, $snapshot);
 
     return run_client_cmd(
 	$self,
@@ -409,7 +411,7 @@ sub file_restore_extract_prepare {
 sub file_restore_extract {
     my ($self, $output_file, $snapshot, $filepath, $base64) = @_;
 
-    (my $namespace, $snapshot) = split_namespaced_parameter($snapshot);
+    (my $namespace, $snapshot) = split_namespaced_parameter($self, $snapshot);
 
     my $ret = eval {
 	local $SIG{ALRM} = sub { die "got timeout\n" };
-- 
2.30.2

Re: [pve-devel] [pmg-devel] [PATCH pve-common 3/5] pbs client: default to configured namespace for non-namespaced parameters

[Fiona Ebner]

Am 20.07.22 um 12:59 schrieb Fabian Ebner:
> For get_snapshots(), also set the default when no namespaced parameter
> is present at all.
> 
> This would break any callers that have a namespace in the initial
> config and explicitly don't set it for a later call, but the only
> such caller is restore_pxar() in PMG, which /should/ be using the
> namespace!
> 
> In other words, this implicitly fixes the restore_pxar() call in PMG
> and avoids the need to extract the namespace from the configuration
> (which already is present in the client) on the call site for all
> functions that currently take a namespaced parameter.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>

Ping. I feel like making restoring backups from namespaces work would be
nice to have for the next PMG release (even if there are no actual user
reports about the issue yet AFAIK).

Re: [pmg-devel] [PATCH pve-common 3/5] pbs client: default to configured namespace for non-namespaced parameters

[Fiona Ebner]

Am 20.07.22 um 12:59 schrieb Fabian Ebner:
> For get_snapshots(), also set the default when no namespaced parameter
> is present at all.
> 
> This would break any callers that have a namespace in the initial
> config and explicitly don't set it for a later call, but the only
> such caller is restore_pxar() in PMG, which /should/ be using the
> namespace!
> 
> In other words, this implicitly fixes the restore_pxar() call in PMG
> and avoids the need to extract the namespace from the configuration
> (which already is present in the client) on the call site for all
> functions that currently take a namespaced parameter.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>

Ping. I feel like making restoring backups from namespaces work would be
nice to have for the next PMG release (even if there are no actual user
reports about the issue yet AFAIK).

[pve-devel] [RFC/PATCH pve-common 4/5] pbs client: deprecate namespaced parameters

[Fabian Ebner]

All existing callers for functions with namespaced parameters just
re-use the one that's passed in via the initial configuration already,
so there is no need for namespaced parameters currently.

If the need for one PBS client to handle multiple namespaces arises, a
set_namespace() function could be added, or the relevant functions
could take an additional parameter, either for just the namespace or
like $cmd_opts in restore_pxar().

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/PVE/PBSClient.pm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/PVE/PBSClient.pm b/src/PVE/PBSClient.pm
index dfb8d76..34d3f67 100644
--- a/src/PVE/PBSClient.pm
+++ b/src/PVE/PBSClient.pm
@@ -242,6 +242,8 @@ sub autogen_encryption_key {
     return file_get_contents($encfile);
 };
 
+# TODO remove support for namespaced parameters. Needs Breaks for pmg-api and libpve-storage-perl.
+# Deprecated! The namespace should be passed in as part of the config in new().
 # Snapshot or group parameters can be either just a string and will then default to the namespace
 # that's part of the initial configuration in new(), or a tuple of `[namespace, snapshot]`.
 my sub split_namespaced_parameter : prototype($$) {
-- 
2.30.2

[pmg-devel] [RFC/PATCH pve-common 4/5] pbs client: deprecate namespaced parameters

[Fabian Ebner]

All existing callers for functions with namespaced parameters just
re-use the one that's passed in via the initial configuration already,
so there is no need for namespaced parameters currently.

If the need for one PBS client to handle multiple namespaces arises, a
set_namespace() function could be added, or the relevant functions
could take an additional parameter, either for just the namespace or
like $cmd_opts in restore_pxar().

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/PVE/PBSClient.pm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/PVE/PBSClient.pm b/src/PVE/PBSClient.pm
index dfb8d76..34d3f67 100644
--- a/src/PVE/PBSClient.pm
+++ b/src/PVE/PBSClient.pm
@@ -242,6 +242,8 @@ sub autogen_encryption_key {
     return file_get_contents($encfile);
 };
 
+# TODO remove support for namespaced parameters. Needs Breaks for pmg-api and libpve-storage-perl.
+# Deprecated! The namespace should be passed in as part of the config in new().
 # Snapshot or group parameters can be either just a string and will then default to the namespace
 # that's part of the initial configuration in new(), or a tuple of `[namespace, snapshot]`.
 my sub split_namespaced_parameter : prototype($$) {
-- 
2.30.2

[pve-devel] [RFC/PATCH pve-common 5/5] pbs client: backup fs tree: drop namespace parameter

[Fabian Ebner]

Instead, use the one from the initial configuration. The only current
caller is in PMG and the namespace parameter set there agrees with
the one from the initial configuration, so this is not actually a
breaking change.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/PVE/PBSClient.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/PVE/PBSClient.pm b/src/PVE/PBSClient.pm
index 34d3f67..d7dd6e1 100644
--- a/src/PVE/PBSClient.pm
+++ b/src/PVE/PBSClient.pm
@@ -274,7 +274,7 @@ sub get_snapshots {
 # create a new PXAR backup of a FS directory tree - doesn't cross FS boundary
 # by default.
 sub backup_fs_tree {
-    my ($self, $root, $id, $pxarname, $cmd_opts, $namespace) = @_;
+    my ($self, $root, $id, $pxarname, $cmd_opts) = @_;
 
     die "backup-id not provided\n" if !defined($id);
     die "backup root dir not provided\n" if !defined($root);
@@ -288,7 +288,7 @@ sub backup_fs_tree {
 
     $cmd_opts //= {};
 
-    $cmd_opts->{namespace} = $namespace if defined($namespace);
+    $cmd_opts->{namespace} = $self->{scfg}->{namespace} if defined($self->{scfg}->{namespace});
 
     return run_raw_client_cmd($self, 'backup', $param, %$cmd_opts);
 };
-- 
2.30.2

[pmg-devel] [RFC/PATCH pve-common 5/5] pbs client: backup fs tree: drop namespace parameter

[Fabian Ebner]

Instead, use the one from the initial configuration. The only current
caller is in PMG and the namespace parameter set there agrees with
the one from the initial configuration, so this is not actually a
breaking change.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/PVE/PBSClient.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/PVE/PBSClient.pm b/src/PVE/PBSClient.pm
index 34d3f67..d7dd6e1 100644
--- a/src/PVE/PBSClient.pm
+++ b/src/PVE/PBSClient.pm
@@ -274,7 +274,7 @@ sub get_snapshots {
 # create a new PXAR backup of a FS directory tree - doesn't cross FS boundary
 # by default.
 sub backup_fs_tree {
-    my ($self, $root, $id, $pxarname, $cmd_opts, $namespace) = @_;
+    my ($self, $root, $id, $pxarname, $cmd_opts) = @_;
 
     die "backup-id not provided\n" if !defined($id);
     die "backup root dir not provided\n" if !defined($root);
@@ -288,7 +288,7 @@ sub backup_fs_tree {
 
     $cmd_opts //= {};
 
-    $cmd_opts->{namespace} = $namespace if defined($namespace);
+    $cmd_opts->{namespace} = $self->{scfg}->{namespace} if defined($self->{scfg}->{namespace});
 
     return run_raw_client_cmd($self, 'backup', $param, %$cmd_opts);
 };
-- 
2.30.2

Re: [pve-devel] [RFC/PATCH pve-common 5/5] pbs client: backup fs tree: drop namespace parameter

[Wolfgang Bumiller]

On Wed, Jul 20, 2022 at 12:59:45PM +0200, Fabian Ebner wrote:
> Instead, use the one from the initial configuration. The only current
> caller is in PMG and the namespace parameter set there agrees with
> the one from the initial configuration, so this is not actually a
> breaking change.

Still technically a breaking change ;-)

> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>  src/PVE/PBSClient.pm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/PVE/PBSClient.pm b/src/PVE/PBSClient.pm
> index 34d3f67..d7dd6e1 100644
> --- a/src/PVE/PBSClient.pm
> +++ b/src/PVE/PBSClient.pm
> @@ -274,7 +274,7 @@ sub get_snapshots {
>  # create a new PXAR backup of a FS directory tree - doesn't cross FS boundary
>  # by default.
>  sub backup_fs_tree {
> -    my ($self, $root, $id, $pxarname, $cmd_opts, $namespace) = @_;
> +    my ($self, $root, $id, $pxarname, $cmd_opts) = @_;

And in theory the namespace *could* be overwritable via `$cmd_opts` (if
we used `//=` below.

But even better yet, since it's not used anywhere, maybe we should just
drop `$cmd_opts` entirely?

>  
>      die "backup-id not provided\n" if !defined($id);
>      die "backup root dir not provided\n" if !defined($root);
> @@ -288,7 +288,7 @@ sub backup_fs_tree {
>  
>      $cmd_opts //= {};
>  
> -    $cmd_opts->{namespace} = $namespace if defined($namespace);
> +    $cmd_opts->{namespace} = $self->{scfg}->{namespace} if defined($self->{scfg}->{namespace});
>  
>      return run_raw_client_cmd($self, 'backup', $param, %$cmd_opts);
>  };
> -- 
> 2.30.2

Re: [pmg-devel] [pve-devel] [RFC/PATCH pve-common 5/5] pbs client: backup fs tree: drop namespace parameter

[Wolfgang Bumiller]

On Wed, Jul 20, 2022 at 12:59:45PM +0200, Fabian Ebner wrote:
> Instead, use the one from the initial configuration. The only current
> caller is in PMG and the namespace parameter set there agrees with
> the one from the initial configuration, so this is not actually a
> breaking change.

Still technically a breaking change ;-)

> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>  src/PVE/PBSClient.pm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/PVE/PBSClient.pm b/src/PVE/PBSClient.pm
> index 34d3f67..d7dd6e1 100644
> --- a/src/PVE/PBSClient.pm
> +++ b/src/PVE/PBSClient.pm
> @@ -274,7 +274,7 @@ sub get_snapshots {
>  # create a new PXAR backup of a FS directory tree - doesn't cross FS boundary
>  # by default.
>  sub backup_fs_tree {
> -    my ($self, $root, $id, $pxarname, $cmd_opts, $namespace) = @_;
> +    my ($self, $root, $id, $pxarname, $cmd_opts) = @_;

And in theory the namespace *could* be overwritable via `$cmd_opts` (if
we used `//=` below.

But even better yet, since it's not used anywhere, maybe we should just
drop `$cmd_opts` entirely?

>  
>      die "backup-id not provided\n" if !defined($id);
>      die "backup root dir not provided\n" if !defined($root);
> @@ -288,7 +288,7 @@ sub backup_fs_tree {
>  
>      $cmd_opts //= {};
>  
> -    $cmd_opts->{namespace} = $namespace if defined($namespace);
> +    $cmd_opts->{namespace} = $self->{scfg}->{namespace} if defined($self->{scfg}->{namespace});
>  
>      return run_raw_client_cmd($self, 'backup', $param, %$cmd_opts);
>  };
> -- 
> 2.30.2

Re: [pve-devel] [RFC/PATCH pve-common 5/5] pbs client: backup fs tree: drop namespace parameter

[Fiona Ebner]

Am 22.07.22 um 12:53 schrieb Wolfgang Bumiller:
> On Wed, Jul 20, 2022 at 12:59:45PM +0200, Fabian Ebner wrote:
>> Instead, use the one from the initial configuration. The only current
>> caller is in PMG and the namespace parameter set there agrees with
>> the one from the initial configuration, so this is not actually a
>> breaking change.
> 
> Still technically a breaking change ;-)
> 

Yeah, should've written "not a breaking change in practice" instead.

>>
>> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
>> ---
>>  src/PVE/PBSClient.pm | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/PVE/PBSClient.pm b/src/PVE/PBSClient.pm
>> index 34d3f67..d7dd6e1 100644
>> --- a/src/PVE/PBSClient.pm
>> +++ b/src/PVE/PBSClient.pm
>> @@ -274,7 +274,7 @@ sub get_snapshots {
>>  # create a new PXAR backup of a FS directory tree - doesn't cross FS boundary
>>  # by default.
>>  sub backup_fs_tree {
>> -    my ($self, $root, $id, $pxarname, $cmd_opts, $namespace) = @_;
>> +    my ($self, $root, $id, $pxarname, $cmd_opts) = @_;
> 
> And in theory the namespace *could* be overwritable via `$cmd_opts` (if
> we used `//=` below.
> 
> But even better yet, since it's not used anywhere, maybe we should just
> drop `$cmd_opts` entirely?
> 

Fine by me. If we need more flexibility in the future, we can still add
it back, maybe only exposing specific options rather than everything
run_raw_client_cmd can take.

>>  
>>      die "backup-id not provided\n" if !defined($id);
>>      die "backup root dir not provided\n" if !defined($root);
>> @@ -288,7 +288,7 @@ sub backup_fs_tree {
>>  
>>      $cmd_opts //= {};
>>  
>> -    $cmd_opts->{namespace} = $namespace if defined($namespace);
>> +    $cmd_opts->{namespace} = $self->{scfg}->{namespace} if defined($self->{scfg}->{namespace});
>>  
>>      return run_raw_client_cmd($self, 'backup', $param, %$cmd_opts);
>>  };
>> -- 
>> 2.30.2

Re: [pmg-devel] [pve-devel] [RFC/PATCH pve-common 5/5] pbs client: backup fs tree: drop namespace parameter

[Fiona Ebner]

Am 22.07.22 um 12:53 schrieb Wolfgang Bumiller:
> On Wed, Jul 20, 2022 at 12:59:45PM +0200, Fabian Ebner wrote:
>> Instead, use the one from the initial configuration. The only current
>> caller is in PMG and the namespace parameter set there agrees with
>> the one from the initial configuration, so this is not actually a
>> breaking change.
> 
> Still technically a breaking change ;-)
> 

Yeah, should've written "not a breaking change in practice" instead.

>>
>> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
>> ---
>>  src/PVE/PBSClient.pm | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/PVE/PBSClient.pm b/src/PVE/PBSClient.pm
>> index 34d3f67..d7dd6e1 100644
>> --- a/src/PVE/PBSClient.pm
>> +++ b/src/PVE/PBSClient.pm
>> @@ -274,7 +274,7 @@ sub get_snapshots {
>>  # create a new PXAR backup of a FS directory tree - doesn't cross FS boundary
>>  # by default.
>>  sub backup_fs_tree {
>> -    my ($self, $root, $id, $pxarname, $cmd_opts, $namespace) = @_;
>> +    my ($self, $root, $id, $pxarname, $cmd_opts) = @_;
> 
> And in theory the namespace *could* be overwritable via `$cmd_opts` (if
> we used `//=` below.
> 
> But even better yet, since it's not used anywhere, maybe we should just
> drop `$cmd_opts` entirely?
> 

Fine by me. If we need more flexibility in the future, we can still add
it back, maybe only exposing specific options rather than everything
run_raw_client_cmd can take.

>>  
>>      die "backup-id not provided\n" if !defined($id);
>>      die "backup root dir not provided\n" if !defined($root);
>> @@ -288,7 +288,7 @@ sub backup_fs_tree {
>>  
>>      $cmd_opts //= {};
>>  
>> -    $cmd_opts->{namespace} = $namespace if defined($namespace);
>> +    $cmd_opts->{namespace} = $self->{scfg}->{namespace} if defined($self->{scfg}->{namespace});
>>  
>>      return run_raw_client_cmd($self, 'backup', $param, %$cmd_opts);
>>  };
>> -- 
>> 2.30.2

[pve-devel] applied-series: [RFC/PATCH pve-common 5/5] pbs client: backup fs tree: drop namespace parameter

[Wolfgang Bumiller]

applied common series as is for now & bumped so we can depend on it

the nits can be addressed when dealing with the deprecation

On Wed, Jul 20, 2022 at 12:59:45PM +0200, Fabian Ebner wrote:
> Instead, use the one from the initial configuration. The only current
> caller is in PMG and the namespace parameter set there agrees with
> the one from the initial configuration, so this is not actually a
> breaking change.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>  src/PVE/PBSClient.pm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/PVE/PBSClient.pm b/src/PVE/PBSClient.pm
> index 34d3f67..d7dd6e1 100644
> --- a/src/PVE/PBSClient.pm
> +++ b/src/PVE/PBSClient.pm
> @@ -274,7 +274,7 @@ sub get_snapshots {
>  # create a new PXAR backup of a FS directory tree - doesn't cross FS boundary
>  # by default.
>  sub backup_fs_tree {
> -    my ($self, $root, $id, $pxarname, $cmd_opts, $namespace) = @_;
> +    my ($self, $root, $id, $pxarname, $cmd_opts) = @_;
>  
>      die "backup-id not provided\n" if !defined($id);
>      die "backup root dir not provided\n" if !defined($root);
> @@ -288,7 +288,7 @@ sub backup_fs_tree {
>  
>      $cmd_opts //= {};
>  
> -    $cmd_opts->{namespace} = $namespace if defined($namespace);
> +    $cmd_opts->{namespace} = $self->{scfg}->{namespace} if defined($self->{scfg}->{namespace});
>  
>      return run_raw_client_cmd($self, 'backup', $param, %$cmd_opts);
>  };
> -- 
> 2.30.2

[pmg-devel] applied-series: [RFC/PATCH pve-common 5/5] pbs client: backup fs tree: drop namespace parameter

[Wolfgang Bumiller]

applied common series as is for now & bumped so we can depend on it

the nits can be addressed when dealing with the deprecation

On Wed, Jul 20, 2022 at 12:59:45PM +0200, Fabian Ebner wrote:
> Instead, use the one from the initial configuration. The only current
> caller is in PMG and the namespace parameter set there agrees with
> the one from the initial configuration, so this is not actually a
> breaking change.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>  src/PVE/PBSClient.pm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/PVE/PBSClient.pm b/src/PVE/PBSClient.pm
> index 34d3f67..d7dd6e1 100644
> --- a/src/PVE/PBSClient.pm
> +++ b/src/PVE/PBSClient.pm
> @@ -274,7 +274,7 @@ sub get_snapshots {
>  # create a new PXAR backup of a FS directory tree - doesn't cross FS boundary
>  # by default.
>  sub backup_fs_tree {
> -    my ($self, $root, $id, $pxarname, $cmd_opts, $namespace) = @_;
> +    my ($self, $root, $id, $pxarname, $cmd_opts) = @_;
>  
>      die "backup-id not provided\n" if !defined($id);
>      die "backup root dir not provided\n" if !defined($root);
> @@ -288,7 +288,7 @@ sub backup_fs_tree {
>  
>      $cmd_opts //= {};
>  
> -    $cmd_opts->{namespace} = $namespace if defined($namespace);
> +    $cmd_opts->{namespace} = $self->{scfg}->{namespace} if defined($self->{scfg}->{namespace});
>  
>      return run_raw_client_cmd($self, 'backup', $param, %$cmd_opts);
>  };
> -- 
> 2.30.2

[pve-devel] [PATCH pmg-api 1/2] api: get group snapshots: take backup-id into account

[Fabian Ebner]

instead of assuming that the requested ID is the same as the node in
the API path. This endpoint is not currently used in the UI AFAICS.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/PMG/API2/PBS/Job.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PMG/API2/PBS/Job.pm b/src/PMG/API2/PBS/Job.pm
index 3f775d6..be9cc06 100644
--- a/src/PMG/API2/PBS/Job.pm
+++ b/src/PMG/API2/PBS/Job.pm
@@ -209,7 +209,7 @@ __PACKAGE__->register_method ({
     code => sub {
 	my ($param) = @_;
 
-	return get_snapshots($param->{remote}, "host/$param->{node}");
+	return get_snapshots($param->{remote}, "host/$param->{'backup-id'}");
     }});
 
 __PACKAGE__->register_method ({
-- 
2.30.2

[pmg-devel] [PATCH pmg-api 1/2] api: get group snapshots: take backup-id into account

[Fabian Ebner]

instead of assuming that the requested ID is the same as the node in
the API path. This endpoint is not currently used in the UI AFAICS.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/PMG/API2/PBS/Job.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PMG/API2/PBS/Job.pm b/src/PMG/API2/PBS/Job.pm
index 3f775d6..be9cc06 100644
--- a/src/PMG/API2/PBS/Job.pm
+++ b/src/PMG/API2/PBS/Job.pm
@@ -209,7 +209,7 @@ __PACKAGE__->register_method ({
     code => sub {
 	my ($param) = @_;
 
-	return get_snapshots($param->{remote}, "host/$param->{node}");
+	return get_snapshots($param->{remote}, "host/$param->{'backup-id'}");
     }});
 
 __PACKAGE__->register_method ({
-- 
2.30.2

[pve-devel] applied-series: [PATCH pmg-api 1/2] api: get group snapshots: take backup-id into account

[Wolfgang Bumiller]

applied & bumped common dep accordingly

On Wed, Jul 20, 2022 at 12:59:46PM +0200, Fabian Ebner wrote:
> instead of assuming that the requested ID is the same as the node in
> the API path. This endpoint is not currently used in the UI AFAICS.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>  src/PMG/API2/PBS/Job.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/PMG/API2/PBS/Job.pm b/src/PMG/API2/PBS/Job.pm
> index 3f775d6..be9cc06 100644
> --- a/src/PMG/API2/PBS/Job.pm
> +++ b/src/PMG/API2/PBS/Job.pm
> @@ -209,7 +209,7 @@ __PACKAGE__->register_method ({
>      code => sub {
>  	my ($param) = @_;
>  
> -	return get_snapshots($param->{remote}, "host/$param->{node}");
> +	return get_snapshots($param->{remote}, "host/$param->{'backup-id'}");
>      }});
>  
>  __PACKAGE__->register_method ({
> -- 
> 2.30.2

[pmg-devel] applied-series: [PATCH pmg-api 1/2] api: get group snapshots: take backup-id into account

[Wolfgang Bumiller]

applied & bumped common dep accordingly

On Wed, Jul 20, 2022 at 12:59:46PM +0200, Fabian Ebner wrote:
> instead of assuming that the requested ID is the same as the node in
> the API path. This endpoint is not currently used in the UI AFAICS.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>  src/PMG/API2/PBS/Job.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/PMG/API2/PBS/Job.pm b/src/PMG/API2/PBS/Job.pm
> index 3f775d6..be9cc06 100644
> --- a/src/PMG/API2/PBS/Job.pm
> +++ b/src/PMG/API2/PBS/Job.pm
> @@ -209,7 +209,7 @@ __PACKAGE__->register_method ({
>      code => sub {
>  	my ($param) = @_;
>  
> -	return get_snapshots($param->{remote}, "host/$param->{node}");
> +	return get_snapshots($param->{remote}, "host/$param->{'backup-id'}");
>      }});
>  
>  __PACKAGE__->register_method ({
> -- 
> 2.30.2

[pve-devel] [RFC/PATCH pmg-api 2/2] api: pbs: don't use namespaced parameters

[Fabian Ebner]

Instead, rely on PBSClient to set namespace according to the initial
configuration.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Dependency bump for new libpve-common-perl needed.

 src/PMG/API2/PBS/Job.pm | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/src/PMG/API2/PBS/Job.pm b/src/PMG/API2/PBS/Job.pm
index be9cc06..e49753c 100644
--- a/src/PMG/API2/PBS/Job.pm
+++ b/src/PMG/API2/PBS/Job.pm
@@ -19,14 +19,6 @@ use PMG::PBSSchedule;
 
 use base qw(PVE::RESTHandler);
 
-my sub get_namespace : prototype($) {
-    my ($remote_config) = @_;
-    if (my $ns = $remote_config->{namespace}) {
-	return $ns if length($ns); # don't pass root namespace
-    }
-    return undef;
-}
-
 __PACKAGE__->register_method ({
     name => 'list',
     path => '',
@@ -111,10 +103,9 @@ my sub get_snapshots {
     my $res = [];
     return $res if $remote_config->{disable};
 
-    my $namespace = get_namespace($remote_config);
     my $pbs = PVE::PBSClient->new($remote_config, $remote, $conf->{secret_dir});
 
-    my $snapshots = $pbs->get_snapshots([$namespace, $group]);
+    my $snapshots = $pbs->get_snapshots($group);
     foreach my $item (@$snapshots) {
 	my ($type, $id, $time) = $item->@{qw(backup-type backup-id backup-time)};
 	next if $type ne 'host';
@@ -252,9 +243,8 @@ __PACKAGE__->register_method ({
 	die "PBS remote '$remote' is disabled\n" if $remote_config->{disable};
 
 	my $pbs = PVE::PBSClient->new($remote_config, $remote, $conf->{secret_dir});
-	my $namespace = get_namespace($remote_config);
 
-	eval { $pbs->forget_snapshot([$namespace, "host/$id/$time"]) };
+	eval { $pbs->forget_snapshot("host/$id/$time") };
 	die "Forgetting backup failed: $@" if $@;
 
 	return;
@@ -324,13 +314,11 @@ __PACKAGE__->register_method ({
 
 	    $log->("starting update of current backup state");
 
-	    my $namespace = get_namespace($remote_config);
-
 	    eval {
 		-d $backup_dir || mkdir $backup_dir;
 		PMG::Backup::pmg_backup($backup_dir, $param->{statistic});
 
-		$pbs->backup_fs_tree($backup_dir, $node, 'pmgbackup', undef, $namespace);
+		$pbs->backup_fs_tree($backup_dir, $node, 'pmgbackup');
 
 		rmtree $backup_dir;
 	    };
@@ -345,7 +333,7 @@ __PACKAGE__->register_method ({
 	    my $group = "host/$node";
 	    if (defined(my $prune_opts = $conf->prune_options($remote))) {
 		$log->("starting prune of $group");
-		my $res = eval { $pbs->prune_group(undef, $prune_opts, [$namespace, $group]) };
+		my $res = eval { $pbs->prune_group(undef, $prune_opts, $group) };
 		if (my $err = $@) {
 		    $log->($err);
 		    PMG::Backup::send_backup_notification($notify, $remote, $full_log, $err);
-- 
2.30.2

[pmg-devel] [RFC/PATCH pmg-api 2/2] api: pbs: don't use namespaced parameters

[Fabian Ebner]

Instead, rely on PBSClient to set namespace according to the initial
configuration.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Dependency bump for new libpve-common-perl needed.

 src/PMG/API2/PBS/Job.pm | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/src/PMG/API2/PBS/Job.pm b/src/PMG/API2/PBS/Job.pm
index be9cc06..e49753c 100644
--- a/src/PMG/API2/PBS/Job.pm
+++ b/src/PMG/API2/PBS/Job.pm
@@ -19,14 +19,6 @@ use PMG::PBSSchedule;
 
 use base qw(PVE::RESTHandler);
 
-my sub get_namespace : prototype($) {
-    my ($remote_config) = @_;
-    if (my $ns = $remote_config->{namespace}) {
-	return $ns if length($ns); # don't pass root namespace
-    }
-    return undef;
-}
-
 __PACKAGE__->register_method ({
     name => 'list',
     path => '',
@@ -111,10 +103,9 @@ my sub get_snapshots {
     my $res = [];
     return $res if $remote_config->{disable};
 
-    my $namespace = get_namespace($remote_config);
     my $pbs = PVE::PBSClient->new($remote_config, $remote, $conf->{secret_dir});
 
-    my $snapshots = $pbs->get_snapshots([$namespace, $group]);
+    my $snapshots = $pbs->get_snapshots($group);
     foreach my $item (@$snapshots) {
 	my ($type, $id, $time) = $item->@{qw(backup-type backup-id backup-time)};
 	next if $type ne 'host';
@@ -252,9 +243,8 @@ __PACKAGE__->register_method ({
 	die "PBS remote '$remote' is disabled\n" if $remote_config->{disable};
 
 	my $pbs = PVE::PBSClient->new($remote_config, $remote, $conf->{secret_dir});
-	my $namespace = get_namespace($remote_config);
 
-	eval { $pbs->forget_snapshot([$namespace, "host/$id/$time"]) };
+	eval { $pbs->forget_snapshot("host/$id/$time") };
 	die "Forgetting backup failed: $@" if $@;
 
 	return;
@@ -324,13 +314,11 @@ __PACKAGE__->register_method ({
 
 	    $log->("starting update of current backup state");
 
-	    my $namespace = get_namespace($remote_config);
-
 	    eval {
 		-d $backup_dir || mkdir $backup_dir;
 		PMG::Backup::pmg_backup($backup_dir, $param->{statistic});
 
-		$pbs->backup_fs_tree($backup_dir, $node, 'pmgbackup', undef, $namespace);
+		$pbs->backup_fs_tree($backup_dir, $node, 'pmgbackup');
 
 		rmtree $backup_dir;
 	    };
@@ -345,7 +333,7 @@ __PACKAGE__->register_method ({
 	    my $group = "host/$node";
 	    if (defined(my $prune_opts = $conf->prune_options($remote))) {
 		$log->("starting prune of $group");
-		my $res = eval { $pbs->prune_group(undef, $prune_opts, [$namespace, $group]) };
+		my $res = eval { $pbs->prune_group(undef, $prune_opts, $group) };
 		if (my $err = $@) {
 		    $log->($err);
 		    PMG::Backup::send_backup_notification($notify, $remote, $full_log, $err);
-- 
2.30.2

[pve-devel] [RFC/PATCH pve-storage 1/1] api: pbs: file restore: don't use namespaced parameters

[Fabian Ebner]

Instead, rely on PBSClient to set namespace according to the initial
configuration.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Dependency bump for new libpve-common-perl needed.

 PVE/API2/Storage/FileRestore.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/PVE/API2/Storage/FileRestore.pm b/PVE/API2/Storage/FileRestore.pm
index 5630f52..ccc56e5 100644
--- a/PVE/API2/Storage/FileRestore.pm
+++ b/PVE/API2/Storage/FileRestore.pm
@@ -119,7 +119,7 @@ __PACKAGE__->register_method ({
 	my (undef, $snap) = PVE::Storage::parse_volname($cfg, $volid);
 
 	my $client = PVE::PBSClient->new($scfg, $storeid);
-	my $ret = $client->file_restore_list([$scfg->{namespace}, $snap], $path, $base64);
+	my $ret = $client->file_restore_list($snap, $path, $base64);
 
 	# 'leaf' is a proper JSON boolean, map to perl-y bool
 	# TODO: make PBSClient decode all bools always as 1/0?
@@ -188,7 +188,7 @@ __PACKAGE__->register_method ({
 	$rpcenv->fork_worker('pbs-download', undef, $user, sub {
 	    my $name = decode_base64($path);
 	    print "Starting download of file: $name\n";
-	    $client->file_restore_extract($fifo, [$scfg->{namespace}, $snap], $path, 1);
+	    $client->file_restore_extract($fifo, $snap, $path, 1);
 	});
 
 	my $ret = {
-- 
2.30.2

[pmg-devel] [RFC/PATCH pve-storage 1/1] api: pbs: file restore: don't use namespaced parameters

[Fabian Ebner]

Instead, rely on PBSClient to set namespace according to the initial
configuration.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Dependency bump for new libpve-common-perl needed.

 PVE/API2/Storage/FileRestore.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/PVE/API2/Storage/FileRestore.pm b/PVE/API2/Storage/FileRestore.pm
index 5630f52..ccc56e5 100644
--- a/PVE/API2/Storage/FileRestore.pm
+++ b/PVE/API2/Storage/FileRestore.pm
@@ -119,7 +119,7 @@ __PACKAGE__->register_method ({
 	my (undef, $snap) = PVE::Storage::parse_volname($cfg, $volid);
 
 	my $client = PVE::PBSClient->new($scfg, $storeid);
-	my $ret = $client->file_restore_list([$scfg->{namespace}, $snap], $path, $base64);
+	my $ret = $client->file_restore_list($snap, $path, $base64);
 
 	# 'leaf' is a proper JSON boolean, map to perl-y bool
 	# TODO: make PBSClient decode all bools always as 1/0?
@@ -188,7 +188,7 @@ __PACKAGE__->register_method ({
 	$rpcenv->fork_worker('pbs-download', undef, $user, sub {
 	    my $name = decode_base64($path);
 	    print "Starting download of file: $name\n";
-	    $client->file_restore_extract($fifo, [$scfg->{namespace}, $snap], $path, 1);
+	    $client->file_restore_extract($fifo, $snap, $path, 1);
 	});
 
 	my $ret = {
-- 
2.30.2

[pve-devel] applied: [RFC/PATCH pve-storage 1/1] api: pbs: file restore: don't use namespaced parameters

[Wolfgang Bumiller]

applied & dumped dependency accordingly

On Wed, Jul 20, 2022 at 12:59:48PM +0200, Fabian Ebner wrote:
> Instead, rely on PBSClient to set namespace according to the initial
> configuration.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> 
> Dependency bump for new libpve-common-perl needed.
> 
>  PVE/API2/Storage/FileRestore.pm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/API2/Storage/FileRestore.pm b/PVE/API2/Storage/FileRestore.pm
> index 5630f52..ccc56e5 100644
> --- a/PVE/API2/Storage/FileRestore.pm
> +++ b/PVE/API2/Storage/FileRestore.pm
> @@ -119,7 +119,7 @@ __PACKAGE__->register_method ({
>  	my (undef, $snap) = PVE::Storage::parse_volname($cfg, $volid);
>  
>  	my $client = PVE::PBSClient->new($scfg, $storeid);
> -	my $ret = $client->file_restore_list([$scfg->{namespace}, $snap], $path, $base64);
> +	my $ret = $client->file_restore_list($snap, $path, $base64);
>  
>  	# 'leaf' is a proper JSON boolean, map to perl-y bool
>  	# TODO: make PBSClient decode all bools always as 1/0?
> @@ -188,7 +188,7 @@ __PACKAGE__->register_method ({
>  	$rpcenv->fork_worker('pbs-download', undef, $user, sub {
>  	    my $name = decode_base64($path);
>  	    print "Starting download of file: $name\n";
> -	    $client->file_restore_extract($fifo, [$scfg->{namespace}, $snap], $path, 1);
> +	    $client->file_restore_extract($fifo, $snap, $path, 1);
>  	});
>  
>  	my $ret = {
> -- 
> 2.30.2

[pmg-devel] applied: [RFC/PATCH pve-storage 1/1] api: pbs: file restore: don't use namespaced parameters

[Wolfgang Bumiller]

applied & dumped dependency accordingly

On Wed, Jul 20, 2022 at 12:59:48PM +0200, Fabian Ebner wrote:
> Instead, rely on PBSClient to set namespace according to the initial
> configuration.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> 
> Dependency bump for new libpve-common-perl needed.
> 
>  PVE/API2/Storage/FileRestore.pm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/API2/Storage/FileRestore.pm b/PVE/API2/Storage/FileRestore.pm
> index 5630f52..ccc56e5 100644
> --- a/PVE/API2/Storage/FileRestore.pm
> +++ b/PVE/API2/Storage/FileRestore.pm
> @@ -119,7 +119,7 @@ __PACKAGE__->register_method ({
>  	my (undef, $snap) = PVE::Storage::parse_volname($cfg, $volid);
>  
>  	my $client = PVE::PBSClient->new($scfg, $storeid);
> -	my $ret = $client->file_restore_list([$scfg->{namespace}, $snap], $path, $base64);
> +	my $ret = $client->file_restore_list($snap, $path, $base64);
>  
>  	# 'leaf' is a proper JSON boolean, map to perl-y bool
>  	# TODO: make PBSClient decode all bools always as 1/0?
> @@ -188,7 +188,7 @@ __PACKAGE__->register_method ({
>  	$rpcenv->fork_worker('pbs-download', undef, $user, sub {
>  	    my $name = decode_base64($path);
>  	    print "Starting download of file: $name\n";
> -	    $client->file_restore_extract($fifo, [$scfg->{namespace}, $snap], $path, 1);
> +	    $client->file_restore_extract($fifo, $snap, $path, 1);
>  	});
>  
>  	my $ret = {
> -- 
> 2.30.2

Re: [pve-devel] [pmg-devel] [PATCH-SERIES pve-common/pmg-api/pve-storage] pbs client: rework namespace usage and minor fixes

[Fiona Ebner]

Am 20.07.22 um 12:59 schrieb Fabian Ebner:
> Mostly done in preparation for #3186 (refactor pbs client use in PVE),
> to avoid the need to manually set the namespace for all call-sites in
> PVE, when it's already present in the storage/PBS config.
> 
> pve-common 1/5 and 2/5 and pmg-api 1/1 are improvements touching parts
> of the same infrastructure, but not directly related.
> 
> The other patches change PBSClient to auto-select the namespace from
> its initial configuration if not explicitly overriden with a
> namespaced parameter and deprecate namespaced parameters as a whole.
> 
> Rationale is that essentially all current users of PBSClient are
> configured for one namespace (there is the "status" call, which
> doesn't depend on a namespace, but that doesn't contradict the
> previous claim). It's less work on the call sites and there's no risk
> to forget namespacing a parameter (as happened with pxar_restore in
> PMG) if the PBSClient handles it itself.
> 
> If the need for handling more than one namespace with a single client
> ever arises, we can still add e.g. a set_namespace() function to the
> PBSClient.
> 
> Also makes it possible to restore a backup from a namespace in PMG,
> which currently fails.
> 
> 
> Dependency bump for new pve-common is needed for pve-storage and
> pmg-api.
> 
> 

Ping

Re: [pmg-devel] [PATCH-SERIES pve-common/pmg-api/pve-storage] pbs client: rework namespace usage and minor fixes

[Fiona Ebner]

Am 20.07.22 um 12:59 schrieb Fabian Ebner:
> Mostly done in preparation for #3186 (refactor pbs client use in PVE),
> to avoid the need to manually set the namespace for all call-sites in
> PVE, when it's already present in the storage/PBS config.
> 
> pve-common 1/5 and 2/5 and pmg-api 1/1 are improvements touching parts
> of the same infrastructure, but not directly related.
> 
> The other patches change PBSClient to auto-select the namespace from
> its initial configuration if not explicitly overriden with a
> namespaced parameter and deprecate namespaced parameters as a whole.
> 
> Rationale is that essentially all current users of PBSClient are
> configured for one namespace (there is the "status" call, which
> doesn't depend on a namespace, but that doesn't contradict the
> previous claim). It's less work on the call sites and there's no risk
> to forget namespacing a parameter (as happened with pxar_restore in
> PMG) if the PBSClient handles it itself.
> 
> If the need for handling more than one namespace with a single client
> ever arises, we can still add e.g. a set_namespace() function to the
> PBSClient.
> 
> Also makes it possible to restore a backup from a namespace in PMG,
> which currently fails.
> 
> 
> Dependency bump for new pve-common is needed for pve-storage and
> pmg-api.
> 
> 

Ping