* [pbs-devel] [PATCH proxmox-backup v14 01/25] pbs-api-types: add backing-device to DataStoreConfig
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
@ 2024-11-22 14:46 ` Hannes Laimer
2024-11-25 13:40 ` Fabian Grünbichler
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 02/25] maintenance: make is_offline more generic Hannes Laimer
` (26 subsequent siblings)
27 siblings, 1 reply; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:46 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
changes since v13:
* drop get_mount_point
* update DATASTORE_DIR_NAME_SCHAME description
pbs-api-types/src/datastore.rs | 25 ++++++++++++++++++++++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index 711051d05..b722c9ab7 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -45,7 +45,7 @@ const_regex! {
pub const CHUNK_DIGEST_FORMAT: ApiStringFormat = ApiStringFormat::Pattern(&SHA256_HEX_REGEX);
-pub const DIR_NAME_SCHEMA: Schema = StringSchema::new("Directory name")
+pub const DATASTORE_DIR_NAME_SCHEMA: Schema = StringSchema::new("Either the absolute path to the datastore directory, or an absolute on-device path for removable datastores.")
.min_length(1)
.max_length(4096)
.schema();
@@ -163,6 +163,9 @@ pub const PRUNE_SCHEMA_KEEP_YEARLY: Schema =
.minimum(1)
.schema();
+/// Base directory where datastores are mounted
+pub const DATASTORE_MOUNT_DIR: &str = "/mnt/datastore";
+
#[api]
#[derive(Debug, Default, Copy, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")]
@@ -237,7 +240,7 @@ pub const DATASTORE_TUNING_STRING_SCHEMA: Schema = StringSchema::new("Datastore
schema: DATASTORE_SCHEMA,
},
path: {
- schema: DIR_NAME_SCHEMA,
+ schema: DATASTORE_DIR_NAME_SCHEMA,
},
"notify-user": {
optional: true,
@@ -276,6 +279,12 @@ pub const DATASTORE_TUNING_STRING_SCHEMA: Schema = StringSchema::new("Datastore
format: &ApiStringFormat::PropertyString(&MaintenanceMode::API_SCHEMA),
type: String,
},
+ "backing-device": {
+ description: "The UUID of the filesystem partition for removable datastores.",
+ optional: true,
+ format: &proxmox_schema::api_types::UUID_FORMAT,
+ type: String,
+ }
}
)]
#[derive(Serialize, Deserialize, Updater, Clone, PartialEq)]
@@ -323,6 +332,11 @@ pub struct DataStoreConfig {
/// 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>,
+
+ /// The UUID of the device(for removable datastores)
+ #[updater(skip)]
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub backing_device: Option<String>,
}
#[api]
@@ -357,12 +371,17 @@ impl DataStoreConfig {
notification_mode: None,
tuning: None,
maintenance_mode: None,
+ backing_device: None,
}
}
/// Returns the absolute path to the datastore content.
pub fn absolute_path(&self) -> String {
- self.path.clone()
+ if self.backing_device.is_some() {
+ format!("{DATASTORE_MOUNT_DIR}/{}", self.name)
+ } else {
+ self.path.clone()
+ }
}
pub fn get_maintenance_mode(&self) -> Option<MaintenanceMode> {
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v14 01/25] pbs-api-types: add backing-device to DataStoreConfig
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 01/25] pbs-api-types: add backing-device to DataStoreConfig Hannes Laimer
@ 2024-11-25 13:40 ` Fabian Grünbichler
0 siblings, 0 replies; 45+ messages in thread
From: Fabian Grünbichler @ 2024-11-25 13:40 UTC (permalink / raw)
To: Proxmox Backup Server development discussion
On November 22, 2024 3:46 pm, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> changes since v13:
> * drop get_mount_point
> * update DATASTORE_DIR_NAME_SCHAME description
but you didn't just change the description, this also changed semantics
quite a bit? in v13, it was a relative path for removable datastores
(which makes a lot of sense - the "path" is the subdir on the mounted
device). could we go back to that please?
>
> pbs-api-types/src/datastore.rs | 25 ++++++++++++++++++++++---
> 1 file changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
> index 711051d05..b722c9ab7 100644
> --- a/pbs-api-types/src/datastore.rs
> +++ b/pbs-api-types/src/datastore.rs
> @@ -45,7 +45,7 @@ const_regex! {
>
> pub const CHUNK_DIGEST_FORMAT: ApiStringFormat = ApiStringFormat::Pattern(&SHA256_HEX_REGEX);
>
> -pub const DIR_NAME_SCHEMA: Schema = StringSchema::new("Directory name")
> +pub const DATASTORE_DIR_NAME_SCHEMA: Schema = StringSchema::new("Either the absolute path to the datastore directory, or an absolute on-device path for removable datastores.")
because an "absolute on-device path" is an oxymoron and confusing..
> .min_length(1)
> .max_length(4096)
> .schema();
> @@ -163,6 +163,9 @@ pub const PRUNE_SCHEMA_KEEP_YEARLY: Schema =
> .minimum(1)
> .schema();
>
> +/// Base directory where datastores are mounted
> +pub const DATASTORE_MOUNT_DIR: &str = "/mnt/datastore";
> +
> #[api]
> #[derive(Debug, Default, Copy, Clone, PartialEq, Eq, Serialize, Deserialize)]
> #[serde(rename_all = "lowercase")]
> @@ -237,7 +240,7 @@ pub const DATASTORE_TUNING_STRING_SCHEMA: Schema = StringSchema::new("Datastore
> schema: DATASTORE_SCHEMA,
> },
> path: {
> - schema: DIR_NAME_SCHEMA,
> + schema: DATASTORE_DIR_NAME_SCHEMA,
> },
> "notify-user": {
> optional: true,
> @@ -276,6 +279,12 @@ pub const DATASTORE_TUNING_STRING_SCHEMA: Schema = StringSchema::new("Datastore
> format: &ApiStringFormat::PropertyString(&MaintenanceMode::API_SCHEMA),
> type: String,
> },
> + "backing-device": {
> + description: "The UUID of the filesystem partition for removable datastores.",
> + optional: true,
> + format: &proxmox_schema::api_types::UUID_FORMAT,
> + type: String,
> + }
> }
> )]
> #[derive(Serialize, Deserialize, Updater, Clone, PartialEq)]
> @@ -323,6 +332,11 @@ pub struct DataStoreConfig {
> /// 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>,
> +
> + /// The UUID of the device(for removable datastores)
> + #[updater(skip)]
> + #[serde(skip_serializing_if = "Option::is_none")]
> + pub backing_device: Option<String>,
> }
>
> #[api]
> @@ -357,12 +371,17 @@ impl DataStoreConfig {
> notification_mode: None,
> tuning: None,
> maintenance_mode: None,
> + backing_device: None,
> }
> }
>
> /// Returns the absolute path to the datastore content.
> pub fn absolute_path(&self) -> String {
> - self.path.clone()
> + if self.backing_device.is_some() {
> + format!("{DATASTORE_MOUNT_DIR}/{}", self.name)
> + } else {
> + self.path.clone()
> + }
> }
>
> pub fn get_maintenance_mode(&self) -> Option<MaintenanceMode> {
> --
> 2.39.5
>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
>
>
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v14 02/25] maintenance: make is_offline more generic
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 01/25] pbs-api-types: add backing-device to DataStoreConfig Hannes Laimer
@ 2024-11-22 14:46 ` Hannes Laimer
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 03/26] maintenance: add 'Unmount' maintenance type Hannes Laimer
` (25 subsequent siblings)
27 siblings, 0 replies; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:46 UTC (permalink / raw)
To: pbs-devel
... and add MaintenanceType::Delete to it. We also want to clear any
cach entries if we are deleting the datastore, not just if it is marked
as offline.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
* new in v14
pbs-api-types/src/maintenance.rs | 7 +++----
pbs-datastore/src/datastore.rs | 5 +++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/pbs-api-types/src/maintenance.rs b/pbs-api-types/src/maintenance.rs
index 1e3413dca..a7b8b078d 100644
--- a/pbs-api-types/src/maintenance.rs
+++ b/pbs-api-types/src/maintenance.rs
@@ -77,10 +77,9 @@ pub struct MaintenanceMode {
}
impl MaintenanceMode {
- /// Used for deciding whether the datastore is cleared from the internal cache after the last
- /// task finishes, so all open files are closed.
- pub fn is_offline(&self) -> bool {
- self.ty == MaintenanceType::Offline
+ /// Used for deciding whether the datastore is cleared from the internal cache
+ pub fn clear_from_cache(&self) -> bool {
+ self.ty == MaintenanceType::Offline || self.ty == MaintenanceType::Delete
}
pub fn check(&self, operation: Option<Operation>) -> Result<(), Error> {
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 2755fed8c..2bf2b8437 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -116,7 +116,8 @@ impl Drop for DataStore {
&& pbs_config::datastore::config()
.and_then(|(s, _)| s.lookup::<DataStoreConfig>("datastore", self.name()))
.map_or(false, |c| {
- c.get_maintenance_mode().map_or(false, |m| m.is_offline())
+ c.get_maintenance_mode()
+ .map_or(false, |m| m.clear_from_cache())
});
if remove_from_cache {
@@ -216,7 +217,7 @@ impl DataStore {
let datastore: DataStoreConfig = config.lookup("datastore", name)?;
if datastore
.get_maintenance_mode()
- .map_or(false, |m| m.is_offline())
+ .map_or(false, |m| m.clear_from_cache())
{
// the datastore drop handler does the checking if tasks are running and clears the
// cache entry, so we just have to trigger it here
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* [pbs-devel] [PATCH proxmox-backup v14 03/26] maintenance: add 'Unmount' maintenance type
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 01/25] pbs-api-types: add backing-device to DataStoreConfig Hannes Laimer
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 02/25] maintenance: make is_offline more generic Hannes Laimer
@ 2024-11-22 14:46 ` Hannes Laimer
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 04/25] datastore: add helper for checking if a datastore is mounted Hannes Laimer
` (24 subsequent siblings)
27 siblings, 0 replies; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:46 UTC (permalink / raw)
To: pbs-devel
From: Dietmar Maurer <dietmar@proxmox.com>
Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
---
pbs-api-types/src/datastore.rs | 3 +++
pbs-api-types/src/maintenance.rs | 9 +++++++--
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index b722c9ab7..ba75ebaba 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -401,6 +401,9 @@ impl DataStoreConfig {
match current_type {
Some(MaintenanceType::ReadOnly) => { /* always OK */ }
Some(MaintenanceType::Offline) => { /* always OK */ }
+ Some(MaintenanceType::Unmount) => {
+ bail!("datastore is being unmounted");
+ }
Some(MaintenanceType::Delete) => {
match new_type {
Some(MaintenanceType::Delete) => { /* allow to delete a deleted storage */ }
diff --git a/pbs-api-types/src/maintenance.rs b/pbs-api-types/src/maintenance.rs
index a7b8b078d..3c9aa8190 100644
--- a/pbs-api-types/src/maintenance.rs
+++ b/pbs-api-types/src/maintenance.rs
@@ -38,7 +38,6 @@ pub enum Operation {
/// Maintenance type.
pub enum MaintenanceType {
// TODO:
- // - Add "unmounting" once we got pluggable datastores
// - Add "GarbageCollection" or "DeleteOnly" as type and track GC (or all deletes) as separate
// operation, so that one can enable a mode where nothing new can be added but stuff can be
// cleaned
@@ -48,6 +47,8 @@ pub enum MaintenanceType {
Offline,
/// The datastore is being deleted.
Delete,
+ /// The (removable) datastore is being unmounted.
+ Unmount,
}
serde_plain::derive_display_from_serialize!(MaintenanceType);
serde_plain::derive_fromstr_from_deserialize!(MaintenanceType);
@@ -79,7 +80,9 @@ pub struct MaintenanceMode {
impl MaintenanceMode {
/// Used for deciding whether the datastore is cleared from the internal cache
pub fn clear_from_cache(&self) -> bool {
- self.ty == MaintenanceType::Offline || self.ty == MaintenanceType::Delete
+ self.ty == MaintenanceType::Offline
+ || self.ty == MaintenanceType::Delete
+ || self.ty == MaintenanceType::Unmount
}
pub fn check(&self, operation: Option<Operation>) -> Result<(), Error> {
@@ -93,6 +96,8 @@ impl MaintenanceMode {
if let Some(Operation::Lookup) = operation {
return Ok(());
+ } else if self.ty == MaintenanceType::Unmount {
+ bail!("datastore is being unmounted");
} else if self.ty == MaintenanceType::Offline {
bail!("offline maintenance mode: {}", message);
} else if self.ty == MaintenanceType::ReadOnly {
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* [pbs-devel] [PATCH proxmox-backup v14 04/25] datastore: add helper for checking if a datastore is mounted
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
` (2 preceding siblings ...)
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 03/26] maintenance: add 'Unmount' maintenance type Hannes Laimer
@ 2024-11-22 14:46 ` Hannes Laimer
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 05/25] api: admin: add (un)mount endpoint for removable datastores Hannes Laimer
` (23 subsequent siblings)
27 siblings, 0 replies; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:46 UTC (permalink / raw)
To: pbs-devel
... at a specific location. Also adds two additional functions to
get the mount status, and ensuring a removable datastore is mounted.
Co-authored-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
change since v13:
* add two helpers to not have to have the same code in a amount a
million places
pbs-datastore/src/datastore.rs | 74 +++++++++++++++++++++++++++++
pbs-datastore/src/lib.rs | 4 +-
src/server/metric_collection/mod.rs | 4 ++
3 files changed, 81 insertions(+), 1 deletion(-)
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 2bf2b8437..6a9fc2dc0 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -1,5 +1,6 @@
use std::collections::{HashMap, HashSet};
use std::io::{self, Write};
+use std::os::unix::ffi::OsStrExt;
use std::os::unix::io::AsRawFd;
use std::path::{Path, PathBuf};
use std::sync::{Arc, LazyLock, Mutex};
@@ -14,6 +15,7 @@ use proxmox_schema::ApiType;
use proxmox_sys::error::SysError;
use proxmox_sys::fs::{file_read_optional_string, replace_file, CreateOptions};
use proxmox_sys::fs::{lock_dir_noblock, DirLockGuard};
+use proxmox_sys::linux::procfs::MountInfo;
use proxmox_sys::process_locker::ProcessLockSharedGuard;
use proxmox_worker_task::WorkerTaskContext;
@@ -46,6 +48,70 @@ pub fn check_backup_owner(owner: &Authid, auth_id: &Authid) -> Result<(), Error>
Ok(())
}
+/// Check if a device with a given UUID is currently mounted at store_mount_point by
+/// comparing the `st_rdev` values of `/dev/disk/by-uuid/<uuid>` and the source device in
+/// /proc/self/mountinfo.
+///
+/// If we can't check if it is mounted, we treat that as not mounted,
+/// returning false.
+///
+/// Reasons it could fail other than not being mounted where expected:
+/// - could not read /proc/self/mountinfo
+/// - could not stat /dev/disk/by-uuid/<uuid>
+/// - /dev/disk/by-uuid/<uuid> is not a block device
+///
+/// Since these are very much out of our control, there is no real value in distinguishing
+/// between them, so for this function they all are treated as 'device not mounted'
+fn is_datastore_mounted_at(store_mount_point: String, device_uuid: &str) -> bool {
+ use nix::sys::stat::SFlag;
+
+ let store_mount_point = Path::new(&store_mount_point);
+
+ let dev_node = match nix::sys::stat::stat(format!("/dev/disk/by-uuid/{device_uuid}").as_str()) {
+ Ok(stat) if SFlag::from_bits_truncate(stat.st_mode) == SFlag::S_IFBLK => stat.st_rdev,
+ _ => return false,
+ };
+
+ let Ok(mount_info) = MountInfo::read() else {
+ return false;
+ };
+
+ for (_, entry) in mount_info {
+ let Some(source) = entry.mount_source else {
+ continue;
+ };
+
+ if entry.mount_point != store_mount_point || !source.as_bytes().starts_with(b"/") {
+ continue;
+ }
+
+ if let Ok(stat) = nix::sys::stat::stat(source.as_os_str()) {
+ let sflag = SFlag::from_bits_truncate(stat.st_mode);
+
+ if sflag == SFlag::S_IFBLK && stat.st_rdev == dev_node {
+ return true;
+ }
+ }
+ }
+
+ false
+}
+
+pub fn get_datastore_mount_status(config: &DataStoreConfig) -> Option<bool> {
+ let Some(ref device_uuid) = config.backing_device else {
+ return None;
+ };
+ Some(is_datastore_mounted_at(config.absolute_path(), device_uuid))
+}
+
+pub fn ensure_datastore_is_mounted(config: &DataStoreConfig) -> Result<(), Error> {
+ match get_datastore_mount_status(config) {
+ Some(true) => Ok(()),
+ Some(false) => Err(format_err!("Datastore '{}' is not mounted", config.name)),
+ None => Ok(()),
+ }
+}
+
/// Datastore Management
///
/// A Datastore can store severals backups, and provides the
@@ -156,6 +222,12 @@ impl DataStore {
}
}
+ if get_datastore_mount_status(&config) == Some(false) {
+ let mut datastore_cache = DATASTORE_MAP.lock().unwrap();
+ datastore_cache.remove(&config.name);
+ bail!("datastore '{}' is not mounted", config.name);
+ }
+
let mut datastore_cache = DATASTORE_MAP.lock().unwrap();
let entry = datastore_cache.get(name);
@@ -259,6 +331,8 @@ impl DataStore {
) -> Result<Arc<Self>, Error> {
let name = config.name.clone();
+ ensure_datastore_is_mounted(&config)?;
+
let tuning: DatastoreTuning = serde_json::from_value(
DatastoreTuning::API_SCHEMA
.parse_property_string(config.tuning.as_deref().unwrap_or(""))?,
diff --git a/pbs-datastore/src/lib.rs b/pbs-datastore/src/lib.rs
index 8050cf4d0..5014b6c09 100644
--- a/pbs-datastore/src/lib.rs
+++ b/pbs-datastore/src/lib.rs
@@ -201,7 +201,9 @@ pub use manifest::BackupManifest;
pub use store_progress::StoreProgress;
mod datastore;
-pub use datastore::{check_backup_owner, DataStore};
+pub use datastore::{
+ check_backup_owner, ensure_datastore_is_mounted, get_datastore_mount_status, DataStore,
+};
mod hierarchy;
pub use hierarchy::{
diff --git a/src/server/metric_collection/mod.rs b/src/server/metric_collection/mod.rs
index b95dba203..2ede8408f 100644
--- a/src/server/metric_collection/mod.rs
+++ b/src/server/metric_collection/mod.rs
@@ -176,6 +176,10 @@ fn collect_disk_stats_sync() -> (DiskStat, Vec<DiskStat>) {
continue;
}
+ if pbs_datastore::get_datastore_mount_status(&config) == Some(false) {
+ continue;
+ }
+
datastores.push(gather_disk_stats(
disk_manager.clone(),
Path::new(&config.absolute_path()),
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* [pbs-devel] [PATCH proxmox-backup v14 05/25] api: admin: add (un)mount endpoint for removable datastores
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
` (3 preceding siblings ...)
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 04/25] datastore: add helper for checking if a datastore is mounted Hannes Laimer
@ 2024-11-22 14:46 ` Hannes Laimer
2024-11-25 13:24 ` Fabian Grünbichler
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 06/25] api: removable datastore creation Hannes Laimer
` (22 subsequent siblings)
27 siblings, 1 reply; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:46 UTC (permalink / raw)
To: pbs-devel
Removable datastores can be mounted unless
- they are already
- their device is not present
For unmounting the maintenance mode is set to `unmount`,
which prohibits the starting of any new tasks envolving any
IO, this mode is unset either
- on completion of the unmount
- on abort of the unmount tasks
If the unmounting itself should fail, the maintenance mode stays in
place and requires manual intervention by unsetting it in the config
file directly. This is intentional, as unmounting should not fail,
and if it should the situation should be looked at.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
changes since v13:
* improve logging
* fix racy unmount
* (manually) changing maintenance during unmount will prevent unmounting and
result in failed unmount task
src/api2/admin/datastore.rs | 294 ++++++++++++++++++++++++++++++++++--
1 file changed, 283 insertions(+), 11 deletions(-)
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 3b863c06b..85522345e 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -4,7 +4,7 @@ use std::collections::HashSet;
use std::ffi::OsStr;
use std::ops::Deref;
use std::os::unix::ffi::OsStrExt;
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
use std::sync::Arc;
use anyhow::{bail, format_err, Error};
@@ -14,7 +14,7 @@ use hyper::{header, Body, Response, StatusCode};
use serde::Deserialize;
use serde_json::{json, Value};
use tokio_stream::wrappers::ReceiverStream;
-use tracing::{info, warn};
+use tracing::{debug, info, warn};
use proxmox_async::blocking::WrappedReaderStream;
use proxmox_async::{io::AsyncChannelWriter, stream::AsyncReaderStream};
@@ -30,6 +30,7 @@ use proxmox_sys::fs::{
file_read_firstline, file_read_optional_string, replace_file, CreateOptions,
};
use proxmox_time::CalendarEvent;
+use proxmox_worker_task::WorkerTaskContext;
use pxar::accessor::aio::Accessor;
use pxar::EntryKind;
@@ -38,13 +39,13 @@ use pbs_api_types::{
print_ns_and_snapshot, print_store_and_ns, ArchiveType, Authid, BackupArchiveName,
BackupContent, BackupGroupDeleteStats, BackupNamespace, BackupType, Counts, CryptMode,
DataStoreConfig, DataStoreListItem, DataStoreStatus, GarbageCollectionJobStatus, GroupListItem,
- JobScheduleStatus, KeepOptions, Operation, PruneJobOptions, SnapshotListItem,
- SnapshotVerifyState, BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA,
- BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA, CATALOG_NAME, CLIENT_LOG_BLOB_NAME, DATASTORE_SCHEMA,
- IGNORE_VERIFIED_BACKUPS_SCHEMA, MANIFEST_BLOB_NAME, MAX_NAMESPACE_DEPTH, NS_MAX_DEPTH_SCHEMA,
- PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE,
- PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY, UPID, UPID_SCHEMA,
- VERIFICATION_OUTDATED_AFTER_SCHEMA,
+ JobScheduleStatus, KeepOptions, MaintenanceMode, MaintenanceType, Operation, PruneJobOptions,
+ SnapshotListItem, SnapshotVerifyState, BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA,
+ BACKUP_NAMESPACE_SCHEMA, BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA, CATALOG_NAME,
+ CLIENT_LOG_BLOB_NAME, DATASTORE_SCHEMA, IGNORE_VERIFIED_BACKUPS_SCHEMA, MANIFEST_BLOB_NAME,
+ MAX_NAMESPACE_DEPTH, NS_MAX_DEPTH_SCHEMA, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP,
+ PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE, PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY, UPID,
+ UPID_SCHEMA, VERIFICATION_OUTDATED_AFTER_SCHEMA,
};
use pbs_client::pxar::{create_tar, create_zip};
use pbs_config::CachedUserInfo;
@@ -59,8 +60,8 @@ use pbs_datastore::index::IndexFile;
use pbs_datastore::manifest::BackupManifest;
use pbs_datastore::prune::compute_prune_info;
use pbs_datastore::{
- check_backup_owner, task_tracking, BackupDir, BackupGroup, DataStore, LocalChunkReader,
- StoreProgress,
+ check_backup_owner, ensure_datastore_is_mounted, task_tracking, BackupDir, BackupGroup,
+ DataStore, LocalChunkReader, StoreProgress,
};
use pbs_tools::json::required_string_param;
use proxmox_rest_server::{formatter, WorkerTask};
@@ -2394,6 +2395,275 @@ pub async fn set_backup_owner(
.await?
}
+/// Here we
+///
+/// 1. mount the removable device to `<PBS_RUN_DIR>/mount/<RANDOM_UUID>`
+/// 2. bind mount `<PBS_RUN_DIR>/mount/<RANDOM_UUID>/<datastore.path>` to `/mnt/datastore/<datastore.name>`
+/// 3. unmount `<PBS_RUN_DIR>/mount/<RANDOM_UUID>`
+///
+/// leaving us with the datastore being mounted directly with its name under /mnt/datastore/...
+///
+/// The reason for the randomized device mounting paths is to avoid two tasks trying to mount to
+/// the same path, this is *very* unlikely since the device is only mounted really shortly, but
+/// technically possible.
+pub fn do_mount_device(datastore: DataStoreConfig) -> Result<(), Error> {
+ if let Some(uuid) = datastore.backing_device.as_ref() {
+ let mount_point = datastore.absolute_path();
+ if pbs_datastore::get_datastore_mount_status(&datastore) == Some(true) {
+ bail!("device is already mounted at '{}'", mount_point);
+ }
+ let tmp_mount_path = format!(
+ "{}/{:x}",
+ pbs_buildcfg::rundir!("/mount"),
+ proxmox_uuid::Uuid::generate()
+ );
+
+ let default_options = proxmox_sys::fs::CreateOptions::new();
+ proxmox_sys::fs::create_path(
+ &tmp_mount_path,
+ Some(default_options.clone()),
+ Some(default_options.clone()),
+ )?;
+
+ info!("temporarily mounting '{uuid}' to '{}'", tmp_mount_path);
+ crate::tools::disks::mount_by_uuid(uuid, Path::new(&tmp_mount_path))
+ .map_err(|e| format_err!("mounting to tmp path failed: {e}"))?;
+
+ let full_store_path = format!(
+ "{tmp_mount_path}/{}",
+ datastore.path.trim_start_matches('/')
+ );
+ let backup_user = pbs_config::backup_user()?;
+ let options = CreateOptions::new()
+ .owner(backup_user.uid)
+ .group(backup_user.gid);
+
+ proxmox_sys::fs::create_path(
+ &mount_point,
+ Some(default_options.clone()),
+ Some(options.clone()),
+ )
+ .map_err(|e| format_err!("creating mountpoint '{mount_point}' failed: {e}"))?;
+
+ // can't be created before it is mounted, so we have to do it here
+ proxmox_sys::fs::create_path(
+ &full_store_path,
+ Some(default_options.clone()),
+ Some(options.clone()),
+ )
+ .map_err(|e| format_err!("creating datastore path '{full_store_path}' failed: {e}"))?;
+
+ info!(
+ "bind mount '{}'({}) to '{}'",
+ datastore.name, datastore.path, mount_point
+ );
+ if let Err(err) =
+ crate::tools::disks::bind_mount(Path::new(&full_store_path), Path::new(&mount_point))
+ {
+ debug!("unmounting '{}'", tmp_mount_path);
+ let _ = crate::tools::disks::unmount_by_mountpoint(Path::new(&tmp_mount_path))
+ .inspect_err(|e| warn!("unmounting from tmp path '{tmp_mount_path} failed: {e}'"));
+ let _ = std::fs::remove_dir(std::path::Path::new(&tmp_mount_path))
+ .inspect_err(|e| warn!("removing tmp path '{tmp_mount_path} failed: {e}'"));
+ return Err(format_err!(
+ "Datastore '{}' cound not be mounted: {}.",
+ datastore.name,
+ err
+ ));
+ }
+
+ debug!("unmounting '{}'", tmp_mount_path);
+ let _ = crate::tools::disks::unmount_by_mountpoint(Path::new(&tmp_mount_path))
+ .map_err(|e| format_err!("unmounting from tmp path '{tmp_mount_path} failed: {e}'"));
+ let _ = std::fs::remove_dir(std::path::Path::new(&tmp_mount_path))
+ .map_err(|e| format_err!("removing tmp path '{tmp_mount_path} failed: {e}'"));
+
+ Ok(())
+ } else {
+ Err(format_err!(
+ "Datastore '{}' cannot be mounted because it is not removable.",
+ datastore.name
+ ))
+ }
+}
+
+#[api(
+ protected: true,
+ input: {
+ properties: {
+ store: {
+ schema: DATASTORE_SCHEMA,
+ },
+ }
+ },
+ returns: {
+ schema: UPID_SCHEMA,
+ },
+ access: {
+ permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT, false),
+ },
+)]
+/// Mount removable datastore.
+pub fn mount(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
+ let (section_config, _digest) = pbs_config::datastore::config()?;
+ let datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
+
+ if datastore.backing_device.is_none() {
+ bail!("datastore '{store}' is not removable");
+ }
+
+ let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+ let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
+
+ let upid = WorkerTask::new_thread(
+ "mount-device",
+ Some(store),
+ auth_id.to_string(),
+ to_stdout,
+ move |_worker| do_mount_device(datastore),
+ )?;
+
+ Ok(json!(upid))
+}
+
+fn expect_maintanance_unmounting(
+ store: &str,
+) -> Result<(pbs_config::BackupLockGuard, DataStoreConfig), Error> {
+ let lock = pbs_config::datastore::lock_config()?;
+ let (section_config, _digest) = pbs_config::datastore::config()?;
+ let store_config: DataStoreConfig = section_config.lookup("datastore", store)?;
+
+ if store_config
+ .get_maintenance_mode()
+ .map_or(true, |m| m.ty != MaintenanceType::Unmount)
+ {
+ bail!("maintenance mode is not 'Unmount'");
+ }
+
+ Ok((lock, store_config))
+}
+
+fn unset_maintenance(
+ _lock: pbs_config::BackupLockGuard,
+ mut config: DataStoreConfig,
+) -> Result<(), Error> {
+ let (mut section_config, _digest) = pbs_config::datastore::config()?;
+ config.maintenance_mode = None;
+ section_config.set_data(&config.name, "datastore", &config)?;
+ pbs_config::datastore::save_config(§ion_config)?;
+ Ok(())
+}
+
+fn do_unmount_device(
+ datastore: DataStoreConfig,
+ worker: Option<&dyn WorkerTaskContext>,
+) -> Result<(), Error> {
+ if datastore.backing_device.is_none() {
+ bail!("can't unmount non-removable datastore");
+ }
+ let mount_point = datastore.absolute_path();
+
+ let mut active_operations = task_tracking::get_active_operations(&datastore.name)?;
+ let mut old_status = String::new();
+ let mut aborted = false;
+ while active_operations.read + active_operations.write > 0 {
+ if let Some(worker) = worker {
+ if worker.abort_requested() {
+ aborted = true;
+ break;
+ }
+ let status = format!(
+ "cannot unmount yet, still {} read and {} write operations active",
+ active_operations.read, active_operations.write
+ );
+ if status != old_status {
+ info!("{status}");
+ old_status = status;
+ }
+ }
+ std::thread::sleep(std::time::Duration::from_secs(1));
+ active_operations = task_tracking::get_active_operations(&datastore.name)?;
+ }
+
+ if aborted {
+ let _ = expect_maintanance_unmounting(&datastore.name)
+ .inspect_err(|e| warn!("maintenance mode was not as expected: {e}"))
+ .and_then(|(lock, config)| {
+ unset_maintenance(lock, config)
+ .inspect_err(|e| warn!("could not reset maintenance mode: {e}"))
+ });
+ bail!("aborted, due to user request");
+ } else {
+ let (lock, config) = expect_maintanance_unmounting(&datastore.name)?;
+ crate::tools::disks::unmount_by_mountpoint(Path::new(&mount_point))?;
+ let _ = unset_maintenance(lock, config)
+ .inspect_err(|e| warn!("could not reset maintenance mode: {e}"));
+ }
+ Ok(())
+}
+
+#[api(
+ protected: true,
+ input: {
+ properties: {
+ store: { schema: DATASTORE_SCHEMA },
+ },
+ },
+ returns: {
+ schema: UPID_SCHEMA,
+ },
+ access: {
+ permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_MODIFY, true),
+ }
+)]
+/// Unmount a removable device that is associated with the datastore
+pub async fn unmount(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
+ let _lock = pbs_config::datastore::lock_config()?;
+ let (mut section_config, _digest) = pbs_config::datastore::config()?;
+ let mut datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
+
+ if datastore.backing_device.is_none() {
+ bail!("datastore '{store}' is not removable");
+ }
+
+ ensure_datastore_is_mounted(&datastore)?;
+
+ datastore.set_maintenance_mode(Some(MaintenanceMode {
+ ty: MaintenanceType::Unmount,
+ message: None,
+ }))?;
+ section_config.set_data(&store, "datastore", &datastore)?;
+ pbs_config::datastore::save_config(§ion_config)?;
+
+ drop(_lock);
+
+ let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+ let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
+
+ if let Ok(proxy_pid) = proxmox_rest_server::read_pid(pbs_buildcfg::PROXMOX_BACKUP_PROXY_PID_FN)
+ {
+ let sock = proxmox_daemon::command_socket::path_from_pid(proxy_pid);
+ let _ = proxmox_daemon::command_socket::send_raw(
+ sock,
+ &format!(
+ "{{\"command\":\"update-datastore-cache\",\"args\":\"{}\"}}\n",
+ &store
+ ),
+ )
+ .await;
+ }
+
+ let upid = WorkerTask::new_thread(
+ "unmount-device",
+ Some(store),
+ auth_id.to_string(),
+ to_stdout,
+ move |worker| do_unmount_device(datastore, Some(&worker)),
+ )?;
+
+ Ok(json!(upid))
+}
+
#[sortable]
const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
(
@@ -2432,6 +2702,7 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
.get(&API_METHOD_LIST_GROUPS)
.delete(&API_METHOD_DELETE_GROUP),
),
+ ("mount", &Router::new().post(&API_METHOD_MOUNT)),
(
"namespace",
// FIXME: move into datastore:: sub-module?!
@@ -2466,6 +2737,7 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
.delete(&API_METHOD_DELETE_SNAPSHOT),
),
("status", &Router::new().get(&API_METHOD_STATUS)),
+ ("unmount", &Router::new().post(&API_METHOD_UNMOUNT)),
(
"upload-backup-log",
&Router::new().upload(&API_METHOD_UPLOAD_BACKUP_LOG),
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v14 05/25] api: admin: add (un)mount endpoint for removable datastores
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 05/25] api: admin: add (un)mount endpoint for removable datastores Hannes Laimer
@ 2024-11-25 13:24 ` Fabian Grünbichler
0 siblings, 0 replies; 45+ messages in thread
From: Fabian Grünbichler @ 2024-11-25 13:24 UTC (permalink / raw)
To: Proxmox Backup Server development discussion
On November 22, 2024 3:46 pm, Hannes Laimer wrote:
> Removable datastores can be mounted unless
> - they are already
> - their device is not present
> For unmounting the maintenance mode is set to `unmount`,
> which prohibits the starting of any new tasks envolving any
> IO, this mode is unset either
> - on completion of the unmount
> - on abort of the unmount tasks
> If the unmounting itself should fail, the maintenance mode stays in
> place and requires manual intervention by unsetting it in the config
> file directly. This is intentional, as unmounting should not fail,
> and if it should the situation should be looked at.
>
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> changes since v13:
> * improve logging
> * fix racy unmount
> * (manually) changing maintenance during unmount will prevent unmounting and
> result in failed unmount task
>
> src/api2/admin/datastore.rs | 294 ++++++++++++++++++++++++++++++++++--
> 1 file changed, 283 insertions(+), 11 deletions(-)
>
> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
> index 3b863c06b..85522345e 100644
> --- a/src/api2/admin/datastore.rs
> +++ b/src/api2/admin/datastore.rs
> @@ -4,7 +4,7 @@ use std::collections::HashSet;
> use std::ffi::OsStr;
> use std::ops::Deref;
> use std::os::unix::ffi::OsStrExt;
> -use std::path::PathBuf;
> +use std::path::{Path, PathBuf};
> use std::sync::Arc;
>
> use anyhow::{bail, format_err, Error};
> @@ -14,7 +14,7 @@ use hyper::{header, Body, Response, StatusCode};
> use serde::Deserialize;
> use serde_json::{json, Value};
> use tokio_stream::wrappers::ReceiverStream;
> -use tracing::{info, warn};
> +use tracing::{debug, info, warn};
>
> use proxmox_async::blocking::WrappedReaderStream;
> use proxmox_async::{io::AsyncChannelWriter, stream::AsyncReaderStream};
> @@ -30,6 +30,7 @@ use proxmox_sys::fs::{
> file_read_firstline, file_read_optional_string, replace_file, CreateOptions,
> };
> use proxmox_time::CalendarEvent;
> +use proxmox_worker_task::WorkerTaskContext;
>
> use pxar::accessor::aio::Accessor;
> use pxar::EntryKind;
> @@ -38,13 +39,13 @@ use pbs_api_types::{
> print_ns_and_snapshot, print_store_and_ns, ArchiveType, Authid, BackupArchiveName,
> BackupContent, BackupGroupDeleteStats, BackupNamespace, BackupType, Counts, CryptMode,
> DataStoreConfig, DataStoreListItem, DataStoreStatus, GarbageCollectionJobStatus, GroupListItem,
> - JobScheduleStatus, KeepOptions, Operation, PruneJobOptions, SnapshotListItem,
> - SnapshotVerifyState, BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA,
> - BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA, CATALOG_NAME, CLIENT_LOG_BLOB_NAME, DATASTORE_SCHEMA,
> - IGNORE_VERIFIED_BACKUPS_SCHEMA, MANIFEST_BLOB_NAME, MAX_NAMESPACE_DEPTH, NS_MAX_DEPTH_SCHEMA,
> - PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE,
> - PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY, UPID, UPID_SCHEMA,
> - VERIFICATION_OUTDATED_AFTER_SCHEMA,
> + JobScheduleStatus, KeepOptions, MaintenanceMode, MaintenanceType, Operation, PruneJobOptions,
> + SnapshotListItem, SnapshotVerifyState, BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA,
> + BACKUP_NAMESPACE_SCHEMA, BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA, CATALOG_NAME,
> + CLIENT_LOG_BLOB_NAME, DATASTORE_SCHEMA, IGNORE_VERIFIED_BACKUPS_SCHEMA, MANIFEST_BLOB_NAME,
> + MAX_NAMESPACE_DEPTH, NS_MAX_DEPTH_SCHEMA, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP,
> + PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE, PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY, UPID,
> + UPID_SCHEMA, VERIFICATION_OUTDATED_AFTER_SCHEMA,
> };
> use pbs_client::pxar::{create_tar, create_zip};
> use pbs_config::CachedUserInfo;
> @@ -59,8 +60,8 @@ use pbs_datastore::index::IndexFile;
> use pbs_datastore::manifest::BackupManifest;
> use pbs_datastore::prune::compute_prune_info;
> use pbs_datastore::{
> - check_backup_owner, task_tracking, BackupDir, BackupGroup, DataStore, LocalChunkReader,
> - StoreProgress,
> + check_backup_owner, ensure_datastore_is_mounted, task_tracking, BackupDir, BackupGroup,
> + DataStore, LocalChunkReader, StoreProgress,
> };
> use pbs_tools::json::required_string_param;
> use proxmox_rest_server::{formatter, WorkerTask};
> @@ -2394,6 +2395,275 @@ pub async fn set_backup_owner(
> .await?
> }
>
> +/// Here we
> +///
> +/// 1. mount the removable device to `<PBS_RUN_DIR>/mount/<RANDOM_UUID>`
> +/// 2. bind mount `<PBS_RUN_DIR>/mount/<RANDOM_UUID>/<datastore.path>` to `/mnt/datastore/<datastore.name>`
> +/// 3. unmount `<PBS_RUN_DIR>/mount/<RANDOM_UUID>`
> +///
> +/// leaving us with the datastore being mounted directly with its name under /mnt/datastore/...
> +///
> +/// The reason for the randomized device mounting paths is to avoid two tasks trying to mount to
> +/// the same path, this is *very* unlikely since the device is only mounted really shortly, but
> +/// technically possible.
> +pub fn do_mount_device(datastore: DataStoreConfig) -> Result<(), Error> {
> + if let Some(uuid) = datastore.backing_device.as_ref() {
> + let mount_point = datastore.absolute_path();
> + if pbs_datastore::get_datastore_mount_status(&datastore) == Some(true) {
> + bail!("device is already mounted at '{}'", mount_point);
> + }
> + let tmp_mount_path = format!(
> + "{}/{:x}",
> + pbs_buildcfg::rundir!("/mount"),
> + proxmox_uuid::Uuid::generate()
> + );
> +
> + let default_options = proxmox_sys::fs::CreateOptions::new();
> + proxmox_sys::fs::create_path(
> + &tmp_mount_path,
> + Some(default_options.clone()),
> + Some(default_options.clone()),
> + )?;
> +
> + info!("temporarily mounting '{uuid}' to '{}'", tmp_mount_path);
> + crate::tools::disks::mount_by_uuid(uuid, Path::new(&tmp_mount_path))
> + .map_err(|e| format_err!("mounting to tmp path failed: {e}"))?;
after this point, any error should trigger an unmount before being
bubbled up..
> +
> + let full_store_path = format!(
> + "{tmp_mount_path}/{}",
> + datastore.path.trim_start_matches('/')
> + );
> + let backup_user = pbs_config::backup_user()?;
> + let options = CreateOptions::new()
> + .owner(backup_user.uid)
> + .group(backup_user.gid);
> +
> + proxmox_sys::fs::create_path(
> + &mount_point,
> + Some(default_options.clone()),
> + Some(options.clone()),
> + )
> + .map_err(|e| format_err!("creating mountpoint '{mount_point}' failed: {e}"))?;
> +
> + // can't be created before it is mounted, so we have to do it here
> + proxmox_sys::fs::create_path(
> + &full_store_path,
> + Some(default_options.clone()),
> + Some(options.clone()),
> + )
> + .map_err(|e| format_err!("creating datastore path '{full_store_path}' failed: {e}"))?;
> +
> + info!(
> + "bind mount '{}'({}) to '{}'",
> + datastore.name, datastore.path, mount_point
> + );
> + if let Err(err) =
> + crate::tools::disks::bind_mount(Path::new(&full_store_path), Path::new(&mount_point))
> + {
> + debug!("unmounting '{}'", tmp_mount_path);
> + let _ = crate::tools::disks::unmount_by_mountpoint(Path::new(&tmp_mount_path))
> + .inspect_err(|e| warn!("unmounting from tmp path '{tmp_mount_path} failed: {e}'"));
> + let _ = std::fs::remove_dir(std::path::Path::new(&tmp_mount_path))
> + .inspect_err(|e| warn!("removing tmp path '{tmp_mount_path} failed: {e}'"));
this doesn't log the error, so adding context doesn't help at all..
> + return Err(format_err!(
> + "Datastore '{}' cound not be mounted: {}.",
> + datastore.name,
> + err
> + ));
> + }
> +
> + debug!("unmounting '{}'", tmp_mount_path);
> + let _ = crate::tools::disks::unmount_by_mountpoint(Path::new(&tmp_mount_path))
> + .map_err(|e| format_err!("unmounting from tmp path '{tmp_mount_path} failed: {e}'"));
> + let _ = std::fs::remove_dir(std::path::Path::new(&tmp_mount_path))
> + .map_err(|e| format_err!("removing tmp path '{tmp_mount_path} failed: {e}'"));
same here
> +
> + Ok(())
> + } else {
> + Err(format_err!(
> + "Datastore '{}' cannot be mounted because it is not removable.",
> + datastore.name
> + ))
> + }
> +}
> +
> +#[api(
> + protected: true,
> + input: {
> + properties: {
> + store: {
> + schema: DATASTORE_SCHEMA,
> + },
> + }
> + },
> + returns: {
> + schema: UPID_SCHEMA,
> + },
> + access: {
> + permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT, false),
> + },
> +)]
> +/// Mount removable datastore.
> +pub fn mount(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
> + let (section_config, _digest) = pbs_config::datastore::config()?;
> + let datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
> +
> + if datastore.backing_device.is_none() {
> + bail!("datastore '{store}' is not removable");
> + }
> +
> + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> + let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
> +
> + let upid = WorkerTask::new_thread(
> + "mount-device",
> + Some(store),
> + auth_id.to_string(),
> + to_stdout,
> + move |_worker| do_mount_device(datastore),
> + )?;
> +
> + Ok(json!(upid))
> +}
> +
> +fn expect_maintanance_unmounting(
> + store: &str,
> +) -> Result<(pbs_config::BackupLockGuard, DataStoreConfig), Error> {
> + let lock = pbs_config::datastore::lock_config()?;
> + let (section_config, _digest) = pbs_config::datastore::config()?;
> + let store_config: DataStoreConfig = section_config.lookup("datastore", store)?;
> +
> + if store_config
> + .get_maintenance_mode()
> + .map_or(true, |m| m.ty != MaintenanceType::Unmount)
> + {
> + bail!("maintenance mode is not 'Unmount'");
> + }
> +
> + Ok((lock, store_config))
> +}
> +
> +fn unset_maintenance(
> + _lock: pbs_config::BackupLockGuard,
> + mut config: DataStoreConfig,
> +) -> Result<(), Error> {
> + let (mut section_config, _digest) = pbs_config::datastore::config()?;
> + config.maintenance_mode = None;
> + section_config.set_data(&config.name, "datastore", &config)?;
> + pbs_config::datastore::save_config(§ion_config)?;
> + Ok(())
> +}
> +
> +fn do_unmount_device(
> + datastore: DataStoreConfig,
> + worker: Option<&dyn WorkerTaskContext>,
> +) -> Result<(), Error> {
> + if datastore.backing_device.is_none() {
> + bail!("can't unmount non-removable datastore");
> + }
> + let mount_point = datastore.absolute_path();
> +
> + let mut active_operations = task_tracking::get_active_operations(&datastore.name)?;
> + let mut old_status = String::new();
> + let mut aborted = false;
> + while active_operations.read + active_operations.write > 0 {
> + if let Some(worker) = worker {
> + if worker.abort_requested() {
> + aborted = true;
> + break;
> + }
> + let status = format!(
> + "cannot unmount yet, still {} read and {} write operations active",
> + active_operations.read, active_operations.write
> + );
> + if status != old_status {
> + info!("{status}");
> + old_status = status;
> + }
> + }
> + std::thread::sleep(std::time::Duration::from_secs(1));
> + active_operations = task_tracking::get_active_operations(&datastore.name)?;
> + }
> +
> + if aborted {
this still doesn't re-check whether the request was aborted.. the loop
above sleeps for a second, it's possible the worker got aborted in that
time frame..
> + let _ = expect_maintanance_unmounting(&datastore.name)
> + .inspect_err(|e| warn!("maintenance mode was not as expected: {e}"))
> + .and_then(|(lock, config)| {
> + unset_maintenance(lock, config)
> + .inspect_err(|e| warn!("could not reset maintenance mode: {e}"))
> + });
> + bail!("aborted, due to user request");
> + } else {
> + let (lock, config) = expect_maintanance_unmounting(&datastore.name)?;
> + crate::tools::disks::unmount_by_mountpoint(Path::new(&mount_point))?;
> + let _ = unset_maintenance(lock, config)
> + .inspect_err(|e| warn!("could not reset maintenance mode: {e}"));
this should return the error..
> + }
> + Ok(())
> +}
> +
> +#[api(
> + protected: true,
> + input: {
> + properties: {
> + store: { schema: DATASTORE_SCHEMA },
> + },
> + },
> + returns: {
> + schema: UPID_SCHEMA,
> + },
> + access: {
> + permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_MODIFY, true),
> + }
> +)]
> +/// Unmount a removable device that is associated with the datastore
> +pub async fn unmount(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
> + let _lock = pbs_config::datastore::lock_config()?;
> + let (mut section_config, _digest) = pbs_config::datastore::config()?;
> + let mut datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
> +
> + if datastore.backing_device.is_none() {
> + bail!("datastore '{store}' is not removable");
> + }
> +
> + ensure_datastore_is_mounted(&datastore)?;
> +
> + datastore.set_maintenance_mode(Some(MaintenanceMode {
> + ty: MaintenanceType::Unmount,
> + message: None,
> + }))?;
> + section_config.set_data(&store, "datastore", &datastore)?;
> + pbs_config::datastore::save_config(§ion_config)?;
> +
> + drop(_lock);
> +
> + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> + let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
> +
> + if let Ok(proxy_pid) = proxmox_rest_server::read_pid(pbs_buildcfg::PROXMOX_BACKUP_PROXY_PID_FN)
> + {
> + let sock = proxmox_daemon::command_socket::path_from_pid(proxy_pid);
> + let _ = proxmox_daemon::command_socket::send_raw(
> + sock,
> + &format!(
> + "{{\"command\":\"update-datastore-cache\",\"args\":\"{}\"}}\n",
> + &store
> + ),
> + )
> + .await;
> + }
> +
> + let upid = WorkerTask::new_thread(
> + "unmount-device",
> + Some(store),
> + auth_id.to_string(),
> + to_stdout,
> + move |worker| do_unmount_device(datastore, Some(&worker)),
> + )?;
> +
> + Ok(json!(upid))
> +}
> +
> #[sortable]
> const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
> (
> @@ -2432,6 +2702,7 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
> .get(&API_METHOD_LIST_GROUPS)
> .delete(&API_METHOD_DELETE_GROUP),
> ),
> + ("mount", &Router::new().post(&API_METHOD_MOUNT)),
> (
> "namespace",
> // FIXME: move into datastore:: sub-module?!
> @@ -2466,6 +2737,7 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
> .delete(&API_METHOD_DELETE_SNAPSHOT),
> ),
> ("status", &Router::new().get(&API_METHOD_STATUS)),
> + ("unmount", &Router::new().post(&API_METHOD_UNMOUNT)),
> (
> "upload-backup-log",
> &Router::new().upload(&API_METHOD_UPLOAD_BACKUP_LOG),
> --
> 2.39.5
>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
>
>
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v14 06/25] api: removable datastore creation
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
` (4 preceding siblings ...)
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 05/25] api: admin: add (un)mount endpoint for removable datastores Hannes Laimer
@ 2024-11-22 14:46 ` Hannes Laimer
2024-11-25 13:40 ` Fabian Grünbichler
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 07/25] api: add check for nested datastores on creation Hannes Laimer
` (21 subsequent siblings)
27 siblings, 1 reply; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:46 UTC (permalink / raw)
To: pbs-devel
Devices can contains multiple datastores.
If the specified path already contains a datastore, `reuse datastore` has
to be set so it'll be added without creating a chunckstore.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
change since v13:
* cleanup
src/api2/config/datastore.rs | 54 ++++++++++++++++++++++++++----------
1 file changed, 40 insertions(+), 14 deletions(-)
diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 37d1528c7..420f8ddd0 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -1,7 +1,7 @@
use std::path::PathBuf;
use ::serde::{Deserialize, Serialize};
-use anyhow::{bail, Error};
+use anyhow::{bail, format_err, Error};
use hex::FromHex;
use serde_json::Value;
use tracing::warn;
@@ -21,7 +21,8 @@ use pbs_config::BackupLockGuard;
use pbs_datastore::chunk_store::ChunkStore;
use crate::api2::admin::{
- prune::list_prune_jobs, sync::list_config_sync_jobs, verify::list_verification_jobs,
+ datastore::do_mount_device, prune::list_prune_jobs, sync::list_config_sync_jobs,
+ verify::list_verification_jobs,
};
use crate::api2::config::prune::{delete_prune_job, do_create_prune_job};
use crate::api2::config::sync::delete_sync_job;
@@ -32,6 +33,7 @@ use pbs_config::CachedUserInfo;
use proxmox_rest_server::WorkerTask;
use crate::server::jobstate;
+use crate::tools::disks::unmount_by_mountpoint;
#[api(
input: {
@@ -73,37 +75,57 @@ pub(crate) fn do_create_datastore(
datastore: DataStoreConfig,
reuse_datastore: bool,
) -> Result<(), Error> {
- let path: PathBuf = datastore.path.clone().into();
+ let path: PathBuf = datastore.absolute_path().into();
if path.parent().is_none() {
bail!("cannot create datastore in root path");
}
+ let need_unmount = datastore.backing_device.is_some();
+ if need_unmount {
+ do_mount_device(datastore.clone())?;
+ };
+
let tuning: DatastoreTuning = serde_json::from_value(
DatastoreTuning::API_SCHEMA
.parse_property_string(datastore.tuning.as_deref().unwrap_or(""))?,
)?;
- if reuse_datastore {
- ChunkStore::verify_chunkstore(&path)?;
+ let res = if reuse_datastore {
+ ChunkStore::verify_chunkstore(&path)
} else {
+ let mut is_empty = true;
if let Ok(dir) = std::fs::read_dir(&path) {
for file in dir {
let name = file?.file_name();
let name = name.to_str();
if !name.map_or(false, |name| name.starts_with('.') || name == "lost+found") {
- bail!("datastore path is not empty");
+ is_empty = false;
+ break;
}
}
}
- let backup_user = pbs_config::backup_user()?;
- let _store = ChunkStore::create(
- &datastore.name,
- path,
- backup_user.uid,
- backup_user.gid,
- tuning.sync_level.unwrap_or_default(),
- )?;
+ if is_empty {
+ let backup_user = pbs_config::backup_user()?;
+ ChunkStore::create(
+ &datastore.name,
+ path.clone(),
+ backup_user.uid,
+ backup_user.gid,
+ tuning.sync_level.unwrap_or_default(),
+ )
+ .map(|_| ())
+ } else {
+ Err(format_err!("datastore path not empty"))
+ }
+ };
+
+ if res.is_err() {
+ if need_unmount {
+ let _ = unmount_by_mountpoint(&path)
+ .inspect_err(|e| warn!("could not unmount device: {e}"));
+ }
+ return res;
}
config.set_data(&datastore.name, "datastore", &datastore)?;
@@ -147,6 +169,10 @@ pub fn create_datastore(
param_bail!("name", "datastore '{}' already exists.", config.name);
}
+ if !config.path.starts_with("/") {
+ param_bail!("path", "expected an abolute path, '{}' is not", config.path);
+ }
+
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v14 06/25] api: removable datastore creation
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 06/25] api: removable datastore creation Hannes Laimer
@ 2024-11-25 13:40 ` Fabian Grünbichler
0 siblings, 0 replies; 45+ messages in thread
From: Fabian Grünbichler @ 2024-11-25 13:40 UTC (permalink / raw)
To: Proxmox Backup Server development discussion
On November 22, 2024 3:46 pm, Hannes Laimer wrote:
> Devices can contains multiple datastores.
> If the specified path already contains a datastore, `reuse datastore` has
> to be set so it'll be added without creating a chunckstore.
>
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> change since v13:
> * cleanup
>
> src/api2/config/datastore.rs | 54 ++++++++++++++++++++++++++----------
> 1 file changed, 40 insertions(+), 14 deletions(-)
>
> diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
> index 37d1528c7..420f8ddd0 100644
> --- a/src/api2/config/datastore.rs
> +++ b/src/api2/config/datastore.rs
> @@ -1,7 +1,7 @@
> use std::path::PathBuf;
>
> use ::serde::{Deserialize, Serialize};
> -use anyhow::{bail, Error};
> +use anyhow::{bail, format_err, Error};
> use hex::FromHex;
> use serde_json::Value;
> use tracing::warn;
> @@ -21,7 +21,8 @@ use pbs_config::BackupLockGuard;
> use pbs_datastore::chunk_store::ChunkStore;
>
> use crate::api2::admin::{
> - prune::list_prune_jobs, sync::list_config_sync_jobs, verify::list_verification_jobs,
> + datastore::do_mount_device, prune::list_prune_jobs, sync::list_config_sync_jobs,
> + verify::list_verification_jobs,
> };
> use crate::api2::config::prune::{delete_prune_job, do_create_prune_job};
> use crate::api2::config::sync::delete_sync_job;
> @@ -32,6 +33,7 @@ use pbs_config::CachedUserInfo;
> use proxmox_rest_server::WorkerTask;
>
> use crate::server::jobstate;
> +use crate::tools::disks::unmount_by_mountpoint;
>
> #[api(
> input: {
> @@ -73,37 +75,57 @@ pub(crate) fn do_create_datastore(
> datastore: DataStoreConfig,
> reuse_datastore: bool,
> ) -> Result<(), Error> {
> - let path: PathBuf = datastore.path.clone().into();
> + let path: PathBuf = datastore.absolute_path().into();
>
> if path.parent().is_none() {
> bail!("cannot create datastore in root path");
> }
>
> + let need_unmount = datastore.backing_device.is_some();
> + if need_unmount {
> + do_mount_device(datastore.clone())?;
> + };
> +
> let tuning: DatastoreTuning = serde_json::from_value(
> DatastoreTuning::API_SCHEMA
> .parse_property_string(datastore.tuning.as_deref().unwrap_or(""))?,
> )?;
>
> - if reuse_datastore {
> - ChunkStore::verify_chunkstore(&path)?;
> + let res = if reuse_datastore {
> + ChunkStore::verify_chunkstore(&path)
> } else {
> + let mut is_empty = true;
> if let Ok(dir) = std::fs::read_dir(&path) {
> for file in dir {
> let name = file?.file_name();
> let name = name.to_str();
> if !name.map_or(false, |name| name.starts_with('.') || name == "lost+found") {
> - bail!("datastore path is not empty");
> + is_empty = false;
> + break;
> }
> }
> }
> - let backup_user = pbs_config::backup_user()?;
> - let _store = ChunkStore::create(
> - &datastore.name,
> - path,
> - backup_user.uid,
> - backup_user.gid,
> - tuning.sync_level.unwrap_or_default(),
> - )?;
> + if is_empty {
> + let backup_user = pbs_config::backup_user()?;
> + ChunkStore::create(
> + &datastore.name,
> + path.clone(),
> + backup_user.uid,
> + backup_user.gid,
> + tuning.sync_level.unwrap_or_default(),
> + )
> + .map(|_| ())
> + } else {
> + Err(format_err!("datastore path not empty"))
> + }
> + };
> +
> + if res.is_err() {
> + if need_unmount {
> + let _ = unmount_by_mountpoint(&path)
> + .inspect_err(|e| warn!("could not unmount device: {e}"));
I think I prefer
if let Err(err) = .. {
warn!(..);
}
it's more obvious that an error is "caught" and logged when I read that
(inspect_err and no bubbling up vs. map_err and bubbling up look very
similar when quickly browsing code)
}
> + }
> + return res;
> }
>
> config.set_data(&datastore.name, "datastore", &datastore)?;
> @@ -147,6 +169,10 @@ pub fn create_datastore(
> param_bail!("name", "datastore '{}' already exists.", config.name);
> }
>
> + if !config.path.starts_with("/") {
> + param_bail!("path", "expected an abolute path, '{}' is not", config.path);
> + }
see comment on the first patch, I think you misunderstood my feedback
there ;) this check here should only be done for non-removable
datastores. removable datastores should continue to have their relative
path in the config..
> +
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
>
> --
> 2.39.5
>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
>
>
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v14 07/25] api: add check for nested datastores on creation
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
` (5 preceding siblings ...)
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 06/25] api: removable datastore creation Hannes Laimer
@ 2024-11-22 14:46 ` Hannes Laimer
2024-11-25 13:44 ` Fabian Grünbichler
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 08/25] pbs-api-types: add mount_status field to DataStoreListItem Hannes Laimer
` (20 subsequent siblings)
27 siblings, 1 reply; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:46 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
* new in v14, and not removable datastore specific
src/api2/config/datastore.rs | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 420f8ddd0..75e1a1a56 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -81,6 +81,20 @@ pub(crate) fn do_create_datastore(
bail!("cannot create datastore in root path");
}
+ for store in config.convert_to_typed_array::<DataStoreConfig>("datastore")? {
+ if store.backing_device != datastore.backing_device {
+ continue;
+ }
+ if store.path.starts_with(&datastore.path) || datastore.path.starts_with(&store.path) {
+ param_bail!(
+ "path",
+ "nested datastores not allowed: '{}' already in '{}'",
+ store.name,
+ store.path
+ );
+ }
+ }
+
let need_unmount = datastore.backing_device.is_some();
if need_unmount {
do_mount_device(datastore.clone())?;
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v14 07/25] api: add check for nested datastores on creation
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 07/25] api: add check for nested datastores on creation Hannes Laimer
@ 2024-11-25 13:44 ` Fabian Grünbichler
2024-11-25 14:48 ` Hannes Laimer
0 siblings, 1 reply; 45+ messages in thread
From: Fabian Grünbichler @ 2024-11-25 13:44 UTC (permalink / raw)
To: Proxmox Backup Server development discussion
On November 22, 2024 3:46 pm, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> * new in v14, and not removable datastore specific
>
> src/api2/config/datastore.rs | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
> index 420f8ddd0..75e1a1a56 100644
> --- a/src/api2/config/datastore.rs
> +++ b/src/api2/config/datastore.rs
> @@ -81,6 +81,20 @@ pub(crate) fn do_create_datastore(
> bail!("cannot create datastore in root path");
> }
>
> + for store in config.convert_to_typed_array::<DataStoreConfig>("datastore")? {
> + if store.backing_device != datastore.backing_device {
> + continue;
> + }
this is not needed, if you compare absolute_paths below?
> + if store.path.starts_with(&datastore.path) || datastore.path.starts_with(&store.path) {
this is broken, as `path` is a String here, and not a Path, so
`starts_with` doesn't properly match on path components, but on
arbitrary substrings..
i.e., I can't configure two removable datastores, one using "removable"
and one using "removable2" as path atm..
> + param_bail!(
> + "path",
> + "nested datastores not allowed: '{}' already in '{}'",
> + store.name,
> + store.path
> + );
> + }
> + }
> +
> let need_unmount = datastore.backing_device.is_some();
> if need_unmount {
> do_mount_device(datastore.clone())?;
> --
> 2.39.5
>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
>
>
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v14 07/25] api: add check for nested datastores on creation
2024-11-25 13:44 ` Fabian Grünbichler
@ 2024-11-25 14:48 ` Hannes Laimer
2024-11-25 14:53 ` Fabian Grünbichler
0 siblings, 1 reply; 45+ messages in thread
From: Hannes Laimer @ 2024-11-25 14:48 UTC (permalink / raw)
To: pbs-devel
On 11/25/24 14:44, Fabian Grünbichler wrote:
> On November 22, 2024 3:46 pm, Hannes Laimer wrote:
>> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
>> ---
>> * new in v14, and not removable datastore specific
>>
>> src/api2/config/datastore.rs | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
>> index 420f8ddd0..75e1a1a56 100644
>> --- a/src/api2/config/datastore.rs
>> +++ b/src/api2/config/datastore.rs
>> @@ -81,6 +81,20 @@ pub(crate) fn do_create_datastore(
>> bail!("cannot create datastore in root path");
>> }
>>
>> + for store in config.convert_to_typed_array::<DataStoreConfig>("datastore")? {
>> + if store.backing_device != datastore.backing_device {
>> + continue;
>> + }
>
> this is not needed, if you compare absolute_paths below?
>
absolute path is DS_MNT_DIR/{name}, it does not contain the on-device
path, we need this
>> + if store.path.starts_with(&datastore.path) || datastore.path.starts_with(&store.path) {
>
> this is broken, as `path` is a String here, and not a Path, so
> `starts_with` doesn't properly match on path components, but on
> arbitrary substrings..
>
> i.e., I can't configure two removable datastores, one using "removable"
> and one using "removable2" as path atm..
>
>> + param_bail!(
>> + "path",
>> + "nested datastores not allowed: '{}' already in '{}'",
>> + store.name,
>> + store.path
>> + );
>> + }
>> + }
>> +
>> let need_unmount = datastore.backing_device.is_some();
>> if need_unmount {
>> do_mount_device(datastore.clone())?;
>> --
>> 2.39.5
>>
>>
>>
>> _______________________________________________
>> pbs-devel mailing list
>> pbs-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>>
>>
>>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
>
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v14 07/25] api: add check for nested datastores on creation
2024-11-25 14:48 ` Hannes Laimer
@ 2024-11-25 14:53 ` Fabian Grünbichler
0 siblings, 0 replies; 45+ messages in thread
From: Fabian Grünbichler @ 2024-11-25 14:53 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
> Hannes Laimer <h.laimer@proxmox.com> hat am 25.11.2024 15:48 CET geschrieben:
>
>
> On 11/25/24 14:44, Fabian Grünbichler wrote:
> > On November 22, 2024 3:46 pm, Hannes Laimer wrote:
> >> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> >> ---
> >> * new in v14, and not removable datastore specific
> >>
> >> src/api2/config/datastore.rs | 14 ++++++++++++++
> >> 1 file changed, 14 insertions(+)
> >>
> >> diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
> >> index 420f8ddd0..75e1a1a56 100644
> >> --- a/src/api2/config/datastore.rs
> >> +++ b/src/api2/config/datastore.rs
> >> @@ -81,6 +81,20 @@ pub(crate) fn do_create_datastore(
> >> bail!("cannot create datastore in root path");
> >> }
> >>
> >> + for store in config.convert_to_typed_array::<DataStoreConfig>("datastore")? {
> >> + if store.backing_device != datastore.backing_device {
> >> + continue;
> >> + }
> >
> > this is not needed, if you compare absolute_paths below?
> >
>
> absolute path is DS_MNT_DIR/{name}, it does not contain the on-device
> path, we need this
fair enough, but the current form is still broken across the board..
we then actually need two checks here:
- absolute paths not overlapping for removable and regular datastores
- on device paths not overlapping within a device for removable datastores
>
> >> + if store.path.starts_with(&datastore.path) || datastore.path.starts_with(&store.path) {
> >
> > this is broken, as `path` is a String here, and not a Path, so
> > `starts_with` doesn't properly match on path components, but on
> > arbitrary substrings..
> >
> > i.e., I can't configure two removable datastores, one using "removable"
> > and one using "removable2" as path atm..
and this still needs to be addressed..
> >
> >> + param_bail!(
> >> + "path",
> >> + "nested datastores not allowed: '{}' already in '{}'",
> >> + store.name,
> >> + store.path
> >> + );
> >> + }
> >> + }
> >> +
> >> let need_unmount = datastore.backing_device.is_some();
> >> if need_unmount {
> >> do_mount_device(datastore.clone())?;
> >> --
> >> 2.39.5
> >>
> >>
> >>
> >> _______________________________________________
> >> pbs-devel mailing list
> >> pbs-devel@lists.proxmox.com
> >> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> >>
> >>
> >>
> >
> >
> > _______________________________________________
> > pbs-devel mailing list
> > pbs-devel@lists.proxmox.com
> > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> >
> >
>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v14 08/25] pbs-api-types: add mount_status field to DataStoreListItem
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
` (6 preceding siblings ...)
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 07/25] api: add check for nested datastores on creation Hannes Laimer
@ 2024-11-22 14:46 ` Hannes Laimer
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 09/26] bin: manager: add (un)mount command Hannes Laimer
` (19 subsequent siblings)
27 siblings, 0 replies; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:46 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
changdes since v13:
* drop commit message, seemed unnecessary, enum is pretty
self-explainatory
* use enum instead of Option<bool>
pbs-api-types/src/datastore.rs | 19 ++++++++++++++++-
src/api2/admin/datastore.rs | 38 ++++++++++++++++++++--------------
src/api2/status/mod.rs | 30 +++++++++++++++++++++++----
3 files changed, 66 insertions(+), 21 deletions(-)
diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index ba75ebaba..b445e10e5 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -178,6 +178,20 @@ pub enum ChunkOrder {
Inode,
}
+#[api]
+#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
+#[serde(rename_all = "lowercase")]
+/// Current mounting status of a datastore, useful for removable datastores.
+pub enum DataStoreMountStatus {
+ /// Removable datastore is currently mounted correctly.
+ Mounted,
+ /// Removable datastore is currebtly not mounted.
+ NotMounted,
+ /// Datastore is not removable, so there is no mount status.
+ #[default]
+ NonRemovable,
+}
+
#[api]
#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")]
@@ -451,6 +465,7 @@ impl DataStoreConfig {
pub struct DataStoreListItem {
pub store: String,
pub comment: Option<String>,
+ pub mount_status: DataStoreMountStatus,
/// If the datastore is in maintenance mode, information about it
#[serde(skip_serializing_if = "Option::is_none")]
pub maintenance: Option<String>,
@@ -1456,6 +1471,7 @@ pub struct DataStoreStatusListItem {
/// The available bytes of the underlying storage. (-1 on error)
#[serde(skip_serializing_if = "Option::is_none")]
pub avail: Option<u64>,
+ pub mount_status: DataStoreMountStatus,
/// A list of usages of the past (last Month).
#[serde(skip_serializing_if = "Option::is_none")]
pub history: Option<Vec<Option<f64>>>,
@@ -1480,12 +1496,13 @@ pub struct DataStoreStatusListItem {
}
impl DataStoreStatusListItem {
- pub fn empty(store: &str, err: Option<String>) -> Self {
+ pub fn empty(store: &str, err: Option<String>, mount_status: DataStoreMountStatus) -> Self {
DataStoreStatusListItem {
store: store.to_owned(),
total: None,
used: None,
avail: None,
+ mount_status,
history: None,
history_start: None,
history_delta: None,
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 85522345e..f41024e42 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -38,14 +38,15 @@ use pxar::EntryKind;
use pbs_api_types::{
print_ns_and_snapshot, print_store_and_ns, ArchiveType, Authid, BackupArchiveName,
BackupContent, BackupGroupDeleteStats, BackupNamespace, BackupType, Counts, CryptMode,
- DataStoreConfig, DataStoreListItem, DataStoreStatus, GarbageCollectionJobStatus, GroupListItem,
- JobScheduleStatus, KeepOptions, MaintenanceMode, MaintenanceType, Operation, PruneJobOptions,
- SnapshotListItem, SnapshotVerifyState, BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA,
- BACKUP_NAMESPACE_SCHEMA, BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA, CATALOG_NAME,
- CLIENT_LOG_BLOB_NAME, DATASTORE_SCHEMA, IGNORE_VERIFIED_BACKUPS_SCHEMA, MANIFEST_BLOB_NAME,
- MAX_NAMESPACE_DEPTH, NS_MAX_DEPTH_SCHEMA, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP,
- PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE, PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY, UPID,
- UPID_SCHEMA, VERIFICATION_OUTDATED_AFTER_SCHEMA,
+ DataStoreConfig, DataStoreListItem, DataStoreMountStatus, DataStoreStatus,
+ GarbageCollectionJobStatus, GroupListItem, JobScheduleStatus, KeepOptions, MaintenanceMode,
+ MaintenanceType, Operation, PruneJobOptions, SnapshotListItem, SnapshotVerifyState,
+ BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA, BACKUP_TIME_SCHEMA,
+ BACKUP_TYPE_SCHEMA, CATALOG_NAME, CLIENT_LOG_BLOB_NAME, DATASTORE_SCHEMA,
+ IGNORE_VERIFIED_BACKUPS_SCHEMA, MANIFEST_BLOB_NAME, MAX_NAMESPACE_DEPTH, NS_MAX_DEPTH_SCHEMA,
+ PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE,
+ PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY, UPID, UPID_SCHEMA,
+ VERIFICATION_OUTDATED_AFTER_SCHEMA,
};
use pbs_client::pxar::{create_tar, create_zip};
use pbs_config::CachedUserInfo;
@@ -1325,8 +1326,8 @@ pub fn get_datastore_list(
let mut list = Vec::new();
- for (store, (_, data)) in &config.sections {
- let acl_path = &["datastore", store];
+ for (store, (_, data)) in config.sections {
+ let acl_path = &["datastore", &store];
let user_privs = user_info.lookup_privs(&auth_id, acl_path);
let allowed = (user_privs & (PRIV_DATASTORE_AUDIT | PRIV_DATASTORE_BACKUP)) != 0;
@@ -1337,15 +1338,20 @@ pub fn get_datastore_list(
}
}
+ let store_config: DataStoreConfig = serde_json::from_value(data)?;
+
+ let mount_status = match pbs_datastore::get_datastore_mount_status(&store_config) {
+ Some(true) => DataStoreMountStatus::Mounted,
+ Some(false) => DataStoreMountStatus::NotMounted,
+ None => DataStoreMountStatus::NonRemovable,
+ };
+
if allowed || allow_id {
list.push(DataStoreListItem {
store: store.clone(),
- comment: if !allowed {
- None
- } else {
- data["comment"].as_str().map(String::from)
- },
- maintenance: data["maintenance-mode"].as_str().map(String::from),
+ comment: store_config.comment.filter(|_| allowed),
+ mount_status,
+ maintenance: store_config.maintenance_mode,
});
}
}
diff --git a/src/api2/status/mod.rs b/src/api2/status/mod.rs
index 113aa9852..5efde9c3d 100644
--- a/src/api2/status/mod.rs
+++ b/src/api2/status/mod.rs
@@ -10,11 +10,12 @@ use proxmox_schema::api;
use proxmox_sortable_macro::sortable;
use pbs_api_types::{
- Authid, DataStoreStatusListItem, Operation, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP,
+ Authid, DataStoreConfig, DataStoreMountStatus, DataStoreStatusListItem, Operation,
+ PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP,
};
use pbs_config::CachedUserInfo;
-use pbs_datastore::DataStore;
+use pbs_datastore::{get_datastore_mount_status, DataStore};
use crate::server::metric_collection::rrd::extract_rrd_data;
use crate::tools::statistics::linear_regression;
@@ -51,10 +52,26 @@ pub async fn datastore_status(
for (store, (_, _)) in &config.sections {
let user_privs = user_info.lookup_privs(&auth_id, &["datastore", store]);
let allowed = (user_privs & (PRIV_DATASTORE_AUDIT | PRIV_DATASTORE_BACKUP)) != 0;
+
+ let store_config = config.lookup::<DataStoreConfig>("datastore", store)?;
+
+ let mount_status = match get_datastore_mount_status(&store_config) {
+ Some(true) => DataStoreMountStatus::Mounted,
+ Some(false) => {
+ list.push(DataStoreStatusListItem::empty(
+ store,
+ None,
+ DataStoreMountStatus::NotMounted,
+ ));
+ continue;
+ }
+ None => DataStoreMountStatus::NonRemovable,
+ };
+
if !allowed {
if let Ok(datastore) = DataStore::lookup_datastore(store, Some(Operation::Lookup)) {
if can_access_any_namespace(datastore, &auth_id, &user_info) {
- list.push(DataStoreStatusListItem::empty(store, None));
+ list.push(DataStoreStatusListItem::empty(store, None, mount_status));
}
}
continue;
@@ -63,7 +80,11 @@ pub async fn datastore_status(
let datastore = match DataStore::lookup_datastore(store, Some(Operation::Read)) {
Ok(datastore) => datastore,
Err(err) => {
- list.push(DataStoreStatusListItem::empty(store, Some(err.to_string())));
+ list.push(DataStoreStatusListItem::empty(
+ store,
+ Some(err.to_string()),
+ mount_status,
+ ));
continue;
}
};
@@ -74,6 +95,7 @@ pub async fn datastore_status(
total: Some(status.total),
used: Some(status.used),
avail: Some(status.available),
+ mount_status,
history: None,
history_start: None,
history_delta: None,
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* [pbs-devel] [PATCH proxmox-backup v14 09/26] bin: manager: add (un)mount command
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
` (7 preceding siblings ...)
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 08/25] pbs-api-types: add mount_status field to DataStoreListItem Hannes Laimer
@ 2024-11-22 14:46 ` Hannes Laimer
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 10/25] add auto-mounting for removable datastores Hannes Laimer
` (18 subsequent siblings)
27 siblings, 0 replies; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:46 UTC (permalink / raw)
To: pbs-devel
We can't just directly delegate these commands to the API endpoints
since both mounting and unmounting are done in a worker, and that one
would be killed when the parent ends. In this case that would be the CLI
process, which basically ends right after spwaning the worker.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
pbs-config/src/datastore.rs | 14 ++++
src/bin/proxmox_backup_manager/datastore.rs | 74 +++++++++++++++++++++
2 files changed, 88 insertions(+)
diff --git a/pbs-config/src/datastore.rs b/pbs-config/src/datastore.rs
index dc5bb3da9..396dcb371 100644
--- a/pbs-config/src/datastore.rs
+++ b/pbs-config/src/datastore.rs
@@ -62,6 +62,20 @@ pub fn complete_datastore_name(_arg: &str, _param: &HashMap<String, String>) ->
}
}
+pub fn complete_removable_datastore_name(
+ _arg: &str,
+ _param: &HashMap<String, String>,
+) -> Vec<String> {
+ match config() {
+ Ok((data, _digest)) => data
+ .sections
+ .into_iter()
+ .filter_map(|(name, (_, c))| c.get("backing-device").map(|_| name))
+ .collect(),
+ Err(_) => Vec::new(),
+ }
+}
+
pub fn complete_acl_path(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> {
let mut list = vec![
String::from("/"),
diff --git a/src/bin/proxmox_backup_manager/datastore.rs b/src/bin/proxmox_backup_manager/datastore.rs
index 3a349451f..32a55fb9c 100644
--- a/src/bin/proxmox_backup_manager/datastore.rs
+++ b/src/bin/proxmox_backup_manager/datastore.rs
@@ -42,6 +42,34 @@ fn list_datastores(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Valu
Ok(Value::Null)
}
+#[api(
+ protected: true,
+ input: {
+ properties: {
+ store: {
+ schema: DATASTORE_SCHEMA,
+ },
+ digest: {
+ optional: true,
+ schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
+ },
+ },
+ },
+)]
+/// Mount a removable datastore.
+async fn mount_datastore(mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
+ param["node"] = "localhost".into();
+
+ let info = &api2::admin::datastore::API_METHOD_MOUNT;
+ let result = match info.handler {
+ ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
+ _ => unreachable!(),
+ };
+
+ crate::wait_for_local_worker(result.as_str().unwrap()).await?;
+ Ok(())
+}
+
#[api(
input: {
properties: {
@@ -101,6 +129,34 @@ async fn create_datastore(mut param: Value) -> Result<Value, Error> {
Ok(Value::Null)
}
+#[api(
+ protected: true,
+ input: {
+ properties: {
+ store: {
+ schema: DATASTORE_SCHEMA,
+ },
+ digest: {
+ optional: true,
+ schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
+ },
+ },
+ },
+)]
+/// Unmount a removable datastore.
+async fn unmount_datastore(mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
+ param["node"] = "localhost".into();
+
+ let info = &api2::admin::datastore::API_METHOD_UNMOUNT;
+ let result = match info.handler {
+ ApiHandler::Async(handler) => (handler)(param, info, rpcenv).await?,
+ _ => unreachable!(),
+ };
+
+ crate::wait_for_local_worker(result.as_str().unwrap()).await?;
+ Ok(())
+}
+
#[api(
protected: true,
input: {
@@ -191,6 +247,15 @@ async fn update_datastore(name: String, mut param: Value) -> Result<(), Error> {
pub fn datastore_commands() -> CommandLineInterface {
let cmd_def = CliCommandMap::new()
.insert("list", CliCommand::new(&API_METHOD_LIST_DATASTORES))
+ .insert(
+ "mount",
+ CliCommand::new(&API_METHOD_MOUNT_DATASTORE)
+ .arg_param(&["store"])
+ .completion_cb(
+ "store",
+ pbs_config::datastore::complete_removable_datastore_name,
+ ),
+ )
.insert(
"show",
CliCommand::new(&API_METHOD_SHOW_DATASTORE)
@@ -201,6 +266,15 @@ pub fn datastore_commands() -> CommandLineInterface {
"create",
CliCommand::new(&API_METHOD_CREATE_DATASTORE).arg_param(&["name", "path"]),
)
+ .insert(
+ "unmount",
+ CliCommand::new(&API_METHOD_UNMOUNT_DATASTORE)
+ .arg_param(&["store"])
+ .completion_cb(
+ "store",
+ pbs_config::datastore::complete_removable_datastore_name,
+ ),
+ )
.insert(
"update",
CliCommand::new(&API_METHOD_UPDATE_DATASTORE)
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* [pbs-devel] [PATCH proxmox-backup v14 10/25] add auto-mounting for removable datastores
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
` (8 preceding siblings ...)
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 09/26] bin: manager: add (un)mount command Hannes Laimer
@ 2024-11-22 14:46 ` Hannes Laimer
2024-11-25 13:47 ` Fabian Grünbichler
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 11/25] datastore: handle deletion of removable datastore properly Hannes Laimer
` (17 subsequent siblings)
27 siblings, 1 reply; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:46 UTC (permalink / raw)
To: pbs-devel
If a device houses multiple datastore, none of them will be mounted
automatically. If a device only contains a single datastore it will be
mounted automatically. The reason for not mounting multiple datastore
automatically is that we don't know which is actually wanted, and since
mounting all means also all have to be unmounted manually, it made sense
to have the user choose which to mount.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
changes since v13:
* skip API alltogether and use mounting wrapper directly
* load datastore config directly
debian/proxmox-backup-server.install | 1 +
debian/proxmox-backup-server.udev | 3 ++
etc/Makefile | 1 +
etc/removable-device-attach@.service | 8 ++++
src/bin/proxmox_backup_manager/datastore.rs | 53 ++++++++++++++++++++-
5 files changed, 65 insertions(+), 1 deletion(-)
create mode 100644 etc/removable-device-attach@.service
diff --git a/debian/proxmox-backup-server.install b/debian/proxmox-backup-server.install
index 79757eadb..ff581e3dd 100644
--- a/debian/proxmox-backup-server.install
+++ b/debian/proxmox-backup-server.install
@@ -4,6 +4,7 @@ etc/proxmox-backup-daily-update.service /lib/systemd/system/
etc/proxmox-backup-daily-update.timer /lib/systemd/system/
etc/proxmox-backup-proxy.service /lib/systemd/system/
etc/proxmox-backup.service /lib/systemd/system/
+etc/removable-device-attach@.service /lib/systemd/system/
usr/bin/pmt
usr/bin/pmtx
usr/bin/proxmox-tape
diff --git a/debian/proxmox-backup-server.udev b/debian/proxmox-backup-server.udev
index afdfb2bc7..e21b8bc71 100644
--- a/debian/proxmox-backup-server.udev
+++ b/debian/proxmox-backup-server.udev
@@ -16,3 +16,6 @@ SUBSYSTEM=="scsi_generic", SUBSYSTEMS=="scsi", ATTRS{type}=="1", ENV{ID_SCSI_SER
SYMLINK+="tape/by-id/scsi-$env{ID_SCSI_SERIAL}-sg"
LABEL="persistent_storage_tape_end"
+
+# triggers the mounting of a removable device
+ACTION=="add", SUBSYSTEM=="block", ENV{ID_FS_UUID}!="", TAG+="systemd", ENV{SYSTEMD_WANTS}="removable-device-attach@$env{ID_FS_UUID}"
\ No newline at end of file
diff --git a/etc/Makefile b/etc/Makefile
index 42f639f62..26e91684e 100644
--- a/etc/Makefile
+++ b/etc/Makefile
@@ -2,6 +2,7 @@ include ../defines.mk
UNITS := \
proxmox-backup-daily-update.timer \
+ removable-device-attach@.service
DYNAMIC_UNITS := \
proxmox-backup-banner.service \
diff --git a/etc/removable-device-attach@.service b/etc/removable-device-attach@.service
new file mode 100644
index 000000000..e10d1ea3c
--- /dev/null
+++ b/etc/removable-device-attach@.service
@@ -0,0 +1,8 @@
+[Unit]
+Description=Try to mount the removable device of a datastore with uuid '%i'.
+After=proxmox-backup-proxy.service
+Requires=proxmox-backup-proxy.service
+
+[Service]
+Type=simple
+ExecStart=/usr/sbin/proxmox-backup-manager datastore uuid-mount %i
diff --git a/src/bin/proxmox_backup_manager/datastore.rs b/src/bin/proxmox_backup_manager/datastore.rs
index 32a55fb9c..bcfdae786 100644
--- a/src/bin/proxmox_backup_manager/datastore.rs
+++ b/src/bin/proxmox_backup_manager/datastore.rs
@@ -9,7 +9,7 @@ use proxmox_backup::api2;
use proxmox_backup::api2::config::datastore::DeletableProperty;
use proxmox_backup::client_helpers::connect_to_localhost;
-use anyhow::Error;
+use anyhow::{format_err, Error};
use serde_json::Value;
#[api(
@@ -244,6 +244,53 @@ async fn update_datastore(name: String, mut param: Value) -> Result<(), Error> {
Ok(())
}
+#[api(
+ protected: true,
+ input: {
+ properties: {
+ uuid: {
+ type: String,
+ description: "The UUID of the device that should be mounted",
+ },
+ "output-format": {
+ schema: OUTPUT_FORMAT,
+ optional: true,
+ },
+ },
+ },
+)]
+/// Try mounting a removable datastore given the UUID.
+async fn uuid_mount(param: Value, _rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
+ let uuid = param["uuid"]
+ .as_str()
+ .ok_or_else(|| format_err!("uuid has to be specified"))?;
+
+ let (config, _digest) = pbs_config::datastore::config()?;
+ let list: Vec<DataStoreConfig> = config.convert_to_typed_array("datastore")?;
+ let matching_stores: Vec<DataStoreConfig> = list
+ .into_iter()
+ .filter(|store: &DataStoreConfig| {
+ store
+ .backing_device
+ .clone()
+ .map_or(false, |device| device.eq(&uuid))
+ })
+ .collect();
+
+ if matching_stores.len() != 1 {
+ return Ok(Value::Null);
+ }
+
+ if let Some(store) = matching_stores.get(0) {
+ api2::admin::datastore::do_mount_device(store.clone())?;
+ return Ok(Value::Null);
+ }
+
+ // we don't want to fail for UUIDs that are not associated with datastores, as that produces
+ // quite some noise in the logs, given this is check for every device that is plugged in.
+ Ok(Value::Null)
+}
+
pub fn datastore_commands() -> CommandLineInterface {
let cmd_def = CliCommandMap::new()
.insert("list", CliCommand::new(&API_METHOD_LIST_DATASTORES))
@@ -289,6 +336,10 @@ pub fn datastore_commands() -> CommandLineInterface {
pbs_config::datastore::complete_calendar_event,
),
)
+ .insert(
+ "uuid-mount",
+ CliCommand::new(&API_METHOD_UUID_MOUNT).arg_param(&["uuid"]),
+ )
.insert(
"remove",
CliCommand::new(&API_METHOD_DELETE_DATASTORE)
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v14 10/25] add auto-mounting for removable datastores
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 10/25] add auto-mounting for removable datastores Hannes Laimer
@ 2024-11-25 13:47 ` Fabian Grünbichler
0 siblings, 0 replies; 45+ messages in thread
From: Fabian Grünbichler @ 2024-11-25 13:47 UTC (permalink / raw)
To: Proxmox Backup Server development discussion
On November 22, 2024 3:46 pm, Hannes Laimer wrote:
> If a device houses multiple datastore, none of them will be mounted
> automatically. If a device only contains a single datastore it will be
> mounted automatically. The reason for not mounting multiple datastore
> automatically is that we don't know which is actually wanted, and since
> mounting all means also all have to be unmounted manually, it made sense
> to have the user choose which to mount.
>
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> changes since v13:
> * skip API alltogether and use mounting wrapper directly
> * load datastore config directly
>
> debian/proxmox-backup-server.install | 1 +
> debian/proxmox-backup-server.udev | 3 ++
> etc/Makefile | 1 +
> etc/removable-device-attach@.service | 8 ++++
> src/bin/proxmox_backup_manager/datastore.rs | 53 ++++++++++++++++++++-
> 5 files changed, 65 insertions(+), 1 deletion(-)
> create mode 100644 etc/removable-device-attach@.service
>
> diff --git a/debian/proxmox-backup-server.install b/debian/proxmox-backup-server.install
> index 79757eadb..ff581e3dd 100644
> --- a/debian/proxmox-backup-server.install
> +++ b/debian/proxmox-backup-server.install
> @@ -4,6 +4,7 @@ etc/proxmox-backup-daily-update.service /lib/systemd/system/
> etc/proxmox-backup-daily-update.timer /lib/systemd/system/
> etc/proxmox-backup-proxy.service /lib/systemd/system/
> etc/proxmox-backup.service /lib/systemd/system/
> +etc/removable-device-attach@.service /lib/systemd/system/
> usr/bin/pmt
> usr/bin/pmtx
> usr/bin/proxmox-tape
> diff --git a/debian/proxmox-backup-server.udev b/debian/proxmox-backup-server.udev
> index afdfb2bc7..e21b8bc71 100644
> --- a/debian/proxmox-backup-server.udev
> +++ b/debian/proxmox-backup-server.udev
> @@ -16,3 +16,6 @@ SUBSYSTEM=="scsi_generic", SUBSYSTEMS=="scsi", ATTRS{type}=="1", ENV{ID_SCSI_SER
> SYMLINK+="tape/by-id/scsi-$env{ID_SCSI_SERIAL}-sg"
>
> LABEL="persistent_storage_tape_end"
> +
> +# triggers the mounting of a removable device
> +ACTION=="add", SUBSYSTEM=="block", ENV{ID_FS_UUID}!="", TAG+="systemd", ENV{SYSTEMD_WANTS}="removable-device-attach@$env{ID_FS_UUID}"
> \ No newline at end of file
> diff --git a/etc/Makefile b/etc/Makefile
> index 42f639f62..26e91684e 100644
> --- a/etc/Makefile
> +++ b/etc/Makefile
> @@ -2,6 +2,7 @@ include ../defines.mk
>
> UNITS := \
> proxmox-backup-daily-update.timer \
> + removable-device-attach@.service
>
> DYNAMIC_UNITS := \
> proxmox-backup-banner.service \
> diff --git a/etc/removable-device-attach@.service b/etc/removable-device-attach@.service
> new file mode 100644
> index 000000000..e10d1ea3c
> --- /dev/null
> +++ b/etc/removable-device-attach@.service
> @@ -0,0 +1,8 @@
> +[Unit]
> +Description=Try to mount the removable device of a datastore with uuid '%i'.
> +After=proxmox-backup-proxy.service
> +Requires=proxmox-backup-proxy.service
> +
> +[Service]
> +Type=simple
> +ExecStart=/usr/sbin/proxmox-backup-manager datastore uuid-mount %i
> diff --git a/src/bin/proxmox_backup_manager/datastore.rs b/src/bin/proxmox_backup_manager/datastore.rs
> index 32a55fb9c..bcfdae786 100644
> --- a/src/bin/proxmox_backup_manager/datastore.rs
> +++ b/src/bin/proxmox_backup_manager/datastore.rs
> @@ -9,7 +9,7 @@ use proxmox_backup::api2;
> use proxmox_backup::api2::config::datastore::DeletableProperty;
> use proxmox_backup::client_helpers::connect_to_localhost;
>
> -use anyhow::Error;
> +use anyhow::{format_err, Error};
> use serde_json::Value;
>
> #[api(
> @@ -244,6 +244,53 @@ async fn update_datastore(name: String, mut param: Value) -> Result<(), Error> {
> Ok(())
> }
>
> +#[api(
> + protected: true,
> + input: {
> + properties: {
> + uuid: {
> + type: String,
> + description: "The UUID of the device that should be mounted",
> + },
> + "output-format": {
> + schema: OUTPUT_FORMAT,
> + optional: true,
> + },
> + },
> + },
> +)]
> +/// Try mounting a removable datastore given the UUID.
> +async fn uuid_mount(param: Value, _rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
> + let uuid = param["uuid"]
> + .as_str()
> + .ok_or_else(|| format_err!("uuid has to be specified"))?;
> +
> + let (config, _digest) = pbs_config::datastore::config()?;
> + let list: Vec<DataStoreConfig> = config.convert_to_typed_array("datastore")?;
> + let matching_stores: Vec<DataStoreConfig> = list
> + .into_iter()
> + .filter(|store: &DataStoreConfig| {
> + store
> + .backing_device
> + .clone()
> + .map_or(false, |device| device.eq(&uuid))
> + })
> + .collect();
> +
> + if matching_stores.len() != 1 {
> + return Ok(Value::Null);
> + }
> +
> + if let Some(store) = matching_stores.get(0) {
> + api2::admin::datastore::do_mount_device(store.clone())?;
> + return Ok(Value::Null);
this return is redundant ;)
> + }
> +
> + // we don't want to fail for UUIDs that are not associated with datastores, as that produces
> + // quite some noise in the logs, given this is check for every device that is plugged in.
> + Ok(Value::Null)
> +}
> +
> pub fn datastore_commands() -> CommandLineInterface {
> let cmd_def = CliCommandMap::new()
> .insert("list", CliCommand::new(&API_METHOD_LIST_DATASTORES))
> @@ -289,6 +336,10 @@ pub fn datastore_commands() -> CommandLineInterface {
> pbs_config::datastore::complete_calendar_event,
> ),
> )
> + .insert(
> + "uuid-mount",
> + CliCommand::new(&API_METHOD_UUID_MOUNT).arg_param(&["uuid"]),
> + )
> .insert(
> "remove",
> CliCommand::new(&API_METHOD_DELETE_DATASTORE)
> --
> 2.39.5
>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
>
>
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v14 11/25] datastore: handle deletion of removable datastore properly
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
` (9 preceding siblings ...)
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 10/25] add auto-mounting for removable datastores Hannes Laimer
@ 2024-11-22 14:46 ` Hannes Laimer
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 12/25] docs: add removable datastores section Hannes Laimer
` (16 subsequent siblings)
27 siblings, 0 replies; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:46 UTC (permalink / raw)
To: pbs-devel
Data deletion is only possible if the datastore is mounted, won't attempt
mounting it for the purpose of deleting data.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
changes since v13:
* log warn! on errors in cleanup
* also unmount without destroy_data
pbs-datastore/src/datastore.rs | 4 +++-
src/api2/config/datastore.rs | 39 +++++++++++++++++++++++++++++++++-
2 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 6a9fc2dc0..adf29f183 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -1535,7 +1535,9 @@ impl DataStore {
// weird, but ok
}
Err(err) if err.is_errno(nix::errno::Errno::EBUSY) => {
- warn!("Cannot delete datastore directory (is it a mount point?).")
+ if datastore_config.backing_device.is_none() {
+ warn!("Cannot delete datastore directory (is it a mount point?).")
+ }
}
Err(err) if err.is_errno(nix::errno::Errno::ENOTEMPTY) => {
warn!("Datastore directory not empty, not deleting.")
diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 75e1a1a56..5c2fd2573 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -1,4 +1,4 @@
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
use ::serde::{Deserialize, Serialize};
use anyhow::{bail, format_err, Error};
@@ -30,6 +30,7 @@ use crate::api2::config::tape_backup_job::{delete_tape_backup_job, list_tape_bac
use crate::api2::config::verify::delete_verification_job;
use pbs_config::CachedUserInfo;
+use pbs_datastore::get_datastore_mount_status;
use proxmox_rest_server::WorkerTask;
use crate::server::jobstate;
@@ -561,6 +562,15 @@ pub async fn delete_datastore(
http_bail!(NOT_FOUND, "datastore '{}' does not exist.", name);
}
+ let store_config: DataStoreConfig = config.lookup("datastore", &name)?;
+
+ if destroy_data && get_datastore_mount_status(&store_config) == Some(false) {
+ http_bail!(
+ BAD_REQUEST,
+ "cannot destroy data on '{name}' unless the datastore is mounted"
+ );
+ }
+
if !keep_job_configs {
for job in list_verification_jobs(Some(name.clone()), Value::Null, rpcenv)? {
delete_verification_job(job.config.id, None, rpcenv)?
@@ -591,6 +601,18 @@ pub async fn delete_datastore(
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
+ if let Ok(proxy_pid) = proxmox_rest_server::read_pid(pbs_buildcfg::PROXMOX_BACKUP_PROXY_PID_FN)
+ {
+ let sock = proxmox_daemon::command_socket::path_from_pid(proxy_pid);
+ let _ = proxmox_daemon::command_socket::send_raw(
+ sock,
+ &format!(
+ "{{\"command\":\"update-datastore-cache\",\"args\":\"{}\"}}\n",
+ name.clone()
+ ),
+ )
+ .await;
+ };
let upid = WorkerTask::new_thread(
"delete-datastore",
@@ -610,6 +632,21 @@ pub async fn delete_datastore(
warn!("failed to notify after datastore removal: {err}");
}
+ // cleanup for removable datastores
+ // - unmount
+ // - remove mount dir, if destroy_data
+ if store_config.backing_device.is_some() {
+ let mount_point = store_config.absolute_path();
+ if get_datastore_mount_status(&store_config) == Some(true) {
+ let _ = unmount_by_mountpoint(Path::new(&mount_point))
+ .inspect_err(|e| warn!("could not unmount device after deletion: {e}"));
+ }
+ if destroy_data {
+ let _ = std::fs::remove_dir(&mount_point)
+ .inspect_err(|e| warn!("could not remove directory after deletion: {e}"));
+ }
+ }
+
Ok(())
},
)?;
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* [pbs-devel] [PATCH proxmox-backup v14 12/25] docs: add removable datastores section
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
` (10 preceding siblings ...)
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 11/25] datastore: handle deletion of removable datastore properly Hannes Laimer
@ 2024-11-22 14:46 ` Hannes Laimer
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 13/26] ui: add partition selector form Hannes Laimer
` (15 subsequent siblings)
27 siblings, 0 replies; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:46 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
changes since v13:
* fix typo
docs/storage.rst | 38 ++++++++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/docs/storage.rst b/docs/storage.rst
index f1e15d522..361af4420 100644
--- a/docs/storage.rst
+++ b/docs/storage.rst
@@ -165,6 +165,44 @@ following command creates a new datastore called ``store1`` on
# proxmox-backup-manager datastore create store1 /backup/disk1/store1
+Removable Datastores
+^^^^^^^^^^^^^^^^^^^^
+Removable datastores have a ``backing-device`` associated with them, they can be
+mounted and unmounted. Other than that they behave the same way a normal datastore
+would.
+
+They can be created on already correctly formatted partitions, which, as with normal
+datastores, should be either ``ext4`` or ``xfs``. It is also possible to create them
+on completely unused disks through "Administration" > "Disks / Storage" > "Directory",
+using this method the disk will be partitioned and formatted automatically for the datastore.
+
+Devices with only one datastore on them will be mounted automatically. It is possible to create a
+removable datastore on one PBS and use it on multiple instances, the device just has to be added
+on each instance as a removable datastore by checking "reuse datastore" on creation.
+If the device already contains a datastore at the specified path it'll just be added as
+a new datastore to the PBS instance and will be mounted whenever plugged in. Unmounting has
+to be done through the UI by clicking "Unmount" on the summary page or using the CLI.
+
+A single device can house multiple datastores, they only limitation is that they are not
+allowed to be nested.
+
+.. code-block:: console
+
+ # proxmox-backup-manager datastore unmount store1
+
+both will wait for any running tasks to finish and unmount the device.
+
+All removable datastores are mounted under /mnt/datastore/<name>, and the specified path
+refers to the path on the device.
+
+All datastores present on a device can be listed using ``proxmox-backup-debug``.
+
+.. code-block:: console
+
+ # proxmox-backup-debug inspect device /dev/...
+
+
+
Managing Datastores
^^^^^^^^^^^^^^^^^^^
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* [pbs-devel] [PATCH proxmox-backup v14 13/26] ui: add partition selector form
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
` (11 preceding siblings ...)
2024-11-22 14:46 ` [pbs-devel] [PATCH proxmox-backup v14 12/25] docs: add removable datastores section Hannes Laimer
@ 2024-11-22 14:47 ` Hannes Laimer
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 14/26] ui: add removable datastore creation support Hannes Laimer
` (14 subsequent siblings)
27 siblings, 0 replies; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:47 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
www/Makefile | 1 +
www/form/PartitionSelector.js | 81 +++++++++++++++++++++++++++++++++++
2 files changed, 82 insertions(+)
create mode 100644 www/form/PartitionSelector.js
diff --git a/www/Makefile b/www/Makefile
index d35e81283..3defe3428 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -49,6 +49,7 @@ JSSRC= \
form/NamespaceMaxDepth.js \
form/CalendarEvent.js \
form/PermissionPathSelector.js \
+ form/PartitionSelector.js \
form/GroupSelector.js \
form/GroupFilter.js \
form/VerifyOutdatedAfter.js \
diff --git a/www/form/PartitionSelector.js b/www/form/PartitionSelector.js
new file mode 100644
index 000000000..162dbe418
--- /dev/null
+++ b/www/form/PartitionSelector.js
@@ -0,0 +1,81 @@
+Ext.define('pbs-partition-list', {
+ extend: 'Ext.data.Model',
+ fields: ['name', 'uuid', 'filesystem', 'devpath', 'size', 'model'],
+ proxy: {
+ type: 'proxmox',
+ url: "/api2/json/nodes/localhost/disks/list?skipsmart=1&include-partitions=1",
+ reader: {
+ transform: (rawData) => rawData.data
+ .flatMap(disk => (disk.partitions
+ .map(part => ({ ...part, model: disk.model })) ?? [])
+ .filter(partition => partition.used === 'filesystem')),
+ },
+ },
+ idProperty: 'devpath',
+
+});
+
+Ext.define('PBS.form.PartitionSelector', {
+ extend: 'Proxmox.form.ComboGrid',
+ alias: 'widget.pbsPartitionSelector',
+
+ allowBlank: false,
+ autoSelect: false,
+ submitEmpty: false,
+ valueField: 'uuid',
+ displayField: 'devpath',
+
+ store: {
+ model: 'pbs-partition-list',
+ autoLoad: true,
+ sorters: 'devpath',
+ },
+ getSubmitData: function() {
+ let me = this;
+ let data = null;
+ if (!me.disabled && me.submitValue && !me.isFileUpload()) {
+ let val = me.getSubmitValue();
+ if (val !== undefined && val !== null && val !== '') {
+ data = {};
+ data[me.getName()] = val;
+ } else if (me.getDeleteEmpty()) {
+ data = {};
+ data.delete = me.getName();
+ }
+ }
+ return data;
+ },
+ listConfig: {
+ columns: [
+ {
+ header: gettext('Path'),
+ sortable: true,
+ dataIndex: 'devpath',
+ renderer: (v, metaData, rec) => Ext.String.htmlEncode(v),
+ flex: 1,
+ },
+ {
+ header: gettext('Filesystem'),
+ sortable: true,
+ dataIndex: 'filesystem',
+ flex: 1,
+ },
+ {
+ header: gettext('Size'),
+ sortable: true,
+ dataIndex: 'size',
+ renderer: Proxmox.Utils.format_size,
+ flex: 1,
+ },
+ {
+ header: gettext('Model'),
+ sortable: true,
+ dataIndex: 'model',
+ flex: 1,
+ },
+ ],
+ viewConfig: {
+ emptyText: 'No usable partitions present',
+ },
+ },
+});
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* [pbs-devel] [PATCH proxmox-backup v14 14/26] ui: add removable datastore creation support
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
` (12 preceding siblings ...)
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 13/26] ui: add partition selector form Hannes Laimer
@ 2024-11-22 14:47 ` Hannes Laimer
2024-11-25 12:44 ` Dominik Csapak
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 15/26] ui: add (un)mount button to summary Hannes Laimer
` (13 subsequent siblings)
27 siblings, 1 reply; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:47 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
www/window/DataStoreEdit.js | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/www/window/DataStoreEdit.js b/www/window/DataStoreEdit.js
index b8e866df2..7b6aff1e7 100644
--- a/www/window/DataStoreEdit.js
+++ b/www/window/DataStoreEdit.js
@@ -63,6 +63,20 @@ Ext.define('PBS.DataStoreEdit', {
emptyText: gettext('An absolute path'),
validator: val => val?.trim() !== '/',
},
+ {
+ xtype: 'pmxDisplayEditField',
+ fieldLabel: gettext('Device'),
+ name: 'backing-device',
+ disabled: true,
+ cbind: {
+ editable: '{isCreate}',
+ },
+ editConfig: {
+ xtype: 'pbsPartitionSelector',
+ allowBlank: true,
+ },
+ emptyText: gettext('Device path'),
+ },
],
column2: [
{
@@ -88,6 +102,29 @@ Ext.define('PBS.DataStoreEdit', {
},
],
columnB: [
+ {
+ xtype: 'checkbox',
+ boxLabel: gettext('Removable datastore'),
+ submitValue: false,
+ listeners: {
+ change: function(checkbox, isRemovable) {
+ let inputPanel = checkbox.up('inputpanel');
+ let pathField = inputPanel.down('[name=path]');
+ let uuidField = inputPanel.down('pbsPartitionSelector[name=backing-device]');
+ let uuidEditField = inputPanel.down('[name=backing-device]');
+
+ uuidField.allowBlank = !isRemovable;
+ uuidEditField.setDisabled(!isRemovable);
+ uuidField.setDisabled(!isRemovable);
+ uuidField.setValue('');
+ if (isRemovable) {
+ pathField.setFieldLabel(gettext('On device path'));
+ } else {
+ pathField.setFieldLabel(gettext('Backing Path'));
+ }
+ },
+ },
+ },
{
xtype: 'textfield',
name: 'comment',
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v14 14/26] ui: add removable datastore creation support
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 14/26] ui: add removable datastore creation support Hannes Laimer
@ 2024-11-25 12:44 ` Dominik Csapak
0 siblings, 0 replies; 45+ messages in thread
From: Dominik Csapak @ 2024-11-25 12:44 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
comments inline:
On 11/22/24 15:47, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> www/window/DataStoreEdit.js | 37 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
>
> diff --git a/www/window/DataStoreEdit.js b/www/window/DataStoreEdit.js
> index b8e866df2..7b6aff1e7 100644
> --- a/www/window/DataStoreEdit.js
> +++ b/www/window/DataStoreEdit.js
> @@ -63,6 +63,20 @@ Ext.define('PBS.DataStoreEdit', {
> emptyText: gettext('An absolute path'),
> validator: val => val?.trim() !== '/',
> },
> + {
> + xtype: 'pmxDisplayEditField',
> + fieldLabel: gettext('Device'),
> + name: 'backing-device',
> + disabled: true,
> + cbind: {
> + editable: '{isCreate}',
> + },
> + editConfig: {
> + xtype: 'pbsPartitionSelector',
> + allowBlank: true,
> + },
> + emptyText: gettext('Device path'),
> + },
it's a bit tricky to see from the code, but this editwindow is never
actually called in an editable context, so the displayedit field here
would actually not be necessary, because if you omit this
> ],
> column2: [
> {
> @@ -88,6 +102,29 @@ Ext.define('PBS.DataStoreEdit', {
> },
> ],
> columnB: [
> + {
> + xtype: 'checkbox',
> + boxLabel: gettext('Removable datastore'),
> + submitValue: false,
> + listeners: {
> + change: function(checkbox, isRemovable) {
> + let inputPanel = checkbox.up('inputpanel');
> + let pathField = inputPanel.down('[name=path]');
> + let uuidField = inputPanel.down('pbsPartitionSelector[name=backing-device]');
> + let uuidEditField = inputPanel.down('[name=backing-device]');
this 'double' field modifying would also not be necessary.
even if we leave the displayedit field, this code here does not make much sense,
since the checkbox is always visible/editable, but the device itself is not?
either this is editable on an existing datastore (then the device should also
be editable afterwards), or this is not changeable, then the checkbox
must also vanish on edit
in any case, the checkbox change can/should only happen when the device is editable
so we can omit trying to modifying the display field here?
> +
> + uuidField.allowBlank = !isRemovable;
> + uuidEditField.setDisabled(!isRemovable);
> + uuidField.setDisabled(!isRemovable);
> + uuidField.setValue('');
> + if (isRemovable) {
> + pathField.setFieldLabel(gettext('On device path'));
> + } else {
> + pathField.setFieldLabel(gettext('Backing Path'));
> + }
> + },
> + },
> + },
> {
> xtype: 'textfield',
> name: 'comment',
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v14 15/26] ui: add (un)mount button to summary
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
` (13 preceding siblings ...)
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 14/26] ui: add removable datastore creation support Hannes Laimer
@ 2024-11-22 14:47 ` Hannes Laimer
2024-11-25 13:00 ` Dominik Csapak
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 16/26] ui: tree: render unmounted datastores correctly Hannes Laimer
` (12 subsequent siblings)
27 siblings, 1 reply; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:47 UTC (permalink / raw)
To: pbs-devel
And only try to load datastore information if the datastore is
available.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
changes since v13:
* stop statusStore update on first failed request, start again on mount
www/datastore/Summary.js | 94 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 92 insertions(+), 2 deletions(-)
diff --git a/www/datastore/Summary.js b/www/datastore/Summary.js
index a932b4e01..2d79a7951 100644
--- a/www/datastore/Summary.js
+++ b/www/datastore/Summary.js
@@ -309,7 +309,84 @@ Ext.define('PBS.DataStoreSummary', {
model: 'pve-rrd-datastore',
});
- me.callParent();
+ me.statusStore = Ext.create('Proxmox.data.ObjectStore', {
+ url: `/api2/json/admin/datastore/${me.datastore}/status`,
+ interval: 1000,
+ });
+
+ let unmountBtn = Ext.create('Ext.Button', {
+ text: gettext('Unmount'),
+ hidden: true,
+ handler: () => {
+ Proxmox.Utils.API2Request({
+ url: `/admin/datastore/${me.datastore}/unmount`,
+ method: 'POST',
+ failure: function(response) {
+ Ext.Msg.alert(gettext('Error'), response.htmlStatus);
+ },
+ success: function(response, options) {
+ Ext.create('Proxmox.window.TaskViewer', {
+ upid: response.result.data,
+ }).show();
+ },
+ });
+ },
+ });
+
+ let mountBtn = Ext.create('Ext.Button', {
+ text: gettext('Mount'),
+ hidden: true,
+ handler: () => {
+ Proxmox.Utils.API2Request({
+ url: `/admin/datastore/${me.datastore}/mount`,
+ method: 'POST',
+ failure: function(response) {
+ Ext.Msg.alert(gettext('Error'), response.htmlStatus);
+ },
+ success: function(response, options) {
+ me.statusStore.startUpdate();
+ Ext.create('Proxmox.window.TaskViewer', {
+ upid: response.result.data,
+ }).show();
+ },
+ });
+ },
+ });
+
+ Ext.apply(me, {
+ tbar: [unmountBtn, mountBtn, '->', { xtype: 'proxmoxRRDTypeSelector' }],
+ });
+
+ me.mon(me.statusStore, 'load', (s, records, success) => {
+ if (!success) {
+ me.statusStore.stopUpdate();
+ me.down('pbsDataStoreInfo').fireEvent('deactivate');
+ Proxmox.Utils.API2Request({
+ url: `/config/datastore/${me.datastore}`,
+ success: response => {
+ let mode = response.result.data['maintenance-mode'];
+ let [type, _message] = PBS.Utils.parseMaintenanceMode(mode);
+ if (!response.result.data['backing-device']) {
+ return;
+ }
+ if (!type || type === 'read-only') {
+ unmountBtn.setDisabled(true);
+ mountBtn.setDisabled(false);
+ } else if (type === 'unmount') {
+ unmountBtn.setDisabled(true);
+ mountBtn.setDisabled(true);
+ } else {
+ unmountBtn.setDisabled(false);
+ mountBtn.setDisabled(false);
+ }
+ },
+ });
+ } else {
+ me.down('pbsDataStoreInfo').fireEvent('activate');
+ unmountBtn.setDisabled(false);
+ mountBtn.setDisabled(true);
+ }
+ });
let sp = Ext.state.Manager.getProvider();
me.mon(sp, 'statechange', function(provider, key, value) {
@@ -322,11 +399,17 @@ Ext.define('PBS.DataStoreSummary', {
Proxmox.Utils.updateColumns(me);
});
+ me.callParent();
+
Proxmox.Utils.API2Request({
url: `/config/datastore/${me.datastore}`,
waitMsgTarget: me.down('pbsDataStoreInfo'),
success: function(response) {
- let path = Ext.htmlEncode(response.result.data.path);
+ let data = response.result.data;
+ let path = Ext.htmlEncode(data.path);
+ const removable = Object.prototype.hasOwnProperty.call(data, "backing-device");
+ unmountBtn.setHidden(!removable);
+ mountBtn.setHidden(!removable);
me.down('pbsDataStoreInfo').setTitle(`${me.datastore} (${path})`);
me.down('pbsDataStoreNotes').setNotes(response.result.data.comment);
},
@@ -344,6 +427,13 @@ Ext.define('PBS.DataStoreSummary', {
let hasIoTicks = records?.some((rec) => rec?.data?.io_ticks !== undefined);
me.down('#ioDelayChart').setVisible(!success || hasIoTicks);
}, undefined, { single: true });
+ me.on('afterrender', () => {
+ me.statusStore.startUpdate();
+ });
+
+ me.on('destroy', () => {
+ me.statusStore.stopUpdate();
+ });
me.query('proxmoxRRDChart').forEach((chart) => {
chart.setStore(me.rrdstore);
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v14 15/26] ui: add (un)mount button to summary
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 15/26] ui: add (un)mount button to summary Hannes Laimer
@ 2024-11-25 13:00 ` Dominik Csapak
0 siblings, 0 replies; 45+ messages in thread
From: Dominik Csapak @ 2024-11-25 13:00 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
this patch removes the 'connection summary' from the overview, because...
On 11/22/24 15:47, Hannes Laimer wrote:
> And only try to load datastore information if the datastore is
> available.
>
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> changes since v13:
> * stop statusStore update on first failed request, start again on mount
>
> www/datastore/Summary.js | 94 +++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 92 insertions(+), 2 deletions(-)
>
> diff --git a/www/datastore/Summary.js b/www/datastore/Summary.js
> index a932b4e01..2d79a7951 100644
> --- a/www/datastore/Summary.js
> +++ b/www/datastore/Summary.js
> @@ -309,7 +309,84 @@ Ext.define('PBS.DataStoreSummary', {
> model: 'pve-rrd-datastore',
> });
>
> - me.callParent();
> + me.statusStore = Ext.create('Proxmox.data.ObjectStore', {
> + url: `/api2/json/admin/datastore/${me.datastore}/status`,
> + interval: 1000,
> + });
> +
> + let unmountBtn = Ext.create('Ext.Button', {
> + text: gettext('Unmount'),
> + hidden: true,
> + handler: () => {
> + Proxmox.Utils.API2Request({
> + url: `/admin/datastore/${me.datastore}/unmount`,
> + method: 'POST',
> + failure: function(response) {
> + Ext.Msg.alert(gettext('Error'), response.htmlStatus);
> + },
> + success: function(response, options) {
> + Ext.create('Proxmox.window.TaskViewer', {
> + upid: response.result.data,
> + }).show();
> + },
> + });
> + },
> + });
> +
> + let mountBtn = Ext.create('Ext.Button', {
> + text: gettext('Mount'),
> + hidden: true,
> + handler: () => {
> + Proxmox.Utils.API2Request({
> + url: `/admin/datastore/${me.datastore}/mount`,
> + method: 'POST',
> + failure: function(response) {
> + Ext.Msg.alert(gettext('Error'), response.htmlStatus);
> + },
> + success: function(response, options) {
> + me.statusStore.startUpdate();
> + Ext.create('Proxmox.window.TaskViewer', {
> + upid: response.result.data,
> + }).show();
> + },
> + });
> + },
> + });
> +
> + Ext.apply(me, {
> + tbar: [unmountBtn, mountBtn, '->', { xtype: 'proxmoxRRDTypeSelector' }],
> + });
you define a few buttons and overwrite the 'tbar' config, here, but forgot to add
the 'connection summary' again.
please don't do it this way, but rather put the buttons to the remaining tbar
config above. since you don't actually need any info from the initComponent
(AFAICS) this should not be a problem
you should be able to add a 'referenceHolder' property on the view to be able
to use 'reference' and 'lookup' to get to the components in the listener below
If it's really not possible to get to the tbar, please remove the original 'tbar'
definition at least, since it'll not be used anyway.
> +
> + me.mon(me.statusStore, 'load', (s, records, success) => {
> + if (!success) {
> + me.statusStore.stopUpdate();
> + me.down('pbsDataStoreInfo').fireEvent('deactivate');
> + Proxmox.Utils.API2Request({
> + url: `/config/datastore/${me.datastore}`,
> + success: response => {
> + let mode = response.result.data['maintenance-mode'];
> + let [type, _message] = PBS.Utils.parseMaintenanceMode(mode);
> + if (!response.result.data['backing-device']) {
> + return;
> + }
> + if (!type || type === 'read-only') {
> + unmountBtn.setDisabled(true);
> + mountBtn.setDisabled(false);
> + } else if (type === 'unmount') {
> + unmountBtn.setDisabled(true);
> + mountBtn.setDisabled(true);
> + } else {
> + unmountBtn.setDisabled(false);
> + mountBtn.setDisabled(false);
> + }
> + },
> + });
> + } else {
> + me.down('pbsDataStoreInfo').fireEvent('activate');
> + unmountBtn.setDisabled(false);
> + mountBtn.setDisabled(true);
> + }
> + });
i'm not completely sure about that, but i think one should call
'me.mon' only after the callParents call. I may be mistaken though.
(you should be able to move the code below that, since it'll trigger only
after the initcomponent anyway)
>
> let sp = Ext.state.Manager.getProvider();
> me.mon(sp, 'statechange', function(provider, key, value) {
> @@ -322,11 +399,17 @@ Ext.define('PBS.DataStoreSummary', {
> Proxmox.Utils.updateColumns(me);
> });
>
> + me.callParent();
> +
> Proxmox.Utils.API2Request({
> url: `/config/datastore/${me.datastore}`,
> waitMsgTarget: me.down('pbsDataStoreInfo'),
> success: function(response) {
> - let path = Ext.htmlEncode(response.result.data.path);
> + let data = response.result.data;
> + let path = Ext.htmlEncode(data.path);
> + const removable = Object.prototype.hasOwnProperty.call(data, "backing-device");
i mean it works, but our usual way to code that would be something like:
let removable = !!data['backing-device'];
is there a special reason for calling the hasOwnProperty method?
> + unmountBtn.setHidden(!removable);
> + mountBtn.setHidden(!removable);
> me.down('pbsDataStoreInfo').setTitle(`${me.datastore} (${path})`);
> me.down('pbsDataStoreNotes').setNotes(response.result.data.comment);
> },
> @@ -344,6 +427,13 @@ Ext.define('PBS.DataStoreSummary', {
> let hasIoTicks = records?.some((rec) => rec?.data?.io_ticks !== undefined);
> me.down('#ioDelayChart').setVisible(!success || hasIoTicks);
> }, undefined, { single: true });
> + me.on('afterrender', () => {
> + me.statusStore.startUpdate();
> + });
> +
> + me.on('destroy', () => {
> + me.statusStore.stopUpdate();
> + });
any special reason to put this here instead of the activate/deactivate/destroy handlers above, were
we also handle the rrdstore?
>
> me.query('proxmoxRRDChart').forEach((chart) => {
> chart.setStore(me.rrdstore);
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v14 16/26] ui: tree: render unmounted datastores correctly
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
` (14 preceding siblings ...)
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 15/26] ui: add (un)mount button to summary Hannes Laimer
@ 2024-11-22 14:47 ` Hannes Laimer
2024-11-25 8:47 ` [pbs-devel] [PATCH proxmox-backup v14] fixup! " Hannes Laimer
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 17/26] ui: utils: make parseMaintenanceMode more robust Hannes Laimer
` (11 subsequent siblings)
27 siblings, 1 reply; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:47 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
www/NavigationTree.js | 17 +++++++++++++----
www/css/ext6-pbs.css | 8 ++++++++
www/datastore/DataStoreListSummary.js | 1 +
3 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/www/NavigationTree.js b/www/NavigationTree.js
index 53c8daff9..dd03fbd62 100644
--- a/www/NavigationTree.js
+++ b/www/NavigationTree.js
@@ -266,14 +266,23 @@ Ext.define('PBS.view.main.NavigationTree', {
while (name.localeCompare(getChildTextAt(j)) > 0 && (j+1) < list.childNodes.length) {
j++;
}
-
- let [qtip, iconCls] = ['', 'fa fa-database'];
+ let mainIcon = `fa fa-${records[i].data.mount-status !== 'nonremovable' ? 'plug' : 'database'}`;
+ let [qtip, iconCls] = ['', mainIcon];
const maintenance = records[i].data.maintenance;
+
+ const removable_not_mounted = records[i].data['mount-status'] === 'notmounted';
+ if (removable_not_mounted) {
+ iconCls = `${mainIcon} pmx-tree-icon-custom unplugged`;
+ qtip = gettext('Removable datastore not mounted');
+ }
if (maintenance) {
const [type, message] = PBS.Utils.parseMaintenanceMode(maintenance);
qtip = `${type}${message ? ': ' + message : ''}`;
- let maintenanceTypeCls = type === 'delete' ? 'destroying' : 'maintenance';
- iconCls = `fa fa-database pmx-tree-icon-custom ${maintenanceTypeCls}`;
+ let maintenanceTypeCls = 'maintenance';
+ if (type === 'delete') {
+ maintenanceTypeCls = 'destroying';
+ }
+ iconCls = `${mainIcon} pmx-tree-icon-custom ${maintenanceTypeCls}`;
}
if (getChildTextAt(j).localeCompare(name) !== 0) {
diff --git a/www/css/ext6-pbs.css b/www/css/ext6-pbs.css
index c33ce6845..706e681e9 100644
--- a/www/css/ext6-pbs.css
+++ b/www/css/ext6-pbs.css
@@ -271,6 +271,10 @@ span.snapshot-comment-column {
content: "\ ";
}
+.x-treelist-item-icon.fa-plug, .pmx-tree-icon-custom.fa-plug {
+ font-size: 12px;
+}
+
/* datastore maintenance */
.pmx-tree-icon-custom.maintenance:after {
content: "\f0ad";
@@ -290,6 +294,10 @@ span.snapshot-comment-column {
color: #888;
}
+.pmx-tree-icon-custom.unplugged:before {
+ color: #888;
+}
+
/*' PBS specific icons */
.pbs-icon-tape {
diff --git a/www/datastore/DataStoreListSummary.js b/www/datastore/DataStoreListSummary.js
index b908034d8..f7ea83e7b 100644
--- a/www/datastore/DataStoreListSummary.js
+++ b/www/datastore/DataStoreListSummary.js
@@ -22,6 +22,7 @@ Ext.define('PBS.datastore.DataStoreListSummary', {
stillbad: 0,
deduplication: 1.0,
error: "",
+ removable: false,
maintenance: '',
},
},
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* [pbs-devel] [PATCH proxmox-backup v14] fixup! ui: tree: render unmounted datastores correctly
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 16/26] ui: tree: render unmounted datastores correctly Hannes Laimer
@ 2024-11-25 8:47 ` Hannes Laimer
2024-11-25 13:06 ` Dominik Csapak
0 siblings, 1 reply; 45+ messages in thread
From: Hannes Laimer @ 2024-11-25 8:47 UTC (permalink / raw)
To: pbs-devel
---
fixes small problem with the original patch where all datastores were rendered with the plug
icon in the UI
sent in reply to original patch since it is a really minor or change and
can just be squashed in
www/NavigationTree.js | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/www/NavigationTree.js b/www/NavigationTree.js
index dd03fbd62..29ecae7fe 100644
--- a/www/NavigationTree.js
+++ b/www/NavigationTree.js
@@ -266,7 +266,8 @@ Ext.define('PBS.view.main.NavigationTree', {
while (name.localeCompare(getChildTextAt(j)) > 0 && (j+1) < list.childNodes.length) {
j++;
}
- let mainIcon = `fa fa-${records[i].data.mount-status !== 'nonremovable' ? 'plug' : 'database'}`;
+ const isRemovable = records[i].data['mount-status'] !== 'nonremovable';
+ let mainIcon = `fa fa-${isRemovable ? 'plug' : 'database'}`;
let [qtip, iconCls] = ['', mainIcon];
const maintenance = records[i].data.maintenance;
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v14] fixup! ui: tree: render unmounted datastores correctly
2024-11-25 8:47 ` [pbs-devel] [PATCH proxmox-backup v14] fixup! " Hannes Laimer
@ 2024-11-25 13:06 ` Dominik Csapak
0 siblings, 0 replies; 45+ messages in thread
From: Dominik Csapak @ 2024-11-25 13:06 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
one comment inline
On 11/25/24 09:47, Hannes Laimer wrote:
> ---
> fixes small problem with the original patch where all datastores were rendered with the plug
> icon in the UI
>
> sent in reply to original patch since it is a really minor or change and
> can just be squashed in
> www/NavigationTree.js | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/www/NavigationTree.js b/www/NavigationTree.js
> index dd03fbd62..29ecae7fe 100644
> --- a/www/NavigationTree.js
> +++ b/www/NavigationTree.js
> @@ -266,7 +266,8 @@ Ext.define('PBS.view.main.NavigationTree', {
> while (name.localeCompare(getChildTextAt(j)) > 0 && (j+1) < list.childNodes.length) {
> j++;
> }
> - let mainIcon = `fa fa-${records[i].data.mount-status !== 'nonremovable' ? 'plug' : 'database'}`;
> + const isRemovable = records[i].data['mount-status'] !== 'nonremovable';
> + let mainIcon = `fa fa-${isRemovable ? 'plug' : 'database'}`;
i think it'll not happen, but if an 'old' api returns no 'mount-status' at all, i think
we should default to the 'normal' symbol, not the plug one
so e.g.
let mountStatus = records[i].data['mount-status'] ?? 'nonremovable';
let isRemoveable = mountStatus !== 'nonremovable';
or something like this
> let [qtip, iconCls] = ['', mainIcon];
> const maintenance = records[i].data.maintenance;
>
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v14 17/26] ui: utils: make parseMaintenanceMode more robust
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
` (15 preceding siblings ...)
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 16/26] ui: tree: render unmounted datastores correctly Hannes Laimer
@ 2024-11-22 14:47 ` Hannes Laimer
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 18/26] ui: add datastore status mask for unmounted removable datastores Hannes Laimer
` (10 subsequent siblings)
27 siblings, 0 replies; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:47 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
www/Utils.js | 29 ++++++++++++++++++++++-------
1 file changed, 22 insertions(+), 7 deletions(-)
diff --git a/www/Utils.js b/www/Utils.js
index 4853be36c..7756e9b5d 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -740,14 +740,29 @@ Ext.define('PBS.Utils', {
return `${icon} ${value}`;
},
- // FIXME: this "parser" is brittle and relies on the order the arguments will appear in
+ /**
+ * Parses maintenance mode property string.
+ * Examples:
+ * "offline,message=foo" -> ["offline", "foo"]
+ * "offline" -> ["offline", null]
+ * "message=foo,offline" -> ["offline", "foo"]
+ * null/undefined -> [null, null]
+ *
+ * @param {string|null} mode - Maintenance mode string to parse.
+ * @return {Array<string|null>} - Parsed maintenance mode values.
+ */
parseMaintenanceMode: function(mode) {
- let [type, message] = mode.split(/,(.+)/);
- type = type.split("=").pop();
- message = message ? message.split("=")[1]
- .replace(/^"(.*)"$/, '$1')
- .replaceAll('\\"', '"') : null;
- return [type, message];
+ if (!mode) {
+ return [null, null];
+ }
+ return mode.split(',').reduce(([m, msg], pair) => {
+ const [key, value] = pair.split('=');
+ if (key === 'message') {
+ return [m, value.replace(/^"(.*)"$/, '$1').replace(/\\"/g, '"')];
+ } else {
+ return [value ?? key, msg];
+ }
+ }, [null, null]);
},
renderMaintenance: function(mode, activeTasks) {
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* [pbs-devel] [PATCH proxmox-backup v14 18/26] ui: add datastore status mask for unmounted removable datastores
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
` (16 preceding siblings ...)
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 17/26] ui: utils: make parseMaintenanceMode more robust Hannes Laimer
@ 2024-11-22 14:47 ` Hannes Laimer
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 19/26] ui: maintenance: fix disable msg field if no type is selected Hannes Laimer
` (9 subsequent siblings)
27 siblings, 0 replies; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:47 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
www/css/ext6-pbs.css | 12 ++++++++++++
www/datastore/Summary.js | 21 +++++++++++++--------
2 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/www/css/ext6-pbs.css b/www/css/ext6-pbs.css
index 706e681e9..891189ae3 100644
--- a/www/css/ext6-pbs.css
+++ b/www/css/ext6-pbs.css
@@ -261,6 +261,18 @@ span.snapshot-comment-column {
content: "\f0ad";
}
+.pbs-unplugged-mask div.x-mask-msg-text {
+ background: None;
+ padding: 12px 0 0;
+}
+
+.pbs-unplugged-mask:before {
+ font-size: 3em;
+ display: flex;
+ justify-content: center;
+ content: "\f1e6";
+}
+
/* the small icons TODO move to proxmox-widget-toolkit */
.pmx-tree-icon-custom:after {
position: relative;
diff --git a/www/datastore/Summary.js b/www/datastore/Summary.js
index 2d79a7951..c41c55423 100644
--- a/www/datastore/Summary.js
+++ b/www/datastore/Summary.js
@@ -61,16 +61,21 @@ Ext.define('PBS.DataStoreInfo', {
Proxmox.Utils.API2Request({
url: `/config/datastore/${me.view.datastore}`,
success: function(response) {
- const config = response.result.data;
- if (config['maintenance-mode']) {
- const [_type, msg] = PBS.Utils.parseMaintenanceMode(config['maintenance-mode']);
- me.view.el.mask(
- `${gettext('Datastore is in maintenance mode')}${msg ? ': ' + msg : ''}`,
- 'fa pbs-maintenance-mask',
- );
- } else {
+ let maintenanceString = response.result.data['maintenance-mode'];
+ let removable = !!response.result.data['backing-device'];
+ if (!maintenanceString && !removable) {
me.view.el.mask(gettext('Datastore is not available'));
+ return;
}
+
+ let [_type, msg] = PBS.Utils.parseMaintenanceMode(maintenanceString);
+ let isUnplugged = !maintenanceString && removable;
+ let maskMessage = isUnplugged
+ ? gettext('Datastore is not mounted')
+ : `${gettext('Datastore is in maintenance mode')}${msg ? ': ' + msg : ''}`;
+
+ let maskIcon = isUnplugged ? 'fa pbs-unplugged-mask' : 'fa pbs-maintenance-mask';
+ me.view.el.mask(maskMessage, maskIcon);
},
});
return;
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* [pbs-devel] [PATCH proxmox-backup v14 19/26] ui: maintenance: fix disable msg field if no type is selected
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
` (17 preceding siblings ...)
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 18/26] ui: add datastore status mask for unmounted removable datastores Hannes Laimer
@ 2024-11-22 14:47 ` Hannes Laimer
2024-11-25 13:23 ` Dominik Csapak
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 20/26] ui: render 'unmount' maintenance mode correctly Hannes Laimer
` (8 subsequent siblings)
27 siblings, 1 reply; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:47 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
www/window/MaintenanceOptions.js | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/www/window/MaintenanceOptions.js b/www/window/MaintenanceOptions.js
index 1ee92542e..527c36987 100644
--- a/www/window/MaintenanceOptions.js
+++ b/www/window/MaintenanceOptions.js
@@ -56,12 +56,17 @@ Ext.define('PBS.window.MaintenanceOptions', {
fieldLabel: gettext('Maintenance Type'),
value: '__default__',
deleteEmpty: true,
+ listeners: {
+ change: (field, newValue) => {
+ Ext.getCmp('message-field').setDisabled(newValue === '__default__');
+ },
+ },
},
{
xtype: 'proxmoxtextfield',
+ id: 'message-field',
name: 'maintenance-msg',
fieldLabel: gettext('Description'),
- // FIXME: disable if maintenance type is none
},
],
},
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v14 19/26] ui: maintenance: fix disable msg field if no type is selected
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 19/26] ui: maintenance: fix disable msg field if no type is selected Hannes Laimer
@ 2024-11-25 13:23 ` Dominik Csapak
0 siblings, 0 replies; 45+ messages in thread
From: Dominik Csapak @ 2024-11-25 13:23 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
one comment inline
On 11/22/24 15:47, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> www/window/MaintenanceOptions.js | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/www/window/MaintenanceOptions.js b/www/window/MaintenanceOptions.js
> index 1ee92542e..527c36987 100644
> --- a/www/window/MaintenanceOptions.js
> +++ b/www/window/MaintenanceOptions.js
> @@ -56,12 +56,17 @@ Ext.define('PBS.window.MaintenanceOptions', {
> fieldLabel: gettext('Maintenance Type'),
> value: '__default__',
> deleteEmpty: true,
> + listeners: {
> + change: (field, newValue) => {
> + Ext.getCmp('message-field').setDisabled(newValue === '__default__');
i'd rather you use 'itemId' instead and use our 'up().down()' mechanism like we do everywhere else,
or use a 'referenceHolder' and 'reference'+'lookup' instead
the reason is that (html) id's have to be globally unique, and having one that is
generically named 'message-field' might clash with something else at one point...
> + },
> + },
> },
> {
> xtype: 'proxmoxtextfield',
> + id: 'message-field',
> name: 'maintenance-msg',
> fieldLabel: gettext('Description'),
> - // FIXME: disable if maintenance type is none
> },
> ],
> },
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v14 20/26] ui: render 'unmount' maintenance mode correctly
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
` (18 preceding siblings ...)
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 19/26] ui: maintenance: fix disable msg field if no type is selected Hannes Laimer
@ 2024-11-22 14:47 ` Hannes Laimer
2024-11-25 13:24 ` Dominik Csapak
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 21/25] api: node: allow creation of removable datastore through directory endpoint Hannes Laimer
` (7 subsequent siblings)
27 siblings, 1 reply; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:47 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
www/Utils.js | 4 +++-
www/window/MaintenanceOptions.js | 10 ++++++++++
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/www/Utils.js b/www/Utils.js
index 7756e9b5d..6bae9b709 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -775,7 +775,7 @@ Ext.define('PBS.Utils', {
let extra = '';
if (activeTasks !== undefined) {
- const conflictingTasks = activeTasks.write + (type === 'offline' ? activeTasks.read : 0);
+ const conflictingTasks = activeTasks.write + (type === 'offline' || type === 'unmount' ? activeTasks.read : 0);
if (conflictingTasks > 0) {
extra += '| <i class="fa fa-spinner fa-pulse fa-fw"></i> ';
@@ -795,6 +795,8 @@ Ext.define('PBS.Utils', {
break;
case 'offline': modeText = gettext("Offline");
break;
+ case 'unmount': modeText = gettext("Unmounting");
+ break;
}
return `${modeText} ${extra}`;
},
diff --git a/www/window/MaintenanceOptions.js b/www/window/MaintenanceOptions.js
index 527c36987..d7348cb4f 100644
--- a/www/window/MaintenanceOptions.js
+++ b/www/window/MaintenanceOptions.js
@@ -52,6 +52,7 @@ Ext.define('PBS.window.MaintenanceOptions', {
items: [
{
xtype: 'pbsMaintenanceType',
+ id: 'type-field',
name: 'maintenance-type',
fieldLabel: gettext('Maintenance Type'),
value: '__default__',
@@ -85,6 +86,15 @@ Ext.define('PBS.window.MaintenanceOptions', {
};
}
+ let unmounting = options['maintenance-type'] === 'unmount';
+ let defaultType = options['maintenance-type'] === '__default__';
+ if (unmounting) {
+ options['maintenance-type'] = '';
+ }
+
me.callParent([options]);
+
+ Ext.ComponentManager.get('type-field').setDisabled(unmounting);
+ Ext.ComponentManager.get('message-field').setDisabled(unmounting || defaultType);
},
});
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v14 20/26] ui: render 'unmount' maintenance mode correctly
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 20/26] ui: render 'unmount' maintenance mode correctly Hannes Laimer
@ 2024-11-25 13:24 ` Dominik Csapak
0 siblings, 0 replies; 45+ messages in thread
From: Dominik Csapak @ 2024-11-25 13:24 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
comments inline
On 11/22/24 15:47, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> www/Utils.js | 4 +++-
> www/window/MaintenanceOptions.js | 10 ++++++++++
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/www/Utils.js b/www/Utils.js
> index 7756e9b5d..6bae9b709 100644
> --- a/www/Utils.js
> +++ b/www/Utils.js
> @@ -775,7 +775,7 @@ Ext.define('PBS.Utils', {
> let extra = '';
>
> if (activeTasks !== undefined) {
> - const conflictingTasks = activeTasks.write + (type === 'offline' ? activeTasks.read : 0);
> + const conflictingTasks = activeTasks.write + (type === 'offline' || type === 'unmount' ? activeTasks.read : 0);
>
> if (conflictingTasks > 0) {
> extra += '| <i class="fa fa-spinner fa-pulse fa-fw"></i> ';
> @@ -795,6 +795,8 @@ Ext.define('PBS.Utils', {
> break;
> case 'offline': modeText = gettext("Offline");
> break;
> + case 'unmount': modeText = gettext("Unmounting");
> + break;
> }
> return `${modeText} ${extra}`;
> },
> diff --git a/www/window/MaintenanceOptions.js b/www/window/MaintenanceOptions.js
> index 527c36987..d7348cb4f 100644
> --- a/www/window/MaintenanceOptions.js
> +++ b/www/window/MaintenanceOptions.js
> @@ -52,6 +52,7 @@ Ext.define('PBS.window.MaintenanceOptions', {
> items: [
> {
> xtype: 'pbsMaintenanceType',
> + id: 'type-field',
same as previous patch: please don't use 'id' for this
(especially using such generic names)
> name: 'maintenance-type',
> fieldLabel: gettext('Maintenance Type'),
> value: '__default__',
> @@ -85,6 +86,15 @@ Ext.define('PBS.window.MaintenanceOptions', {
> };
> }
>
> + let unmounting = options['maintenance-type'] === 'unmount';
> + let defaultType = options['maintenance-type'] === '__default__';
> + if (unmounting) {
> + options['maintenance-type'] = '';
> + }
> +
> me.callParent([options]);
> +
> + Ext.ComponentManager.get('type-field').setDisabled(unmounting);
> + Ext.ComponentManager.get('message-field').setDisabled(unmounting || defaultType);
> },
> });
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v14 21/25] api: node: allow creation of removable datastore through directory endpoint
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
` (19 preceding siblings ...)
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 20/26] ui: render 'unmount' maintenance mode correctly Hannes Laimer
@ 2024-11-22 14:47 ` Hannes Laimer
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 22/25] api: node: include removable datastores in directory list Hannes Laimer
` (6 subsequent siblings)
27 siblings, 0 replies; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:47 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
changes since v13:
* use one worker with few conditionals instead of two
src/api2/node/disks/directory.rs | 32 +++++++++++++++++++++-----------
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/src/api2/node/disks/directory.rs b/src/api2/node/disks/directory.rs
index 7f5402207..b6006b47c 100644
--- a/src/api2/node/disks/directory.rs
+++ b/src/api2/node/disks/directory.rs
@@ -123,6 +123,11 @@ pub fn list_datastore_mounts() -> Result<Vec<DatastoreMountInfo>, Error> {
description: "Configure a datastore using the directory.",
type: bool,
optional: true,
+ default: false,
+ },
+ "removable-datastore": {
+ description: "The added datastore is removable.",
+ type: bool,
},
filesystem: {
type: FileSystemType,
@@ -141,7 +146,8 @@ pub fn list_datastore_mounts() -> Result<Vec<DatastoreMountInfo>, Error> {
pub fn create_datastore_disk(
name: String,
disk: String,
- add_datastore: Option<bool>,
+ add_datastore: bool,
+ removable_datastore: bool,
filesystem: Option<FileSystemType>,
rpcenv: &mut dyn RpcEnvironment,
) -> Result<String, Error> {
@@ -156,7 +162,6 @@ pub fn create_datastore_disk(
}
let mount_point = format!("{}{}", BASE_MOUNT_DIR, &name);
-
// check if the default path exists already.
// bail if it is not empty or another filesystem mounted on top
let default_path = std::path::PathBuf::from(&mount_point);
@@ -183,7 +188,6 @@ pub fn create_datastore_disk(
move |_worker| {
info!("create datastore '{name}' on disk {disk}");
- let add_datastore = add_datastore.unwrap_or(false);
let filesystem = filesystem.unwrap_or(FileSystemType::Ext4);
let manager = DiskManage::new();
@@ -196,18 +200,24 @@ pub fn create_datastore_disk(
let uuid = get_fs_uuid(&partition)?;
let uuid_path = format!("/dev/disk/by-uuid/{}", uuid);
- let mount_unit_name =
- create_datastore_mount_unit(&name, &mount_point, filesystem, &uuid_path)?;
+ if !removable_datastore {
+ let mount_unit_name =
+ create_datastore_mount_unit(&name, &mount_point, filesystem, &uuid_path)?;
- crate::tools::systemd::reload_daemon()?;
- crate::tools::systemd::enable_unit(&mount_unit_name)?;
- crate::tools::systemd::start_unit(&mount_unit_name)?;
+ crate::tools::systemd::reload_daemon()?;
+ crate::tools::systemd::enable_unit(&mount_unit_name)?;
+ crate::tools::systemd::start_unit(&mount_unit_name)?;
+ }
if add_datastore {
let lock = pbs_config::datastore::lock_config()?;
- let datastore: DataStoreConfig =
- serde_json::from_value(json!({ "name": name, "path": mount_point }))?;
-
+ let datastore: DataStoreConfig = if removable_datastore {
+ serde_json::from_value(
+ json!({ "name": name, "path": format!("/{name}"), "backing-device": uuid }),
+ )?
+ } else {
+ serde_json::from_value(json!({ "name": name, "path": mount_point }))?
+ };
let (config, _digest) = pbs_config::datastore::config()?;
if config.sections.contains_key(&datastore.name) {
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* [pbs-devel] [PATCH proxmox-backup v14 22/25] api: node: include removable datastores in directory list
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
` (20 preceding siblings ...)
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 21/25] api: node: allow creation of removable datastore through directory endpoint Hannes Laimer
@ 2024-11-22 14:47 ` Hannes Laimer
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 23/26] node: disks: replace BASE_MOUNT_DIR with DATASTORE_MOUNT_DIR Hannes Laimer
` (5 subsequent siblings)
27 siblings, 0 replies; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:47 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
changes since v13:
* drop check with get_mount_point
src/api2/node/disks/directory.rs | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/src/api2/node/disks/directory.rs b/src/api2/node/disks/directory.rs
index b6006b47c..11d07af42 100644
--- a/src/api2/node/disks/directory.rs
+++ b/src/api2/node/disks/directory.rs
@@ -45,6 +45,8 @@ pub struct DatastoreMountInfo {
pub path: String,
/// The mounted device.
pub device: String,
+ /// This is removable
+ pub removable: bool,
/// File system type
pub filesystem: Option<String>,
/// Mount options
@@ -61,7 +63,7 @@ pub struct DatastoreMountInfo {
}
},
returns: {
- description: "List of systemd datastore mount units.",
+ description: "List of removable-datastore devices and systemd datastore mount units.",
type: Array,
items: {
type: DatastoreMountInfo,
@@ -100,6 +102,28 @@ pub fn list_datastore_mounts() -> Result<Vec<DatastoreMountInfo>, Error> {
path: data.Where,
filesystem: data.Type,
options: data.Options,
+ removable: false,
+ });
+ }
+
+ let (config, _digest) = pbs_config::datastore::config()?;
+ let store_list: Vec<DataStoreConfig> = config.convert_to_typed_array("datastore")?;
+
+ for item in store_list
+ .into_iter()
+ .filter(|store| store.backing_device.is_some())
+ {
+ let Some(backing_device) = item.backing_device.as_deref() else {
+ continue;
+ };
+ list.push(DatastoreMountInfo {
+ unitfile: "datastore config".to_string(),
+ name: item.name.clone(),
+ device: format!("/dev/disk/by-uuid/{backing_device}"),
+ path: item.absolute_path(),
+ filesystem: None,
+ options: None,
+ removable: true,
});
}
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* [pbs-devel] [PATCH proxmox-backup v14 23/26] node: disks: replace BASE_MOUNT_DIR with DATASTORE_MOUNT_DIR
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
` (21 preceding siblings ...)
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 22/25] api: node: include removable datastores in directory list Hannes Laimer
@ 2024-11-22 14:47 ` Hannes Laimer
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 24/26] ui: support create removable datastore through directory creation Hannes Laimer
` (4 subsequent siblings)
27 siblings, 0 replies; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:47 UTC (permalink / raw)
To: pbs-devel
... since they do have the same value.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/api2/node/disks/directory.rs | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/src/api2/node/disks/directory.rs b/src/api2/node/disks/directory.rs
index 11d07af42..ff817b253 100644
--- a/src/api2/node/disks/directory.rs
+++ b/src/api2/node/disks/directory.rs
@@ -11,8 +11,8 @@ use proxmox_schema::api;
use proxmox_section_config::SectionConfigData;
use pbs_api_types::{
- DataStoreConfig, BLOCKDEVICE_NAME_SCHEMA, DATASTORE_SCHEMA, NODE_SCHEMA, PRIV_SYS_AUDIT,
- PRIV_SYS_MODIFY, UPID_SCHEMA,
+ DataStoreConfig, BLOCKDEVICE_NAME_SCHEMA, DATASTORE_MOUNT_DIR, DATASTORE_SCHEMA, NODE_SCHEMA,
+ PRIV_SYS_AUDIT, PRIV_SYS_MODIFY, UPID_SCHEMA,
};
use crate::tools::disks::{
@@ -23,8 +23,6 @@ use crate::tools::systemd::{self, types::*};
use proxmox_rest_server::WorkerTask;
-const BASE_MOUNT_DIR: &str = "/mnt/datastore/";
-
#[api(
properties: {
"filesystem": {
@@ -91,7 +89,7 @@ pub fn list_datastore_mounts() -> Result<Vec<DatastoreMountInfo>, Error> {
let name = data
.Where
- .strip_prefix(BASE_MOUNT_DIR)
+ .strip_prefix(DATASTORE_MOUNT_DIR)
.unwrap_or(&data.Where)
.to_string();
@@ -185,7 +183,7 @@ pub fn create_datastore_disk(
bail!("disk '{}' is already in use.", disk);
}
- let mount_point = format!("{}{}", BASE_MOUNT_DIR, &name);
+ let mount_point = format!("{}/{}", DATASTORE_MOUNT_DIR, &name);
// check if the default path exists already.
// bail if it is not empty or another filesystem mounted on top
let default_path = std::path::PathBuf::from(&mount_point);
@@ -193,7 +191,7 @@ pub fn create_datastore_disk(
match std::fs::metadata(&default_path) {
Err(_) => {} // path does not exist
Ok(stat) => {
- let basedir_dev = std::fs::metadata(BASE_MOUNT_DIR)?.st_dev();
+ let basedir_dev = std::fs::metadata(DATASTORE_MOUNT_DIR)?.st_dev();
if stat.st_dev() != basedir_dev {
bail!("path {default_path:?} already exists and is mountpoint");
}
@@ -278,7 +276,7 @@ pub fn create_datastore_disk(
)]
/// Remove a Filesystem mounted under `/mnt/datastore/<name>`.
pub fn delete_datastore_disk(name: String) -> Result<(), Error> {
- let path = format!("{}{}", BASE_MOUNT_DIR, name);
+ let path = format!("{}/{}", DATASTORE_MOUNT_DIR, name);
// path of datastore cannot be changed
let (config, _) = pbs_config::datastore::config()?;
let datastores: Vec<DataStoreConfig> = config.convert_to_typed_array("datastore")?;
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* [pbs-devel] [PATCH proxmox-backup v14 24/26] ui: support create removable datastore through directory creation
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
` (22 preceding siblings ...)
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 23/26] node: disks: replace BASE_MOUNT_DIR with DATASTORE_MOUNT_DIR Hannes Laimer
@ 2024-11-22 14:47 ` Hannes Laimer
2024-11-25 13:28 ` Dominik Csapak
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 25/26] bin: debug: add inspect device command Hannes Laimer
` (3 subsequent siblings)
27 siblings, 1 reply; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:47 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/api2/node/disks/directory.rs | 2 ++
www/DirectoryList.js | 13 +++++++++++++
www/window/CreateDirectory.js | 14 ++++++++++++++
3 files changed, 29 insertions(+)
diff --git a/src/api2/node/disks/directory.rs b/src/api2/node/disks/directory.rs
index ff817b253..2f7cc7a27 100644
--- a/src/api2/node/disks/directory.rs
+++ b/src/api2/node/disks/directory.rs
@@ -150,6 +150,8 @@ pub fn list_datastore_mounts() -> Result<Vec<DatastoreMountInfo>, Error> {
"removable-datastore": {
description: "The added datastore is removable.",
type: bool,
+ optional: true,
+ default: false,
},
filesystem: {
type: FileSystemType,
diff --git a/www/DirectoryList.js b/www/DirectoryList.js
index adefa9abf..25921a623 100644
--- a/www/DirectoryList.js
+++ b/www/DirectoryList.js
@@ -121,6 +121,19 @@ Ext.define('PBS.admin.Directorylist', {
],
columns: [
+ {
+ text: '<span class="fa fa-plug"/>',
+ flex: 0,
+ width: 35,
+ dataIndex: 'removable',
+ renderer: function(_text, _, row) {
+ if (row.data.removable) {
+ return `<i class="fa fa-check"/>`;
+ } else {
+ return '';
+ }
+ },
+ },
{
text: gettext('Path'),
dataIndex: 'path',
diff --git a/www/window/CreateDirectory.js b/www/window/CreateDirectory.js
index 6aabe21ab..38d6979d9 100644
--- a/www/window/CreateDirectory.js
+++ b/www/window/CreateDirectory.js
@@ -43,6 +43,20 @@ Ext.define('PBS.window.CreateDirectory', {
name: 'add-datastore',
fieldLabel: gettext('Add as Datastore'),
value: '1',
+ listeners: {
+ change(field, newValue, _oldValue) {
+ let form = field.up('form');
+ let rmBox = form.down('[name=removable-datastore]');
+
+ rmBox.setDisabled(!newValue);
+ rmBox.setValue(false);
+ },
+ },
+ },
+ {
+ xtype: 'proxmoxcheckbox',
+ name: 'removable-datastore',
+ fieldLabel: gettext('is removable'),
},
],
});
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v14 24/26] ui: support create removable datastore through directory creation
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 24/26] ui: support create removable datastore through directory creation Hannes Laimer
@ 2024-11-25 13:28 ` Dominik Csapak
2024-11-25 16:21 ` Hannes Laimer
0 siblings, 1 reply; 45+ messages in thread
From: Dominik Csapak @ 2024-11-25 13:28 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
a bit offtopic, but why only directory storages and not also zfs based ones?
e.g. i can imagine users wanting a zpool on an external disk too (for
checksumming, send/receive, snapshotting, etc)
On 11/22/24 15:47, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> src/api2/node/disks/directory.rs | 2 ++
> www/DirectoryList.js | 13 +++++++++++++
> www/window/CreateDirectory.js | 14 ++++++++++++++
> 3 files changed, 29 insertions(+)
>
> diff --git a/src/api2/node/disks/directory.rs b/src/api2/node/disks/directory.rs
> index ff817b253..2f7cc7a27 100644
> --- a/src/api2/node/disks/directory.rs
> +++ b/src/api2/node/disks/directory.rs
> @@ -150,6 +150,8 @@ pub fn list_datastore_mounts() -> Result<Vec<DatastoreMountInfo>, Error> {
> "removable-datastore": {
> description: "The added datastore is removable.",
> type: bool,
> + optional: true,
> + default: false,
> },
> filesystem: {
> type: FileSystemType,
> diff --git a/www/DirectoryList.js b/www/DirectoryList.js
> index adefa9abf..25921a623 100644
> --- a/www/DirectoryList.js
> +++ b/www/DirectoryList.js
> @@ -121,6 +121,19 @@ Ext.define('PBS.admin.Directorylist', {
> ],
>
> columns: [
> + {
> + text: '<span class="fa fa-plug"/>',
> + flex: 0,
> + width: 35,
> + dataIndex: 'removable',
> + renderer: function(_text, _, row) {
> + if (row.data.removable) {
> + return `<i class="fa fa-check"/>`;
> + } else {
> + return '';
> + }
> + },
> + },
> {
> text: gettext('Path'),
> dataIndex: 'path',
> diff --git a/www/window/CreateDirectory.js b/www/window/CreateDirectory.js
> index 6aabe21ab..38d6979d9 100644
> --- a/www/window/CreateDirectory.js
> +++ b/www/window/CreateDirectory.js
> @@ -43,6 +43,20 @@ Ext.define('PBS.window.CreateDirectory', {
> name: 'add-datastore',
> fieldLabel: gettext('Add as Datastore'),
> value: '1',
> + listeners: {
> + change(field, newValue, _oldValue) {
> + let form = field.up('form');
> + let rmBox = form.down('[name=removable-datastore]');
> +
> + rmBox.setDisabled(!newValue);
> + rmBox.setValue(false);
> + },
> + },
> + },
> + {
> + xtype: 'proxmoxcheckbox',
> + name: 'removable-datastore',
> + fieldLabel: gettext('is removable'),
> },
> ],
> });
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v14 24/26] ui: support create removable datastore through directory creation
2024-11-25 13:28 ` Dominik Csapak
@ 2024-11-25 16:21 ` Hannes Laimer
0 siblings, 0 replies; 45+ messages in thread
From: Hannes Laimer @ 2024-11-25 16:21 UTC (permalink / raw)
To: Dominik Csapak, Proxmox Backup Server development discussion
On 11/25/24 14:28, Dominik Csapak wrote:
> a bit offtopic, but why only directory storages and not also zfs based
> ones?
>
no reason, should be pretty easy to add. I'll do a follow-up
> e.g. i can imagine users wanting a zpool on an external disk too (for
> checksumming, send/receive, snapshotting, etc)
>
> On 11/22/24 15:47, Hannes Laimer wrote:
>> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
>> ---
>> src/api2/node/disks/directory.rs | 2 ++
>> www/DirectoryList.js | 13 +++++++++++++
>> www/window/CreateDirectory.js | 14 ++++++++++++++
>> 3 files changed, 29 insertions(+)
>>
>> diff --git a/src/api2/node/disks/directory.rs b/src/api2/node/disks/
>> directory.rs
>> index ff817b253..2f7cc7a27 100644
>> --- a/src/api2/node/disks/directory.rs
>> +++ b/src/api2/node/disks/directory.rs
>> @@ -150,6 +150,8 @@ pub fn list_datastore_mounts() ->
>> Result<Vec<DatastoreMountInfo>, Error> {
>> "removable-datastore": {
>> description: "The added datastore is removable.",
>> type: bool,
>> + optional: true,
>> + default: false,
>> },
>> filesystem: {
>> type: FileSystemType,
>> diff --git a/www/DirectoryList.js b/www/DirectoryList.js
>> index adefa9abf..25921a623 100644
>> --- a/www/DirectoryList.js
>> +++ b/www/DirectoryList.js
>> @@ -121,6 +121,19 @@ Ext.define('PBS.admin.Directorylist', {
>> ],
>> columns: [
>> + {
>> + text: '<span class="fa fa-plug"/>',
>> + flex: 0,
>> + width: 35,
>> + dataIndex: 'removable',
>> + renderer: function(_text, _, row) {
>> + if (row.data.removable) {
>> + return `<i class="fa fa-check"/>`;
>> + } else {
>> + return '';
>> + }
>> + },
>> + },
>> {
>> text: gettext('Path'),
>> dataIndex: 'path',
>> diff --git a/www/window/CreateDirectory.js b/www/window/
>> CreateDirectory.js
>> index 6aabe21ab..38d6979d9 100644
>> --- a/www/window/CreateDirectory.js
>> +++ b/www/window/CreateDirectory.js
>> @@ -43,6 +43,20 @@ Ext.define('PBS.window.CreateDirectory', {
>> name: 'add-datastore',
>> fieldLabel: gettext('Add as Datastore'),
>> value: '1',
>> + listeners: {
>> + change(field, newValue, _oldValue) {
>> + let form = field.up('form');
>> + let rmBox = form.down('[name=removable-datastore]');
>> +
>> + rmBox.setDisabled(!newValue);
>> + rmBox.setValue(false);
>> + },
>> + },
>> + },
>> + {
>> + xtype: 'proxmoxcheckbox',
>> + name: 'removable-datastore',
>> + fieldLabel: gettext('is removable'),
>> },
>> ],
>> });
>
>
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v14 25/26] bin: debug: add inspect device command
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
` (23 preceding siblings ...)
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 24/26] ui: support create removable datastore through directory creation Hannes Laimer
@ 2024-11-22 14:47 ` Hannes Laimer
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 26/26] api: disks: only return UUID of partitions if it actually is one Hannes Laimer
` (2 subsequent siblings)
27 siblings, 0 replies; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:47 UTC (permalink / raw)
To: pbs-devel
... to get information about (removable) datastores a device contains
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/bin/proxmox_backup_debug/inspect.rs | 149 ++++++++++++++++++++++++
1 file changed, 149 insertions(+)
diff --git a/src/bin/proxmox_backup_debug/inspect.rs b/src/bin/proxmox_backup_debug/inspect.rs
index 28a472b0f..17df09be2 100644
--- a/src/bin/proxmox_backup_debug/inspect.rs
+++ b/src/bin/proxmox_backup_debug/inspect.rs
@@ -331,6 +331,151 @@ fn inspect_file(
Ok(())
}
+/// Return the count of VM, CT and host backup groups and the count of namespaces
+/// as this tuple (vm, ct, host, ns)
+fn get_basic_ds_info(path: String) -> Result<(i64, i64, i64, i64), Error> {
+ let mut vms = 0;
+ let mut cts = 0;
+ let mut hosts = 0;
+ let mut ns = 0;
+ let mut walker = WalkDir::new(path).into_iter();
+
+ while let Some(entry_result) = walker.next() {
+ let entry = entry_result?;
+ if !entry.file_type().is_dir() {
+ continue;
+ }
+
+ let Some(name) = entry.path().file_name().and_then(|a| a.to_str()) else {
+ continue;
+ };
+
+ if name == ".chunks" {
+ walker.skip_current_dir();
+ continue;
+ }
+
+ let dir_count = std::fs::read_dir(entry.path())?
+ .filter_map(Result::ok)
+ .filter(|entry| entry.path().is_dir())
+ .count() as i64;
+
+ match name {
+ "ns" => ns += dir_count,
+ "vm" => {
+ vms += dir_count;
+ walker.skip_current_dir();
+ }
+ "ct" => {
+ cts += dir_count;
+ walker.skip_current_dir();
+ }
+ "host" => {
+ hosts += dir_count;
+ walker.skip_current_dir();
+ }
+ _ => {
+ // root or ns dir
+ }
+ }
+ }
+
+ Ok((vms, cts, hosts, ns))
+}
+
+#[api(
+ input: {
+ properties: {
+ device: {
+ description: "Device path, usually /dev/...",
+ type: String,
+ },
+ "output-format": {
+ schema: OUTPUT_FORMAT,
+ optional: true,
+ },
+ }
+ }
+)]
+/// Inspect a device for possible datastores on it
+fn inspect_device(device: String, param: Value) -> Result<(), Error> {
+ let output_format = get_output_format(¶m);
+ let tmp_mount_path = format!(
+ "{}/{:x}",
+ pbs_buildcfg::rundir!("/mount"),
+ proxmox_uuid::Uuid::generate()
+ );
+
+ let default_options = proxmox_sys::fs::CreateOptions::new();
+ proxmox_sys::fs::create_path(
+ &tmp_mount_path,
+ Some(default_options.clone()),
+ Some(default_options.clone()),
+ )?;
+ let mut mount_cmd = std::process::Command::new("mount");
+ mount_cmd.arg(device.clone());
+ mount_cmd.arg(tmp_mount_path.clone());
+ proxmox_sys::command::run_command(mount_cmd, None)?;
+
+ let mut walker = WalkDir::new(tmp_mount_path.clone()).into_iter();
+
+ let mut stores = Vec::new();
+
+ let mut ds_count = 0;
+ while let Some(entry_result) = walker.next() {
+ let entry = entry_result?;
+
+ if entry.file_type().is_dir()
+ && entry
+ .file_name()
+ .to_str()
+ .map_or(false, |name| name == ".chunks")
+ {
+ let store_path = entry
+ .path()
+ .to_str()
+ .and_then(|n| n.strip_suffix("/.chunks"));
+
+ if let Some(store_path) = store_path {
+ ds_count += 1;
+ let (vm, ct, host, ns) = get_basic_ds_info(store_path.to_string())?;
+ stores.push(json!({
+ "path": store_path.strip_prefix(&tmp_mount_path).unwrap_or("???"),
+ "vm-count": vm,
+ "ct-count": ct,
+ "host-count": host,
+ "ns-count": ns,
+ }));
+ };
+
+ walker.skip_current_dir();
+ }
+ }
+
+ let mut umount_cmd = std::process::Command::new("umount");
+ umount_cmd.arg(tmp_mount_path.clone());
+ proxmox_sys::command::run_command(umount_cmd, None)?;
+ std::fs::remove_dir(std::path::Path::new(&tmp_mount_path))?;
+
+ if output_format == "text" {
+ println!("Device containes {} stores", ds_count);
+ println!("---------------");
+ for s in stores {
+ println!(
+ "Datastore at {} | VM: {}, CT: {}, HOST: {}, NS: {}",
+ s["path"], s["vm-count"], s["ct-count"], s["host-count"], s["ns-count"]
+ );
+ }
+ } else {
+ format_and_print_result(
+ &json!({"store_count": stores.len(), "stores": stores}),
+ &output_format,
+ );
+ }
+
+ Ok(())
+}
+
pub fn inspect_commands() -> CommandLineInterface {
let cmd_def = CliCommandMap::new()
.insert(
@@ -340,6 +485,10 @@ pub fn inspect_commands() -> CommandLineInterface {
.insert(
"file",
CliCommand::new(&API_METHOD_INSPECT_FILE).arg_param(&["file"]),
+ )
+ .insert(
+ "device",
+ CliCommand::new(&API_METHOD_INSPECT_DEVICE).arg_param(&["device"]),
);
cmd_def.into()
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* [pbs-devel] [PATCH proxmox-backup v14 26/26] api: disks: only return UUID of partitions if it actually is one
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
` (24 preceding siblings ...)
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 25/26] bin: debug: add inspect device command Hannes Laimer
@ 2024-11-22 14:47 ` Hannes Laimer
2024-11-25 6:19 ` [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
2024-11-25 13:15 ` Lukas Wagner
27 siblings, 0 replies; 45+ messages in thread
From: Hannes Laimer @ 2024-11-22 14:47 UTC (permalink / raw)
To: pbs-devel
Some filesystems like FAT don't include a concept of UUIDs.
Instead, tools like blkid tools like blkid derive these
identifiers based on certain filesystem metadata, such as
volume serial numbers or other unique information. This does
however not follow the format specified in RFC 9562[1].
[1] https://datatracker.ietf.org/doc/html/rfc9562
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/tools/disks/mod.rs | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/tools/disks/mod.rs b/src/tools/disks/mod.rs
index 6345fde7c..61aceccd6 100644
--- a/src/tools/disks/mod.rs
+++ b/src/tools/disks/mod.rs
@@ -898,7 +898,10 @@ fn get_partitions_info(
let mut uuid = None;
if let Some(devpath) = devpath.as_ref() {
for info in lsblk_infos.iter().filter(|i| i.path.eq(devpath)) {
- uuid = info.uuid.clone();
+ uuid = info
+ .uuid
+ .clone()
+ .filter(|uuid| pbs_api_types::UUID_REGEX.is_match(uuid));
used = match info.partition_type.as_deref() {
Some("21686148-6449-6e6f-744e-656564454649") => PartitionUsageType::BIOS,
Some("c12a7328-f81f-11d2-ba4b-00a0c93ec93b") => PartitionUsageType::EFI,
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
` (25 preceding siblings ...)
2024-11-22 14:47 ` [pbs-devel] [PATCH proxmox-backup v14 26/26] api: disks: only return UUID of partitions if it actually is one Hannes Laimer
@ 2024-11-25 6:19 ` Hannes Laimer
2024-11-25 13:15 ` Lukas Wagner
27 siblings, 0 replies; 45+ messages in thread
From: Hannes Laimer @ 2024-11-25 6:19 UTC (permalink / raw)
To: pbs-devel
On 11/22/24 15:46, Hannes Laimer wrote:
> These patches add support for removable datastores. All removable
> datastores have a backing-device(a UUID) associated with them. Removable
> datastores work like normal ones, just that they can be unplugged. It is
> possible to create a removable datastore, sync backups onto it, unplug
> it and use it on a different PBS.
>
> The datastore path specified is relative to the root of the used device.
> Removable datastores are bind mounted to /mnt/datastore/<NAME>.
> Multiple datastores can be created on a single device, but only device with
> a single datastore on them will be auto-mounted.
>
> When a removable datastore is deleted and 'destroy-data' is set, the
> device has to be mounted. If 'destroy-data' is not set the datastore
> can be deleted even if the device is not present. Removable datastores
> are automatically mounted when plugged in.
>
> v14: thanks @Fabian and @Maximiliano
> * add two functions to get mount status, or ensure mounted for rm.ds.,
> avoiding repeating things
> * use enum for `mount_status` instead of Option<bool>
> * fix problme with unmounting, now check for unmounting maintenance mode before actually unmounting,
> manually chaning the maintenance mode during unmounting will not prevent unmounting
^ *now
> * improve logging for mounting: add context and adjust logging levels
> * improve uuid_mount function: load config file directly and call do_mount function directly without
> going through the API
> * add logging for cleanup on ds deletion
> * move check for nesting into do_create_datastore, and check fir all datastore(not just removable)
> * remove redundant check when deleting through directory
> * use single worker when creating removable datastore through dir endpoint
> * drop get_mount_point function
> * ui: stop loading status after first failed attempt, prevents logs spamming
>
> v13: thanks @Fabian
> * allow multiple datastore on devices
> * replace `is_datastore_available` by a more specific function, it is now
> removable datastore specific and won't be called for normal ones
> * replace removable/is_available in status structs with mount_state,
> which is `None` for normal datastore as it makes it
> less ambiguous what is meant
> * remove notion of 'available' from normal datastores and replace it with
> mounted/mount_status for removable ones, as it never really made sense
> in the first place
> * abort of an unmount task will now reset the maintanance mode
> * add check for race when setting maintenance at end of unmounting task
> * improve documentation and commit messages
> * remove not needed tokio::spawn
> * only auto mount devices with single datastore on them
> * drop ptach that added flag for excluding used partitions
> * make auto mount service not dynamic
> * add debug command to scan devices for datastores they may contain
> * rebase onto master
>
> v12: thanks @Wolfgang
> * use bind mounts, so now
> <DEVICE>/path/to/ds is mounted to /mnt/datastore/<NAME>
> this is a bit cleaner and allows for multiple datastores
> on a single device to be mounted individually, if we
> want to allow that in the future
> * small code improvements
>
>
> v11:
> * rebase onto master
>
> v10: thanks @Gabriel and @Wolfgang
> * make is_datastore_available more robust
> * fix a lot of wording
> * drop format on uuid_mount command for UUID
> * only gather_disk_stats if datastore is available
> * overall code improvements
> * ui: include model in partition selector
> * rebased onto master
>
> v9:
> * change mount point to `/mnt/datastore/<name>`
> * update "Directory" list UI
> * add `absolute_path()` from Dietmar's RFC
> * update docs
>
> v8:
> * still depends on [1]
> * paths for removable datastores are now relative to
> `/mnt/removable_datastore/<UUID>`
> * add support for creation of removable datastore through the
> "create directory" endpoint (last 3 patches)
> * update datastore creation UI
> * update docs
>
> v7:
> * depends on [1]
> * improve logging when waiting for tasks
> * drop `update-datatore-cache` refactoring
> * fix some commit messages
>
> [1] https://lists.proxmox.com/pipermail/pbs-devel/2024-April/008739.html
>
> v6:
> * remove 'drop' flag in datastore cache
> * use maintenance-mode 'unmount' for unmounting process, only for the
> unmounting not for being unmounted
> * rename/simplify update-datastore-cache command
> * ui: integrate new unmounting maintenance mode
> * basically a mix of v3 and v4
>
> v5: thanks @Dietmar and @Christian
> * drop --force for unmount since it'll always fail if tasks are still running, and if
> there are not normal unount will work
> * improve several commit messages
> * improve error message wording
> * add removable datastore section to docs
> * add documentation for is_datastore_available
>
> v4: thanks a lot @Dietmar and @Christian
> * make check if mounted wayyy faster
> * don't keep track of mounting state
> * drop Unplugged maintenance mode
> * use UUID_FORMAT for uuid field
> * a lot of small things, like use of bail!, inline format!, ...
> * include improvement to cache handling
>
> v3:
> * remove lazy unmounting (since 9cba51ac782d04085c0af55128f32178e5132358 is applied)
> * fix CLI (un)mount command, thanks @Gabriel
> * add removable datastore CLI autocomplete helper
> * rebase onto master
> * move ui patches to the end
>
> thanks @Lukas and @Thomas for the feedback
> v2:
> * fix datastore 'add' button in the UI
> * some format!("{}", a) -> format!("{a}")
> * replace `const` with `let` in js code
> * change icon `fa-usb` -> `fa-plug`
> * add some docs
> * add JDoc for parseMaintenanceMode
> * proxmox-schema dep bump
>
> Dietmar Maurer (1):
> maintenance: add 'Unmount' maintenance type
>
> Hannes Laimer (25):
> pbs-api-types: add backing-device to DataStoreConfig
> maintenance: make is_offline more generic
> datastore: add helper for checking if a datastore is mounted
> api: admin: add (un)mount endpoint for removable datastores
> api: removable datastore creation
> api: add check for nested datastores on creation
> pbs-api-types: add mount_status field to DataStoreListItem
> bin: manager: add (un)mount command
> add auto-mounting for removable datastores
> datastore: handle deletion of removable datastore properly
> docs: add removable datastores section
> ui: add partition selector form
> ui: add removable datastore creation support
> ui: add (un)mount button to summary
> ui: tree: render unmounted datastores correctly
> ui: utils: make parseMaintenanceMode more robust
> ui: add datastore status mask for unmounted removable datastores
> ui: maintenance: fix disable msg field if no type is selected
> ui: render 'unmount' maintenance mode correctly
> api: node: allow creation of removable datastore through directory
> endpoint
> api: node: include removable datastores in directory list
> node: disks: replace BASE_MOUNT_DIR with DATASTORE_MOUNT_DIR
> ui: support create removable datastore through directory creation
> bin: debug: add inspect device command
> api: disks: only return UUID of partitions if it actually is one
>
> debian/proxmox-backup-server.install | 1 +
> debian/proxmox-backup-server.udev | 3 +
> docs/storage.rst | 38 +++
> etc/Makefile | 1 +
> etc/removable-device-attach@.service | 8 +
> pbs-api-types/src/datastore.rs | 47 ++-
> pbs-api-types/src/maintenance.rs | 12 +-
> pbs-config/src/datastore.rs | 14 +
> pbs-datastore/src/datastore.rs | 83 +++++-
> pbs-datastore/src/lib.rs | 4 +-
> src/api2/admin/datastore.rs | 310 +++++++++++++++++++-
> src/api2/config/datastore.rs | 107 ++++++-
> src/api2/node/disks/directory.rs | 74 +++--
> src/api2/status/mod.rs | 30 +-
> src/bin/proxmox_backup_debug/inspect.rs | 149 ++++++++++
> src/bin/proxmox_backup_manager/datastore.rs | 127 +++++++-
> src/server/metric_collection/mod.rs | 4 +
> src/tools/disks/mod.rs | 5 +-
> www/DirectoryList.js | 13 +
> www/Makefile | 1 +
> www/NavigationTree.js | 17 +-
> www/Utils.js | 33 ++-
> www/css/ext6-pbs.css | 20 ++
> www/datastore/DataStoreListSummary.js | 1 +
> www/datastore/Summary.js | 115 +++++++-
> www/form/PartitionSelector.js | 81 +++++
> www/window/CreateDirectory.js | 14 +
> www/window/DataStoreEdit.js | 37 +++
> www/window/MaintenanceOptions.js | 17 +-
> 29 files changed, 1274 insertions(+), 92 deletions(-)
> create mode 100644 etc/removable-device-attach@.service
> create mode 100644 www/form/PartitionSelector.js
>
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores
2024-11-22 14:46 [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
` (26 preceding siblings ...)
2024-11-25 6:19 ` [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores Hannes Laimer
@ 2024-11-25 13:15 ` Lukas Wagner
2024-11-25 13:32 ` Dominik Csapak
27 siblings, 1 reply; 45+ messages in thread
From: Lukas Wagner @ 2024-11-25 13:15 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
On 2024-11-22 15:46, Hannes Laimer wrote:
> These patches add support for removable datastores. All removable
> datastores have a backing-device(a UUID) associated with them. Removable
> datastores work like normal ones, just that they can be unplugged. It is
> possible to create a removable datastore, sync backups onto it, unplug
> it and use it on a different PBS.
>
> The datastore path specified is relative to the root of the used device.
> Removable datastores are bind mounted to /mnt/datastore/<NAME>.
> Multiple datastores can be created on a single device, but only device with
> a single datastore on them will be auto-mounted.
>
> When a removable datastore is deleted and 'destroy-data' is set, the
> device has to be mounted. If 'destroy-data' is not set the datastore
> can be deleted even if the device is not present. Removable datastores
> are automatically mounted when plugged in.
>
Tested these patches against the latest state on master. Works fine generally,
but there were a couple of rough edges that were at least unexpected (at least to me, not being
very familiar with this new feature)
In no particular order, they were:
- When creating a removable datastore, it struck me as odd that the "On device path" has to be absolute
(start with a /), considering that the path you enter is relative to the root directory of the
partition.
- a removable datastore cannot be ZFS. I guess there are technical reasons for this, but it was a
bit unexpected, especially considering that you can create a non-removable datastore on ZFS without
any issues. Maybe the docs could give some background on this, or at least mention that
removable datastores cannot be created on ZFS formatted disks
- ran into an issue with the 'unmounting' maintenance mode. I pressed 'unmount' in the UI.
One of my bash session still had its working directory in the mounted datastore, leading to umount failing
with a 'device busy' message. After this, the datastore was in the 'unmounting' maintenance mode
which I could not clear anymore, neither through the UI, nor through the proxmox-backup-manager
CLI tool. Eventually I had to remove the `maintenance-mode` line from `datastores.cfg` by hand.
- When a datastore is unmounted, accessing the GC job view or content view, the system logs are spammed by HTTP 400 error messages, e.g.
GET /api2/json/admin/datastore/disk2/status: 400 Bad Request: [client [::xxxxxxxxx]:34382] datastore 'disk2' is not mounted
Might be annoying and/or confusing for system admins looking through the logs when they debug some other issue.
- Similarly, if a removeable-datastore is added as a storage in PVE, pvestatd logs errors every 10 seconds if the
datastore is not mounted. Not sure if this is possible, but maybe we could handle this more graceful to not
spam the logs?
- Something that also was a bit confusing was the following:
- Attached new disk to my PBS test VM
- Formatted the disk as ext4 using the webui (name: disk1, device: sdb1), opting to NOT create a datastore as part of the formatting process
- Created a removable datastore on the new partition (name: disk1-store, device: sdb1)
-> This led to the partition being mounted twice:
/dev/sdb1 on /mnt/datastore/disk1 type ext4 (rw,relatime)
/dev/sdb1 on /mnt/datastore/disk1-store type ext4 (rw,relatime)
-> if 'unmount' is pressed for the datastore, only the second mount is unmounted. This could be confusing
to users who expect to be able to safely remove the disk after unmounting the datastore.
- For contrast, if while creating the partition one opts to create the removable datastore immediately,
the partition is only mounted once.
- Also, when trying to mount a datastore without the disk being available, the error message in the task
log might not be super clear to some users:
TASK ERROR: mounting to tmp path failed: command "mount" "UUID=a264c664-9e0e-47ad-abf5-960e0aabfe0b" "/run/proxmox-backup/mount/5292c7d2-a54b-42f1-be87-88810b2e90cd" failed - status code: 1 - mount: /run/proxmox-backup/mount/5292c7d2-a54b-42f1-be87-88810b2e90cd: can't find UUID=a264c664-9e0e-47ad-abf5-960e0aabfe0b.)
Maybe we could add clearer error messages for the more common error cases, like disks being not available?
I don't think any of these are blockers, but nevertheless I thought I'd bring these up.
--
- Lukas
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v14 00/26] add removable datastores
2024-11-25 13:15 ` Lukas Wagner
@ 2024-11-25 13:32 ` Dominik Csapak
0 siblings, 0 replies; 45+ messages in thread
From: Dominik Csapak @ 2024-11-25 13:32 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Lukas Wagner,
Hannes Laimer
On 11/25/24 14:15, Lukas Wagner wrote:
> On 2024-11-22 15:46, Hannes Laimer wrote:
>> These patches add support for removable datastores. All removable
>> datastores have a backing-device(a UUID) associated with them. Removable
>> datastores work like normal ones, just that they can be unplugged. It is
>> possible to create a removable datastore, sync backups onto it, unplug
>> it and use it on a different PBS.
>>
>> The datastore path specified is relative to the root of the used device.
>> Removable datastores are bind mounted to /mnt/datastore/<NAME>.
>> Multiple datastores can be created on a single device, but only device with
>> a single datastore on them will be auto-mounted.
>>
>> When a removable datastore is deleted and 'destroy-data' is set, the
>> device has to be mounted. If 'destroy-data' is not set the datastore
>> can be deleted even if the device is not present. Removable datastores
>> are automatically mounted when plugged in.
>>
>
[snip]
>
> - Something that also was a bit confusing was the following:
> - Attached new disk to my PBS test VM
> - Formatted the disk as ext4 using the webui (name: disk1, device: sdb1), opting to NOT create a datastore as part of the formatting process
> - Created a removable datastore on the new partition (name: disk1-store, device: sdb1)
> -> This led to the partition being mounted twice:
>
> /dev/sdb1 on /mnt/datastore/disk1 type ext4 (rw,relatime)
> /dev/sdb1 on /mnt/datastore/disk1-store type ext4 (rw,relatime)
>
> -> if 'unmount' is pressed for the datastore, only the second mount is unmounted. This could be confusing
> to users who expect to be able to safely remove the disk after unmounting the datastore.
>
> - For contrast, if while creating the partition one opts to create the removable datastore immediately,
> the partition is only mounted once.
>
>
i had slightly different, but also confusing experience
did basically the same as Lukas, but my target path was the exact same as the already mounted disk
which lead to an 'it's already mounted' error
(which yeah was expected, i just created the disk)
in that case we could e.g. try to simply use the already mounted path for that?
(idk how much work checking if it's the right on, etc. that would involve though...)
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 45+ messages in thread