* [pbs-devel] [PATCH proxmox-backup 0/5] removable datastore follow-up @ 2024-11-26 11:43 Hannes Laimer 2024-11-26 11:43 ` [pbs-devel] [PATCH proxmox-backup 1/5] api: mainatenance: allow setting of maintenance mode if 'unmounting' Hannes Laimer ` (5 more replies) 0 siblings, 6 replies; 13+ messages in thread From: Hannes Laimer @ 2024-11-26 11:43 UTC (permalink / raw) To: pbs-devel Some follow-ups for removable datastore, including * fix sync with older pbs versions * add Sys.Modify permission for endpoints * add docs * allow resetting 'unmount' maintenance mode trough API/CLI last patch also allows it through UI, just put it in, in case we want to allow that Hannes Laimer (5): api: mainatenance: allow setting of maintenance mode if 'unmounting' api: add Sys.Modify on /system/disks as permission to endpoints handling removable datastores api: types: add 'mount_status' to schema docs: add information for removable datastores ui: allow resetting unmounting maintenance docs/storage.rst | 31 ++++++++++++++++++++++++++----- pbs-api-types/src/datastore.rs | 10 +++++++++- src/api2/admin/datastore.rs | 12 +++++++++--- src/api2/config/datastore.rs | 12 +++++++++--- www/window/MaintenanceOptions.js | 3 +-- 5 files changed, 54 insertions(+), 14 deletions(-) -- 2.39.5 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 1/5] api: mainatenance: allow setting of maintenance mode if 'unmounting' 2024-11-26 11:43 [pbs-devel] [PATCH proxmox-backup 0/5] removable datastore follow-up Hannes Laimer @ 2024-11-26 11:43 ` Hannes Laimer 2024-11-26 11:43 ` [pbs-devel] [PATCH proxmox-backup 2/5] api: add Sys.Modify on /system/disks as permission to endpoints handling removable datastores Hannes Laimer ` (4 subsequent siblings) 5 siblings, 0 replies; 13+ messages in thread From: Hannes Laimer @ 2024-11-26 11:43 UTC (permalink / raw) To: pbs-devel So it is possible to reset it after a failed unmount, or abort an unmount task by restting it through the API. Signed-off-by: Hannes Laimer <h.laimer@proxmox.com> --- pbs-api-types/src/datastore.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs index 4927f3724..203e75e38 100644 --- a/pbs-api-types/src/datastore.rs +++ b/pbs-api-types/src/datastore.rs @@ -416,7 +416,7 @@ impl DataStoreConfig { Some(MaintenanceType::ReadOnly) => { /* always OK */ } Some(MaintenanceType::Offline) => { /* always OK */ } Some(MaintenanceType::Unmount) => { - bail!("datastore is being unmounted"); + /* used to reset it after failed unmount, or alternative for aborting unmount task */ } Some(MaintenanceType::Delete) => { match new_type { -- 2.39.5 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 2/5] api: add Sys.Modify on /system/disks as permission to endpoints handling removable datastores 2024-11-26 11:43 [pbs-devel] [PATCH proxmox-backup 0/5] removable datastore follow-up Hannes Laimer 2024-11-26 11:43 ` [pbs-devel] [PATCH proxmox-backup 1/5] api: mainatenance: allow setting of maintenance mode if 'unmounting' Hannes Laimer @ 2024-11-26 11:43 ` Hannes Laimer 2024-11-26 12:07 ` Fabian Grünbichler 2024-11-26 11:43 ` [pbs-devel] [PATCH proxmox-backup 3/5] api: types: add 'mount_status' to schema Hannes Laimer ` (3 subsequent siblings) 5 siblings, 1 reply; 13+ messages in thread From: Hannes Laimer @ 2024-11-26 11:43 UTC (permalink / raw) To: pbs-devel Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com> --- src/api2/admin/datastore.rs | 12 +++++++++--- src/api2/config/datastore.rs | 12 +++++++++--- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs index 1c939bc20..cae7eb89c 100644 --- a/src/api2/admin/datastore.rs +++ b/src/api2/admin/datastore.rs @@ -45,7 +45,7 @@ use pbs_api_types::{ BACKUP_TYPE_SCHEMA, CATALOG_NAME, CLIENT_LOG_BLOB_NAME, DATASTORE_SCHEMA, IGNORE_VERIFIED_BACKUPS_SCHEMA, MANIFEST_BLOB_NAME, MAX_NAMESPACE_DEPTH, NS_MAX_DEPTH_SCHEMA, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE, - PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY, UPID, UPID_SCHEMA, + PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY, PRIV_SYS_MODIFY, UPID, UPID_SCHEMA, VERIFICATION_OUTDATED_AFTER_SCHEMA, }; use pbs_client::pxar::{create_tar, create_zip}; @@ -2512,7 +2512,10 @@ pub fn do_mount_device(datastore: DataStoreConfig) -> Result<(), Error> { schema: UPID_SCHEMA, }, access: { - permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT, false), + permission: &Permission::And(&[ + &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT, false), + &Permission::Privilege(&["system", "disks"], PRIV_SYS_MODIFY, false) + ]), }, )] /// Mount removable datastore. @@ -2625,7 +2628,10 @@ fn do_unmount_device( schema: UPID_SCHEMA, }, access: { - permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_MODIFY, true), + permission: &Permission::And(&[ + &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_MODIFY, true), + &Permission::Privilege(&["system", "disks"], PRIV_SYS_MODIFY, false) + ]), } )] /// Unmount a removable device that is associated with the datastore diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs index 121222c40..359b676a5 100644 --- a/src/api2/config/datastore.rs +++ b/src/api2/config/datastore.rs @@ -14,7 +14,7 @@ use proxmox_uuid::Uuid; use pbs_api_types::{ Authid, DataStoreConfig, DataStoreConfigUpdater, DatastoreNotify, DatastoreTuning, KeepOptions, MaintenanceMode, PruneJobConfig, PruneJobOptions, SyncDirection, DATASTORE_SCHEMA, - PRIV_DATASTORE_ALLOCATE, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_MODIFY, + PRIV_DATASTORE_ALLOCATE, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_MODIFY, PRIV_SYS_MODIFY, PROXMOX_CONFIG_DIGEST_SCHEMA, UPID_SCHEMA, }; use pbs_config::BackupLockGuard; @@ -173,7 +173,10 @@ pub(crate) fn do_create_datastore( }, }, access: { - permission: &Permission::Privilege(&["datastore"], PRIV_DATASTORE_ALLOCATE, false), + permission: &Permission::And(&[ + &Permission::Privilege(&["datastore"], PRIV_DATASTORE_ALLOCATE, false), + &Permission::Privilege(&["system", "disks"], PRIV_SYS_MODIFY, false) + ]), }, )] /// Create new datastore config. @@ -551,7 +554,10 @@ pub fn update_datastore( }, }, access: { - permission: &Permission::Privilege(&["datastore", "{name}"], PRIV_DATASTORE_ALLOCATE, false), + permission: &Permission::And(&[ + &Permission::Privilege(&["datastore", "{name}"], PRIV_DATASTORE_ALLOCATE, false), + &Permission::Privilege(&["system", "disks"], PRIV_SYS_MODIFY, false) + ]), }, returns: { schema: UPID_SCHEMA, -- 2.39.5 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 2/5] api: add Sys.Modify on /system/disks as permission to endpoints handling removable datastores 2024-11-26 11:43 ` [pbs-devel] [PATCH proxmox-backup 2/5] api: add Sys.Modify on /system/disks as permission to endpoints handling removable datastores Hannes Laimer @ 2024-11-26 12:07 ` Fabian Grünbichler 2024-11-26 12:26 ` Thomas Lamprecht 2024-11-26 13:53 ` Hannes Laimer 0 siblings, 2 replies; 13+ messages in thread From: Fabian Grünbichler @ 2024-11-26 12:07 UTC (permalink / raw) To: Proxmox Backup Server development discussion On November 26, 2024 12:43 pm, Hannes Laimer wrote: > Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> > Signed-off-by: Hannes Laimer <h.laimer@proxmox.com> > --- > src/api2/admin/datastore.rs | 12 +++++++++--- > src/api2/config/datastore.rs | 12 +++++++++--- > 2 files changed, 18 insertions(+), 6 deletions(-) > > diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs > index 1c939bc20..cae7eb89c 100644 > --- a/src/api2/admin/datastore.rs > +++ b/src/api2/admin/datastore.rs > @@ -45,7 +45,7 @@ use pbs_api_types::{ > BACKUP_TYPE_SCHEMA, CATALOG_NAME, CLIENT_LOG_BLOB_NAME, DATASTORE_SCHEMA, > IGNORE_VERIFIED_BACKUPS_SCHEMA, MANIFEST_BLOB_NAME, MAX_NAMESPACE_DEPTH, NS_MAX_DEPTH_SCHEMA, > PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE, > - PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY, UPID, UPID_SCHEMA, > + PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY, PRIV_SYS_MODIFY, UPID, UPID_SCHEMA, > VERIFICATION_OUTDATED_AFTER_SCHEMA, > }; > use pbs_client::pxar::{create_tar, create_zip}; > @@ -2512,7 +2512,10 @@ pub fn do_mount_device(datastore: DataStoreConfig) -> Result<(), Error> { > schema: UPID_SCHEMA, > }, > access: { > - permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT, false), > + permission: &Permission::And(&[ > + &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT, false), > + &Permission::Privilege(&["system", "disks"], PRIV_SYS_MODIFY, false) > + ]), I am not 100% sure this part should require Sys.Modify.. somebody needs to have set up the datastore already, just mounting seems benign in that case? > }, > )] > /// Mount removable datastore. > @@ -2625,7 +2628,10 @@ fn do_unmount_device( > schema: UPID_SCHEMA, > }, > access: { > - permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_MODIFY, true), > + permission: &Permission::And(&[ > + &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_MODIFY, true), > + &Permission::Privilege(&["system", "disks"], PRIV_SYS_MODIFY, false) > + ]), same logic would apply here.. > } > )] > /// Unmount a removable device that is associated with the datastore > diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs > index 121222c40..359b676a5 100644 > --- a/src/api2/config/datastore.rs > +++ b/src/api2/config/datastore.rs > @@ -14,7 +14,7 @@ use proxmox_uuid::Uuid; > use pbs_api_types::{ > Authid, DataStoreConfig, DataStoreConfigUpdater, DatastoreNotify, DatastoreTuning, KeepOptions, > MaintenanceMode, PruneJobConfig, PruneJobOptions, SyncDirection, DATASTORE_SCHEMA, > - PRIV_DATASTORE_ALLOCATE, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_MODIFY, > + PRIV_DATASTORE_ALLOCATE, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_MODIFY, PRIV_SYS_MODIFY, > PROXMOX_CONFIG_DIGEST_SCHEMA, UPID_SCHEMA, > }; > use pbs_config::BackupLockGuard; > @@ -173,7 +173,10 @@ pub(crate) fn do_create_datastore( > }, > }, > access: { > - permission: &Permission::Privilege(&["datastore"], PRIV_DATASTORE_ALLOCATE, false), > + permission: &Permission::And(&[ > + &Permission::Privilege(&["datastore"], PRIV_DATASTORE_ALLOCATE, false), > + &Permission::Privilege(&["system", "disks"], PRIV_SYS_MODIFY, false) > + ]), this now affects regular datastores as well, it should probably be inside the API handler and conditionalized on backing_device being set? > }, > )] > /// Create new datastore config. > @@ -551,7 +554,10 @@ pub fn update_datastore( > }, > }, > access: { > - permission: &Permission::Privilege(&["datastore", "{name}"], PRIV_DATASTORE_ALLOCATE, false), > + permission: &Permission::And(&[ > + &Permission::Privilege(&["datastore", "{name}"], PRIV_DATASTORE_ALLOCATE, false), > + &Permission::Privilege(&["system", "disks"], PRIV_SYS_MODIFY, false) > + ]), and this is not needed at all, since path and backing_device are fixed after creation? > }, > returns: { > schema: UPID_SCHEMA, > -- > 2.39.5 > > > > _______________________________________________ > pbs-devel mailing list > pbs-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel > _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 2/5] api: add Sys.Modify on /system/disks as permission to endpoints handling removable datastores 2024-11-26 12:07 ` Fabian Grünbichler @ 2024-11-26 12:26 ` Thomas Lamprecht 2024-11-26 13:53 ` Hannes Laimer 1 sibling, 0 replies; 13+ messages in thread From: Thomas Lamprecht @ 2024-11-26 12:26 UTC (permalink / raw) To: Proxmox Backup Server development discussion, Fabian Grünbichler This is missing a commit message explaining the rationale. Am 26.11.24 um 13:07 schrieb Fabian Grünbichler: >> @@ -2512,7 +2512,10 @@ pub fn do_mount_device(datastore: DataStoreConfig) -> Result<(), Error> { >> schema: UPID_SCHEMA, >> }, >> access: { >> - permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT, false), >> + permission: &Permission::And(&[ >> + &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT, false), >> + &Permission::Privilege(&["system", "disks"], PRIV_SYS_MODIFY, false) >> + ]), > I am not 100% sure this part should require Sys.Modify.. somebody needs > to have set up the datastore already, just mounting seems benign in that > case? Mounting is always a bit of an involved operation as it can result in IO hangs, just requiring audit on the store seems IMO rather to low of a requirement. The Audit privs are not for things that alter the system state, but rather for pure observation. Sys.Modify might not be ideal but IMO definitively better than the status quo, "Datastore.Modify" might be fine too though. >> }, >> )] >> /// Mount removable datastore. >> @@ -2625,7 +2628,10 @@ fn do_unmount_device( >> schema: UPID_SCHEMA, >> }, >> access: { >> - permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_MODIFY, true), >> + permission: &Permission::And(&[ >> + &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_MODIFY, true), >> + &Permission::Privilege(&["system", "disks"], PRIV_SYS_MODIFY, false) >> + ]), > same logic would apply here.. > >> } >> )] >> /// Unmount a removable device that is associated with the datastore here the status quo requires "Datastore.Modify", which is better, but if we go for Sys.Modify above I'd not have any objection to use it also here. _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 2/5] api: add Sys.Modify on /system/disks as permission to endpoints handling removable datastores 2024-11-26 12:07 ` Fabian Grünbichler 2024-11-26 12:26 ` Thomas Lamprecht @ 2024-11-26 13:53 ` Hannes Laimer 2024-11-26 14:14 ` Fabian Grünbichler 1 sibling, 1 reply; 13+ messages in thread From: Hannes Laimer @ 2024-11-26 13:53 UTC (permalink / raw) To: pbs-devel On 11/26/24 13:07, Fabian Grünbichler wrote: > On November 26, 2024 12:43 pm, Hannes Laimer wrote: >> Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> >> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com> >> --- >> src/api2/admin/datastore.rs | 12 +++++++++--- >> src/api2/config/datastore.rs | 12 +++++++++--- >> 2 files changed, 18 insertions(+), 6 deletions(-) >> >> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs >> index 1c939bc20..cae7eb89c 100644 >> --- a/src/api2/admin/datastore.rs >> +++ b/src/api2/admin/datastore.rs >> @@ -45,7 +45,7 @@ use pbs_api_types::{ >> BACKUP_TYPE_SCHEMA, CATALOG_NAME, CLIENT_LOG_BLOB_NAME, DATASTORE_SCHEMA, >> IGNORE_VERIFIED_BACKUPS_SCHEMA, MANIFEST_BLOB_NAME, MAX_NAMESPACE_DEPTH, NS_MAX_DEPTH_SCHEMA, >> PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE, >> - PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY, UPID, UPID_SCHEMA, >> + PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY, PRIV_SYS_MODIFY, UPID, UPID_SCHEMA, >> VERIFICATION_OUTDATED_AFTER_SCHEMA, >> }; >> use pbs_client::pxar::{create_tar, create_zip}; >> @@ -2512,7 +2512,10 @@ pub fn do_mount_device(datastore: DataStoreConfig) -> Result<(), Error> { >> schema: UPID_SCHEMA, >> }, >> access: { >> - permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT, false), >> + permission: &Permission::And(&[ >> + &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT, false), >> + &Permission::Privilege(&["system", "disks"], PRIV_SYS_MODIFY, false) >> + ]), > > I am not 100% sure this part should require Sys.Modify.. somebody needs > to have set up the datastore already, just mounting seems benign in that > case? > >> }, >> )] >> /// Mount removable datastore. >> @@ -2625,7 +2628,10 @@ fn do_unmount_device( >> schema: UPID_SCHEMA, >> }, >> access: { >> - permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_MODIFY, true), >> + permission: &Permission::And(&[ >> + &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_MODIFY, true), >> + &Permission::Privilege(&["system", "disks"], PRIV_SYS_MODIFY, false) >> + ]), > > same logic would apply here.. > >> } >> )] >> /// Unmount a removable device that is associated with the datastore >> diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs >> index 121222c40..359b676a5 100644 >> --- a/src/api2/config/datastore.rs >> +++ b/src/api2/config/datastore.rs >> @@ -14,7 +14,7 @@ use proxmox_uuid::Uuid; >> use pbs_api_types::{ >> Authid, DataStoreConfig, DataStoreConfigUpdater, DatastoreNotify, DatastoreTuning, KeepOptions, >> MaintenanceMode, PruneJobConfig, PruneJobOptions, SyncDirection, DATASTORE_SCHEMA, >> - PRIV_DATASTORE_ALLOCATE, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_MODIFY, >> + PRIV_DATASTORE_ALLOCATE, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_MODIFY, PRIV_SYS_MODIFY, >> PROXMOX_CONFIG_DIGEST_SCHEMA, UPID_SCHEMA, >> }; >> use pbs_config::BackupLockGuard; >> @@ -173,7 +173,10 @@ pub(crate) fn do_create_datastore( >> }, >> }, >> access: { >> - permission: &Permission::Privilege(&["datastore"], PRIV_DATASTORE_ALLOCATE, false), >> + permission: &Permission::And(&[ >> + &Permission::Privilege(&["datastore"], PRIV_DATASTORE_ALLOCATE, false), >> + &Permission::Privilege(&["system", "disks"], PRIV_SYS_MODIFY, false) >> + ]), > > this now affects regular datastores as well, it should probably be > inside the API handler and conditionalized on backing_device being set? > >> }, >> )] >> /// Create new datastore config. >> @@ -551,7 +554,10 @@ pub fn update_datastore( >> }, >> }, >> access: { >> - permission: &Permission::Privilege(&["datastore", "{name}"], PRIV_DATASTORE_ALLOCATE, false), >> + permission: &Permission::And(&[ >> + &Permission::Privilege(&["datastore", "{name}"], PRIV_DATASTORE_ALLOCATE, false), >> + &Permission::Privilege(&["system", "disks"], PRIV_SYS_MODIFY, false) >> + ]), > > and this is not needed at all, since path and backing_device are fixed > after creation? > not sure why git diff shows `update_datastore` this is for the delete endpoint. But I'll chnage that to only check when it is actually removable(as above) >> }, >> returns: { >> schema: UPID_SCHEMA, >> -- >> 2.39.5 >> >> >> >> _______________________________________________ >> pbs-devel mailing list >> pbs-devel@lists.proxmox.com >> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel >> > > > _______________________________________________ > pbs-devel mailing list > pbs-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 2/5] api: add Sys.Modify on /system/disks as permission to endpoints handling removable datastores 2024-11-26 13:53 ` Hannes Laimer @ 2024-11-26 14:14 ` Fabian Grünbichler 0 siblings, 0 replies; 13+ messages in thread From: Fabian Grünbichler @ 2024-11-26 14:14 UTC (permalink / raw) To: Proxmox Backup Server development discussion On November 26, 2024 2:53 pm, Hannes Laimer wrote: > > > On 11/26/24 13:07, Fabian Grünbichler wrote: >> On November 26, 2024 12:43 pm, Hannes Laimer wrote: >>> @@ -551,7 +554,10 @@ pub fn update_datastore( >>> }, >>> }, >>> access: { >>> - permission: &Permission::Privilege(&["datastore", "{name}"], PRIV_DATASTORE_ALLOCATE, false), >>> + permission: &Permission::And(&[ >>> + &Permission::Privilege(&["datastore", "{name}"], PRIV_DATASTORE_ALLOCATE, false), >>> + &Permission::Privilege(&["system", "disks"], PRIV_SYS_MODIFY, false) >>> + ]), >> >> and this is not needed at all, since path and backing_device are fixed >> after creation? >> > > not sure why git diff shows `update_datastore` this is for the delete > endpoint. But I'll chnage that to only check when it is actually > removable(as above) oh, missed that. yeah, for deletion one can argue that mirroring the creation ACL checks makes sense.. I think the API macro often confuses `git diff/format-patch` and gets the context wrong, not sure whether a different diff algorithm or other settings might help? _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 3/5] api: types: add 'mount_status' to schema 2024-11-26 11:43 [pbs-devel] [PATCH proxmox-backup 0/5] removable datastore follow-up Hannes Laimer 2024-11-26 11:43 ` [pbs-devel] [PATCH proxmox-backup 1/5] api: mainatenance: allow setting of maintenance mode if 'unmounting' Hannes Laimer 2024-11-26 11:43 ` [pbs-devel] [PATCH proxmox-backup 2/5] api: add Sys.Modify on /system/disks as permission to endpoints handling removable datastores Hannes Laimer @ 2024-11-26 11:43 ` Hannes Laimer 2024-11-26 11:43 ` [pbs-devel] [PATCH proxmox-backup 4/5] docs: add information for removable datastores Hannes Laimer ` (2 subsequent siblings) 5 siblings, 0 replies; 13+ messages in thread From: Hannes Laimer @ 2024-11-26 11:43 UTC (permalink / raw) To: pbs-devel ... and deserialize with default if field is missing in data. Reported-by: Aaron Lauterer <a.lauterer@proxmox.com> Fixes: 76609915d6 ("pbs-api-types: add mount_status field to DataStoreListItem") Signed-off-by: Hannes Laimer <h.laimer@proxmox.com> --- pbs-api-types/src/datastore.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs index 203e75e38..90f1195bf 100644 --- a/pbs-api-types/src/datastore.rs +++ b/pbs-api-types/src/datastore.rs @@ -452,6 +452,9 @@ impl DataStoreConfig { optional: true, schema: SINGLE_LINE_COMMENT_SCHEMA, }, + "mount-status": { + type: DataStoreMountStatus, + }, maintenance: { optional: true, format: &ApiStringFormat::PropertyString(&MaintenanceMode::API_SCHEMA), @@ -465,6 +468,7 @@ impl DataStoreConfig { pub struct DataStoreListItem { pub store: String, pub comment: Option<String>, + #[serde(default)] pub mount_status: DataStoreMountStatus, /// If the datastore is in maintenance mode, information about it #[serde(skip_serializing_if = "Option::is_none")] @@ -1447,6 +1451,9 @@ pub struct DataStoreStatus { store: { schema: DATASTORE_SCHEMA, }, + "mount-status": { + type: DataStoreMountStatus, + }, history: { type: Array, optional: true, @@ -1471,6 +1478,7 @@ pub struct DataStoreStatusListItem { /// The available bytes of the underlying storage. (-1 on error) #[serde(skip_serializing_if = "Option::is_none")] pub avail: Option<u64>, + #[serde(default)] pub mount_status: DataStoreMountStatus, /// A list of usages of the past (last Month). #[serde(skip_serializing_if = "Option::is_none")] -- 2.39.5 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 4/5] docs: add information for removable datastores 2024-11-26 11:43 [pbs-devel] [PATCH proxmox-backup 0/5] removable datastore follow-up Hannes Laimer ` (2 preceding siblings ...) 2024-11-26 11:43 ` [pbs-devel] [PATCH proxmox-backup 3/5] api: types: add 'mount_status' to schema Hannes Laimer @ 2024-11-26 11:43 ` Hannes Laimer 2024-11-26 11:43 ` [pbs-devel] [PATCH proxmox-backup 5/5] ui: allow resetting unmounting maintenance Hannes Laimer 2024-11-26 12:09 ` [pbs-devel] partially applied: [PATCH proxmox-backup 0/5] removable datastore follow-up Fabian Grünbichler 5 siblings, 0 replies; 13+ messages in thread From: Hannes Laimer @ 2024-11-26 11:43 UTC (permalink / raw) To: pbs-devel Specifically about jobs and how they behave when the datastore is not mounted, how to create and use deivices with multiple datatstores on multiple PBS instances and options how to handle failed unmounts. Signed-off-by: Hannes Laimer <h.laimer@proxmox.com> --- docs/storage.rst | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/docs/storage.rst b/docs/storage.rst index 361af4420..5cd8704c4 100644 --- a/docs/storage.rst +++ b/docs/storage.rst @@ -176,16 +176,32 @@ datastores, should be either ``ext4`` or ``xfs``. It is also possible to create on completely unused disks through "Administration" > "Disks / Storage" > "Directory", using this method the disk will be partitioned and formatted automatically for the datastore. -Devices with only one datastore on them will be mounted automatically. It is possible to create a -removable datastore on one PBS and use it on multiple instances, the device just has to be added -on each instance as a removable datastore by checking "reuse datastore" on creation. -If the device already contains a datastore at the specified path it'll just be added as -a new datastore to the PBS instance and will be mounted whenever plugged in. Unmounting has +Devices with only one datastore on them will be mounted automatically. Unmounting has to be done through the UI by clicking "Unmount" on the summary page or using the CLI. +If unmounting should fail, the reason is logged in the unmount-task, and the datastore +will stay in maintenance mode ``unmounting``, which prevents any IO operations. If that should +happen, the maintenace mode has to be reset manually using: + +.. code-block:: console + + # proxmox-backup-manager datastore update --maintenance-mode offline + +to prevent any IO, or to clear it use: + +.. code-block:: console + + # proxmox-backup-manager datastore update --delete maintenance-mode + A single device can house multiple datastores, they only limitation is that they are not allowed to be nested. +Removable datastores are created on the the device with the given relative path that is specified +on creation. In order to use a datastore on multiple PBS instances, it has to be created on one, +and added with ``Reuse existing datastore`` checked on the others. The path you set on creation +is how multiple datastores on a signle device are identified. So When adding on a new PBS instance, +it has to match what was set on creation. + .. code-block:: console # proxmox-backup-manager datastore unmount store1 @@ -202,6 +218,11 @@ All datastores present on a device can be listed using ``proxmox-backup-debug``. # proxmox-backup-debug inspect device /dev/... +Verify jobs are skipped if the removable datastore should not be mounted when they are scheduled, +Sync jobs start, but fail with an error saying the datastore was not mounted. The reason is that +syncs not happening as schduled should at least be noticable. GC and pruning, like verification, +is skipped without a failed task if the datastore should not be mounted. + Managing Datastores ^^^^^^^^^^^^^^^^^^^ -- 2.39.5 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 5/5] ui: allow resetting unmounting maintenance 2024-11-26 11:43 [pbs-devel] [PATCH proxmox-backup 0/5] removable datastore follow-up Hannes Laimer ` (3 preceding siblings ...) 2024-11-26 11:43 ` [pbs-devel] [PATCH proxmox-backup 4/5] docs: add information for removable datastores Hannes Laimer @ 2024-11-26 11:43 ` Hannes Laimer 2024-11-26 12:11 ` Fabian Grünbichler 2024-11-26 15:35 ` [pbs-devel] applied: " Thomas Lamprecht 2024-11-26 12:09 ` [pbs-devel] partially applied: [PATCH proxmox-backup 0/5] removable datastore follow-up Fabian Grünbichler 5 siblings, 2 replies; 13+ messages in thread From: Hannes Laimer @ 2024-11-26 11:43 UTC (permalink / raw) To: pbs-devel Signed-off-by: Hannes Laimer <h.laimer@proxmox.com> --- optional, just added it in case we want it www/window/MaintenanceOptions.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/www/window/MaintenanceOptions.js b/www/window/MaintenanceOptions.js index 896d6a58e..8a88ba2b7 100644 --- a/www/window/MaintenanceOptions.js +++ b/www/window/MaintenanceOptions.js @@ -89,12 +89,11 @@ Ext.define('PBS.window.MaintenanceOptions', { let unmounting = options['maintenance-type'] === 'unmount'; let defaultType = options['maintenance-type'] === '__default__'; if (unmounting) { - options['maintenance-type'] = ''; + options['maintenance-type'] = gettext('Unmounting'); } me.callParent([options]); - me.lookupReference('type-field').setDisabled(unmounting); me.lookupReference('message-field').setDisabled(unmounting || defaultType); }, }); -- 2.39.5 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 5/5] ui: allow resetting unmounting maintenance 2024-11-26 11:43 ` [pbs-devel] [PATCH proxmox-backup 5/5] ui: allow resetting unmounting maintenance Hannes Laimer @ 2024-11-26 12:11 ` Fabian Grünbichler 2024-11-26 15:35 ` [pbs-devel] applied: " Thomas Lamprecht 1 sibling, 0 replies; 13+ messages in thread From: Fabian Grünbichler @ 2024-11-26 12:11 UTC (permalink / raw) To: Proxmox Backup Server development discussion Acked-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> but not applied in case somebody else wants to keep this away from the UI for now ;) On November 26, 2024 12:43 pm, Hannes Laimer wrote: > Signed-off-by: Hannes Laimer <h.laimer@proxmox.com> > --- > optional, just added it in case we want it > > www/window/MaintenanceOptions.js | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/www/window/MaintenanceOptions.js b/www/window/MaintenanceOptions.js > index 896d6a58e..8a88ba2b7 100644 > --- a/www/window/MaintenanceOptions.js > +++ b/www/window/MaintenanceOptions.js > @@ -89,12 +89,11 @@ Ext.define('PBS.window.MaintenanceOptions', { > let unmounting = options['maintenance-type'] === 'unmount'; > let defaultType = options['maintenance-type'] === '__default__'; > if (unmounting) { > - options['maintenance-type'] = ''; > + options['maintenance-type'] = gettext('Unmounting'); > } > > me.callParent([options]); > > - me.lookupReference('type-field').setDisabled(unmounting); > me.lookupReference('message-field').setDisabled(unmounting || defaultType); > }, > }); > -- > 2.39.5 > > > > _______________________________________________ > pbs-devel mailing list > pbs-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel > > > _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* [pbs-devel] applied: [PATCH proxmox-backup 5/5] ui: allow resetting unmounting maintenance 2024-11-26 11:43 ` [pbs-devel] [PATCH proxmox-backup 5/5] ui: allow resetting unmounting maintenance Hannes Laimer 2024-11-26 12:11 ` Fabian Grünbichler @ 2024-11-26 15:35 ` Thomas Lamprecht 1 sibling, 0 replies; 13+ messages in thread From: Thomas Lamprecht @ 2024-11-26 15:35 UTC (permalink / raw) To: Proxmox Backup Server development discussion, Hannes Laimer Am 26.11.24 um 12:43 schrieb Hannes Laimer: > Signed-off-by: Hannes Laimer <h.laimer@proxmox.com> > --- > optional, just added it in case we want it > > www/window/MaintenanceOptions.js | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > applied, thanks! _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* [pbs-devel] partially applied: [PATCH proxmox-backup 0/5] removable datastore follow-up 2024-11-26 11:43 [pbs-devel] [PATCH proxmox-backup 0/5] removable datastore follow-up Hannes Laimer ` (4 preceding siblings ...) 2024-11-26 11:43 ` [pbs-devel] [PATCH proxmox-backup 5/5] ui: allow resetting unmounting maintenance Hannes Laimer @ 2024-11-26 12:09 ` Fabian Grünbichler 5 siblings, 0 replies; 13+ messages in thread From: Fabian Grünbichler @ 2024-11-26 12:09 UTC (permalink / raw) To: Proxmox Backup Server development discussion all but patch #2 and #5 and with a small follow-up for the docs patch and some typos in commit one fixed up ;) On November 26, 2024 12:43 pm, Hannes Laimer wrote: > Some follow-ups for removable datastore, including > * fix sync with older pbs versions > * add Sys.Modify permission for endpoints > * add docs > * allow resetting 'unmount' maintenance mode trough API/CLI > last patch also allows it through UI, just put it in, in case > we want to allow that > > > Hannes Laimer (5): > api: mainatenance: allow setting of maintenance mode if 'unmounting' > api: add Sys.Modify on /system/disks as permission to endpoints > handling removable datastores > api: types: add 'mount_status' to schema > docs: add information for removable datastores > ui: allow resetting unmounting maintenance > > docs/storage.rst | 31 ++++++++++++++++++++++++++----- > pbs-api-types/src/datastore.rs | 10 +++++++++- > src/api2/admin/datastore.rs | 12 +++++++++--- > src/api2/config/datastore.rs | 12 +++++++++--- > www/window/MaintenanceOptions.js | 3 +-- > 5 files changed, 54 insertions(+), 14 deletions(-) > > -- > 2.39.5 > > > > _______________________________________________ > pbs-devel mailing list > pbs-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel > > > _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2024-11-26 15:35 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-11-26 11:43 [pbs-devel] [PATCH proxmox-backup 0/5] removable datastore follow-up Hannes Laimer 2024-11-26 11:43 ` [pbs-devel] [PATCH proxmox-backup 1/5] api: mainatenance: allow setting of maintenance mode if 'unmounting' Hannes Laimer 2024-11-26 11:43 ` [pbs-devel] [PATCH proxmox-backup 2/5] api: add Sys.Modify on /system/disks as permission to endpoints handling removable datastores Hannes Laimer 2024-11-26 12:07 ` Fabian Grünbichler 2024-11-26 12:26 ` Thomas Lamprecht 2024-11-26 13:53 ` Hannes Laimer 2024-11-26 14:14 ` Fabian Grünbichler 2024-11-26 11:43 ` [pbs-devel] [PATCH proxmox-backup 3/5] api: types: add 'mount_status' to schema Hannes Laimer 2024-11-26 11:43 ` [pbs-devel] [PATCH proxmox-backup 4/5] docs: add information for removable datastores Hannes Laimer 2024-11-26 11:43 ` [pbs-devel] [PATCH proxmox-backup 5/5] ui: allow resetting unmounting maintenance Hannes Laimer 2024-11-26 12:11 ` Fabian Grünbichler 2024-11-26 15:35 ` [pbs-devel] applied: " Thomas Lamprecht 2024-11-26 12:09 ` [pbs-devel] partially applied: [PATCH proxmox-backup 0/5] removable datastore follow-up Fabian Grünbichler
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox