* [pbs-devel] [PATCH proxmox-backup v9 0/6] closes #3071: maintenance mode for datastore
@ 2022-02-17 17:14 Hannes Laimer
2022-02-17 17:14 ` [pbs-devel] [PATCH proxmox-backup v9 1/6] api-types: add maintenance type Hannes Laimer
` (6 more replies)
0 siblings, 7 replies; 12+ messages in thread
From: Hannes Laimer @ 2022-02-17 17:14 UTC (permalink / raw)
To: pbs-devel
Adds maintenance mode and tracking of active reading/writing operations.
The maintenance mode prevents the start of new operations if the type of
operation they would perform on the datastore would conflict with the
maintenance type that is currently set. This check is performed when
lookup_datastore is called. Tasks only call this function once at the
beginning, therefore updating the maintenance type cannot interfere with
already running tasks.
active operations tracking:
Changed file layout to now also keep track of the pid+starttime and the
counts of operations that that pid started, like this it is possible to
not count operations that were started by a dead process, since they
are also not active anymore. Whenever the file is updated, also entries
of dead processes are removed. When the file is read, only entries of
active processes are counted.
The UI shows a spinner with the count of conflictintg tasks (the tasks
that were started before the maintenance type was updated) next to it.
As soon as all conflicting tasks are finished a checkmark appears.
v9:
- bump proxmox-schema dep to 1.2.1
- loosen MAINTENANCE_MESSAGE_REGEX
- MaintenanceMode fn check: use and_then()
- ui: properly (un)escape double quotes in message
v8:
- replace enum with MaintenanceMode struct
- impl check function on MaintenanceMode struct
- api now encodes type+msg in one string
- message has to be encoded to ensure proper parsing
(everywhere except the UI the message has to be already encoded
when passed, API+CLI)
v7:
- tracking: fix counting on clone
- ui: correct gettext usages + remove usage of capitalize
v6:
- also use process start time in order to avoid pid clashes(as suggested
by Thomas and somehow missed by me in the last version)
- now a single call of get_active_operations return reads and writes
- improved code structure
- don't lock when reading
v5:
- use simple struct and serde instead of manual parsing for file
- move tracking related stuff into new file (task_tracking.rs)
v4:
- clones are not also tracked
- use lockfile, instead of locking the file
- track pid of the process which started smth
- updating maintenance mode is now always possible
- add get_active_operations endpoint for datastore
- ui: show count of conflicting tasks (or checkmark if no conflicting
operations are active)
v3, based on Dominik Csapak <d.csapak@proxmox.com>'s feedback:
- added Operation enum(r/w), as suggested by
- added active operation tracking
- combine type and message into on field
v2:
- check for maintenance now directly in lookup_datastore
- parameter for checking is now the last acceptable maintenance type,
description in commit msg of 2nd patch
- ui cleanup
Hannes Laimer (6):
api-types: add maintenance type
datastore: add check for maintenance in lookup
pbs-datastore: add active operations tracking
api: make maintenance_type updatable
api: add get_active_operations endpoint
ui: add option to change the maintenance type
debian/control | 6 +-
pbs-api-types/Cargo.toml | 2 +-
pbs-api-types/src/datastore.rs | 24 +++++-
pbs-api-types/src/lib.rs | 3 +
pbs-api-types/src/maintenance.rs | 78 +++++++++++++++++
pbs-datastore/Cargo.toml | 1 +
pbs-datastore/src/datastore.rs | 121 +++++++++++++++++++--------
pbs-datastore/src/lib.rs | 4 +
pbs-datastore/src/snapshot_reader.rs | 6 +-
pbs-datastore/src/task_tracking.rs | 110 ++++++++++++++++++++++++
src/api2/admin/datastore.rs | 81 ++++++++++++------
src/api2/backup/mod.rs | 4 +-
src/api2/config/datastore.rs | 5 ++
src/api2/reader/mod.rs | 6 +-
src/api2/status.rs | 4 +-
src/api2/tape/backup.rs | 6 +-
src/api2/tape/restore.rs | 6 +-
src/bin/proxmox-backup-api.rs | 1 +
src/bin/proxmox-backup-proxy.rs | 4 +-
src/server/mod.rs | 16 +++-
src/server/prune_job.rs | 4 +-
src/server/pull.rs | 4 +-
src/server/verify_job.rs | 4 +-
www/Makefile | 1 +
www/Utils.js | 23 +++++
www/datastore/OptionView.js | 30 +++++++
www/window/MaintenanceOptions.js | 77 +++++++++++++++++
27 files changed, 540 insertions(+), 91 deletions(-)
create mode 100644 pbs-api-types/src/maintenance.rs
create mode 100644 pbs-datastore/src/task_tracking.rs
create mode 100644 www/window/MaintenanceOptions.js
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v9 1/6] api-types: add maintenance type
2022-02-17 17:14 [pbs-devel] [PATCH proxmox-backup v9 0/6] closes #3071: maintenance mode for datastore Hannes Laimer
@ 2022-02-17 17:14 ` Hannes Laimer
2022-04-06 13:08 ` Dylan Whyte
2022-02-17 17:14 ` [pbs-devel] [PATCH proxmox-backup v9 2/6] datastore: add check for maintenance in lookup Hannes Laimer
` (5 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Hannes Laimer @ 2022-02-17 17:14 UTC (permalink / raw)
To: pbs-devel
+ bump proxmox-schema dep to 1.2.1 (for quoted property string)
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
debian/control | 6 ++--
pbs-api-types/Cargo.toml | 2 +-
pbs-api-types/src/datastore.rs | 24 ++++++++++---
pbs-api-types/src/lib.rs | 3 ++
pbs-api-types/src/maintenance.rs | 59 ++++++++++++++++++++++++++++++++
5 files changed, 86 insertions(+), 8 deletions(-)
create mode 100644 pbs-api-types/src/maintenance.rs
diff --git a/debian/control b/debian/control
index 02a32977..f869d32d 100644
--- a/debian/control
+++ b/debian/control
@@ -56,9 +56,9 @@ Build-Depends: debhelper (>= 12),
librust-proxmox-openid-0.9+default-dev,
librust-proxmox-router-1+cli-dev (>= 1.1-~~),
librust-proxmox-router-1+default-dev (>= 1.1-~~),
- librust-proxmox-schema-1+api-macro-dev (>= 1.1-~~),
- librust-proxmox-schema-1+default-dev (>= 1.1-~~),
- librust-proxmox-schema-1+upid-api-impl-dev (>= 1.1-~~),
+ librust-proxmox-schema-1+api-macro-dev (>= 1.2.1-~~),
+ librust-proxmox-schema-1+default-dev (>= 1.2.1-~~),
+ librust-proxmox-schema-1+upid-api-impl-dev (>= 1.2.1-~~),
librust-proxmox-section-config-1+default-dev,
librust-proxmox-serde-0.1+default-dev,
librust-proxmox-shared-memory-0.2+default-dev,
diff --git a/pbs-api-types/Cargo.toml b/pbs-api-types/Cargo.toml
index e77d8bc4..1a98b09d 100644
--- a/pbs-api-types/Cargo.toml
+++ b/pbs-api-types/Cargo.toml
@@ -14,7 +14,7 @@ regex = "1.2"
serde = { version = "1.0", features = ["derive"] }
proxmox-lang = "1.0.0"
-proxmox-schema = { version = "1.1", features = [ "api-macro" ] }
+proxmox-schema = { version = "1.2.1", features = [ "api-macro" ] }
proxmox-serde = "0.1"
proxmox-time = "1.1.1"
proxmox-uuid = { version = "1.0.0", features = [ "serde" ] }
diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index 36279b3a..7a2d6ae2 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -6,10 +6,9 @@ use proxmox_schema::{
};
use crate::{
- PROXMOX_SAFE_ID_FORMAT, SHA256_HEX_REGEX, SINGLE_LINE_COMMENT_SCHEMA, CryptMode, UPID,
- Fingerprint, Userid, Authid,
- GC_SCHEDULE_SCHEMA, DATASTORE_NOTIFY_STRING_SCHEMA, PRUNE_SCHEDULE_SCHEMA,
-
+ Authid, CryptMode, Fingerprint, MaintenanceMode, Userid, DATASTORE_NOTIFY_STRING_SCHEMA,
+ GC_SCHEDULE_SCHEMA, PROXMOX_SAFE_ID_FORMAT, PRUNE_SCHEDULE_SCHEMA, SHA256_HEX_REGEX,
+ SINGLE_LINE_COMMENT_SCHEMA, UPID,
};
const_regex!{
@@ -224,6 +223,11 @@ pub struct PruneOptions {
optional: true,
type: bool,
},
+ "maintenance-mode": {
+ optional: true,
+ format: &ApiStringFormat::PropertyString(&MaintenanceMode::API_SCHEMA),
+ type: String,
+ },
}
)]
#[derive(Serialize,Deserialize,Updater)]
@@ -261,6 +265,18 @@ pub struct DataStoreConfig {
/// Send notification only for job errors
#[serde(skip_serializing_if="Option::is_none")]
pub notify: Option<String>,
+ /// Maintenance mode, type is either 'offline' or 'read-only', message should be enclosed in "
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub maintenance_mode: Option<String>,
+}
+
+impl DataStoreConfig {
+ pub fn get_maintenance_mode(&self) -> Option<MaintenanceMode> {
+ self.maintenance_mode
+ .as_ref()
+ .and_then(|str| MaintenanceMode::API_SCHEMA.parse_property_string(str).ok())
+ .and_then(|value| MaintenanceMode::deserialize(value).ok())
+ }
}
#[api(
diff --git a/pbs-api-types/src/lib.rs b/pbs-api-types/src/lib.rs
index 754e7b22..efb01c3e 100644
--- a/pbs-api-types/src/lib.rs
+++ b/pbs-api-types/src/lib.rs
@@ -49,6 +49,9 @@ pub use jobs::*;
mod key_derivation;
pub use key_derivation::{Kdf, KeyInfo};
+mod maintenance;
+pub use maintenance::*;
+
mod network;
pub use network::*;
diff --git a/pbs-api-types/src/maintenance.rs b/pbs-api-types/src/maintenance.rs
new file mode 100644
index 00000000..19883e46
--- /dev/null
+++ b/pbs-api-types/src/maintenance.rs
@@ -0,0 +1,59 @@
+use serde::{Deserialize, Serialize};
+
+use proxmox_schema::{api, ApiStringFormat, const_regex, Schema, StringSchema};
+
+const_regex!{
+ pub MAINTENANCE_MESSAGE_REGEX = r"^[[:^cntrl:]]*$";
+}
+
+pub const MAINTENANCE_MESSAGE_FORMAT: ApiStringFormat =
+ ApiStringFormat::Pattern(&MAINTENANCE_MESSAGE_REGEX);
+
+
+pub const MAINTENANCE_MESSAGE_SCHEMA: Schema =
+ StringSchema::new("Message describing the reason for the maintenance.")
+ .format(&MAINTENANCE_MESSAGE_FORMAT)
+ .max_length(32)
+ .schema();
+
+#[derive(Clone, Copy, Debug)]
+/// Operation requirements, used when checking for maintenance mode.
+pub enum Operation {
+ Read,
+ Write,
+}
+
+#[api]
+#[derive(Deserialize, Serialize)]
+#[serde(rename_all="kebab-case")]
+/// Maintenance type.
+pub enum MaintenanceType {
+ /// Only reading operations are allowed on the datastore.
+ ReadOnly,
+ /// Neither reading nor writing operations are allowed on the datastore.
+ Offline,
+}
+
+#[api(
+ properties: {
+ type: {
+ type: MaintenanceType,
+ },
+ message: {
+ optional: true,
+ schema: MAINTENANCE_MESSAGE_SCHEMA,
+ }
+ },
+ default_key: "type",
+)]
+#[derive(Deserialize, Serialize)]
+/// Maintenance mode
+pub struct MaintenanceMode {
+ /// Type of the maintenance
+ #[serde(rename = "type")]
+ ty: MaintenanceType,
+
+ /// Reason for the maintenance mode.
+ #[serde(skip_serializing_if = "Option::is_none")]
+ message: Option<String>,
+}
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v9 2/6] datastore: add check for maintenance in lookup
2022-02-17 17:14 [pbs-devel] [PATCH proxmox-backup v9 0/6] closes #3071: maintenance mode for datastore Hannes Laimer
2022-02-17 17:14 ` [pbs-devel] [PATCH proxmox-backup v9 1/6] api-types: add maintenance type Hannes Laimer
@ 2022-02-17 17:14 ` Hannes Laimer
2022-04-06 13:08 ` Dylan Whyte
2022-02-17 17:14 ` [pbs-devel] [PATCH proxmox-backup v9 3/6] pbs-datastore: add active operations tracking Hannes Laimer
` (4 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Hannes Laimer @ 2022-02-17 17:14 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
pbs-api-types/src/maintenance.rs | 23 +++++++++++--
pbs-datastore/src/datastore.rs | 17 +++++++---
pbs-datastore/src/snapshot_reader.rs | 6 +++-
src/api2/admin/datastore.rs | 50 ++++++++++++++--------------
src/api2/backup/mod.rs | 4 +--
src/api2/reader/mod.rs | 6 ++--
src/api2/status.rs | 4 +--
src/api2/tape/backup.rs | 6 ++--
src/api2/tape/restore.rs | 6 ++--
src/bin/proxmox-backup-proxy.rs | 4 +--
src/server/prune_job.rs | 4 +--
src/server/pull.rs | 4 +--
src/server/verify_job.rs | 4 +--
13 files changed, 85 insertions(+), 53 deletions(-)
diff --git a/pbs-api-types/src/maintenance.rs b/pbs-api-types/src/maintenance.rs
index 19883e46..55d66964 100644
--- a/pbs-api-types/src/maintenance.rs
+++ b/pbs-api-types/src/maintenance.rs
@@ -1,3 +1,5 @@
+use std::borrow::Cow;
+use anyhow::{bail, Error};
use serde::{Deserialize, Serialize};
use proxmox_schema::{api, ApiStringFormat, const_regex, Schema, StringSchema};
@@ -24,7 +26,7 @@ pub enum Operation {
}
#[api]
-#[derive(Deserialize, Serialize)]
+#[derive(Deserialize, Serialize, PartialEq)]
#[serde(rename_all="kebab-case")]
/// Maintenance type.
pub enum MaintenanceType {
@@ -49,7 +51,7 @@ pub enum MaintenanceType {
#[derive(Deserialize, Serialize)]
/// Maintenance mode
pub struct MaintenanceMode {
- /// Type of the maintenance
+ /// Type of the maintenance.
#[serde(rename = "type")]
ty: MaintenanceType,
@@ -57,3 +59,20 @@ pub struct MaintenanceMode {
#[serde(skip_serializing_if = "Option::is_none")]
message: Option<String>,
}
+
+impl MaintenanceMode {
+ pub fn check(&self, operation: Option<Operation>) -> Result<(), Error> {
+ let message = percent_encoding::percent_decode_str(self.message.as_deref().unwrap_or(""))
+ .decode_utf8()
+ .unwrap_or(Cow::Borrowed(""));
+
+ if self.ty == MaintenanceType::Offline {
+ bail!("offline maintenance mode: {}", message);
+ } else if self.ty == MaintenanceType::ReadOnly {
+ if let Some(Operation::Write) = operation {
+ bail!("read-only maintenance mode: {}", message);
+ }
+ }
+ Ok(())
+ }
+}
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 7044e074..902848d6 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -15,7 +15,9 @@ use proxmox_sys::WorkerTaskContext;
use proxmox_sys::{task_log, task_warn};
use proxmox_sys::fs::{lock_dir_noblock, DirLockGuard};
-use pbs_api_types::{UPID, DataStoreConfig, Authid, GarbageCollectionStatus, HumanByte};
+use pbs_api_types::{
+ UPID, DataStoreConfig, Authid, Operation, GarbageCollectionStatus, HumanByte
+};
use pbs_config::{open_backup_lockfile, BackupLockGuard};
use crate::DataBlob;
@@ -60,13 +62,20 @@ pub struct DataStore {
}
impl DataStore {
-
- pub fn lookup_datastore(name: &str) -> Result<Arc<DataStore>, Error> {
-
+ pub fn lookup_datastore(
+ name: &str,
+ operation: Option<Operation>,
+ ) -> Result<Arc<DataStore>, Error> {
let (config, _digest) = pbs_config::datastore::config()?;
let config: DataStoreConfig = config.lookup("datastore", name)?;
let path = PathBuf::from(&config.path);
+ if let Some(maintenance_mode) = config.get_maintenance_mode() {
+ if let Err(error) = maintenance_mode.check(operation) {
+ bail!("datastore '{}' is in {}", name, error);
+ }
+ }
+
let mut map = DATASTORE_MAP.lock().unwrap();
if let Some(datastore) = map.get(name) {
diff --git a/pbs-datastore/src/snapshot_reader.rs b/pbs-datastore/src/snapshot_reader.rs
index 18bc0d83..65abac7d 100644
--- a/pbs-datastore/src/snapshot_reader.rs
+++ b/pbs-datastore/src/snapshot_reader.rs
@@ -14,6 +14,7 @@ use crate::fixed_index::FixedIndexReader;
use crate::dynamic_index::DynamicIndexReader;
use crate::manifest::{archive_type, ArchiveType, CLIENT_LOG_BLOB_NAME, MANIFEST_BLOB_NAME};
use crate::DataStore;
+use pbs_api_types::Operation;
/// Helper to access the contents of a datastore backup snapshot
///
@@ -120,7 +121,10 @@ impl <'a> Iterator for SnapshotChunkIterator<'a> {
};
let datastore =
- DataStore::lookup_datastore(self.snapshot_reader.datastore_name())?;
+ DataStore::lookup_datastore(
+ self.snapshot_reader.datastore_name(),
+ Some(Operation::Read)
+ )?;
let order = datastore.get_chunks_in_order(&index, |_| false, |_| Ok(()))?;
self.current_index = Some((Arc::new(index), 0, order));
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 263ea96f..ce710938 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -30,7 +30,7 @@ use pxar::EntryKind;
use pbs_api_types::{ Authid, BackupContent, Counts, CryptMode,
DataStoreListItem, GarbageCollectionStatus, GroupListItem,
- SnapshotListItem, SnapshotVerifyState, PruneOptions,
+ Operation, SnapshotListItem, SnapshotVerifyState, PruneOptions,
DataStoreStatus, RRDMode, RRDTimeFrame,
BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_TIME_SCHEMA,
BACKUP_TYPE_SCHEMA, DATASTORE_SCHEMA,
@@ -170,7 +170,7 @@ pub fn list_groups(
let user_info = CachedUserInfo::new()?;
let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]);
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
let list_all = (user_privs & PRIV_DATASTORE_AUDIT) != 0;
let backup_groups = BackupInfo::list_backup_groups(&datastore.base_path())?;
@@ -268,7 +268,7 @@ pub fn delete_group(
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let group = BackupGroup::new(backup_type, backup_id);
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
check_priv_or_backup_owner(&datastore, &group, &auth_id, PRIV_DATASTORE_MODIFY)?;
@@ -315,7 +315,7 @@ pub fn list_snapshot_files(
) -> Result<Vec<BackupContent>, Error> {
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
let snapshot = BackupDir::new(backup_type, backup_id, backup_time)?;
@@ -365,7 +365,7 @@ pub fn delete_snapshot(
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let snapshot = BackupDir::new(backup_type, backup_id, backup_time)?;
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
check_priv_or_backup_owner(&datastore, snapshot.group(), &auth_id, PRIV_DATASTORE_MODIFY)?;
@@ -414,7 +414,7 @@ pub fn list_snapshots (
let list_all = (user_privs & PRIV_DATASTORE_AUDIT) != 0;
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
let base_path = datastore.base_path();
@@ -615,7 +615,7 @@ pub fn status(
_info: &ApiMethod,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<DataStoreStatus, Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
let storage = crate::tools::disks::disk_usage(&datastore.base_path())?;
let (counts, gc_status) = if verbose {
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
@@ -693,7 +693,7 @@ pub fn verify(
outdated_after: Option<i64>,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<Value, Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
let ignore_verified = ignore_verified.unwrap_or(true);
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
@@ -838,7 +838,7 @@ pub fn prune(
let group = BackupGroup::new(&backup_type, &backup_id);
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
check_priv_or_backup_owner(&datastore, &group, &auth_id, PRIV_DATASTORE_MODIFY)?;
@@ -963,7 +963,7 @@ pub fn prune_datastore(
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
@@ -1007,7 +1007,7 @@ pub fn start_garbage_collection(
rpcenv: &mut dyn RpcEnvironment,
) -> Result<Value, Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let job = Job::new("garbage_collection", &store)
@@ -1043,7 +1043,7 @@ pub fn garbage_collection_status(
_rpcenv: &mut dyn RpcEnvironment,
) -> Result<GarbageCollectionStatus, Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
let status = datastore.last_gc_status();
@@ -1119,7 +1119,7 @@ pub fn download_file(
async move {
let store = required_string_param(¶m, "store")?;
- let datastore = DataStore::lookup_datastore(store)?;
+ let datastore = DataStore::lookup_datastore(store, Some(Operation::Read))?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
@@ -1189,7 +1189,7 @@ pub fn download_file_decoded(
async move {
let store = required_string_param(¶m, "store")?;
- let datastore = DataStore::lookup_datastore(store)?;
+ let datastore = DataStore::lookup_datastore(store, Some(Operation::Read))?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
@@ -1303,7 +1303,7 @@ pub fn upload_backup_log(
async move {
let store = required_string_param(¶m, "store")?;
- let datastore = DataStore::lookup_datastore(store)?;
+ let datastore = DataStore::lookup_datastore(store, Some(Operation::Write))?;
let file_name = CLIENT_LOG_BLOB_NAME;
@@ -1380,7 +1380,7 @@ pub fn catalog(
filepath: String,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<Vec<ArchiveEntry>, Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
@@ -1450,7 +1450,7 @@ pub fn pxar_file_download(
async move {
let store = required_string_param(¶m, "store")?;
- let datastore = DataStore::lookup_datastore(store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
@@ -1567,7 +1567,7 @@ pub fn get_rrd_stats(
_param: Value,
) -> Result<Value, Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
let disk_manager = crate::tools::disks::DiskManage::new();
let mut rrd_fields = vec![
@@ -1615,7 +1615,7 @@ pub fn get_group_notes(
backup_id: String,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<String, Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let backup_group = BackupGroup::new(backup_type, backup_id);
@@ -1657,7 +1657,7 @@ pub fn set_group_notes(
notes: String,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<(), Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let backup_group = BackupGroup::new(backup_type, backup_id);
@@ -1699,7 +1699,7 @@ pub fn get_notes(
backup_time: i64,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<String, Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?;
@@ -1750,7 +1750,7 @@ pub fn set_notes(
notes: String,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<(), Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?;
@@ -1793,7 +1793,7 @@ pub fn get_protection(
backup_time: i64,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<bool, Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?;
@@ -1838,7 +1838,7 @@ pub fn set_protection(
protected: bool,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<(), Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?;
@@ -1879,7 +1879,7 @@ pub fn set_backup_owner(
rpcenv: &mut dyn RpcEnvironment,
) -> Result<(), Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
let backup_group = BackupGroup::new(backup_type, backup_id);
diff --git a/src/api2/backup/mod.rs b/src/api2/backup/mod.rs
index 395edd3d..9d62dc52 100644
--- a/src/api2/backup/mod.rs
+++ b/src/api2/backup/mod.rs
@@ -16,7 +16,7 @@ use proxmox_router::{
use proxmox_schema::*;
use pbs_api_types::{
- Authid, VerifyState, SnapshotVerifyState,
+ Authid, Operation, VerifyState, SnapshotVerifyState,
BACKUP_ID_SCHEMA, BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA, DATASTORE_SCHEMA,
CHUNK_DIGEST_SCHEMA, PRIV_DATASTORE_BACKUP, BACKUP_ARCHIVE_NAME_SCHEMA,
};
@@ -77,7 +77,7 @@ async move {
let user_info = CachedUserInfo::new()?;
user_info.check_privs(&auth_id, &["datastore", &store], PRIV_DATASTORE_BACKUP, false)?;
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
let backup_type = required_string_param(¶m, "backup-type")?;
let backup_id = required_string_param(¶m, "backup-id")?;
diff --git a/src/api2/reader/mod.rs b/src/api2/reader/mod.rs
index 2b11d1b1..45cefe5d 100644
--- a/src/api2/reader/mod.rs
+++ b/src/api2/reader/mod.rs
@@ -16,8 +16,8 @@ use proxmox_router::{
use proxmox_schema::{BooleanSchema, ObjectSchema};
use pbs_api_types::{
- Authid, DATASTORE_SCHEMA, BACKUP_TYPE_SCHEMA, BACKUP_TIME_SCHEMA, BACKUP_ID_SCHEMA,
- CHUNK_DIGEST_SCHEMA, PRIV_DATASTORE_READ, PRIV_DATASTORE_BACKUP,
+ Authid, Operation, DATASTORE_SCHEMA, BACKUP_TYPE_SCHEMA, BACKUP_TIME_SCHEMA,
+ BACKUP_ID_SCHEMA, CHUNK_DIGEST_SCHEMA, PRIV_DATASTORE_READ, PRIV_DATASTORE_BACKUP,
BACKUP_ARCHIVE_NAME_SCHEMA,
};
use proxmox_sys::fs::lock_dir_noblock_shared;
@@ -81,7 +81,7 @@ fn upgrade_to_backup_reader_protocol(
bail!("no permissions on /datastore/{}", store);
}
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
let backup_type = required_string_param(¶m, "backup-type")?;
let backup_id = required_string_param(¶m, "backup-id")?;
diff --git a/src/api2/status.rs b/src/api2/status.rs
index 029529ac..b589951d 100644
--- a/src/api2/status.rs
+++ b/src/api2/status.rs
@@ -14,7 +14,7 @@ use proxmox_router::{
use proxmox_router::list_subdirs_api_method;
use pbs_api_types::{
- Authid, DATASTORE_SCHEMA, RRDMode, RRDTimeFrame,
+ Authid, Operation, DATASTORE_SCHEMA, RRDMode, RRDTimeFrame,
PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP,
};
@@ -97,7 +97,7 @@ pub fn datastore_status(
continue;
}
- let datastore = match DataStore::lookup_datastore(store) {
+ let datastore = match DataStore::lookup_datastore(&store, Some(Operation::Read)) {
Ok(datastore) => datastore,
Err(err) => {
list.push(json!({
diff --git a/src/api2/tape/backup.rs b/src/api2/tape/backup.rs
index 1462f200..28b24168 100644
--- a/src/api2/tape/backup.rs
+++ b/src/api2/tape/backup.rs
@@ -10,7 +10,7 @@ use proxmox_schema::api;
use proxmox_sys::{task_log, task_warn, WorkerTaskContext};
use pbs_api_types::{
- Authid, Userid, TapeBackupJobConfig, TapeBackupJobSetup, TapeBackupJobStatus, MediaPoolConfig,
+ Authid, Userid, Operation, TapeBackupJobConfig, TapeBackupJobSetup, TapeBackupJobStatus, MediaPoolConfig,
UPID_SCHEMA, JOB_ID_SCHEMA, PRIV_DATASTORE_READ, PRIV_TAPE_AUDIT, PRIV_TAPE_WRITE,
GroupFilter,
};
@@ -168,7 +168,7 @@ pub fn do_tape_backup_job(
let worker_type = job.jobtype().to_string();
- let datastore = DataStore::lookup_datastore(&setup.store)?;
+ let datastore = DataStore::lookup_datastore(&setup.store, Some(Operation::Read))?;
let (config, _digest) = pbs_config::media_pool::config()?;
let pool_config: MediaPoolConfig = config.lookup("pool", &setup.pool)?;
@@ -349,7 +349,7 @@ pub fn backup(
&setup.drive,
)?;
- let datastore = DataStore::lookup_datastore(&setup.store)?;
+ let datastore = DataStore::lookup_datastore(&setup.store, Some(Operation::Read))?;
let (config, _digest) = pbs_config::media_pool::config()?;
let pool_config: MediaPoolConfig = config.lookup("pool", &setup.pool)?;
diff --git a/src/api2/tape/restore.rs b/src/api2/tape/restore.rs
index 2ce16c9d..59f098d9 100644
--- a/src/api2/tape/restore.rs
+++ b/src/api2/tape/restore.rs
@@ -17,7 +17,7 @@ use proxmox_uuid::Uuid;
use proxmox_sys::{task_log, task_warn, WorkerTaskContext};
use pbs_api_types::{
- Authid, Userid, CryptMode,
+ Authid, Operation, Userid, CryptMode,
DATASTORE_MAP_ARRAY_SCHEMA, DATASTORE_MAP_LIST_SCHEMA, DRIVE_NAME_SCHEMA,
UPID_SCHEMA, TAPE_RESTORE_SNAPSHOT_SCHEMA,
PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_TAPE_READ,
@@ -93,10 +93,10 @@ impl TryFrom<String> for DataStoreMap {
if let Some(index) = store.find('=') {
let mut target = store.split_off(index);
target.remove(0); // remove '='
- let datastore = DataStore::lookup_datastore(&target)?;
+ let datastore = DataStore::lookup_datastore(&target, Some(Operation::Write))?;
map.insert(store, datastore);
} else if default.is_none() {
- default = Some(DataStore::lookup_datastore(&store)?);
+ default = Some(DataStore::lookup_datastore(&store, Some(Operation::Write))?);
} else {
bail!("multiple default stores given");
}
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index a4f20af1..e8315e5f 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -48,7 +48,7 @@ use proxmox_time::CalendarEvent;
use pbs_api_types::{
Authid, DataStoreConfig, PruneOptions, SyncJobConfig, TapeBackupJobConfig,
- VerificationJobConfig,
+ VerificationJobConfig, Operation
};
use proxmox_rest_server::daemon;
@@ -560,7 +560,7 @@ async fn schedule_datastore_garbage_collection() {
};
for (store, (_, store_config)) in config.sections {
- let datastore = match DataStore::lookup_datastore(&store) {
+ let datastore = match DataStore::lookup_datastore(&store, Some(Operation::Write)) {
Ok(datastore) => datastore,
Err(err) => {
eprintln!("lookup_datastore failed - {}", err);
diff --git a/src/server/prune_job.rs b/src/server/prune_job.rs
index 825fd60d..d9a6651e 100644
--- a/src/server/prune_job.rs
+++ b/src/server/prune_job.rs
@@ -7,7 +7,7 @@ use proxmox_sys::{task_log, task_warn};
use pbs_datastore::backup_info::BackupInfo;
use pbs_datastore::prune::compute_prune_info;
use pbs_datastore::DataStore;
-use pbs_api_types::{Authid, PRIV_DATASTORE_MODIFY, PruneOptions};
+use pbs_api_types::{Authid, Operation, PRIV_DATASTORE_MODIFY, PruneOptions};
use pbs_config::CachedUserInfo;
use proxmox_rest_server::WorkerTask;
@@ -97,7 +97,7 @@ pub fn do_prune_job(
auth_id: &Authid,
schedule: Option<String>,
) -> Result<String, Error> {
- let datastore = DataStore::lookup_datastore(&store)?;
+ let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
let worker_type = job.jobtype().to_string();
let auth_id = auth_id.clone();
diff --git a/src/server/pull.rs b/src/server/pull.rs
index a2a5f1f1..559533a6 100644
--- a/src/server/pull.rs
+++ b/src/server/pull.rs
@@ -16,7 +16,7 @@ use proxmox_sys::task_log;
use pbs_api_types::{
Authid, GroupFilter, GroupListItem, RateLimitConfig, Remote,
- SnapshotListItem,
+ Operation, SnapshotListItem,
};
use pbs_datastore::{BackupDir, BackupInfo, BackupGroup, DataStore, StoreProgress};
@@ -57,7 +57,7 @@ impl PullParameters {
group_filter: Option<Vec<GroupFilter>>,
limit: RateLimitConfig,
) -> Result<Self, Error> {
- let store = DataStore::lookup_datastore(store)?;
+ let store = DataStore::lookup_datastore(store, Some(Operation::Write))?;
let (remote_config, _digest) = pbs_config::remote::config()?;
let remote: Remote = remote_config.lookup("remote", remote)?;
diff --git a/src/server/verify_job.rs b/src/server/verify_job.rs
index a23eb04e..e472ed91 100644
--- a/src/server/verify_job.rs
+++ b/src/server/verify_job.rs
@@ -1,7 +1,7 @@
use anyhow::{format_err, Error};
use proxmox_sys::task_log;
-use pbs_api_types::{Authid, VerificationJobConfig};
+use pbs_api_types::{Authid, Operation, VerificationJobConfig};
use proxmox_rest_server::WorkerTask;
use pbs_datastore::DataStore;
@@ -22,7 +22,7 @@ pub fn do_verification_job(
to_stdout: bool,
) -> Result<String, Error> {
- let datastore = DataStore::lookup_datastore(&verification_job.store)?;
+ let datastore = DataStore::lookup_datastore(&verification_job.store, Some(Operation::Read))?;
let outdated_after = verification_job.outdated_after;
let ignore_verified_snapshots = verification_job.ignore_verified.unwrap_or(true);
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v9 3/6] pbs-datastore: add active operations tracking
2022-02-17 17:14 [pbs-devel] [PATCH proxmox-backup v9 0/6] closes #3071: maintenance mode for datastore Hannes Laimer
2022-02-17 17:14 ` [pbs-devel] [PATCH proxmox-backup v9 1/6] api-types: add maintenance type Hannes Laimer
2022-02-17 17:14 ` [pbs-devel] [PATCH proxmox-backup v9 2/6] datastore: add check for maintenance in lookup Hannes Laimer
@ 2022-02-17 17:14 ` Hannes Laimer
2022-02-17 17:14 ` [pbs-devel] [PATCH proxmox-backup v9 4/6] api: make maintenance_type updatable Hannes Laimer
` (3 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Hannes Laimer @ 2022-02-17 17:14 UTC (permalink / raw)
To: pbs-devel
Saves the currently active read/write operation counts in a file. The
file is updated whenever a reference returned by lookup_datastore is
dropped and whenever a reference is returned by lookup_datastore. The
files are locked before every access, there is one file per datastore.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
pbs-datastore/Cargo.toml | 1 +
pbs-datastore/src/datastore.rs | 106 ++++++++++++++++++++---------
pbs-datastore/src/lib.rs | 4 ++
pbs-datastore/src/task_tracking.rs | 81 ++++++++++++++++++++++
src/bin/proxmox-backup-api.rs | 1 +
src/server/mod.rs | 16 ++++-
6 files changed, 177 insertions(+), 32 deletions(-)
create mode 100644 pbs-datastore/src/task_tracking.rs
diff --git a/pbs-datastore/Cargo.toml b/pbs-datastore/Cargo.toml
index d8cefa00..0c703d44 100644
--- a/pbs-datastore/Cargo.toml
+++ b/pbs-datastore/Cargo.toml
@@ -35,5 +35,6 @@ proxmox-uuid = "1"
proxmox-sys = "0.2"
pbs-api-types = { path = "../pbs-api-types" }
+pbs-buildcfg = { path = "../pbs-buildcfg" }
pbs-tools = { path = "../pbs-tools" }
pbs-config = { path = "../pbs-config" }
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 902848d6..95399c34 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -9,11 +9,12 @@ use std::time::Duration;
use anyhow::{bail, format_err, Error};
use lazy_static::lazy_static;
-use proxmox_sys::fs::{replace_file, file_read_optional_string, CreateOptions};
+use proxmox_sys::fs::{replace_file, file_read_optional_string,
+ CreateOptions, lock_dir_noblock, DirLockGuard,
+};
use proxmox_sys::process_locker::ProcessLockSharedGuard;
use proxmox_sys::WorkerTaskContext;
use proxmox_sys::{task_log, task_warn};
-use proxmox_sys::fs::{lock_dir_noblock, DirLockGuard};
use pbs_api_types::{
UPID, DataStoreConfig, Authid, Operation, GarbageCollectionStatus, HumanByte
@@ -31,9 +32,10 @@ use crate::manifest::{
ArchiveType, BackupManifest,
archive_type,
};
+use crate::task_tracking::update_active_operations;
lazy_static! {
- static ref DATASTORE_MAP: Mutex<HashMap<String, Arc<DataStore>>> = Mutex::new(HashMap::new());
+ static ref DATASTORE_MAP: Mutex<HashMap<String, Arc<DataStoreImpl>>> = Mutex::new(HashMap::new());
}
/// checks if auth_id is owner, or, if owner is a token, if
@@ -54,13 +56,45 @@ pub fn check_backup_owner(
///
/// A Datastore can store severals backups, and provides the
/// management interface for backup.
-pub struct DataStore {
+pub struct DataStoreImpl {
chunk_store: Arc<ChunkStore>,
gc_mutex: Mutex<()>,
last_gc_status: Mutex<GarbageCollectionStatus>,
verify_new: bool,
}
+pub struct DataStore {
+ inner: Arc<DataStoreImpl>,
+ operation: Option<Operation>,
+}
+
+impl Clone for DataStore {
+ fn clone(&self) -> Self {
+ let mut new_operation = self.operation;
+ if let Some(operation) = self.operation {
+ if let Err(e) = update_active_operations(self.name(), operation, 1) {
+ log::error!("could not update active operations - {}", e);
+ new_operation = None;
+ }
+ }
+
+ DataStore {
+ inner: self.inner.clone(),
+ operation: new_operation,
+ }
+ }
+}
+
+impl Drop for DataStore {
+ fn drop(&mut self) {
+ if let Some(operation) = self.operation {
+ if let Err(e) = update_active_operations(self.name(), operation, -1) {
+ log::error!("could not update active operations - {}", e);
+ }
+ }
+ }
+}
+
impl DataStore {
pub fn lookup_datastore(
name: &str,
@@ -76,6 +110,10 @@ impl DataStore {
}
}
+ if let Some(operation) = operation {
+ update_active_operations(name, operation, 1)?;
+ }
+
let mut map = DATASTORE_MAP.lock().unwrap();
if let Some(datastore) = map.get(name) {
@@ -83,7 +121,10 @@ impl DataStore {
if datastore.chunk_store.base() == path &&
datastore.verify_new == config.verify_new.unwrap_or(false)
{
- return Ok(datastore.clone());
+ return Ok(Arc::new(Self {
+ inner: datastore.clone(),
+ operation,
+ }))
}
}
@@ -92,7 +133,10 @@ impl DataStore {
let datastore = Arc::new(datastore);
map.insert(name.to_string(), datastore.clone());
- Ok(datastore)
+ Ok(Arc::new(Self {
+ inner: datastore,
+ operation,
+ }))
}
/// removes all datastores that are not configured anymore
@@ -107,7 +151,7 @@ impl DataStore {
Ok(())
}
- fn open_with_path(store_name: &str, path: &Path, config: DataStoreConfig) -> Result<Self, Error> {
+ fn open_with_path(store_name: &str, path: &Path, config: DataStoreConfig) -> Result<DataStoreImpl, Error> {
let chunk_store = ChunkStore::open(store_name, path)?;
let mut gc_status_path = chunk_store.base_path();
@@ -125,7 +169,7 @@ impl DataStore {
GarbageCollectionStatus::default()
};
- Ok(Self {
+ Ok(DataStoreImpl {
chunk_store: Arc::new(chunk_store),
gc_mutex: Mutex::new(()),
last_gc_status: Mutex::new(gc_status),
@@ -139,19 +183,19 @@ impl DataStore {
impl Iterator<Item = (Result<proxmox_sys::fs::ReadDirEntry, Error>, usize, bool)>,
Error
> {
- self.chunk_store.get_chunk_iterator()
+ self.inner.chunk_store.get_chunk_iterator()
}
pub fn create_fixed_writer<P: AsRef<Path>>(&self, filename: P, size: usize, chunk_size: usize) -> Result<FixedIndexWriter, Error> {
- let index = FixedIndexWriter::create(self.chunk_store.clone(), filename.as_ref(), size, chunk_size)?;
+ let index = FixedIndexWriter::create(self.inner.chunk_store.clone(), filename.as_ref(), size, chunk_size)?;
Ok(index)
}
pub fn open_fixed_reader<P: AsRef<Path>>(&self, filename: P) -> Result<FixedIndexReader, Error> {
- let full_path = self.chunk_store.relative_path(filename.as_ref());
+ let full_path = self.inner.chunk_store.relative_path(filename.as_ref());
let index = FixedIndexReader::open(&full_path)?;
@@ -163,14 +207,14 @@ impl DataStore {
) -> Result<DynamicIndexWriter, Error> {
let index = DynamicIndexWriter::create(
- self.chunk_store.clone(), filename.as_ref())?;
+ self.inner.chunk_store.clone(), filename.as_ref())?;
Ok(index)
}
pub fn open_dynamic_reader<P: AsRef<Path>>(&self, filename: P) -> Result<DynamicIndexReader, Error> {
- let full_path = self.chunk_store.relative_path(filename.as_ref());
+ let full_path = self.inner.chunk_store.relative_path(filename.as_ref());
let index = DynamicIndexReader::open(&full_path)?;
@@ -220,11 +264,11 @@ impl DataStore {
}
pub fn name(&self) -> &str {
- self.chunk_store.name()
+ self.inner.chunk_store.name()
}
pub fn base_path(&self) -> PathBuf {
- self.chunk_store.base_path()
+ self.inner.chunk_store.base_path()
}
/// Cleanup a backup directory
@@ -527,7 +571,7 @@ impl DataStore {
worker.check_abort()?;
worker.fail_on_shutdown()?;
let digest = index.index_digest(pos).unwrap();
- if !self.chunk_store.cond_touch_chunk(digest, false)? {
+ if !self.inner.chunk_store.cond_touch_chunk(digest, false)? {
task_warn!(
worker,
"warning: unable to access non-existent chunk {}, required by {:?}",
@@ -543,7 +587,7 @@ impl DataStore {
let mut bad_path = PathBuf::new();
bad_path.push(self.chunk_path(digest).0);
bad_path.set_extension(bad_ext);
- self.chunk_store.cond_touch_path(&bad_path, false)?;
+ self.inner.chunk_store.cond_touch_path(&bad_path, false)?;
}
}
}
@@ -623,24 +667,24 @@ impl DataStore {
}
pub fn last_gc_status(&self) -> GarbageCollectionStatus {
- self.last_gc_status.lock().unwrap().clone()
+ self.inner.last_gc_status.lock().unwrap().clone()
}
pub fn garbage_collection_running(&self) -> bool {
- !matches!(self.gc_mutex.try_lock(), Ok(_))
+ !matches!(self.inner.gc_mutex.try_lock(), Ok(_))
}
pub fn garbage_collection(&self, worker: &dyn WorkerTaskContext, upid: &UPID) -> Result<(), Error> {
- if let Ok(ref mut _mutex) = self.gc_mutex.try_lock() {
+ if let Ok(ref mut _mutex) = self.inner.gc_mutex.try_lock() {
// avoids that we run GC if an old daemon process has still a
// running backup writer, which is not save as we have no "oldest
// writer" information and thus no safe atime cutoff
- let _exclusive_lock = self.chunk_store.try_exclusive_lock()?;
+ let _exclusive_lock = self.inner.chunk_store.try_exclusive_lock()?;
let phase1_start_time = proxmox_time::epoch_i64();
- let oldest_writer = self.chunk_store.oldest_writer().unwrap_or(phase1_start_time);
+ let oldest_writer = self.inner.chunk_store.oldest_writer().unwrap_or(phase1_start_time);
let mut gc_status = GarbageCollectionStatus::default();
gc_status.upid = Some(upid.to_string());
@@ -650,7 +694,7 @@ impl DataStore {
self.mark_used_chunks(&mut gc_status, worker)?;
task_log!(worker, "Start GC phase2 (sweep unused chunks)");
- self.chunk_store.sweep_unused_chunks(
+ self.inner.chunk_store.sweep_unused_chunks(
oldest_writer,
phase1_start_time,
&mut gc_status,
@@ -727,7 +771,7 @@ impl DataStore {
let _ = replace_file(path, serialized.as_bytes(), options, false);
}
- *self.last_gc_status.lock().unwrap() = gc_status;
+ *self.inner.last_gc_status.lock().unwrap() = gc_status;
} else {
bail!("Start GC failed - (already running/locked)");
@@ -737,15 +781,15 @@ impl DataStore {
}
pub fn try_shared_chunk_store_lock(&self) -> Result<ProcessLockSharedGuard, Error> {
- self.chunk_store.try_shared_lock()
+ self.inner.chunk_store.try_shared_lock()
}
pub fn chunk_path(&self, digest:&[u8; 32]) -> (PathBuf, String) {
- self.chunk_store.chunk_path(digest)
+ self.inner.chunk_store.chunk_path(digest)
}
pub fn cond_touch_chunk(&self, digest: &[u8; 32], fail_if_not_exist: bool) -> Result<bool, Error> {
- self.chunk_store.cond_touch_chunk(digest, fail_if_not_exist)
+ self.inner.chunk_store.cond_touch_chunk(digest, fail_if_not_exist)
}
pub fn insert_chunk(
@@ -753,7 +797,7 @@ impl DataStore {
chunk: &DataBlob,
digest: &[u8; 32],
) -> Result<(bool, u64), Error> {
- self.chunk_store.insert_chunk(chunk, digest)
+ self.inner.chunk_store.insert_chunk(chunk, digest)
}
pub fn load_blob(&self, backup_dir: &BackupDir, filename: &str) -> Result<DataBlob, Error> {
@@ -769,13 +813,13 @@ impl DataStore {
pub fn stat_chunk(&self, digest: &[u8; 32]) -> Result<std::fs::Metadata, Error> {
- let (chunk_path, _digest_str) = self.chunk_store.chunk_path(digest);
+ let (chunk_path, _digest_str) = self.inner.chunk_store.chunk_path(digest);
std::fs::metadata(chunk_path).map_err(Error::from)
}
pub fn load_chunk(&self, digest: &[u8; 32]) -> Result<DataBlob, Error> {
- let (chunk_path, digest_str) = self.chunk_store.chunk_path(digest);
+ let (chunk_path, digest_str) = self.inner.chunk_store.chunk_path(digest);
proxmox_lang::try_block!({
let mut file = std::fs::File::open(&chunk_path)?;
@@ -889,7 +933,7 @@ impl DataStore {
}
pub fn verify_new(&self) -> bool {
- self.verify_new
+ self.inner.verify_new
}
/// returns a list of chunks sorted by their inode number on disk
diff --git a/pbs-datastore/src/lib.rs b/pbs-datastore/src/lib.rs
index d50a64a5..131040c6 100644
--- a/pbs-datastore/src/lib.rs
+++ b/pbs-datastore/src/lib.rs
@@ -145,6 +145,9 @@
// Note: .pcat1 => Proxmox Catalog Format version 1
pub const CATALOG_NAME: &str = "catalog.pcat1.didx";
+/// Directory path where active operations counters are saved.
+pub const ACTIVE_OPERATIONS_DIR: &str = concat!(pbs_buildcfg::PROXMOX_BACKUP_RUN_DIR_M!(), "/active-operations");
+
#[macro_export]
macro_rules! PROXMOX_BACKUP_PROTOCOL_ID_V1 {
() => {
@@ -179,6 +182,7 @@ pub mod paperkey;
pub mod prune;
pub mod read_chunk;
pub mod store_progress;
+pub mod task_tracking;
pub mod dynamic_index;
pub mod fixed_index;
diff --git a/pbs-datastore/src/task_tracking.rs b/pbs-datastore/src/task_tracking.rs
new file mode 100644
index 00000000..a02d9a17
--- /dev/null
+++ b/pbs-datastore/src/task_tracking.rs
@@ -0,0 +1,81 @@
+use anyhow::Error;
+use libc::pid_t;
+use nix::unistd::Pid;
+use std::path::PathBuf;
+
+use pbs_api_types::Operation;
+use proxmox_sys::fs::{file_read_optional_string, open_file_locked, replace_file, CreateOptions};
+use proxmox_sys::linux::procfs;
+use serde::{Deserialize, Serialize};
+
+#[derive(Deserialize, Serialize, Clone)]
+struct TaskOperations {
+ pid: u32,
+ starttime: u64,
+ reading_operations: i64,
+ writing_operations: i64,
+}
+
+pub fn update_active_operations(name: &str, operation: Operation, count: i64) -> Result<(), Error> {
+ let path = PathBuf::from(format!("{}/{}", crate::ACTIVE_OPERATIONS_DIR, name));
+ let lock_path = PathBuf::from(format!("{}/{}.lock", crate::ACTIVE_OPERATIONS_DIR, name));
+
+ let user = pbs_config::backup_user()?;
+ let options = CreateOptions::new()
+ .group(user.gid)
+ .owner(user.uid)
+ .perm(nix::sys::stat::Mode::from_bits_truncate(0o660));
+
+ let timeout = std::time::Duration::new(10, 0);
+ open_file_locked(&lock_path, timeout, true, options.clone())?;
+
+ let pid = std::process::id();
+ let starttime = procfs::PidStat::read_from_pid(Pid::from_raw(pid as pid_t))?.starttime;
+ let mut updated = false;
+
+ let mut updated_tasks: Vec<TaskOperations> = match file_read_optional_string(&path)? {
+ Some(data) => serde_json::from_str::<Vec<TaskOperations>>(&data)?
+ .iter_mut()
+ .filter_map(
+ |task| match procfs::check_process_running(task.pid as pid_t) {
+ Some(stat) if pid == task.pid && stat.starttime != task.starttime => None,
+ Some(_) => {
+ if pid == task.pid {
+ updated = true;
+ match operation {
+ Operation::Read => task.reading_operations += count,
+ Operation::Write => task.writing_operations += count,
+ };
+ }
+ Some(task.clone())
+ }
+ _ => None,
+ },
+ )
+ .collect(),
+ None => Vec::new(),
+ };
+
+ if !updated {
+ updated_tasks.push(match operation {
+ Operation::Read => TaskOperations {
+ pid,
+ starttime,
+ reading_operations: 1,
+ writing_operations: 0,
+ },
+ Operation::Write => TaskOperations {
+ pid,
+ starttime,
+ reading_operations: 0,
+ writing_operations: 1,
+ },
+ })
+ }
+ replace_file(
+ &path,
+ serde_json::to_string(&updated_tasks)?.as_bytes(),
+ options,
+ false,
+ )
+}
diff --git a/src/bin/proxmox-backup-api.rs b/src/bin/proxmox-backup-api.rs
index ee037a3b..68aa4863 100644
--- a/src/bin/proxmox-backup-api.rs
+++ b/src/bin/proxmox-backup-api.rs
@@ -75,6 +75,7 @@ async fn run() -> Result<(), Error> {
proxmox_backup::server::create_run_dir()?;
proxmox_backup::server::create_state_dir()?;
+ proxmox_backup::server::create_active_operations_dir()?;
proxmox_backup::server::jobstate::create_jobstate_dir()?;
proxmox_backup::tape::create_tape_status_dir()?;
proxmox_backup::tape::create_drive_state_dir()?;
diff --git a/src/server/mod.rs b/src/server/mod.rs
index 2b54cdf0..5a9884e9 100644
--- a/src/server/mod.rs
+++ b/src/server/mod.rs
@@ -4,7 +4,7 @@
//! services. We want async IO, so this is built on top of
//! tokio/hyper.
-use anyhow::Error;
+use anyhow::{format_err, Error};
use serde_json::Value;
use proxmox_sys::fs::{create_path, CreateOptions};
@@ -71,3 +71,17 @@ pub fn create_state_dir() -> Result<(), Error> {
create_path(pbs_buildcfg::PROXMOX_BACKUP_STATE_DIR_M!(), None, Some(opts))?;
Ok(())
}
+
+/// Create active operations dir with correct permission.
+pub fn create_active_operations_dir() -> Result<(), Error> {
+ let backup_user = pbs_config::backup_user()?;
+ let mode = nix::sys::stat::Mode::from_bits_truncate(0o0750);
+ let options = CreateOptions::new()
+ .perm(mode)
+ .owner(backup_user.uid)
+ .group(backup_user.gid);
+
+ create_path(pbs_datastore::ACTIVE_OPERATIONS_DIR, None, Some(options))
+ .map_err(|err: Error| format_err!("unable to create active operations dir - {}", err))?;
+ Ok(())
+}
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v9 4/6] api: make maintenance_type updatable
2022-02-17 17:14 [pbs-devel] [PATCH proxmox-backup v9 0/6] closes #3071: maintenance mode for datastore Hannes Laimer
` (2 preceding siblings ...)
2022-02-17 17:14 ` [pbs-devel] [PATCH proxmox-backup v9 3/6] pbs-datastore: add active operations tracking Hannes Laimer
@ 2022-02-17 17:14 ` Hannes Laimer
2022-02-17 17:14 ` [pbs-devel] [PATCH proxmox-backup v9 5/6] api: add get_active_operations endpoint Hannes Laimer
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Hannes Laimer @ 2022-02-17 17:14 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/api2/config/datastore.rs | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 60bc3c0e..3adc7fdb 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -184,6 +184,8 @@ pub enum DeletableProperty {
notify_user,
/// Delete the notify property
notify,
+ /// Delete the maintenance-mode property
+ maintenance_mode,
}
#[api(
@@ -250,6 +252,7 @@ pub fn update_datastore(
DeletableProperty::verify_new => { data.verify_new = None; },
DeletableProperty::notify => { data.notify = None; },
DeletableProperty::notify_user => { data.notify_user = None; },
+ DeletableProperty::maintenance_mode => { data.maintenance_mode = None; },
}
}
}
@@ -295,6 +298,8 @@ pub fn update_datastore(
if update.notify_user.is_some() { data.notify_user = update.notify_user; }
+ if update.maintenance_mode.is_some() { data.maintenance_mode = update.maintenance_mode; }
+
config.set_data(&name, "datastore", &data)?;
pbs_config::datastore::save_config(&config)?;
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v9 5/6] api: add get_active_operations endpoint
2022-02-17 17:14 [pbs-devel] [PATCH proxmox-backup v9 0/6] closes #3071: maintenance mode for datastore Hannes Laimer
` (3 preceding siblings ...)
2022-02-17 17:14 ` [pbs-devel] [PATCH proxmox-backup v9 4/6] api: make maintenance_type updatable Hannes Laimer
@ 2022-02-17 17:14 ` Hannes Laimer
2022-02-17 17:14 ` [pbs-devel] [PATCH proxmox-backup v9 6/6] ui: add option to change the maintenance type Hannes Laimer
2022-04-06 13:08 ` [pbs-devel] [PATCH proxmox-backup v9 0/6] closes #3071: maintenance mode for datastore Dylan Whyte
6 siblings, 0 replies; 12+ messages in thread
From: Hannes Laimer @ 2022-02-17 17:14 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
pbs-datastore/src/task_tracking.rs | 61 ++++++++++++++++++++++--------
src/api2/admin/datastore.rs | 31 ++++++++++++++-
2 files changed, 75 insertions(+), 17 deletions(-)
diff --git a/pbs-datastore/src/task_tracking.rs b/pbs-datastore/src/task_tracking.rs
index a02d9a17..c0a419e4 100644
--- a/pbs-datastore/src/task_tracking.rs
+++ b/pbs-datastore/src/task_tracking.rs
@@ -1,6 +1,7 @@
use anyhow::Error;
use libc::pid_t;
use nix::unistd::Pid;
+use std::iter::Sum;
use std::path::PathBuf;
use pbs_api_types::Operation;
@@ -8,12 +9,46 @@ use proxmox_sys::fs::{file_read_optional_string, open_file_locked, replace_file,
use proxmox_sys::linux::procfs;
use serde::{Deserialize, Serialize};
+#[derive(Deserialize, Serialize, Clone, Copy, Default)]
+pub struct ActiveOperationStats {
+ pub read: i64,
+ pub write: i64,
+}
+
+impl Sum<Self> for ActiveOperationStats {
+ fn sum<I>(iter: I) -> Self
+ where
+ I: Iterator<Item = Self>,
+ {
+ iter.fold(Self::default(), |a, b| Self {
+ read: a.read + b.read,
+ write: a.write + b.write,
+ })
+ }
+}
+
#[derive(Deserialize, Serialize, Clone)]
struct TaskOperations {
pid: u32,
starttime: u64,
- reading_operations: i64,
- writing_operations: i64,
+ active_operations: ActiveOperationStats,
+}
+
+pub fn get_active_operations(name: &str) -> Result<ActiveOperationStats, Error> {
+ let path = PathBuf::from(format!("{}/{}", crate::ACTIVE_OPERATIONS_DIR, name));
+
+ Ok(match file_read_optional_string(&path)? {
+ Some(data) => serde_json::from_str::<Vec<TaskOperations>>(&data)?
+ .iter()
+ .filter_map(
+ |task| match procfs::check_process_running(task.pid as pid_t) {
+ Some(stat) if task.starttime == stat.starttime => Some(task.active_operations),
+ _ => None,
+ },
+ )
+ .sum(),
+ None => ActiveOperationStats::default(),
+ })
}
pub fn update_active_operations(name: &str, operation: Operation, count: i64) -> Result<(), Error> {
@@ -43,8 +78,8 @@ pub fn update_active_operations(name: &str, operation: Operation, count: i64) ->
if pid == task.pid {
updated = true;
match operation {
- Operation::Read => task.reading_operations += count,
- Operation::Write => task.writing_operations += count,
+ Operation::Read => task.active_operations.read += count,
+ Operation::Write => task.active_operations.write += count,
};
}
Some(task.clone())
@@ -57,18 +92,12 @@ pub fn update_active_operations(name: &str, operation: Operation, count: i64) ->
};
if !updated {
- updated_tasks.push(match operation {
- Operation::Read => TaskOperations {
- pid,
- starttime,
- reading_operations: 1,
- writing_operations: 0,
- },
- Operation::Write => TaskOperations {
- pid,
- starttime,
- reading_operations: 0,
- writing_operations: 1,
+ updated_tasks.push(TaskOperations {
+ pid,
+ starttime,
+ active_operations: match operation {
+ Operation::Read => ActiveOperationStats { read: 1, write: 0 },
+ Operation::Write => ActiveOperationStats { read: 0, write: 1 },
},
})
}
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index ce710938..aa7c818d 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -43,7 +43,7 @@ use pbs_api_types::{ Authid, BackupContent, Counts, CryptMode,
use pbs_client::pxar::create_zip;
use pbs_datastore::{
check_backup_owner, DataStore, BackupDir, BackupGroup, StoreProgress, LocalChunkReader,
- CATALOG_NAME,
+ CATALOG_NAME, task_tracking
};
use pbs_datastore::backup_info::BackupInfo;
use pbs_datastore::cached_chunk_reader::CachedChunkReader;
@@ -1590,6 +1590,30 @@ pub fn get_rrd_stats(
)
}
+#[api(
+ input: {
+ properties: {
+ store: {
+ schema: DATASTORE_SCHEMA,
+ },
+ },
+ },
+ access: {
+ permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT, true),
+ },
+)]
+/// Read datastore stats
+pub fn get_active_operations(
+ store: String,
+ _param: Value,
+) -> Result<Value, Error> {
+ let active_operations = task_tracking::get_active_operations(&store)?;
+ Ok(json!({
+ "read": active_operations.read,
+ "write": active_operations.write,
+ }))
+}
+
#[api(
input: {
properties: {
@@ -1947,6 +1971,11 @@ pub fn set_backup_owner(
#[sortable]
const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
+ (
+ "active-operations",
+ &Router::new()
+ .get(&API_METHOD_GET_ACTIVE_OPERATIONS)
+ ),
(
"catalog",
&Router::new()
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v9 6/6] ui: add option to change the maintenance type
2022-02-17 17:14 [pbs-devel] [PATCH proxmox-backup v9 0/6] closes #3071: maintenance mode for datastore Hannes Laimer
` (4 preceding siblings ...)
2022-02-17 17:14 ` [pbs-devel] [PATCH proxmox-backup v9 5/6] api: add get_active_operations endpoint Hannes Laimer
@ 2022-02-17 17:14 ` Hannes Laimer
2022-04-06 13:08 ` [pbs-devel] [PATCH proxmox-backup v9 0/6] closes #3071: maintenance mode for datastore Dylan Whyte
6 siblings, 0 replies; 12+ messages in thread
From: Hannes Laimer @ 2022-02-17 17:14 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
www/Makefile | 1 +
www/Utils.js | 23 ++++++++++
www/datastore/OptionView.js | 30 +++++++++++++
www/window/MaintenanceOptions.js | 77 ++++++++++++++++++++++++++++++++
4 files changed, 131 insertions(+)
create mode 100644 www/window/MaintenanceOptions.js
diff --git a/www/Makefile b/www/Makefile
index 455fbeec..0952fb82 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -61,6 +61,7 @@ JSSRC= \
window/BackupGroupChangeOwner.js \
window/CreateDirectory.js \
window/DataStoreEdit.js \
+ window/MaintenanceOptions.js \
window/NotesEdit.js \
window/RemoteEdit.js \
window/TrafficControlEdit.js \
diff --git a/www/Utils.js b/www/Utils.js
index 36a94211..b99bec32 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -640,4 +640,27 @@ Ext.define('PBS.Utils', {
return `${icon} ${value}`;
},
+ renderMaintenance: function(mode, activeTasks) {
+ if (!mode) return gettext('None');
+ let [type, _message] = mode.split(",");
+ type = type.split("=").pop();
+
+ const conflictingTasks = activeTasks.write + (type === 'offline' ? activeTasks.read : 0);
+ const checkmarkIcon = '<i class="fa fa-check"></i>';
+ const spinnerIcon = '<i class="fa fa-spinner fa-pulse fa-fw"></i>';
+ const conflictingTasksMessage = `<i>${conflictingTasks} conflicting tasks still active</i>`;
+ const extra = conflictingTasks > 0 ? `| ${spinnerIcon} ${conflictingTasksMessage}` : checkmarkIcon;
+
+ let modeText = Proxmox.Utils.unknownText;
+ switch (type) {
+ case 'read-only':
+ modeText = gettext("Read-only");
+ break;
+ case 'offline':
+ modeText = gettext("Offline");
+ break;
+ }
+ return `${modeText} ${extra}`;
+ },
+
});
diff --git a/www/datastore/OptionView.js b/www/datastore/OptionView.js
index 5a5e85be..25fe71dc 100644
--- a/www/datastore/OptionView.js
+++ b/www/datastore/OptionView.js
@@ -1,3 +1,4 @@
+
Ext.define('PBS.Datastore.Options', {
extend: 'Proxmox.grid.ObjectGrid',
xtype: 'pbsDatastoreOptionView',
@@ -6,6 +7,10 @@ Ext.define('PBS.Datastore.Options', {
cbindData: function(initial) {
let me = this;
+ me.maintenanceActiveTasks = {
+ read: 0,
+ write: 0,
+ };
me.datastore = encodeURIComponent(me.datastore);
me.url = `/api2/json/config/datastore/${me.datastore}`;
me.editorConfig = {
@@ -18,6 +23,24 @@ Ext.define('PBS.Datastore.Options', {
controller: {
xclass: 'Ext.app.ViewController',
+ init: function(view) {
+ let me = this;
+ me.callParent();
+ view.rows['maintenance-mode'].renderer =
+ (value) => PBS.Utils.renderMaintenance(value, view.maintenanceActiveTasks);
+
+ me.activeOperationsRstore = Ext.create('Proxmox.data.ObjectStore', {
+ url: `/api2/json/admin/datastore/${view.datastore}/active-operations`,
+ interval: 3000,
+ });
+ me.activeOperationsRstore.startUpdate();
+
+ view.mon(me.activeOperationsRstore, 'load', (store, data, success) => {
+ me.view.maintenanceActiveTasks.read = data[0].data.value;
+ me.view.maintenanceActiveTasks.write = data[1].data.value;
+ });
+ },
+
edit: function() {
this.getView().run_editor();
},
@@ -111,5 +134,12 @@ Ext.define('PBS.Datastore.Options', {
},
},
},
+ "maintenance-mode": {
+ required: true,
+ header: gettext('Maintenance mode'),
+ editor: {
+ xtype: 'pbsMaintenanceOptionEdit',
+ },
+ },
},
});
diff --git a/www/window/MaintenanceOptions.js b/www/window/MaintenanceOptions.js
new file mode 100644
index 00000000..1aa03cbd
--- /dev/null
+++ b/www/window/MaintenanceOptions.js
@@ -0,0 +1,77 @@
+Ext.define('PBS.form.maintenanceType', {
+ extend: 'Proxmox.form.KVComboBox',
+ alias: 'widget.pbsMaintenanceType',
+
+ comboItems: [
+ ['__default__', gettext('None')],
+ ['read-only', gettext('Read only')],
+ ['offline', gettext('Offline')],
+ ],
+});
+
+Ext.define('PBS.window.MaintenanceOptions', {
+ extend: 'Proxmox.window.Edit',
+ xtype: 'pbsMaintenanceOptionEdit',
+ mixins: ['Proxmox.Mixin.CBind'],
+
+ subject: gettext('Maintenance mode'),
+
+ width: 450,
+ fieldDefaults: {
+ labelWidth: 120,
+ },
+
+ items: {
+ onGetValues: function(values) {
+ if (values.delete === 'maintenance-type') {
+ values.delete = 'maintenance-mode';
+ } else if (values['maintenance-type']) {
+ const escaped_message = values['maintenance-msg']
+ .replaceAll('\\', '')
+ .replaceAll('"', '\\"');
+ const maybe_message =
+ values['maintenance-msg'] ? `,message="${escaped_message}"` : '';
+ values['maintenance-mode'] = `type=${values['maintenance-type']}${maybe_message}`;
+ }
+ delete values['maintenance-type'];
+ delete values['maintenance-msg'];
+ return values;
+ },
+ xtype: 'inputpanel',
+ items: [
+ {
+ xtype: 'pbsMaintenanceType',
+ name: 'maintenance-type',
+ fieldLabel: gettext('Maintenance Type'),
+ value: '__default__',
+ deleteEmpty: true,
+ },
+ {
+ xtype: 'proxmoxtextfield',
+ name: 'maintenance-msg',
+ fieldLabel: gettext('Description'),
+ },
+ ],
+ },
+ setValues: function(values) {
+ let me = this;
+
+ let options = {
+ 'maintenance-type': '__default__',
+ 'maintenance-msg': '',
+ };
+ if (values['maintenance-mode']) {
+ let [type, message] = values['maintenance-mode'].split(/,(.+)/);
+ type = type.split("=").pop();
+ message = message ? message.split("=")[1]
+ .replace(/^"(.*)"$/, '$1')
+ .replaceAll('\\"', '"') : '';
+ options = {
+ 'maintenance-type': type,
+ 'maintenance-msg': message,
+ };
+ }
+
+ me.callParent([options]);
+ },
+});
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup v9 0/6] closes #3071: maintenance mode for datastore
2022-02-17 17:14 [pbs-devel] [PATCH proxmox-backup v9 0/6] closes #3071: maintenance mode for datastore Hannes Laimer
` (5 preceding siblings ...)
2022-02-17 17:14 ` [pbs-devel] [PATCH proxmox-backup v9 6/6] ui: add option to change the maintenance type Hannes Laimer
@ 2022-04-06 13:08 ` Dylan Whyte
2022-04-11 7:35 ` Thomas Lamprecht
6 siblings, 1 reply; 12+ messages in thread
From: Dylan Whyte @ 2022-04-06 13:08 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
Hi,
On 2/17/22 18:14, Hannes Laimer wrote:
> Adds maintenance mode and tracking of active reading/writing operations.
> The maintenance mode prevents the start of new operations if the type of
> operation they would perform on the datastore would conflict with the
> maintenance type that is currently set. This check is performed when
> lookup_datastore is called. Tasks only call this function once at the
> beginning, therefore updating the maintenance type cannot interfere with
> already running tasks.
>
> active operations tracking:
> Changed file layout to now also keep track of the pid+starttime and the
> counts of operations that that pid started, like this it is possible to
> not count operations that were started by a dead process, since they
> are also not active anymore. Whenever the file is updated, also entries
> of dead processes are removed. When the file is read, only entries of
> active processes are counted.
>
> The UI shows a spinner with the count of conflictintg tasks (the tasks
> that were started before the maintenance type was updated) next to it.
> As soon as all conflicting tasks are finished a checkmark appears.
>
> Hannes Laimer (6):
> api-types: add maintenance type
> datastore: add check for maintenance in lookup
> pbs-datastore: add active operations tracking
> api: make maintenance_type updatable
> api: add get_active_operations endpoint
> ui: add option to change the maintenance type
>
> debian/control | 6 +-
> pbs-api-types/Cargo.toml | 2 +-
> pbs-api-types/src/datastore.rs | 24 +++++-
> pbs-api-types/src/lib.rs | 3 +
> pbs-api-types/src/maintenance.rs | 78 +++++++++++++++++
> pbs-datastore/Cargo.toml | 1 +
> pbs-datastore/src/datastore.rs | 121 +++++++++++++++++++--------
> pbs-datastore/src/lib.rs | 4 +
> pbs-datastore/src/snapshot_reader.rs | 6 +-
> pbs-datastore/src/task_tracking.rs | 110 ++++++++++++++++++++++++
> src/api2/admin/datastore.rs | 81 ++++++++++++------
> src/api2/backup/mod.rs | 4 +-
> src/api2/config/datastore.rs | 5 ++
> src/api2/reader/mod.rs | 6 +-
> src/api2/status.rs | 4 +-
> src/api2/tape/backup.rs | 6 +-
> src/api2/tape/restore.rs | 6 +-
> src/bin/proxmox-backup-api.rs | 1 +
> src/bin/proxmox-backup-proxy.rs | 4 +-
> src/server/mod.rs | 16 +++-
> src/server/prune_job.rs | 4 +-
> src/server/pull.rs | 4 +-
> src/server/verify_job.rs | 4 +-
> www/Makefile | 1 +
> www/Utils.js | 23 +++++
> www/datastore/OptionView.js | 30 +++++++
> www/window/MaintenanceOptions.js | 77 +++++++++++++++++
> 27 files changed, 540 insertions(+), 91 deletions(-)
> create mode 100644 pbs-api-types/src/maintenance.rs
> create mode 100644 pbs-datastore/src/task_tracking.rs
> create mode 100644 www/window/MaintenanceOptions.js
I just wanted to reply to this series, to get the ball rolling again.
However, I can't say that I found much real issues with it on first
glance, so I've just added some very minor suggestions.
As a rebase is now required for it to apply, I haven't done any testing.
I'd be happy to test once it's rebased though.
regards,
Dylan
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup v9 1/6] api-types: add maintenance type
2022-02-17 17:14 ` [pbs-devel] [PATCH proxmox-backup v9 1/6] api-types: add maintenance type Hannes Laimer
@ 2022-04-06 13:08 ` Dylan Whyte
2022-04-11 7:33 ` Thomas Lamprecht
0 siblings, 1 reply; 12+ messages in thread
From: Dylan Whyte @ 2022-04-06 13:08 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
Minor comments inline:
On 2/17/22 18:14, Hannes Laimer wrote:
> + bump proxmox-schema dep to 1.2.1 (for quoted property string)
>
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> debian/control | 6 ++--
> pbs-api-types/Cargo.toml | 2 +-
> pbs-api-types/src/datastore.rs | 24 ++++++++++---
> pbs-api-types/src/lib.rs | 3 ++
> pbs-api-types/src/maintenance.rs | 59 ++++++++++++++++++++++++++++++++
> 5 files changed, 86 insertions(+), 8 deletions(-)
> create mode 100644 pbs-api-types/src/maintenance.rs
>
> diff --git a/debian/control b/debian/control
> index 02a32977..f869d32d 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -56,9 +56,9 @@ Build-Depends: debhelper (>= 12),
> librust-proxmox-openid-0.9+default-dev,
> librust-proxmox-router-1+cli-dev (>= 1.1-~~),
> librust-proxmox-router-1+default-dev (>= 1.1-~~),
> - librust-proxmox-schema-1+api-macro-dev (>= 1.1-~~),
> - librust-proxmox-schema-1+default-dev (>= 1.1-~~),
> - librust-proxmox-schema-1+upid-api-impl-dev (>= 1.1-~~),
> + librust-proxmox-schema-1+api-macro-dev (>= 1.2.1-~~),
> + librust-proxmox-schema-1+default-dev (>= 1.2.1-~~),
> + librust-proxmox-schema-1+upid-api-impl-dev (>= 1.2.1-~~),
> librust-proxmox-section-config-1+default-dev,
> librust-proxmox-serde-0.1+default-dev,
> librust-proxmox-shared-memory-0.2+default-dev,
> diff --git a/pbs-api-types/Cargo.toml b/pbs-api-types/Cargo.toml
> index e77d8bc4..1a98b09d 100644
> --- a/pbs-api-types/Cargo.toml
> +++ b/pbs-api-types/Cargo.toml
> @@ -14,7 +14,7 @@ regex = "1.2"
> serde = { version = "1.0", features = ["derive"] }
>
> proxmox-lang = "1.0.0"
> -proxmox-schema = { version = "1.1", features = [ "api-macro" ] }
> +proxmox-schema = { version = "1.2.1", features = [ "api-macro" ] }
> proxmox-serde = "0.1"
> proxmox-time = "1.1.1"
> proxmox-uuid = { version = "1.0.0", features = [ "serde" ] }
> diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
> index 36279b3a..7a2d6ae2 100644
> --- a/pbs-api-types/src/datastore.rs
> +++ b/pbs-api-types/src/datastore.rs
> @@ -6,10 +6,9 @@ use proxmox_schema::{
> };
>
> use crate::{
> - PROXMOX_SAFE_ID_FORMAT, SHA256_HEX_REGEX, SINGLE_LINE_COMMENT_SCHEMA, CryptMode, UPID,
> - Fingerprint, Userid, Authid,
> - GC_SCHEDULE_SCHEMA, DATASTORE_NOTIFY_STRING_SCHEMA, PRUNE_SCHEDULE_SCHEMA,
> -
> + Authid, CryptMode, Fingerprint, MaintenanceMode, Userid, DATASTORE_NOTIFY_STRING_SCHEMA,
> + GC_SCHEDULE_SCHEMA, PROXMOX_SAFE_ID_FORMAT, PRUNE_SCHEDULE_SCHEMA, SHA256_HEX_REGEX,
> + SINGLE_LINE_COMMENT_SCHEMA, UPID,
> };
>
> const_regex!{
> @@ -224,6 +223,11 @@ pub struct PruneOptions {
> optional: true,
> type: bool,
> },
> + "maintenance-mode": {
> + optional: true,
> + format: &ApiStringFormat::PropertyString(&MaintenanceMode::API_SCHEMA),
> + type: String,
> + },
> }
> )]
> #[derive(Serialize,Deserialize,Updater)]
> @@ -261,6 +265,18 @@ pub struct DataStoreConfig {
> /// Send notification only for job errors
> #[serde(skip_serializing_if="Option::is_none")]
> pub notify: Option<String>,
> + /// Maintenance mode, type is either 'offline' or 'read-only', message should be enclosed in "
> + #[serde(skip_serializing_if = "Option::is_none")]
> + pub maintenance_mode: Option<String>,
> +}
> +
> +impl DataStoreConfig {
> + pub fn get_maintenance_mode(&self) -> Option<MaintenanceMode> {
> + self.maintenance_mode
> + .as_ref()
> + .and_then(|str| MaintenanceMode::API_SCHEMA.parse_property_string(str).ok())
> + .and_then(|value| MaintenanceMode::deserialize(value).ok())
> + }
> }
>
> #[api(
> diff --git a/pbs-api-types/src/lib.rs b/pbs-api-types/src/lib.rs
> index 754e7b22..efb01c3e 100644
> --- a/pbs-api-types/src/lib.rs
> +++ b/pbs-api-types/src/lib.rs
> @@ -49,6 +49,9 @@ pub use jobs::*;
> mod key_derivation;
> pub use key_derivation::{Kdf, KeyInfo};
>
> +mod maintenance;
> +pub use maintenance::*;
> +
> mod network;
> pub use network::*;
>
> diff --git a/pbs-api-types/src/maintenance.rs b/pbs-api-types/src/maintenance.rs
> new file mode 100644
> index 00000000..19883e46
> --- /dev/null
> +++ b/pbs-api-types/src/maintenance.rs
> @@ -0,0 +1,59 @@
> +use serde::{Deserialize, Serialize};
> +
> +use proxmox_schema::{api, ApiStringFormat, const_regex, Schema, StringSchema};
> +
> +const_regex!{
> + pub MAINTENANCE_MESSAGE_REGEX = r"^[[:^cntrl:]]*$";
> +}
> +
> +pub const MAINTENANCE_MESSAGE_FORMAT: ApiStringFormat =
> + ApiStringFormat::Pattern(&MAINTENANCE_MESSAGE_REGEX);
> +
> +
> +pub const MAINTENANCE_MESSAGE_SCHEMA: Schema =
> + StringSchema::new("Message describing the reason for the maintenance.")
> + .format(&MAINTENANCE_MESSAGE_FORMAT)
> + .max_length(32)
> + .schema();
Is 32 characters enough for the message? I would think this is somewhat
limiting, but am open to other opinions.
> +
> +#[derive(Clone, Copy, Debug)]
> +/// Operation requirements, used when checking for maintenance mode.
> +pub enum Operation {
> + Read,
> + Write,
> +}
> +
> +#[api]
> +#[derive(Deserialize, Serialize)]
> +#[serde(rename_all="kebab-case")]
> +/// Maintenance type.
> +pub enum MaintenanceType {
> + /// Only reading operations are allowed on the datastore.
docstring: s/reading/read/
> + ReadOnly,
> + /// Neither reading nor writing operations are allowed on the datastore.
docstring: s/reading/read/
docstring: s/writing/write/
> + Offline,
> +}
> +
> +#[api(
> + properties: {
> + type: {
> + type: MaintenanceType,
> + },
> + message: {
> + optional: true,
> + schema: MAINTENANCE_MESSAGE_SCHEMA,
> + }
> + },
> + default_key: "type",
> +)]
> +#[derive(Deserialize, Serialize)]
> +/// Maintenance mode
> +pub struct MaintenanceMode {
> + /// Type of the maintenance
docstring: Reword to 'Type of maintenance ("read-only" or "offline").'
> + #[serde(rename = "type")]
> + ty: MaintenanceType,
> +
> + /// Reason for the maintenance mode.
docstring: Maybe reword to "Reason for maintenance."
> + #[serde(skip_serializing_if = "Option::is_none")]
> + message: Option<String>,
> +}
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup v9 2/6] datastore: add check for maintenance in lookup
2022-02-17 17:14 ` [pbs-devel] [PATCH proxmox-backup v9 2/6] datastore: add check for maintenance in lookup Hannes Laimer
@ 2022-04-06 13:08 ` Dylan Whyte
0 siblings, 0 replies; 12+ messages in thread
From: Dylan Whyte @ 2022-04-06 13:08 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
Minor comment inline:
On 2/17/22 18:14, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> pbs-api-types/src/maintenance.rs | 23 +++++++++++--
> pbs-datastore/src/datastore.rs | 17 +++++++---
> pbs-datastore/src/snapshot_reader.rs | 6 +++-
> src/api2/admin/datastore.rs | 50 ++++++++++++++--------------
> src/api2/backup/mod.rs | 4 +--
> src/api2/reader/mod.rs | 6 ++--
> src/api2/status.rs | 4 +--
> src/api2/tape/backup.rs | 6 ++--
> src/api2/tape/restore.rs | 6 ++--
> src/bin/proxmox-backup-proxy.rs | 4 +--
> src/server/prune_job.rs | 4 +--
> src/server/pull.rs | 4 +--
> src/server/verify_job.rs | 4 +--
> 13 files changed, 85 insertions(+), 53 deletions(-)
>
> diff --git a/pbs-api-types/src/maintenance.rs b/pbs-api-types/src/maintenance.rs
> index 19883e46..55d66964 100644
> --- a/pbs-api-types/src/maintenance.rs
> +++ b/pbs-api-types/src/maintenance.rs
> @@ -1,3 +1,5 @@
> +use std::borrow::Cow;
> +use anyhow::{bail, Error};
> use serde::{Deserialize, Serialize};
>
> use proxmox_schema::{api, ApiStringFormat, const_regex, Schema, StringSchema};
> @@ -24,7 +26,7 @@ pub enum Operation {
> }
>
> #[api]
> -#[derive(Deserialize, Serialize)]
> +#[derive(Deserialize, Serialize, PartialEq)]
> #[serde(rename_all="kebab-case")]
> /// Maintenance type.
> pub enum MaintenanceType {
> @@ -49,7 +51,7 @@ pub enum MaintenanceType {
> #[derive(Deserialize, Serialize)]
> /// Maintenance mode
> pub struct MaintenanceMode {
> - /// Type of the maintenance
> + /// Type of the maintenance.
> #[serde(rename = "type")]
> ty: MaintenanceType,
>
> @@ -57,3 +59,20 @@ pub struct MaintenanceMode {
> #[serde(skip_serializing_if = "Option::is_none")]
> message: Option<String>,
> }
> +
> +impl MaintenanceMode {
> + pub fn check(&self, operation: Option<Operation>) -> Result<(), Error> {
> + let message = percent_encoding::percent_decode_str(self.message.as_deref().unwrap_or(""))
> + .decode_utf8()
> + .unwrap_or(Cow::Borrowed(""));
You could possibly add a more generic default/fallback message here,
such as "contact the administrator for more info". This adds less
confusion, given the trailing semi-colon below, but is really not a big
deal in any case.
> +
> + if self.ty == MaintenanceType::Offline {
> + bail!("offline maintenance mode: {}", message);
> + } else if self.ty == MaintenanceType::ReadOnly {
> + if let Some(Operation::Write) = operation {
> + bail!("read-only maintenance mode: {}", message);
> + }
> + }
> + Ok(())
> + }
> +}
> diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
> index 7044e074..902848d6 100644
> --- a/pbs-datastore/src/datastore.rs
> +++ b/pbs-datastore/src/datastore.rs
> @@ -15,7 +15,9 @@ use proxmox_sys::WorkerTaskContext;
> use proxmox_sys::{task_log, task_warn};
> use proxmox_sys::fs::{lock_dir_noblock, DirLockGuard};
>
> -use pbs_api_types::{UPID, DataStoreConfig, Authid, GarbageCollectionStatus, HumanByte};
> +use pbs_api_types::{
> + UPID, DataStoreConfig, Authid, Operation, GarbageCollectionStatus, HumanByte
> +};
> use pbs_config::{open_backup_lockfile, BackupLockGuard};
>
> use crate::DataBlob;
> @@ -60,13 +62,20 @@ pub struct DataStore {
> }
>
> impl DataStore {
> -
> - pub fn lookup_datastore(name: &str) -> Result<Arc<DataStore>, Error> {
> -
> + pub fn lookup_datastore(
> + name: &str,
> + operation: Option<Operation>,
> + ) -> Result<Arc<DataStore>, Error> {
> let (config, _digest) = pbs_config::datastore::config()?;
> let config: DataStoreConfig = config.lookup("datastore", name)?;
> let path = PathBuf::from(&config.path);
>
> + if let Some(maintenance_mode) = config.get_maintenance_mode() {
> + if let Err(error) = maintenance_mode.check(operation) {
> + bail!("datastore '{}' is in {}", name, error);
> + }
> + }
> +
> let mut map = DATASTORE_MAP.lock().unwrap();
>
> if let Some(datastore) = map.get(name) {
> diff --git a/pbs-datastore/src/snapshot_reader.rs b/pbs-datastore/src/snapshot_reader.rs
> index 18bc0d83..65abac7d 100644
> --- a/pbs-datastore/src/snapshot_reader.rs
> +++ b/pbs-datastore/src/snapshot_reader.rs
> @@ -14,6 +14,7 @@ use crate::fixed_index::FixedIndexReader;
> use crate::dynamic_index::DynamicIndexReader;
> use crate::manifest::{archive_type, ArchiveType, CLIENT_LOG_BLOB_NAME, MANIFEST_BLOB_NAME};
> use crate::DataStore;
> +use pbs_api_types::Operation;
>
> /// Helper to access the contents of a datastore backup snapshot
> ///
> @@ -120,7 +121,10 @@ impl <'a> Iterator for SnapshotChunkIterator<'a> {
> };
>
> let datastore =
> - DataStore::lookup_datastore(self.snapshot_reader.datastore_name())?;
> + DataStore::lookup_datastore(
> + self.snapshot_reader.datastore_name(),
> + Some(Operation::Read)
> + )?;
> let order = datastore.get_chunks_in_order(&index, |_| false, |_| Ok(()))?;
>
> self.current_index = Some((Arc::new(index), 0, order));
> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
> index 263ea96f..ce710938 100644
> --- a/src/api2/admin/datastore.rs
> +++ b/src/api2/admin/datastore.rs
> @@ -30,7 +30,7 @@ use pxar::EntryKind;
>
> use pbs_api_types::{ Authid, BackupContent, Counts, CryptMode,
> DataStoreListItem, GarbageCollectionStatus, GroupListItem,
> - SnapshotListItem, SnapshotVerifyState, PruneOptions,
> + Operation, SnapshotListItem, SnapshotVerifyState, PruneOptions,
> DataStoreStatus, RRDMode, RRDTimeFrame,
> BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_TIME_SCHEMA,
> BACKUP_TYPE_SCHEMA, DATASTORE_SCHEMA,
> @@ -170,7 +170,7 @@ pub fn list_groups(
> let user_info = CachedUserInfo::new()?;
> let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]);
>
> - let datastore = DataStore::lookup_datastore(&store)?;
> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
> let list_all = (user_privs & PRIV_DATASTORE_AUDIT) != 0;
>
> let backup_groups = BackupInfo::list_backup_groups(&datastore.base_path())?;
> @@ -268,7 +268,7 @@ pub fn delete_group(
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
>
> let group = BackupGroup::new(backup_type, backup_id);
> - let datastore = DataStore::lookup_datastore(&store)?;
> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
>
> check_priv_or_backup_owner(&datastore, &group, &auth_id, PRIV_DATASTORE_MODIFY)?;
>
> @@ -315,7 +315,7 @@ pub fn list_snapshot_files(
> ) -> Result<Vec<BackupContent>, Error> {
>
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> - let datastore = DataStore::lookup_datastore(&store)?;
> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
>
> let snapshot = BackupDir::new(backup_type, backup_id, backup_time)?;
>
> @@ -365,7 +365,7 @@ pub fn delete_snapshot(
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
>
> let snapshot = BackupDir::new(backup_type, backup_id, backup_time)?;
> - let datastore = DataStore::lookup_datastore(&store)?;
> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
>
> check_priv_or_backup_owner(&datastore, snapshot.group(), &auth_id, PRIV_DATASTORE_MODIFY)?;
>
> @@ -414,7 +414,7 @@ pub fn list_snapshots (
>
> let list_all = (user_privs & PRIV_DATASTORE_AUDIT) != 0;
>
> - let datastore = DataStore::lookup_datastore(&store)?;
> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
>
> let base_path = datastore.base_path();
>
> @@ -615,7 +615,7 @@ pub fn status(
> _info: &ApiMethod,
> rpcenv: &mut dyn RpcEnvironment,
> ) -> Result<DataStoreStatus, Error> {
> - let datastore = DataStore::lookup_datastore(&store)?;
> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
> let storage = crate::tools::disks::disk_usage(&datastore.base_path())?;
> let (counts, gc_status) = if verbose {
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> @@ -693,7 +693,7 @@ pub fn verify(
> outdated_after: Option<i64>,
> rpcenv: &mut dyn RpcEnvironment,
> ) -> Result<Value, Error> {
> - let datastore = DataStore::lookup_datastore(&store)?;
> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
> let ignore_verified = ignore_verified.unwrap_or(true);
>
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> @@ -838,7 +838,7 @@ pub fn prune(
>
> let group = BackupGroup::new(&backup_type, &backup_id);
>
> - let datastore = DataStore::lookup_datastore(&store)?;
> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
>
> check_priv_or_backup_owner(&datastore, &group, &auth_id, PRIV_DATASTORE_MODIFY)?;
>
> @@ -963,7 +963,7 @@ pub fn prune_datastore(
>
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
>
> - let datastore = DataStore::lookup_datastore(&store)?;
> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
>
> let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
>
> @@ -1007,7 +1007,7 @@ pub fn start_garbage_collection(
> rpcenv: &mut dyn RpcEnvironment,
> ) -> Result<Value, Error> {
>
> - let datastore = DataStore::lookup_datastore(&store)?;
> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
>
> let job = Job::new("garbage_collection", &store)
> @@ -1043,7 +1043,7 @@ pub fn garbage_collection_status(
> _rpcenv: &mut dyn RpcEnvironment,
> ) -> Result<GarbageCollectionStatus, Error> {
>
> - let datastore = DataStore::lookup_datastore(&store)?;
> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
>
> let status = datastore.last_gc_status();
>
> @@ -1119,7 +1119,7 @@ pub fn download_file(
>
> async move {
> let store = required_string_param(¶m, "store")?;
> - let datastore = DataStore::lookup_datastore(store)?;
> + let datastore = DataStore::lookup_datastore(store, Some(Operation::Read))?;
>
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
>
> @@ -1189,7 +1189,7 @@ pub fn download_file_decoded(
>
> async move {
> let store = required_string_param(¶m, "store")?;
> - let datastore = DataStore::lookup_datastore(store)?;
> + let datastore = DataStore::lookup_datastore(store, Some(Operation::Read))?;
>
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
>
> @@ -1303,7 +1303,7 @@ pub fn upload_backup_log(
>
> async move {
> let store = required_string_param(¶m, "store")?;
> - let datastore = DataStore::lookup_datastore(store)?;
> + let datastore = DataStore::lookup_datastore(store, Some(Operation::Write))?;
>
> let file_name = CLIENT_LOG_BLOB_NAME;
>
> @@ -1380,7 +1380,7 @@ pub fn catalog(
> filepath: String,
> rpcenv: &mut dyn RpcEnvironment,
> ) -> Result<Vec<ArchiveEntry>, Error> {
> - let datastore = DataStore::lookup_datastore(&store)?;
> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
>
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
>
> @@ -1450,7 +1450,7 @@ pub fn pxar_file_download(
>
> async move {
> let store = required_string_param(¶m, "store")?;
> - let datastore = DataStore::lookup_datastore(store)?;
> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
>
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
>
> @@ -1567,7 +1567,7 @@ pub fn get_rrd_stats(
> _param: Value,
> ) -> Result<Value, Error> {
>
> - let datastore = DataStore::lookup_datastore(&store)?;
> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
> let disk_manager = crate::tools::disks::DiskManage::new();
>
> let mut rrd_fields = vec![
> @@ -1615,7 +1615,7 @@ pub fn get_group_notes(
> backup_id: String,
> rpcenv: &mut dyn RpcEnvironment,
> ) -> Result<String, Error> {
> - let datastore = DataStore::lookup_datastore(&store)?;
> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
>
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> let backup_group = BackupGroup::new(backup_type, backup_id);
> @@ -1657,7 +1657,7 @@ pub fn set_group_notes(
> notes: String,
> rpcenv: &mut dyn RpcEnvironment,
> ) -> Result<(), Error> {
> - let datastore = DataStore::lookup_datastore(&store)?;
> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
>
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> let backup_group = BackupGroup::new(backup_type, backup_id);
> @@ -1699,7 +1699,7 @@ pub fn get_notes(
> backup_time: i64,
> rpcenv: &mut dyn RpcEnvironment,
> ) -> Result<String, Error> {
> - let datastore = DataStore::lookup_datastore(&store)?;
> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
>
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?;
> @@ -1750,7 +1750,7 @@ pub fn set_notes(
> notes: String,
> rpcenv: &mut dyn RpcEnvironment,
> ) -> Result<(), Error> {
> - let datastore = DataStore::lookup_datastore(&store)?;
> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
>
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?;
> @@ -1793,7 +1793,7 @@ pub fn get_protection(
> backup_time: i64,
> rpcenv: &mut dyn RpcEnvironment,
> ) -> Result<bool, Error> {
> - let datastore = DataStore::lookup_datastore(&store)?;
> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
>
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?;
> @@ -1838,7 +1838,7 @@ pub fn set_protection(
> protected: bool,
> rpcenv: &mut dyn RpcEnvironment,
> ) -> Result<(), Error> {
> - let datastore = DataStore::lookup_datastore(&store)?;
> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
>
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?;
> @@ -1879,7 +1879,7 @@ pub fn set_backup_owner(
> rpcenv: &mut dyn RpcEnvironment,
> ) -> Result<(), Error> {
>
> - let datastore = DataStore::lookup_datastore(&store)?;
> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
>
> let backup_group = BackupGroup::new(backup_type, backup_id);
>
> diff --git a/src/api2/backup/mod.rs b/src/api2/backup/mod.rs
> index 395edd3d..9d62dc52 100644
> --- a/src/api2/backup/mod.rs
> +++ b/src/api2/backup/mod.rs
> @@ -16,7 +16,7 @@ use proxmox_router::{
> use proxmox_schema::*;
>
> use pbs_api_types::{
> - Authid, VerifyState, SnapshotVerifyState,
> + Authid, Operation, VerifyState, SnapshotVerifyState,
> BACKUP_ID_SCHEMA, BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA, DATASTORE_SCHEMA,
> CHUNK_DIGEST_SCHEMA, PRIV_DATASTORE_BACKUP, BACKUP_ARCHIVE_NAME_SCHEMA,
> };
> @@ -77,7 +77,7 @@ async move {
> let user_info = CachedUserInfo::new()?;
> user_info.check_privs(&auth_id, &["datastore", &store], PRIV_DATASTORE_BACKUP, false)?;
>
> - let datastore = DataStore::lookup_datastore(&store)?;
> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
>
> let backup_type = required_string_param(¶m, "backup-type")?;
> let backup_id = required_string_param(¶m, "backup-id")?;
> diff --git a/src/api2/reader/mod.rs b/src/api2/reader/mod.rs
> index 2b11d1b1..45cefe5d 100644
> --- a/src/api2/reader/mod.rs
> +++ b/src/api2/reader/mod.rs
> @@ -16,8 +16,8 @@ use proxmox_router::{
> use proxmox_schema::{BooleanSchema, ObjectSchema};
>
> use pbs_api_types::{
> - Authid, DATASTORE_SCHEMA, BACKUP_TYPE_SCHEMA, BACKUP_TIME_SCHEMA, BACKUP_ID_SCHEMA,
> - CHUNK_DIGEST_SCHEMA, PRIV_DATASTORE_READ, PRIV_DATASTORE_BACKUP,
> + Authid, Operation, DATASTORE_SCHEMA, BACKUP_TYPE_SCHEMA, BACKUP_TIME_SCHEMA,
> + BACKUP_ID_SCHEMA, CHUNK_DIGEST_SCHEMA, PRIV_DATASTORE_READ, PRIV_DATASTORE_BACKUP,
> BACKUP_ARCHIVE_NAME_SCHEMA,
> };
> use proxmox_sys::fs::lock_dir_noblock_shared;
> @@ -81,7 +81,7 @@ fn upgrade_to_backup_reader_protocol(
> bail!("no permissions on /datastore/{}", store);
> }
>
> - let datastore = DataStore::lookup_datastore(&store)?;
> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
>
> let backup_type = required_string_param(¶m, "backup-type")?;
> let backup_id = required_string_param(¶m, "backup-id")?;
> diff --git a/src/api2/status.rs b/src/api2/status.rs
> index 029529ac..b589951d 100644
> --- a/src/api2/status.rs
> +++ b/src/api2/status.rs
> @@ -14,7 +14,7 @@ use proxmox_router::{
> use proxmox_router::list_subdirs_api_method;
>
> use pbs_api_types::{
> - Authid, DATASTORE_SCHEMA, RRDMode, RRDTimeFrame,
> + Authid, Operation, DATASTORE_SCHEMA, RRDMode, RRDTimeFrame,
> PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP,
> };
>
> @@ -97,7 +97,7 @@ pub fn datastore_status(
> continue;
> }
>
> - let datastore = match DataStore::lookup_datastore(store) {
> + let datastore = match DataStore::lookup_datastore(&store, Some(Operation::Read)) {
> Ok(datastore) => datastore,
> Err(err) => {
> list.push(json!({
> diff --git a/src/api2/tape/backup.rs b/src/api2/tape/backup.rs
> index 1462f200..28b24168 100644
> --- a/src/api2/tape/backup.rs
> +++ b/src/api2/tape/backup.rs
> @@ -10,7 +10,7 @@ use proxmox_schema::api;
> use proxmox_sys::{task_log, task_warn, WorkerTaskContext};
>
> use pbs_api_types::{
> - Authid, Userid, TapeBackupJobConfig, TapeBackupJobSetup, TapeBackupJobStatus, MediaPoolConfig,
> + Authid, Userid, Operation, TapeBackupJobConfig, TapeBackupJobSetup, TapeBackupJobStatus, MediaPoolConfig,
> UPID_SCHEMA, JOB_ID_SCHEMA, PRIV_DATASTORE_READ, PRIV_TAPE_AUDIT, PRIV_TAPE_WRITE,
> GroupFilter,
> };
> @@ -168,7 +168,7 @@ pub fn do_tape_backup_job(
>
> let worker_type = job.jobtype().to_string();
>
> - let datastore = DataStore::lookup_datastore(&setup.store)?;
> + let datastore = DataStore::lookup_datastore(&setup.store, Some(Operation::Read))?;
>
> let (config, _digest) = pbs_config::media_pool::config()?;
> let pool_config: MediaPoolConfig = config.lookup("pool", &setup.pool)?;
> @@ -349,7 +349,7 @@ pub fn backup(
> &setup.drive,
> )?;
>
> - let datastore = DataStore::lookup_datastore(&setup.store)?;
> + let datastore = DataStore::lookup_datastore(&setup.store, Some(Operation::Read))?;
>
> let (config, _digest) = pbs_config::media_pool::config()?;
> let pool_config: MediaPoolConfig = config.lookup("pool", &setup.pool)?;
> diff --git a/src/api2/tape/restore.rs b/src/api2/tape/restore.rs
> index 2ce16c9d..59f098d9 100644
> --- a/src/api2/tape/restore.rs
> +++ b/src/api2/tape/restore.rs
> @@ -17,7 +17,7 @@ use proxmox_uuid::Uuid;
> use proxmox_sys::{task_log, task_warn, WorkerTaskContext};
>
> use pbs_api_types::{
> - Authid, Userid, CryptMode,
> + Authid, Operation, Userid, CryptMode,
> DATASTORE_MAP_ARRAY_SCHEMA, DATASTORE_MAP_LIST_SCHEMA, DRIVE_NAME_SCHEMA,
> UPID_SCHEMA, TAPE_RESTORE_SNAPSHOT_SCHEMA,
> PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_TAPE_READ,
> @@ -93,10 +93,10 @@ impl TryFrom<String> for DataStoreMap {
> if let Some(index) = store.find('=') {
> let mut target = store.split_off(index);
> target.remove(0); // remove '='
> - let datastore = DataStore::lookup_datastore(&target)?;
> + let datastore = DataStore::lookup_datastore(&target, Some(Operation::Write))?;
> map.insert(store, datastore);
> } else if default.is_none() {
> - default = Some(DataStore::lookup_datastore(&store)?);
> + default = Some(DataStore::lookup_datastore(&store, Some(Operation::Write))?);
> } else {
> bail!("multiple default stores given");
> }
> diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
> index a4f20af1..e8315e5f 100644
> --- a/src/bin/proxmox-backup-proxy.rs
> +++ b/src/bin/proxmox-backup-proxy.rs
> @@ -48,7 +48,7 @@ use proxmox_time::CalendarEvent;
>
> use pbs_api_types::{
> Authid, DataStoreConfig, PruneOptions, SyncJobConfig, TapeBackupJobConfig,
> - VerificationJobConfig,
> + VerificationJobConfig, Operation
> };
>
> use proxmox_rest_server::daemon;
> @@ -560,7 +560,7 @@ async fn schedule_datastore_garbage_collection() {
> };
>
> for (store, (_, store_config)) in config.sections {
> - let datastore = match DataStore::lookup_datastore(&store) {
> + let datastore = match DataStore::lookup_datastore(&store, Some(Operation::Write)) {
> Ok(datastore) => datastore,
> Err(err) => {
> eprintln!("lookup_datastore failed - {}", err);
> diff --git a/src/server/prune_job.rs b/src/server/prune_job.rs
> index 825fd60d..d9a6651e 100644
> --- a/src/server/prune_job.rs
> +++ b/src/server/prune_job.rs
> @@ -7,7 +7,7 @@ use proxmox_sys::{task_log, task_warn};
> use pbs_datastore::backup_info::BackupInfo;
> use pbs_datastore::prune::compute_prune_info;
> use pbs_datastore::DataStore;
> -use pbs_api_types::{Authid, PRIV_DATASTORE_MODIFY, PruneOptions};
> +use pbs_api_types::{Authid, Operation, PRIV_DATASTORE_MODIFY, PruneOptions};
> use pbs_config::CachedUserInfo;
> use proxmox_rest_server::WorkerTask;
>
> @@ -97,7 +97,7 @@ pub fn do_prune_job(
> auth_id: &Authid,
> schedule: Option<String>,
> ) -> Result<String, Error> {
> - let datastore = DataStore::lookup_datastore(&store)?;
> + let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
>
> let worker_type = job.jobtype().to_string();
> let auth_id = auth_id.clone();
> diff --git a/src/server/pull.rs b/src/server/pull.rs
> index a2a5f1f1..559533a6 100644
> --- a/src/server/pull.rs
> +++ b/src/server/pull.rs
> @@ -16,7 +16,7 @@ use proxmox_sys::task_log;
>
> use pbs_api_types::{
> Authid, GroupFilter, GroupListItem, RateLimitConfig, Remote,
> - SnapshotListItem,
> + Operation, SnapshotListItem,
> };
>
> use pbs_datastore::{BackupDir, BackupInfo, BackupGroup, DataStore, StoreProgress};
> @@ -57,7 +57,7 @@ impl PullParameters {
> group_filter: Option<Vec<GroupFilter>>,
> limit: RateLimitConfig,
> ) -> Result<Self, Error> {
> - let store = DataStore::lookup_datastore(store)?;
> + let store = DataStore::lookup_datastore(store, Some(Operation::Write))?;
>
> let (remote_config, _digest) = pbs_config::remote::config()?;
> let remote: Remote = remote_config.lookup("remote", remote)?;
> diff --git a/src/server/verify_job.rs b/src/server/verify_job.rs
> index a23eb04e..e472ed91 100644
> --- a/src/server/verify_job.rs
> +++ b/src/server/verify_job.rs
> @@ -1,7 +1,7 @@
> use anyhow::{format_err, Error};
>
> use proxmox_sys::task_log;
> -use pbs_api_types::{Authid, VerificationJobConfig};
> +use pbs_api_types::{Authid, Operation, VerificationJobConfig};
> use proxmox_rest_server::WorkerTask;
> use pbs_datastore::DataStore;
>
> @@ -22,7 +22,7 @@ pub fn do_verification_job(
> to_stdout: bool,
> ) -> Result<String, Error> {
>
> - let datastore = DataStore::lookup_datastore(&verification_job.store)?;
> + let datastore = DataStore::lookup_datastore(&verification_job.store, Some(Operation::Read))?;
>
> let outdated_after = verification_job.outdated_after;
> let ignore_verified_snapshots = verification_job.ignore_verified.unwrap_or(true);
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup v9 1/6] api-types: add maintenance type
2022-04-06 13:08 ` Dylan Whyte
@ 2022-04-11 7:33 ` Thomas Lamprecht
0 siblings, 0 replies; 12+ messages in thread
From: Thomas Lamprecht @ 2022-04-11 7:33 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Dylan Whyte, Hannes Laimer
On 06.04.22 15:08, Dylan Whyte wrote:
>> +pub const MAINTENANCE_MESSAGE_SCHEMA: Schema =
>> + StringSchema::new("Message describing the reason for the maintenance.")
>> + .format(&MAINTENANCE_MESSAGE_FORMAT)
>> + .max_length(32)
>> + .schema();
> Is 32 characters enough for the message? I would think this is somewhat limiting, but am open to other opinions.
The idea is to start more limited, as opening those up once there's real world
requests for legitimate reasons is easy, but we cannot really easily reduce the
allowed length ever without breaking older systems.
Naturally starting out with a overly strict limit is not much of use either, note
that 32 characters is not /that/ strict, infos like "zfs pool disk maintenance"
(26 chars) fit there easily, but yes it may be limitting for more detailed infos.
Maybe we could settle for 64 now, that would allow a info message like:
"zfs pool disk replacement undergoing until 2022-04-11T10:00Z" (61 chars), which
is IMO an OK upper limit of what people can be enforced to condense the relevant
info bits into without being required to play "text golf".
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup v9 0/6] closes #3071: maintenance mode for datastore
2022-04-06 13:08 ` [pbs-devel] [PATCH proxmox-backup v9 0/6] closes #3071: maintenance mode for datastore Dylan Whyte
@ 2022-04-11 7:35 ` Thomas Lamprecht
0 siblings, 0 replies; 12+ messages in thread
From: Thomas Lamprecht @ 2022-04-11 7:35 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Dylan Whyte, Hannes Laimer
On 06.04.22 15:08, Dylan Whyte wrote:
> I just wanted to reply to this series, to get the ball rolling again. However, I can't say that I found much real issues with it on first glance, so I've just added some very minor suggestions.
>
> As a rebase is now required for it to apply, I haven't done any testing. I'd be happy to test once it's rebased though.
Thanks for that, this series is definitively over due, sorry from the appliers
team on my part.
@Hannes: If you can send a rebased version until Wednesday I'll try to apply
it still this week.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-04-11 7:35 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-17 17:14 [pbs-devel] [PATCH proxmox-backup v9 0/6] closes #3071: maintenance mode for datastore Hannes Laimer
2022-02-17 17:14 ` [pbs-devel] [PATCH proxmox-backup v9 1/6] api-types: add maintenance type Hannes Laimer
2022-04-06 13:08 ` Dylan Whyte
2022-04-11 7:33 ` Thomas Lamprecht
2022-02-17 17:14 ` [pbs-devel] [PATCH proxmox-backup v9 2/6] datastore: add check for maintenance in lookup Hannes Laimer
2022-04-06 13:08 ` Dylan Whyte
2022-02-17 17:14 ` [pbs-devel] [PATCH proxmox-backup v9 3/6] pbs-datastore: add active operations tracking Hannes Laimer
2022-02-17 17:14 ` [pbs-devel] [PATCH proxmox-backup v9 4/6] api: make maintenance_type updatable Hannes Laimer
2022-02-17 17:14 ` [pbs-devel] [PATCH proxmox-backup v9 5/6] api: add get_active_operations endpoint Hannes Laimer
2022-02-17 17:14 ` [pbs-devel] [PATCH proxmox-backup v9 6/6] ui: add option to change the maintenance type Hannes Laimer
2022-04-06 13:08 ` [pbs-devel] [PATCH proxmox-backup v9 0/6] closes #3071: maintenance mode for datastore Dylan Whyte
2022-04-11 7:35 ` Thomas Lamprecht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox