* [pbs-devel] [PATCH proxmox-backup 0/2] fixes #6195, add support for moving namespaces
@ 2025-09-03 10:11 Hannes Laimer
2025-09-03 10:11 ` [pbs-devel] [PATCH proxmox-backup 1/2] fix #6195: api: datastore: add endpoint " Hannes Laimer
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Hannes Laimer @ 2025-09-03 10:11 UTC (permalink / raw)
To: pbs-devel
Adds support for moving namespaces within a datastore.
- we don't create non-existing target namespaces
- we don't allow moving if a namespace with the same "name" already
exists. So given:
/a/b/c/x
/1/2/3/4/x
moving /1/2/3/4/x to /a/b/c/ is not allowed.
these are the only real restrictions. Of course moving itself into
sub-namespaces, exeeding depth limit or moving root are also not. But
those are not really artificial.
Hannes Laimer (2):
fix #6195: api: datastore: add endpoint for moving namespaces
fix #6195: ui: add button/window for moving a namespace
pbs-datastore/src/datastore.rs | 121 +++++++++++++++++++++++++++++++++
src/api2/admin/namespace.rs | 43 +++++++++++-
www/Makefile | 1 +
www/datastore/Content.js | 38 +++++++++++
www/window/NamespaceMove.js | 47 +++++++++++++
5 files changed, 248 insertions(+), 2 deletions(-)
create mode 100644 www/window/NamespaceMove.js
--
2.47.2
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 1/2] fix #6195: api: datastore: add endpoint for moving namespaces
2025-09-03 10:11 [pbs-devel] [PATCH proxmox-backup 0/2] fixes #6195, add support for moving namespaces Hannes Laimer
@ 2025-09-03 10:11 ` Hannes Laimer
2025-09-03 10:40 ` Shannon Sterz
2025-09-03 10:11 ` [pbs-devel] [PATCH proxmox-backup 2/2] fix #6195: ui: add button/window for moving a namespace Hannes Laimer
2025-09-03 12:10 ` [pbs-devel] superseded: [PATCH proxmox-backup 0/2] fixes #6195, add support for moving namespaces Hannes Laimer
2 siblings, 1 reply; 6+ messages in thread
From: Hannes Laimer @ 2025-09-03 10:11 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
pbs-datastore/src/datastore.rs | 121 +++++++++++++++++++++++++++++++++
src/api2/admin/namespace.rs | 43 +++++++++++-
2 files changed, 162 insertions(+), 2 deletions(-)
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 7cf020fc..531927ff 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -706,6 +706,127 @@ impl DataStore {
Ok(ns)
}
+ pub fn namespace_move(
+ self: &Arc<Self>,
+ ns: BackupNamespace,
+ mut target: BackupNamespace,
+ ) -> Result<(), Error> {
+ log::info!(
+ "moving ns '{}' to ns '{}' in datastore {}",
+ ns,
+ target,
+ self.name()
+ );
+ if ns.is_root() {
+ bail!("can't move root");
+ }
+ if !self.namespace_exists(&ns) {
+ bail!("cannot move a namespace that does not exist");
+ }
+ if !self.namespace_exists(&target) {
+ bail!("cannot move to a namespace that does not exist");
+ }
+ let Some(base_name) = ns.clone().pop() else {
+ bail!("source namespace can't be root");
+ };
+ target.push(base_name.clone())?;
+ if self.namespace_exists(&target) {
+ bail!("'{}' already exists", target);
+ }
+
+ let source_path = self.namespace_path(&ns);
+ let target_path = self.namespace_path(&target);
+ if target_path.starts_with(&source_path) {
+ bail!("cannot move namespace into one of its sub-namespaces");
+ }
+
+ let iter = self.recursive_iter_backup_ns_ok(ns.to_owned(), None)?;
+ let source_depth = ns.depth();
+ let source_height = iter.map(|ns| ns.depth() - source_depth).max().unwrap_or(1);
+ target.check_max_depth(source_height - 1)?;
+
+ if let DatastoreBackend::S3(s3_client) = self.backend()? {
+ let src_ns_rel = ns.path();
+ let src_ns_rel_str = src_ns_rel
+ .to_str()
+ .ok_or_else(|| format_err!("invalid source namespace path"))?;
+
+ let dst_ns_rel = target.path();
+ let dst_ns_rel_str = dst_ns_rel
+ .to_str()
+ .ok_or_else(|| format_err!("invalid destination namespace path"))?;
+
+ let list_prefix = S3PathPrefix::Some(format!("{S3_CONTENT_PREFIX}/{src_ns_rel_str}"));
+ let mut next_continuation_token: Option<String> = None;
+ let store_prefix = format!("{}/{S3_CONTENT_PREFIX}/", self.name());
+
+ loop {
+ let list_objects_result = proxmox_async::runtime::block_on(
+ s3_client.list_objects_v2(&list_prefix, next_continuation_token.as_deref()),
+ )?;
+
+ let objects_to_move: Vec<S3ObjectKey> = list_objects_result
+ .contents
+ .iter()
+ .map(|c| c.key.clone())
+ .collect();
+
+ for object_key in objects_to_move {
+ let object_key_str = object_key.to_string();
+ let object_path =
+ object_key_str.strip_prefix(&store_prefix).ok_or_else(|| {
+ format_err!(
+ "failed to strip store prefix '{}' from object key '{}'",
+ store_prefix,
+ object_key_str
+ )
+ })?;
+
+ let src_key_rel = S3ObjectKey::try_from(
+ format!("{S3_CONTENT_PREFIX}/{}", object_path).as_str(),
+ )?;
+
+ let src_rel_with_sep = format!("{}/", src_ns_rel_str);
+ let rest = object_path
+ .strip_prefix(&src_rel_with_sep)
+ .ok_or_else(|| format_err!("unexpected object path: {}", object_path))?;
+
+ let dest_rel_path = format!("{}/{}", dst_ns_rel_str, rest);
+ let dest_rel_path = std::path::Path::new(&dest_rel_path);
+ let file_name = dest_rel_path
+ .file_name()
+ .and_then(|n| n.to_str())
+ .ok_or_else(|| format_err!("invalid destination object file name"))?;
+ let parent = dest_rel_path
+ .parent()
+ .unwrap_or_else(|| std::path::Path::new(""));
+ let dest_key = crate::s3::object_key_from_path(parent, file_name)?;
+
+ proxmox_async::runtime::block_on(
+ s3_client.copy_object(src_key_rel.clone(), dest_key),
+ )?;
+ }
+
+ if list_objects_result.is_truncated {
+ next_continuation_token = list_objects_result.next_continuation_token;
+ } else {
+ break;
+ }
+ }
+
+ // delete objects under the old namespace prefix
+ let delete_error =
+ proxmox_async::runtime::block_on(s3_client.delete_objects_by_prefix(&list_prefix))?;
+ if delete_error {
+ bail!("deleting objects under old namespace prefix failed");
+ }
+ }
+
+ std::fs::create_dir_all(&target_path)?;
+ std::fs::rename(source_path, target_path)?;
+ Ok(())
+ }
+
/// Returns if the given namespace exists on the datastore
pub fn namespace_exists(&self, ns: &BackupNamespace) -> bool {
let mut path = self.base_path();
diff --git a/src/api2/admin/namespace.rs b/src/api2/admin/namespace.rs
index 6cf88d89..71992098 100644
--- a/src/api2/admin/namespace.rs
+++ b/src/api2/admin/namespace.rs
@@ -6,7 +6,7 @@ use proxmox_schema::*;
use pbs_api_types::{
Authid, BackupGroupDeleteStats, BackupNamespace, NamespaceListItem, Operation,
- DATASTORE_SCHEMA, NS_MAX_DEPTH_SCHEMA, PROXMOX_SAFE_ID_FORMAT,
+ DATASTORE_SCHEMA, NS_MAX_DEPTH_SCHEMA, PRIV_DATASTORE_MODIFY, PROXMOX_SAFE_ID_FORMAT,
};
use pbs_datastore::DataStore;
@@ -123,6 +123,44 @@ pub fn list_namespaces(
Ok(namespace_list)
}
+#[api(
+ input: {
+ properties: {
+ store: { schema: DATASTORE_SCHEMA },
+ ns: {
+ type: BackupNamespace,
+ },
+ "target-ns": {
+ type: BackupNamespace,
+ optional: true,
+ },
+ },
+ },
+ access: {
+ permission: &Permission::Anybody,
+ description: "Requires DATASTORE_MODIFY on both the source /datastore/{store}/{ns} and\
+ the target /datastore/{store}/[{target-ns}]",
+ },
+)]
+/// Move a namespace into a different one. No target means the root namespace is the target.
+pub fn move_namespace(
+ store: String,
+ ns: BackupNamespace,
+ target_ns: Option<BackupNamespace>,
+ _info: &ApiMethod,
+ rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+ let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+ let target = target_ns.unwrap_or(BackupNamespace::root());
+
+ check_ns_modification_privs(&store, &ns, &auth_id)?;
+ check_ns_privs(&store, &target, &auth_id, PRIV_DATASTORE_MODIFY)?;
+
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
+
+ datastore.namespace_move(ns, target)
+}
+
#[api(
input: {
properties: {
@@ -192,4 +230,5 @@ pub fn delete_namespace(
pub const ROUTER: Router = Router::new()
.get(&API_METHOD_LIST_NAMESPACES)
.post(&API_METHOD_CREATE_NAMESPACE)
- .delete(&API_METHOD_DELETE_NAMESPACE);
+ .delete(&API_METHOD_DELETE_NAMESPACE)
+ .subdirs(&[("move", &Router::new().post(&API_METHOD_MOVE_NAMESPACE))]);
--
2.47.2
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 2/2] fix #6195: ui: add button/window for moving a namespace
2025-09-03 10:11 [pbs-devel] [PATCH proxmox-backup 0/2] fixes #6195, add support for moving namespaces Hannes Laimer
2025-09-03 10:11 ` [pbs-devel] [PATCH proxmox-backup 1/2] fix #6195: api: datastore: add endpoint " Hannes Laimer
@ 2025-09-03 10:11 ` Hannes Laimer
2025-09-03 12:10 ` [pbs-devel] superseded: [PATCH proxmox-backup 0/2] fixes #6195, add support for moving namespaces Hannes Laimer
2 siblings, 0 replies; 6+ messages in thread
From: Hannes Laimer @ 2025-09-03 10:11 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
www/Makefile | 1 +
www/datastore/Content.js | 38 ++++++++++++++++++++++++++++++
www/window/NamespaceMove.js | 47 +++++++++++++++++++++++++++++++++++++
3 files changed, 86 insertions(+)
create mode 100644 www/window/NamespaceMove.js
diff --git a/www/Makefile b/www/Makefile
index 9ebf0445..d0b4592c 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -79,6 +79,7 @@ JSSRC= \
window/CreateDirectory.js \
window/DataStoreEdit.js \
window/NamespaceEdit.js \
+ window/NamespaceMove.js \
window/MaintenanceOptions.js \
window/NotesEdit.js \
window/RemoteEdit.js \
diff --git a/www/datastore/Content.js b/www/datastore/Content.js
index 075022e9..ad9733c1 100644
--- a/www/datastore/Content.js
+++ b/www/datastore/Content.js
@@ -447,6 +447,22 @@ Ext.define('PBS.DataStoreContent', {
win.on('destroy', this.reload, this);
},
+ onMove: function (table, rI, cI, item, e, rec) {
+ let me = this;
+ let view = me.getView();
+ if (!rec.data.ns || rec.data.ty !== 'ns' || !view.datastore) {
+ return;
+ }
+ Ext.create('PBS.NamespaceMove', {
+ autoShow: true,
+ datastore: view.datastore,
+ ns: rec.data.ns,
+ listeners: {
+ destroy: () => me.reload(),
+ },
+ });
+ },
+
onPrune: function (table, rI, cI, item, e, rec) {
let me = this;
let view = me.getView();
@@ -1058,6 +1074,18 @@ Ext.define('PBS.DataStoreContent', {
data.ty === 'group' ? 'fa fa-scissors' : 'pmx-hidden',
isActionDisabled: (v, r, c, i, { data }) => data.ty !== 'group',
},
+ {
+ handler: 'onMove',
+ getTip: (v, m, rec) => Ext.String.format(gettext("Move '{0}'"), v),
+ getClass: (v, m, { data }) => {
+ if (data.parentId !== 'root' && !!data.ns && data.ty === 'ns' && !data.isRootNS) {
+ return 'fa fa-arrow-right';
+ } else {
+ return 'pmx-hidden';
+ }
+ },
+ isActionDisabled: (v, r, c, i, { data }) => data.ty !== 'ns',
+ },
{
handler: 'onProtectionChange',
getTip: (v, m, rec) =>
@@ -1439,6 +1467,16 @@ Ext.define('PBS.datastore.GroupCmdMenu', {
hidden: '{!onPrune}',
},
},
+ {
+ text: gettext('Move'),
+ iconCls: 'fa fa-arrow-right',
+ handler: function () {
+ this.up('menu').onMove();
+ },
+ cbind: {
+ hidden: '{!onMove}',
+ },
+ },
{ xtype: 'menuseparator' },
{
text: gettext('Remove'),
diff --git a/www/window/NamespaceMove.js b/www/window/NamespaceMove.js
new file mode 100644
index 00000000..b294bf6f
--- /dev/null
+++ b/www/window/NamespaceMove.js
@@ -0,0 +1,47 @@
+Ext.define('PBS.NamespaceMove', {
+ extend: 'Proxmox.window.Edit',
+ alias: 'widget.pbsNamespaceMove',
+ mixins: ['Proxmox.Mixin.CBind'],
+
+ onlineHelp: 'changing-backup-owner',
+
+ submitText: gettext('Move Namespace'),
+ width: 350,
+
+ initComponent: function () {
+ let me = this;
+
+ if (!me.datastore) {
+ throw 'no datastore specified';
+ }
+ if (!me.ns) {
+ throw 'no namespace specified';
+ }
+
+ Ext.apply(me, {
+ url: `/api2/extjs/admin/datastore/${me.datastore}/namespace/move`,
+ method: 'POST',
+ subject: gettext('Move Namespace') + ` - ${me.ns}`,
+ items: {
+ xtype: 'inputpanel',
+ onGetValues: function (values) {
+ values.ns = me.ns;
+ return values;
+ },
+
+ items: [
+ {
+ xtype: 'pbsNamespaceSelector',
+ datastore: me.datastore,
+ name: 'target-ns',
+ value: me.ns,
+ fieldLabel: gettext('Target'),
+ allowBlank: true,
+ },
+ ],
+ },
+ });
+
+ me.callParent();
+ },
+});
--
2.47.2
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 1/2] fix #6195: api: datastore: add endpoint for moving namespaces
2025-09-03 10:11 ` [pbs-devel] [PATCH proxmox-backup 1/2] fix #6195: api: datastore: add endpoint " Hannes Laimer
@ 2025-09-03 10:40 ` Shannon Sterz
2025-09-03 10:49 ` Hannes Laimer
0 siblings, 1 reply; 6+ messages in thread
From: Shannon Sterz @ 2025-09-03 10:40 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
On Wed Sep 3, 2025 at 12:11 PM CEST, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> pbs-datastore/src/datastore.rs | 121 +++++++++++++++++++++++++++++++++
> src/api2/admin/namespace.rs | 43 +++++++++++-
> 2 files changed, 162 insertions(+), 2 deletions(-)
>
> diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
> index 7cf020fc..531927ff 100644
> --- a/pbs-datastore/src/datastore.rs
> +++ b/pbs-datastore/src/datastore.rs
> @@ -706,6 +706,127 @@ impl DataStore {
> Ok(ns)
> }
>
> + pub fn namespace_move(
> + self: &Arc<Self>,
> + ns: BackupNamespace,
> + mut target: BackupNamespace,
> + ) -> Result<(), Error> {
> + log::info!(
> + "moving ns '{}' to ns '{}' in datastore {}",
> + ns,
> + target,
> + self.name()
> + );
> + if ns.is_root() {
> + bail!("can't move root");
> + }
> + if !self.namespace_exists(&ns) {
> + bail!("cannot move a namespace that does not exist");
> + }
> + if !self.namespace_exists(&target) {
> + bail!("cannot move to a namespace that does not exist");
> + }
> + let Some(base_name) = ns.clone().pop() else {
> + bail!("source namespace can't be root");
> + };
> + target.push(base_name.clone())?;
> + if self.namespace_exists(&target) {
> + bail!("'{}' already exists", target);
> + }
> +
> + let source_path = self.namespace_path(&ns);
> + let target_path = self.namespace_path(&target);
> + if target_path.starts_with(&source_path) {
> + bail!("cannot move namespace into one of its sub-namespaces");
> + }
> +
> + let iter = self.recursive_iter_backup_ns_ok(ns.to_owned(), None)?;
> + let source_depth = ns.depth();
> + let source_height = iter.map(|ns| ns.depth() - source_depth).max().unwrap_or(1);
> + target.check_max_depth(source_height - 1)?;
> +
> + if let DatastoreBackend::S3(s3_client) = self.backend()? {
> + let src_ns_rel = ns.path();
> + let src_ns_rel_str = src_ns_rel
> + .to_str()
> + .ok_or_else(|| format_err!("invalid source namespace path"))?;
> +
> + let dst_ns_rel = target.path();
> + let dst_ns_rel_str = dst_ns_rel
> + .to_str()
> + .ok_or_else(|| format_err!("invalid destination namespace path"))?;
> +
> + let list_prefix = S3PathPrefix::Some(format!("{S3_CONTENT_PREFIX}/{src_ns_rel_str}"));
> + let mut next_continuation_token: Option<String> = None;
> + let store_prefix = format!("{}/{S3_CONTENT_PREFIX}/", self.name());
> +
> + loop {
> + let list_objects_result = proxmox_async::runtime::block_on(
> + s3_client.list_objects_v2(&list_prefix, next_continuation_token.as_deref()),
> + )?;
> +
> + let objects_to_move: Vec<S3ObjectKey> = list_objects_result
> + .contents
> + .iter()
> + .map(|c| c.key.clone())
> + .collect();
> +
> + for object_key in objects_to_move {
> + let object_key_str = object_key.to_string();
> + let object_path =
> + object_key_str.strip_prefix(&store_prefix).ok_or_else(|| {
> + format_err!(
> + "failed to strip store prefix '{}' from object key '{}'",
> + store_prefix,
> + object_key_str
> + )
> + })?;
> +
> + let src_key_rel = S3ObjectKey::try_from(
> + format!("{S3_CONTENT_PREFIX}/{}", object_path).as_str(),
> + )?;
> +
> + let src_rel_with_sep = format!("{}/", src_ns_rel_str);
> + let rest = object_path
> + .strip_prefix(&src_rel_with_sep)
> + .ok_or_else(|| format_err!("unexpected object path: {}", object_path))?;
> +
> + let dest_rel_path = format!("{}/{}", dst_ns_rel_str, rest);
> + let dest_rel_path = std::path::Path::new(&dest_rel_path);
> + let file_name = dest_rel_path
> + .file_name()
> + .and_then(|n| n.to_str())
> + .ok_or_else(|| format_err!("invalid destination object file name"))?;
> + let parent = dest_rel_path
> + .parent()
> + .unwrap_or_else(|| std::path::Path::new(""));
> + let dest_key = crate::s3::object_key_from_path(parent, file_name)?;
> +
> + proxmox_async::runtime::block_on(
> + s3_client.copy_object(src_key_rel.clone(), dest_key),
> + )?;
> + }
> +
> + if list_objects_result.is_truncated {
> + next_continuation_token = list_objects_result.next_continuation_token;
> + } else {
> + break;
> + }
> + }
> +
> + // delete objects under the old namespace prefix
> + let delete_error =
> + proxmox_async::runtime::block_on(s3_client.delete_objects_by_prefix(&list_prefix))?;
> + if delete_error {
> + bail!("deleting objects under old namespace prefix failed");
> + }
> + }
> +
> + std::fs::create_dir_all(&target_path)?;
> + std::fs::rename(source_path, target_path)?;
hm won't this break running back up tasks that assume they can write
manifests/index into an existing location?
> + Ok(())
> + }
> +
> /// Returns if the given namespace exists on the datastore
> pub fn namespace_exists(&self, ns: &BackupNamespace) -> bool {
> let mut path = self.base_path();
> diff --git a/src/api2/admin/namespace.rs b/src/api2/admin/namespace.rs
> index 6cf88d89..71992098 100644
> --- a/src/api2/admin/namespace.rs
> +++ b/src/api2/admin/namespace.rs
> @@ -6,7 +6,7 @@ use proxmox_schema::*;
>
> use pbs_api_types::{
> Authid, BackupGroupDeleteStats, BackupNamespace, NamespaceListItem, Operation,
> - DATASTORE_SCHEMA, NS_MAX_DEPTH_SCHEMA, PROXMOX_SAFE_ID_FORMAT,
> + DATASTORE_SCHEMA, NS_MAX_DEPTH_SCHEMA, PRIV_DATASTORE_MODIFY, PROXMOX_SAFE_ID_FORMAT,
> };
>
> use pbs_datastore::DataStore;
> @@ -123,6 +123,44 @@ pub fn list_namespaces(
> Ok(namespace_list)
> }
>
> +#[api(
> + input: {
> + properties: {
> + store: { schema: DATASTORE_SCHEMA },
> + ns: {
> + type: BackupNamespace,
> + },
> + "target-ns": {
> + type: BackupNamespace,
> + optional: true,
> + },
> + },
> + },
> + access: {
> + permission: &Permission::Anybody,
> + description: "Requires DATASTORE_MODIFY on both the source /datastore/{store}/{ns} and\
> + the target /datastore/{store}/[{target-ns}]",
> + },
> +)]
> +/// Move a namespace into a different one. No target means the root namespace is the target.
> +pub fn move_namespace(
> + store: String,
> + ns: BackupNamespace,
> + target_ns: Option<BackupNamespace>,
> + _info: &ApiMethod,
> + rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<(), Error> {
> + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> + let target = target_ns.unwrap_or(BackupNamespace::root());
> +
> + check_ns_modification_privs(&store, &ns, &auth_id)?;
> + check_ns_privs(&store, &target, &auth_id, PRIV_DATASTORE_MODIFY)?;
> +
> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
> +
> + datastore.namespace_move(ns, target)
> +}
> +
> #[api(
> input: {
> properties: {
> @@ -192,4 +230,5 @@ pub fn delete_namespace(
> pub const ROUTER: Router = Router::new()
> .get(&API_METHOD_LIST_NAMESPACES)
> .post(&API_METHOD_CREATE_NAMESPACE)
> - .delete(&API_METHOD_DELETE_NAMESPACE);
> + .delete(&API_METHOD_DELETE_NAMESPACE)
> + .subdirs(&[("move", &Router::new().post(&API_METHOD_MOVE_NAMESPACE))]);
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 1/2] fix #6195: api: datastore: add endpoint for moving namespaces
2025-09-03 10:40 ` Shannon Sterz
@ 2025-09-03 10:49 ` Hannes Laimer
0 siblings, 0 replies; 6+ messages in thread
From: Hannes Laimer @ 2025-09-03 10:49 UTC (permalink / raw)
To: Shannon Sterz, Proxmox Backup Server development discussion
On 03.09.25 12:40, Shannon Sterz wrote:
> On Wed Sep 3, 2025 at 12:11 PM CEST, Hannes Laimer wrote:
>> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
>> ---
>> pbs-datastore/src/datastore.rs | 121 +++++++++++++++++++++++++++++++++
>> src/api2/admin/namespace.rs | 43 +++++++++++-
>> 2 files changed, 162 insertions(+), 2 deletions(-)
>>
>> diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
>> index 7cf020fc..531927ff 100644
>> --- a/pbs-datastore/src/datastore.rs
>> +++ b/pbs-datastore/src/datastore.rs
>> @@ -706,6 +706,127 @@ impl DataStore {
>> Ok(ns)
>> }
>>
>> + pub fn namespace_move(
>> + self: &Arc<Self>,
>> + ns: BackupNamespace,
>> + mut target: BackupNamespace,
>> + ) -> Result<(), Error> {
>> + log::info!(
>> + "moving ns '{}' to ns '{}' in datastore {}",
>> + ns,
>> + target,
>> + self.name()
>> + );
>> + if ns.is_root() {
>> + bail!("can't move root");
>> + }
>> + if !self.namespace_exists(&ns) {
>> + bail!("cannot move a namespace that does not exist");
>> + }
>> + if !self.namespace_exists(&target) {
>> + bail!("cannot move to a namespace that does not exist");
>> + }
>> + let Some(base_name) = ns.clone().pop() else {
>> + bail!("source namespace can't be root");
>> + };
>> + target.push(base_name.clone())?;
>> + if self.namespace_exists(&target) {
>> + bail!("'{}' already exists", target);
>> + }
>> +
>> + let source_path = self.namespace_path(&ns);
>> + let target_path = self.namespace_path(&target);
>> + if target_path.starts_with(&source_path) {
>> + bail!("cannot move namespace into one of its sub-namespaces");
>> + }
>> +
>> + let iter = self.recursive_iter_backup_ns_ok(ns.to_owned(), None)?;
>> + let source_depth = ns.depth();
>> + let source_height = iter.map(|ns| ns.depth() - source_depth).max().unwrap_or(1);
>> + target.check_max_depth(source_height - 1)?;
>> +
>> + if let DatastoreBackend::S3(s3_client) = self.backend()? {
>> + let src_ns_rel = ns.path();
>> + let src_ns_rel_str = src_ns_rel
>> + .to_str()
>> + .ok_or_else(|| format_err!("invalid source namespace path"))?;
>> +
>> + let dst_ns_rel = target.path();
>> + let dst_ns_rel_str = dst_ns_rel
>> + .to_str()
>> + .ok_or_else(|| format_err!("invalid destination namespace path"))?;
>> +
>> + let list_prefix = S3PathPrefix::Some(format!("{S3_CONTENT_PREFIX}/{src_ns_rel_str}"));
>> + let mut next_continuation_token: Option<String> = None;
>> + let store_prefix = format!("{}/{S3_CONTENT_PREFIX}/", self.name());
>> +
>> + loop {
>> + let list_objects_result = proxmox_async::runtime::block_on(
>> + s3_client.list_objects_v2(&list_prefix, next_continuation_token.as_deref()),
>> + )?;
>> +
>> + let objects_to_move: Vec<S3ObjectKey> = list_objects_result
>> + .contents
>> + .iter()
>> + .map(|c| c.key.clone())
>> + .collect();
>> +
>> + for object_key in objects_to_move {
>> + let object_key_str = object_key.to_string();
>> + let object_path =
>> + object_key_str.strip_prefix(&store_prefix).ok_or_else(|| {
>> + format_err!(
>> + "failed to strip store prefix '{}' from object key '{}'",
>> + store_prefix,
>> + object_key_str
>> + )
>> + })?;
>> +
>> + let src_key_rel = S3ObjectKey::try_from(
>> + format!("{S3_CONTENT_PREFIX}/{}", object_path).as_str(),
>> + )?;
>> +
>> + let src_rel_with_sep = format!("{}/", src_ns_rel_str);
>> + let rest = object_path
>> + .strip_prefix(&src_rel_with_sep)
>> + .ok_or_else(|| format_err!("unexpected object path: {}", object_path))?;
>> +
>> + let dest_rel_path = format!("{}/{}", dst_ns_rel_str, rest);
>> + let dest_rel_path = std::path::Path::new(&dest_rel_path);
>> + let file_name = dest_rel_path
>> + .file_name()
>> + .and_then(|n| n.to_str())
>> + .ok_or_else(|| format_err!("invalid destination object file name"))?;
>> + let parent = dest_rel_path
>> + .parent()
>> + .unwrap_or_else(|| std::path::Path::new(""));
>> + let dest_key = crate::s3::object_key_from_path(parent, file_name)?;
>> +
>> + proxmox_async::runtime::block_on(
>> + s3_client.copy_object(src_key_rel.clone(), dest_key),
>> + )?;
>> + }
>> +
>> + if list_objects_result.is_truncated {
>> + next_continuation_token = list_objects_result.next_continuation_token;
>> + } else {
>> + break;
>> + }
>> + }
>> +
>> + // delete objects under the old namespace prefix
>> + let delete_error =
>> + proxmox_async::runtime::block_on(s3_client.delete_objects_by_prefix(&list_prefix))?;
>> + if delete_error {
>> + bail!("deleting objects under old namespace prefix failed");
>> + }
>> + }
>> +
>> + std::fs::create_dir_all(&target_path)?;
>> + std::fs::rename(source_path, target_path)?;
>
> hm won't this break running back up tasks that assume they can write
> manifests/index into an existing location?
>
you're right. I'm not sure why, by in my head
`DataStore::lookup_datastore(&store, Some(Operation::Write))`
behaved like a lock, but it obviously does not.
Setting a maintenance mode and waiting for running operations should
fix this. I'll send a v2.
Thanks for taking a look!
>> + Ok(())
>> + }
>> +
>> /// Returns if the given namespace exists on the datastore
>> pub fn namespace_exists(&self, ns: &BackupNamespace) -> bool {
>> let mut path = self.base_path();
>> diff --git a/src/api2/admin/namespace.rs b/src/api2/admin/namespace.rs
>> index 6cf88d89..71992098 100644
>> --- a/src/api2/admin/namespace.rs
>> +++ b/src/api2/admin/namespace.rs
>> @@ -6,7 +6,7 @@ use proxmox_schema::*;
>>
>> use pbs_api_types::{
>> Authid, BackupGroupDeleteStats, BackupNamespace, NamespaceListItem, Operation,
>> - DATASTORE_SCHEMA, NS_MAX_DEPTH_SCHEMA, PROXMOX_SAFE_ID_FORMAT,
>> + DATASTORE_SCHEMA, NS_MAX_DEPTH_SCHEMA, PRIV_DATASTORE_MODIFY, PROXMOX_SAFE_ID_FORMAT,
>> };
>>
>> use pbs_datastore::DataStore;
>> @@ -123,6 +123,44 @@ pub fn list_namespaces(
>> Ok(namespace_list)
>> }
>>
>> +#[api(
>> + input: {
>> + properties: {
>> + store: { schema: DATASTORE_SCHEMA },
>> + ns: {
>> + type: BackupNamespace,
>> + },
>> + "target-ns": {
>> + type: BackupNamespace,
>> + optional: true,
>> + },
>> + },
>> + },
>> + access: {
>> + permission: &Permission::Anybody,
>> + description: "Requires DATASTORE_MODIFY on both the source /datastore/{store}/{ns} and\
>> + the target /datastore/{store}/[{target-ns}]",
>> + },
>> +)]
>> +/// Move a namespace into a different one. No target means the root namespace is the target.
>> +pub fn move_namespace(
>> + store: String,
>> + ns: BackupNamespace,
>> + target_ns: Option<BackupNamespace>,
>> + _info: &ApiMethod,
>> + rpcenv: &mut dyn RpcEnvironment,
>> +) -> Result<(), Error> {
>> + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
>> + let target = target_ns.unwrap_or(BackupNamespace::root());
>> +
>> + check_ns_modification_privs(&store, &ns, &auth_id)?;
>> + check_ns_privs(&store, &target, &auth_id, PRIV_DATASTORE_MODIFY)?;
>> +
>> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
>> +
>> + datastore.namespace_move(ns, target)
>> +}
>> +
>> #[api(
>> input: {
>> properties: {
>> @@ -192,4 +230,5 @@ pub fn delete_namespace(
>> pub const ROUTER: Router = Router::new()
>> .get(&API_METHOD_LIST_NAMESPACES)
>> .post(&API_METHOD_CREATE_NAMESPACE)
>> - .delete(&API_METHOD_DELETE_NAMESPACE);
>> + .delete(&API_METHOD_DELETE_NAMESPACE)
>> + .subdirs(&[("move", &Router::new().post(&API_METHOD_MOVE_NAMESPACE))]);
>
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [pbs-devel] superseded: [PATCH proxmox-backup 0/2] fixes #6195, add support for moving namespaces
2025-09-03 10:11 [pbs-devel] [PATCH proxmox-backup 0/2] fixes #6195, add support for moving namespaces Hannes Laimer
2025-09-03 10:11 ` [pbs-devel] [PATCH proxmox-backup 1/2] fix #6195: api: datastore: add endpoint " Hannes Laimer
2025-09-03 10:11 ` [pbs-devel] [PATCH proxmox-backup 2/2] fix #6195: ui: add button/window for moving a namespace Hannes Laimer
@ 2025-09-03 12:10 ` Hannes Laimer
2 siblings, 0 replies; 6+ messages in thread
From: Hannes Laimer @ 2025-09-03 12:10 UTC (permalink / raw)
To: pbs-devel
superseded-by:
https://lore.proxmox.com/pbs-devel/20250903120844.211292-1-h.laimer@proxmox.com/T/#ma96e762273bef2d51bfbc17cc7a263deef0aed4e
On 03.09.25 12:11, Hannes Laimer wrote:
> Adds support for moving namespaces within a datastore.
> - we don't create non-existing target namespaces
> - we don't allow moving if a namespace with the same "name" already
> exists. So given:
> /a/b/c/x
> /1/2/3/4/x
> moving /1/2/3/4/x to /a/b/c/ is not allowed.
> these are the only real restrictions. Of course moving itself into
> sub-namespaces, exeeding depth limit or moving root are also not. But
> those are not really artificial.
>
>
> Hannes Laimer (2):
> fix #6195: api: datastore: add endpoint for moving namespaces
> fix #6195: ui: add button/window for moving a namespace
>
> pbs-datastore/src/datastore.rs | 121 +++++++++++++++++++++++++++++++++
> src/api2/admin/namespace.rs | 43 +++++++++++-
> www/Makefile | 1 +
> www/datastore/Content.js | 38 +++++++++++
> www/window/NamespaceMove.js | 47 +++++++++++++
> 5 files changed, 248 insertions(+), 2 deletions(-)
> create mode 100644 www/window/NamespaceMove.js
>
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-09-03 12:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-09-03 10:11 [pbs-devel] [PATCH proxmox-backup 0/2] fixes #6195, add support for moving namespaces Hannes Laimer
2025-09-03 10:11 ` [pbs-devel] [PATCH proxmox-backup 1/2] fix #6195: api: datastore: add endpoint " Hannes Laimer
2025-09-03 10:40 ` Shannon Sterz
2025-09-03 10:49 ` Hannes Laimer
2025-09-03 10:11 ` [pbs-devel] [PATCH proxmox-backup 2/2] fix #6195: ui: add button/window for moving a namespace Hannes Laimer
2025-09-03 12:10 ` [pbs-devel] superseded: [PATCH proxmox-backup 0/2] fixes #6195, add support for moving namespaces Hannes Laimer
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.