* [pbs-devel] [PATCH proxmox-backup v6 1/6] api-types: add maintenance type
2022-02-02 15:49 [pbs-devel] [PATCH proxmox-backup v6 0/6] closes #3071: maintenance mode for datastore Hannes Laimer
@ 2022-02-02 15:49 ` Hannes Laimer
2022-02-02 15:49 ` [pbs-devel] [PATCH proxmox-backup v6 2/6] datastore: add check for maintenance in lookup Hannes Laimer
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Hannes Laimer @ 2022-02-02 15:49 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
pbs-api-types/src/datastore.rs | 8 +++-
pbs-api-types/src/lib.rs | 3 ++
pbs-api-types/src/maintenance.rs | 82 ++++++++++++++++++++++++++++++++
3 files changed, 92 insertions(+), 1 deletion(-)
create mode 100644 pbs-api-types/src/maintenance.rs
diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index 36279b3a..e2491a92 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -7,7 +7,7 @@ use proxmox_schema::{
use crate::{
PROXMOX_SAFE_ID_FORMAT, SHA256_HEX_REGEX, SINGLE_LINE_COMMENT_SCHEMA, CryptMode, UPID,
- Fingerprint, Userid, Authid,
+ Fingerprint, Userid, Authid, MaintenanceType,
GC_SCHEDULE_SCHEMA, DATASTORE_NOTIFY_STRING_SCHEMA, PRUNE_SCHEDULE_SCHEMA,
};
@@ -224,6 +224,10 @@ pub struct PruneOptions {
optional: true,
type: bool,
},
+ "maintenance-type": {
+ optional: true,
+ type: MaintenanceType,
+ },
}
)]
#[derive(Serialize,Deserialize,Updater)]
@@ -261,6 +265,8 @@ pub struct DataStoreConfig {
/// Send notification only for job errors
#[serde(skip_serializing_if="Option::is_none")]
pub notify: Option<String>,
+ #[serde(skip_serializing_if="Option::is_none")]
+ pub maintenance_type: Option<MaintenanceType>,
}
#[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..4d3ccb3b
--- /dev/null
+++ b/pbs-api-types/src/maintenance.rs
@@ -0,0 +1,82 @@
+use anyhow::{bail, Error};
+
+use proxmox_schema::{ApiStringFormat, Schema, StringSchema, UpdaterType};
+
+use crate::{PROXMOX_SAFE_ID_FORMAT, SINGLE_LINE_COMMENT_FORMAT};
+
+pub const MAINTENANCE_MODE_SCHEMA: Schema = StringSchema::new("Maintenance mode.")
+ .format(&PROXMOX_SAFE_ID_FORMAT)
+ .min_length(3)
+ .max_length(32)
+ .schema();
+
+pub const MAINTENANCE_MESSAGE_SCHEMA: Schema =
+ StringSchema::new("Message describing the reason for the maintenance.")
+ .format(&SINGLE_LINE_COMMENT_FORMAT)
+ .max_length(32)
+ .schema();
+
+#[derive(UpdaterType)]
+/// Maintenance type and message.
+pub enum MaintenanceType {
+ /// Only reading operations are allowed on the datastore.
+ ReadOnly(String),
+ /// Neither reading nor writing operations are allowed on the datastore.
+ Offline(String),
+}
+
+/// Operation requirments, used when checking for maintenance mode.
+pub enum Operation {
+ Read,
+ Write,
+}
+
+proxmox_serde::forward_deserialize_to_from_str!(MaintenanceType);
+proxmox_serde::forward_serialize_to_display!(MaintenanceType);
+
+impl proxmox_schema::ApiType for MaintenanceType {
+ const API_SCHEMA: Schema = StringSchema::new(
+ "Maintenance type (e.g. 'read-only-<message>', 'offline-<message>')",
+ )
+ .format(&ApiStringFormat::VerifyFn(|text| {
+ let location: MaintenanceType = text.parse()?;
+ match location {
+ MaintenanceType::ReadOnly(ref message) => {
+ MAINTENANCE_MESSAGE_SCHEMA.parse_simple_value(message)?;
+ }
+ MaintenanceType::Offline(ref message) => {
+ MAINTENANCE_MESSAGE_SCHEMA.parse_simple_value(message)?;
+ }
+ }
+ Ok(())
+ }))
+ .schema();
+}
+
+impl std::fmt::Display for MaintenanceType {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ match self {
+ MaintenanceType::ReadOnly(message) => {
+ write!(f, "read-only-{}", message)
+ }
+ MaintenanceType::Offline(message) => {
+ write!(f, "offline-{}", message)
+ }
+ }
+ }
+}
+
+impl std::str::FromStr for MaintenanceType {
+ type Err = Error;
+
+ fn from_str(s: &str) -> Result<Self, Self::Err> {
+ if let Some(message) = s.strip_prefix("read-only-") {
+ return Ok(MaintenanceType::ReadOnly(message.to_string()));
+ }
+ if let Some(message) = s.strip_prefix("offline-") {
+ return Ok(MaintenanceType::Offline(message.to_string()));
+ }
+
+ bail!("Maintenance type parse error");
+ }
+}
--
2.30.2
^ permalink raw reply [flat|nested] 10+ messages in thread* [pbs-devel] [PATCH proxmox-backup v6 2/6] datastore: add check for maintenance in lookup
2022-02-02 15:49 [pbs-devel] [PATCH proxmox-backup v6 0/6] closes #3071: maintenance mode for datastore Hannes Laimer
2022-02-02 15:49 ` [pbs-devel] [PATCH proxmox-backup v6 1/6] api-types: add maintenance type Hannes Laimer
@ 2022-02-02 15:49 ` Hannes Laimer
2022-02-02 15:49 ` [pbs-devel] [PATCH proxmox-backup v6 3/6] pbs-datastore: add active operations tracking Hannes Laimer
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Hannes Laimer @ 2022-02-02 15:49 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
pbs-datastore/src/datastore.rs | 19 ++++++++---
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 | 6 ++--
src/server/prune_job.rs | 4 +--
src/server/pull.rs | 4 +--
src/server/verify_job.rs | 4 +--
12 files changed, 67 insertions(+), 52 deletions(-)
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 37ef753e..c65d4574 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, MaintenanceType, Operation, GarbageCollectionStatus, HumanByte
+};
use pbs_config::{open_backup_lockfile, BackupLockGuard};
use crate::DataBlob;
@@ -60,13 +62,22 @@ 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);
+ match (&config.maintenance_type, operation) {
+ (Some(MaintenanceType::ReadOnly(message)), Some(Operation::Write))
+ | (Some(MaintenanceType::Offline(message)), Some(_)) => {
+ bail!("Datastore '{}' is in maintenance mode: {}", name, message);
+ },
+ _ => {}
+ }
+
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..89273627 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::Write))?;
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::Write))?;
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 aac1826b..8ff28763 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::Read))?;
map.insert(store, datastore);
} else if default.is_none() {
- default = Some(DataStore::lookup_datastore(&store)?);
+ default = Some(DataStore::lookup_datastore(&store, Some(Operation::Read))?);
} else {
bail!("multiple default stores given");
}
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index 8d0033de..83e77237 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -50,8 +50,8 @@ use pbs_buildcfg::configdir;
use proxmox_time::CalendarEvent;
use pbs_api_types::{
- Authid, TapeBackupJobConfig, VerificationJobConfig, SyncJobConfig, DataStoreConfig,
- PruneOptions,
+ Authid, Operation, TapeBackupJobConfig, VerificationJobConfig,
+ SyncJobConfig, DataStoreConfig, PruneOptions,
};
use proxmox_rest_server::daemon;
@@ -555,7 +555,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 8b52e8c2..52819262 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] 10+ messages in thread* [pbs-devel] [PATCH proxmox-backup v6 3/6] pbs-datastore: add active operations tracking
2022-02-02 15:49 [pbs-devel] [PATCH proxmox-backup v6 0/6] closes #3071: maintenance mode for datastore Hannes Laimer
2022-02-02 15:49 ` [pbs-devel] [PATCH proxmox-backup v6 1/6] api-types: add maintenance type Hannes Laimer
2022-02-02 15:49 ` [pbs-devel] [PATCH proxmox-backup v6 2/6] datastore: add check for maintenance in lookup Hannes Laimer
@ 2022-02-02 15:49 ` Hannes Laimer
2022-02-03 10:53 ` Dominik Csapak
2022-02-02 15:49 ` [pbs-devel] [PATCH proxmox-backup v6 4/6] api: make maintenance_type updatable Hannes Laimer
` (3 subsequent siblings)
6 siblings, 1 reply; 10+ messages in thread
From: Hannes Laimer @ 2022-02-02 15:49 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-api-types/src/maintenance.rs | 1 +
pbs-datastore/Cargo.toml | 1 +
pbs-datastore/src/datastore.rs | 114 +++++++++++++++++++----------
pbs-datastore/src/lib.rs | 4 +
pbs-datastore/src/task_tracking.rs | 81 ++++++++++++++++++++
src/api2/tape/backup.rs | 4 +-
src/api2/tape/restore.rs | 4 +-
src/bin/proxmox-backup-api.rs | 1 +
src/server/mod.rs | 16 +++-
9 files changed, 184 insertions(+), 42 deletions(-)
create mode 100644 pbs-datastore/src/task_tracking.rs
diff --git a/pbs-api-types/src/maintenance.rs b/pbs-api-types/src/maintenance.rs
index 4d3ccb3b..1b5753d2 100644
--- a/pbs-api-types/src/maintenance.rs
+++ b/pbs-api-types/src/maintenance.rs
@@ -25,6 +25,7 @@ pub enum MaintenanceType {
Offline(String),
}
+#[derive(Copy ,Clone)]
/// Operation requirments, used when checking for maintenance mode.
pub enum Operation {
Read,
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 c65d4574..c643bc55 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, MaintenanceType, 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,42 @@ 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 {
+ if let Some(operation) = self.operation.clone() {
+ if let Err(e) = update_active_operations(self.name(), operation, -1) {
+ eprintln!("could not update active operations - {}", e);
+ }
+ }
+ DataStore {
+ inner: self.inner.clone(),
+ operation: self.operation.clone(),
+ }
+ }
+}
+
+impl Drop for DataStore {
+ fn drop(&mut self) {
+ if let Some(operation) = self.operation.clone() {
+ if let Err(e) = update_active_operations(self.name(), operation, -1) {
+ eprintln!("could not update active operations - {}", e);
+ }
+ }
+ }
+}
+
impl DataStore {
pub fn lookup_datastore(
name: &str,
@@ -70,12 +101,15 @@ impl DataStore {
let config: DataStoreConfig = config.lookup("datastore", name)?;
let path = PathBuf::from(&config.path);
- match (&config.maintenance_type, operation) {
- (Some(MaintenanceType::ReadOnly(message)), Some(Operation::Write))
- | (Some(MaintenanceType::Offline(message)), Some(_)) => {
- bail!("Datastore '{}' is in maintenance mode: {}", name, message);
- },
- _ => {}
+ if let Some(MaintenanceType::Offline(message)) = &config.maintenance_type {
+ bail!("datastore '{}' is in offline maintenance mode: {}", name, message);
+ } else if let Some(MaintenanceType::ReadOnly(message)) = &config.maintenance_type {
+ if let Some(Operation::Write) = operation {
+ bail!("datastore '{}' is in read-only maintenance mode: {}", name, message);
+ }
+ }
+ if let Some(op) = operation {
+ update_active_operations(name, op, 1)?;
}
let mut map = DATASTORE_MAP.lock().unwrap();
@@ -85,7 +119,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,
+ }))
}
}
@@ -94,7 +131,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
@@ -109,7 +149,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();
@@ -127,7 +167,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),
@@ -141,19 +181,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)?;
@@ -165,14 +205,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)?;
@@ -222,11 +262,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
@@ -529,7 +569,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 {:?}",
@@ -545,7 +585,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)?;
}
}
}
@@ -625,24 +665,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());
@@ -652,7 +692,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,
@@ -729,7 +769,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)");
@@ -739,15 +779,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(
@@ -755,7 +795,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> {
@@ -771,13 +811,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)?;
@@ -891,7 +931,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/api2/tape/backup.rs b/src/api2/tape/backup.rs
index 89273627..28b24168 100644
--- a/src/api2/tape/backup.rs
+++ b/src/api2/tape/backup.rs
@@ -168,7 +168,7 @@ pub fn do_tape_backup_job(
let worker_type = job.jobtype().to_string();
- let datastore = DataStore::lookup_datastore(&setup.store, Some(Operation::Write))?;
+ 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, Some(Operation::Write))?;
+ 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 8ff28763..8fadbeca 100644
--- a/src/api2/tape/restore.rs
+++ b/src/api2/tape/restore.rs
@@ -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, Some(Operation::Read))?;
+ let datastore = DataStore::lookup_datastore(&target, Some(Operation::Write))?;
map.insert(store, datastore);
} else if default.is_none() {
- default = Some(DataStore::lookup_datastore(&store, Some(Operation::Read))?);
+ default = Some(DataStore::lookup_datastore(&store, Some(Operation::Write))?);
} else {
bail!("multiple default stores given");
}
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] 10+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v6 3/6] pbs-datastore: add active operations tracking
2022-02-02 15:49 ` [pbs-devel] [PATCH proxmox-backup v6 3/6] pbs-datastore: add active operations tracking Hannes Laimer
@ 2022-02-03 10:53 ` Dominik Csapak
0 siblings, 0 replies; 10+ messages in thread
From: Dominik Csapak @ 2022-02-03 10:53 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
[..snip..]
On 2/2/22 16:49, Hannes Laimer wrote:
> +impl Clone for DataStore {
> + fn clone(&self) -> Self {
> + if let Some(operation) = self.operation.clone() {
> + if let Err(e) = update_active_operations(self.name(), operation, -1) {
this must have '1' instead of '-1' else a lookup + clone makes it look like
no operation is running and dropping both, leaves the counter at -2
> + eprintln!("could not update active operations - {}", e);
> + }
> + }
> + DataStore {
> + inner: self.inner.clone(),
> + operation: self.operation.clone(),
> + }
> + }
> +}
> +
> +impl Drop for DataStore {
> + fn drop(&mut self) {
> + if let Some(operation) = self.operation.clone() {
> + if let Err(e) = update_active_operations(self.name(), operation, -1) {
> + eprintln!("could not update active operations - {}", e);
> + }
> + }
> + }
> +}
> +
[..snip..]
^ permalink raw reply [flat|nested] 10+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v6 4/6] api: make maintenance_type updatable
2022-02-02 15:49 [pbs-devel] [PATCH proxmox-backup v6 0/6] closes #3071: maintenance mode for datastore Hannes Laimer
` (2 preceding siblings ...)
2022-02-02 15:49 ` [pbs-devel] [PATCH proxmox-backup v6 3/6] pbs-datastore: add active operations tracking Hannes Laimer
@ 2022-02-02 15:49 ` Hannes Laimer
2022-02-02 15:49 ` [pbs-devel] [PATCH proxmox-backup v6 5/6] api: add get_active_operations endpoint Hannes Laimer
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Hannes Laimer @ 2022-02-02 15:49 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..3b74d989 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-type property
+ maintenance_type,
}
#[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_type => { data.maintenance_type = None; },
}
}
}
@@ -295,6 +298,8 @@ pub fn update_datastore(
if update.notify_user.is_some() { data.notify_user = update.notify_user; }
+ if update.maintenance_type.is_some() { data.maintenance_type = update.maintenance_type; }
+
config.set_data(&name, "datastore", &data)?;
pbs_config::datastore::save_config(&config)?;
--
2.30.2
^ permalink raw reply [flat|nested] 10+ messages in thread* [pbs-devel] [PATCH proxmox-backup v6 5/6] api: add get_active_operations endpoint
2022-02-02 15:49 [pbs-devel] [PATCH proxmox-backup v6 0/6] closes #3071: maintenance mode for datastore Hannes Laimer
` (3 preceding siblings ...)
2022-02-02 15:49 ` [pbs-devel] [PATCH proxmox-backup v6 4/6] api: make maintenance_type updatable Hannes Laimer
@ 2022-02-02 15:49 ` Hannes Laimer
2022-02-02 15:49 ` [pbs-devel] [PATCH proxmox-backup v6 6/6] ui: add option to change the maintenance type Hannes Laimer
2022-02-03 10:53 ` [pbs-devel] [PATCH proxmox-backup v6 0/6] closes #3071: maintenance mode for datastore Dominik Csapak
6 siblings, 0 replies; 10+ messages in thread
From: Hannes Laimer @ 2022-02-02 15:49 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] 10+ messages in thread* [pbs-devel] [PATCH proxmox-backup v6 6/6] ui: add option to change the maintenance type
2022-02-02 15:49 [pbs-devel] [PATCH proxmox-backup v6 0/6] closes #3071: maintenance mode for datastore Hannes Laimer
` (4 preceding siblings ...)
2022-02-02 15:49 ` [pbs-devel] [PATCH proxmox-backup v6 5/6] api: add get_active_operations endpoint Hannes Laimer
@ 2022-02-02 15:49 ` Hannes Laimer
2022-02-03 10:53 ` Dominik Csapak
2022-02-03 10:53 ` [pbs-devel] [PATCH proxmox-backup v6 0/6] closes #3071: maintenance mode for datastore Dominik Csapak
6 siblings, 1 reply; 10+ messages in thread
From: Hannes Laimer @ 2022-02-02 15:49 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 | 72 ++++++++++++++++++++++++++++++++
4 files changed, 126 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..db23645a 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -640,4 +640,27 @@ Ext.define('PBS.Utils', {
return `${icon} ${value}`;
},
+ renderMaintenance: function(type, activeTasks) {
+ if (!type) return gettext('None');
+ let at = 0;
+ for (let x of ['read-only-', 'offline-']) {
+ if (type.startsWith(x)) {
+ at = x.length;
+ }
+ }
+
+ const conflictingTasks = activeTasks.write + (type.startsWith('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;
+
+ const mode = type.substring(0, at-1);
+ let msg = type.substring(at);
+ if (msg.length > 0) {
+ msg = `"${msg}"`;
+ }
+ return Ext.String.capitalize(`${gettext(mode)} ${msg} ${extra}`) || gettext('None');
+ },
+
});
diff --git a/www/datastore/OptionView.js b/www/datastore/OptionView.js
index 5a5e85be..50ccce47 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-type'].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-type": {
+ 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..374b69f6
--- /dev/null
+++ b/www/window/MaintenanceOptions.js
@@ -0,0 +1,72 @@
+Ext.define('PBS.form.oaintenanceType', {
+ 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) {
+ console.log(values);
+ if (values['maintenance-type']) {
+ values['maintenance-type'] =
+ `${values['maintenance-type']}-${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-type']) {
+ let type = values['maintenance-type'];
+ let at = 0;
+ for (let x of ['read-only-', 'offline-']) {
+ if (type.startsWith(x)) {
+ at = x.length;
+ }
+ }
+ options = {
+ 'maintenance-type': type.substr(0, at-1) || '__default__',
+ 'maintenance-msg': type.substr(at) || '',
+ };
+ }
+
+ me.callParent([options]);
+ },
+});
--
2.30.2
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v6 6/6] ui: add option to change the maintenance type
2022-02-02 15:49 ` [pbs-devel] [PATCH proxmox-backup v6 6/6] ui: add option to change the maintenance type Hannes Laimer
@ 2022-02-03 10:53 ` Dominik Csapak
0 siblings, 0 replies; 10+ messages in thread
From: Dominik Csapak @ 2022-02-03 10:53 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
comment inline:
On 2/2/22 16:49, Hannes Laimer wrote:
> 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 | 72 ++++++++++++++++++++++++++++++++
> 4 files changed, 126 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..db23645a 100644
> --- a/www/Utils.js
> +++ b/www/Utils.js
> @@ -640,4 +640,27 @@ Ext.define('PBS.Utils', {
> return `${icon} ${value}`;
> },
>
> + renderMaintenance: function(type, activeTasks) {
> + if (!type) return gettext('None');
> + let at = 0;
> + for (let x of ['read-only-', 'offline-']) {
> + if (type.startsWith(x)) {
> + at = x.length;
> + }
> + }
> +
> + const conflictingTasks = activeTasks.write + (type.startsWith('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;
> +
> + const mode = type.substring(0, at-1);
> + let msg = type.substring(at);
> + if (msg.length > 0) {
> + msg = `"${msg}"`;
> + }
> + return Ext.String.capitalize(`${gettext(mode)} ${msg} ${extra}`) || gettext('None');
two small issues here:
i don't think using 'capitalize' is good here because
1. may not work in all translations like you intended
2. the gettext should already be capitalized
second issue is that you use a variable in gettext. while it'll "work"
the actual values you pass will never be picked up by our script in
proxmox-i18n.
instead something like this would work better (untested):
---
let modeText = Proxmox.Utils.unknownText;
switch (mode) {
case 'read-only:
modeText = gettext("Read-only");
break;
case 'offline':
modeText = gettext("Offline");
break;
}
---
with that, the gettexts are properly picked up and you don't need to use 'capitalize'
> + },
> +
> });
>
[..snip..]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup v6 0/6] closes #3071: maintenance mode for datastore
2022-02-02 15:49 [pbs-devel] [PATCH proxmox-backup v6 0/6] closes #3071: maintenance mode for datastore Hannes Laimer
` (5 preceding siblings ...)
2022-02-02 15:49 ` [pbs-devel] [PATCH proxmox-backup v6 6/6] ui: add option to change the maintenance type Hannes Laimer
@ 2022-02-03 10:53 ` Dominik Csapak
6 siblings, 0 replies; 10+ messages in thread
From: Dominik Csapak @ 2022-02-03 10:53 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
hi,
it seems the tape patches are squashed into the wrong patch 3/6 instead
of 2/6, but no big issue imho
i found some issues that were already there last time around (sorry)
but i think those are small enough that you can send a follow up..
aside from the issues in 3/6 and 6/6 consider it
(note, i tested it with my suggestion in 3/6)
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Dominik Csapak <d.csapak@proxmox.com>
On 2/2/22 16:49, 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.
>
> 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
>
> pbs-api-types/src/datastore.rs | 8 +-
> pbs-api-types/src/lib.rs | 3 +
> pbs-api-types/src/maintenance.rs | 83 +++++++++++++++++++
> pbs-datastore/Cargo.toml | 1 +
> pbs-datastore/src/datastore.rs | 119 +++++++++++++++++++--------
> 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 | 6 +-
> 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 | 72 ++++++++++++++++
> 25 files changed, 522 insertions(+), 85 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
>
^ permalink raw reply [flat|nested] 10+ messages in thread