[pbs-devel] [RFC v2 proxmox-backup 18/21] api: admin: implement endpoints to restore trashed contents
Fabian Grünbichler
f.gruenbichler at proxmox.com
Mon May 12 12:03:03 CEST 2025
On May 9, 2025 2:59 pm, Christian Ebner wrote:
> Thanks for feedback, will have a closer look next week.
>
> Allow me two quick questions inline though...
>
> On 5/9/25 14:27, Fabian Grünbichler wrote:
>> On May 8, 2025 3:05 pm, Christian Ebner wrote:
>>> Implements the api endpoints to restore trashed contents contained
>>> within namespaces, backup groups or individual snapshots.
>>>
>>> Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
>>> ---
>>> src/api2/admin/datastore.rs | 173 +++++++++++++++++++++++++++++++++++-
>>> 1 file changed, 172 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
>>> index cbd24c729..eb033c3fc 100644
>>> --- a/src/api2/admin/datastore.rs
>>> +++ b/src/api2/admin/datastore.rs
>>> @@ -51,7 +51,7 @@ use pbs_api_types::{
>>> };
>>> use pbs_client::pxar::{create_tar, create_zip};
>>> use pbs_config::CachedUserInfo;
>>> -use pbs_datastore::backup_info::{BackupInfo, ListBackupFilter};
>>> +use pbs_datastore::backup_info::{BackupInfo, ListBackupFilter, TRASH_MARKER_FILENAME};
>>> use pbs_datastore::cached_chunk_reader::CachedChunkReader;
>>> use pbs_datastore::catalog::{ArchiveEntry, CatalogReader};
>>> use pbs_datastore::data_blob::DataBlob;
>>> @@ -2727,6 +2727,165 @@ pub async fn unmount(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<V
>>> Ok(json!(upid))
>>> }
>>>
>>> +#[api(
>>> + input: {
>>> + properties: {
>>> + store: { schema: DATASTORE_SCHEMA },
>>> + ns: { type: BackupNamespace, },
>>> + },
>>> + },
>>> + access: {
>>> + permission: &Permission::Anybody,
>>> + description: "Requires on /datastore/{store}[/{namespace}] either DATASTORE_MODIFY for any \
>>> + or DATASTORE_BACKUP and being the owner of the group",
>>> + },
>>> +)]
>>> +/// Recover trashed contents of a namespace.
>>> +pub fn recover_namespace(
>>> + store: String,
>>> + ns: BackupNamespace,
>>> + rpcenv: &mut dyn RpcEnvironment,
>>> +) -> Result<(), Error> {
>>> + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
>>> + let limited = check_ns_privs_full(
>>> + &store,
>>> + &ns,
>>> + &auth_id,
>>> + PRIV_DATASTORE_MODIFY,
>>> + PRIV_DATASTORE_BACKUP,
>>> + )?;
>>> +
>>> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
>>> +
>>> + for backup_group in datastore.iter_backup_groups(ns.clone())? {
>>> + let backup_group = backup_group?;
>>> + if limited {
>>> + let owner = datastore.get_owner(&ns, backup_group.group())?;
>>> + if check_backup_owner(&owner, &auth_id).is_err() {
>>> + continue;
>>> + }
>>> + }
>>> + do_recover_group(&backup_group)?;
>>> + }
>>> +
>>> + Ok(())
>>> +}
>>> +
>>> +#[api(
>>> + input: {
>>> + properties: {
>>> + store: { schema: DATASTORE_SCHEMA },
>>> + group: {
>>> + type: pbs_api_types::BackupGroup,
>>> + flatten: true,
>>> + },
>>> + ns: {
>>> + type: BackupNamespace,
>>> + optional: true,
>>> + },
>>> + },
>>> + },
>>> + access: {
>>> + permission: &Permission::Anybody,
>>> + description: "Requires on /datastore/{store}[/{namespace}] either DATASTORE_MODIFY for any \
>>> + or DATASTORE_BACKUP and being the owner of the group",
>>> + },
>>> +)]
>>> +/// Recover trashed contents of a backup group.
>>> +pub fn recover_group(
>>> + store: String,
>>> + group: pbs_api_types::BackupGroup,
>>> + ns: Option<BackupNamespace>,
>>> + rpcenv: &mut dyn RpcEnvironment,
>>> +) -> Result<(), Error> {
>>> + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
>>> + let ns = ns.unwrap_or_default();
>>> + let datastore = check_privs_and_load_store(
>>> + &store,
>>> + &ns,
>>> + &auth_id,
>>> + PRIV_DATASTORE_MODIFY,
>>> + PRIV_DATASTORE_BACKUP,
>>> + Some(Operation::Write),
>>> + &group,
>>> + )?;
>>> +
>>> + let backup_group = datastore.backup_group(ns, group);
>>> + do_recover_group(&backup_group)?;
>>> +
>>> + Ok(())
>>> +}
>>> +
>>> +fn do_recover_group(backup_group: &BackupGroup) -> Result<(), Error> {
>>
>> missing locking for the group?
>
> Not sure about that one. After all the group is trashed at least as long
> as all the snapshots are trashed. And GC will only ever clean up the
> group folder if the trash marker is not set. So I do not see a reason
> why this should be locked.
because logically, this is the inverse of BackupGroup::destroy with
skip_trash=false, and that locks the group..
else you could have a recovery and a full deletion running concurrently
for the same group. also, while you are recovering the group you
probably don't want to start a new backup snapshot, which would also be
possible with the missing lock.
>>> + let trashed_snapshots = backup_group.list_backups(ListBackupFilter::Trashed)?;
>>> + for snapshot in trashed_snapshots {
>>> + do_recover_snapshot(&snapshot.backup_dir)?;
>>> + }
>>> +
>>> + let group_trash_path = backup_group.full_group_path().join(TRASH_MARKER_FILENAME);
>>> + if let Err(err) = std::fs::remove_file(&group_trash_path) {
>>> + if err.kind() != std::io::ErrorKind::NotFound {
>>> + bail!("failed to remove group trash file {group_trash_path:?} - {err}");
>>> + }
>>> + }
>>> + Ok(())
>>> +}
>>> +
>>> +#[api(
>>> + input: {
>>> + properties: {
>>> + store: { schema: DATASTORE_SCHEMA },
>>> + backup_dir: {
>>> + type: pbs_api_types::BackupDir,
>>> + flatten: true,
>>> + },
>>> + ns: {
>>> + type: BackupNamespace,
>>> + optional: true,
>>> + },
>>> + },
>>> + },
>>> + access: {
>>> + permission: &Permission::Anybody,
>>> + description: "Requires on /datastore/{store}[/{namespace}] either DATASTORE_MODIFY for any \
>>> + or DATASTORE_BACKUP and being the owner of the group",
>>> + },
>>> +)]
>>> +/// Recover trashed contents of a backup snapshot.
>>> +pub fn recover_snapshot(
>>> + store: String,
>>> + backup_dir: pbs_api_types::BackupDir,
>>> + ns: Option<BackupNamespace>,
>>> + rpcenv: &mut dyn RpcEnvironment,
>>> +) -> Result<(), Error> {
>>> + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
>>> + let ns = ns.unwrap_or_default();
>>> + let datastore = check_privs_and_load_store(
>>> + &store,
>>> + &ns,
>>> + &auth_id,
>>> + PRIV_DATASTORE_MODIFY,
>>> + PRIV_DATASTORE_BACKUP,
>>> + Some(Operation::Write),
>>> + &backup_dir.group,
>>> + )?;
>>> +
>>> + let snapshot = datastore.backup_dir(ns, backup_dir)?;
>>> + do_recover_snapshot(&snapshot)?;
>>> +
>>> + Ok(())
>>> +}
>>> +
>>> +fn do_recover_snapshot(snapshot_dir: &BackupDir) -> Result<(), Error> {
>>
>> missing locking for the snapshot?
>
> Why? remove_file() should be atomic?
but a skip_trash=true deletion might be going on already or some other
operation holding a lock on the snapshot that doesn't want the 'trash'
status being changed underneath it?
>>
>>> + let trash_path = snapshot_dir.full_path().join(TRASH_MARKER_FILENAME);
>>> + if let Err(err) = std::fs::remove_file(&trash_path) {
>>> + if err.kind() != std::io::ErrorKind::NotFound {
>>> + bail!("failed to remove trash file {trash_path:?} - {err}");
>>> + }
>>> + }
>>> + Ok(())
>>> +}
>>> +
>>> #[sortable]
>>> const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
>>> (
>>> @@ -2792,6 +2951,18 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
>>> "pxar-file-download",
>>> &Router::new().download(&API_METHOD_PXAR_FILE_DOWNLOAD),
>>> ),
>>> + (
>>> + "recover-group",
>>> + &Router::new().post(&API_METHOD_RECOVER_GROUP),
>>
>> I am not sure whether those should be POST or PUT, they are modifying an
>> existing (trashed) group/snapshot/.. after all?
>>
>>> + ),
>>> + (
>>> + "recover-namespace",
>>> + &Router::new().post(&API_METHOD_RECOVER_NAMESPACE),
>>> + ),
>>> + (
>>> + "recover-snapshot",
>>> + &Router::new().post(&API_METHOD_RECOVER_SNAPSHOT),
>>> + ),
>>> ("rrd", &Router::new().get(&API_METHOD_GET_RRD_STATS)),
>>> (
>>> "snapshots",
>>> --
>>> 2.39.5
>>>
>>>
>>>
>>> _______________________________________________
>>> pbs-devel mailing list
>>> pbs-devel at lists.proxmox.com
>>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>>>
>>>
>>>
>>
>>
>> _______________________________________________
>> pbs-devel mailing list
>> pbs-devel at lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>>
>>
>
>
More information about the pbs-devel
mailing list