all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores
@ 2024-04-25  6:52 Hannes Laimer
  2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 01/26] tools: add disks utility functions Hannes Laimer
                   ` (27 more replies)
  0 siblings, 28 replies; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:52 UTC (permalink / raw)
  To: pbs-devel

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 `/mnt/removable_datastore/<name>`.

When a removable datastore is deleted and 'destroy-data' is set, the
device has to be mounted in. 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. 

 
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 (2):
  config: factor out method to get the absolute datastore path
  maintenance: add 'Unmount' maintenance type

Hannes Laimer (24):
  tools: add disks utility functions
  pbs-api-types: add backing-device to DataStoreConfig
  disks: add UUID to partition info
  datastore: add helper for checking if a removable datastore is
    available
  api: admin: add (un)mount endpoint for removable datastores
  api: removable datastore creation
  api: disks list: add exclude-used flag
  pbs-api-types: add removable/is-available flag 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

 debian/proxmox-backup-server.install        |   1 +
 debian/proxmox-backup-server.udev           |   3 +
 docs/storage.rst                            |  29 +++
 etc/Makefile                                |   3 +-
 etc/removable-device-attach@.service.in     |   8 +
 pbs-api-types/src/datastore.rs              |  42 +++-
 pbs-api-types/src/maintenance.rs            |  11 +-
 pbs-config/src/datastore.rs                 |  14 ++
 pbs-datastore/src/datastore.rs              |  79 +++++++-
 pbs-datastore/src/lib.rs                    |   2 +-
 src/api2/admin/datastore.rs                 | 203 ++++++++++++++++++--
 src/api2/config/datastore.rs                |  69 ++++++-
 src/api2/node/disks/directory.rs            | 112 +++++++++--
 src/api2/node/disks/mod.rs                  |   8 +
 src/api2/status.rs                          |  18 +-
 src/bin/proxmox-backup-proxy.rs             |   9 +-
 src/bin/proxmox_backup_manager/datastore.rs | 129 ++++++++++++-
 src/tools/disks/mod.rs                      |  89 ++++++++-
 www/DirectoryList.js                        |  13 ++
 www/Makefile                                |   1 +
 www/NavigationTree.js                       |  15 +-
 www/Utils.js                                |  33 +++-
 www/css/ext6-pbs.css                        |  20 ++
 www/datastore/DataStoreListSummary.js       |   1 +
 www/datastore/Summary.js                    | 113 ++++++++++-
 www/form/PartitionSelector.js               |  81 ++++++++
 www/window/CreateDirectory.js               |  14 ++
 www/window/DataStoreEdit.js                 |  37 ++++
 www/window/MaintenanceOptions.js            |  17 +-
 29 files changed, 1086 insertions(+), 88 deletions(-)
 create mode 100644 etc/removable-device-attach@.service.in
 create mode 100644 www/form/PartitionSelector.js

-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 01/26] tools: add disks utility functions
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
@ 2024-04-25  6:52 ` Hannes Laimer
  2024-05-03 11:33   ` Max Carrara
  2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 02/26] config: factor out method to get the absolute datastore path Hannes Laimer
                   ` (26 subsequent siblings)
  27 siblings, 1 reply; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:52 UTC (permalink / raw)
  To: pbs-devel

... for mounting and unmounting

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/tools/disks/mod.rs | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/src/tools/disks/mod.rs b/src/tools/disks/mod.rs
index 94f89e0a..303fe4f1 100644
--- a/src/tools/disks/mod.rs
+++ b/src/tools/disks/mod.rs
@@ -1323,3 +1323,22 @@ pub fn get_fs_uuid(disk: &Disk) -> Result<String, Error> {
 
     bail!("get_fs_uuid failed - missing UUID");
 }
+
+/// Mount a disk by its UUID and the mount point.
+pub fn mount_by_uuid(uuid: &str, mount_point: &Path) -> Result<(), Error> {
+    let mut command = std::process::Command::new("mount");
+    command.arg(&format!("UUID={uuid}"));
+    command.arg(mount_point);
+
+    proxmox_sys::command::run_command(command, None)?;
+    Ok(())
+}
+
+/// Unmount a disk by its mount point.
+pub fn unmount_by_mountpoint(path: &str) -> Result<(), Error> {
+    let mut command = std::process::Command::new("umount");
+    command.arg(path);
+
+    proxmox_sys::command::run_command(command, None)?;
+    Ok(())
+}
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 02/26] config: factor out method to get the absolute datastore path
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
  2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 01/26] tools: add disks utility functions Hannes Laimer
@ 2024-04-25  6:52 ` Hannes Laimer
  2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 03/26] pbs-api-types: add backing-device to DataStoreConfig Hannes Laimer
                   ` (25 subsequent siblings)
  27 siblings, 0 replies; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:52 UTC (permalink / raw)
  To: pbs-devel

From: Dietmar Maurer <dietmar@proxmox.com>

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 pbs-api-types/src/datastore.rs   |  5 +++++
 pbs-datastore/src/datastore.rs   | 11 +++++++----
 src/api2/node/disks/directory.rs |  4 ++--
 src/bin/proxmox-backup-proxy.rs  |  4 ++--
 4 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index 31767417..a5704c93 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -357,6 +357,11 @@ impl DataStoreConfig {
         }
     }
 
+    /// Returns the absolute path to the datastore content.
+    pub fn absolute_path(&self) -> String {
+        self.path.clone()
+    }
+
     pub fn get_maintenance_mode(&self) -> Option<MaintenanceMode> {
         self.maintenance_mode.as_ref().and_then(|str| {
             MaintenanceMode::deserialize(proxmox_schema::de::SchemaDeserializer::new(
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index f95da761..f9b986c1 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -185,7 +185,7 @@ impl DataStore {
             )?;
             Arc::new(ChunkStore::open(
                 name,
-                &config.path,
+                config.absolute_path(),
                 tuning.sync_level.unwrap_or_default(),
             )?)
         };
@@ -265,8 +265,11 @@ impl DataStore {
             DatastoreTuning::API_SCHEMA
                 .parse_property_string(config.tuning.as_deref().unwrap_or(""))?,
         )?;
-        let chunk_store =
-            ChunkStore::open(&name, &config.path, tuning.sync_level.unwrap_or_default())?;
+        let chunk_store = ChunkStore::open(
+            &name,
+            config.absolute_path(),
+            tuning.sync_level.unwrap_or_default(),
+        )?;
         let inner = Arc::new(Self::with_store_and_config(
             Arc::new(chunk_store),
             config,
@@ -1405,7 +1408,7 @@ impl DataStore {
             bail!("datastore is currently in use");
         }
 
-        let base = PathBuf::from(&datastore_config.path);
+        let base = PathBuf::from(datastore_config.absolute_path());
 
         let mut ok = true;
         if destroy_data {
diff --git a/src/api2/node/disks/directory.rs b/src/api2/node/disks/directory.rs
index 9f1112a9..73af92cc 100644
--- a/src/api2/node/disks/directory.rs
+++ b/src/api2/node/disks/directory.rs
@@ -251,12 +251,12 @@ pub fn delete_datastore_disk(name: String) -> Result<(), Error> {
     let (config, _) = pbs_config::datastore::config()?;
     let datastores: Vec<DataStoreConfig> = config.convert_to_typed_array("datastore")?;
     let conflicting_datastore: Option<DataStoreConfig> =
-        datastores.into_iter().find(|ds| ds.path == path);
+        datastores.into_iter().find(|ds| ds.absolute_path() == path);
 
     if let Some(conflicting_datastore) = conflicting_datastore {
         bail!(
             "Can't remove '{}' since it's required by datastore '{}'",
-            conflicting_datastore.path,
+            conflicting_datastore.absolute_path(),
             conflicting_datastore.name
         );
     }
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index 15444685..3a05b1f3 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -1143,8 +1143,8 @@ fn collect_disk_stats_sync() -> (DiskStat, Vec<DiskStat>) {
                 {
                     continue;
                 }
-                let path = std::path::Path::new(&config.path);
-                datastores.push(gather_disk_stats(disk_manager.clone(), path, &config.name));
+                let path = std::path::PathBuf::from(config.absolute_path());
+                datastores.push(gather_disk_stats(disk_manager.clone(), &path, &config.name));
             }
         }
         Err(err) => {
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 03/26] pbs-api-types: add backing-device to DataStoreConfig
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
  2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 01/26] tools: add disks utility functions Hannes Laimer
  2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 02/26] config: factor out method to get the absolute datastore path Hannes Laimer
@ 2024-04-25  6:52 ` Hannes Laimer
  2024-05-03 11:34   ` Max Carrara
  2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 04/26] maintenance: add 'Unmount' maintenance type Hannes Laimer
                   ` (24 subsequent siblings)
  27 siblings, 1 reply; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:52 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 pbs-api-types/src/datastore.rs | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index a5704c93..e5c5cfcf 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -160,6 +160,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")]
@@ -273,6 +276,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)]
@@ -320,6 +329,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]
@@ -354,12 +368,23 @@ 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 let Some(mount_point) = self.get_mount_point() {
+            format!("{mount_point}/{}", self.path.trim_matches('/'))
+        } else {
+            self.path.clone()
+        }
+    }
+
+    pub fn get_mount_point(&self) -> Option<String> {
+        self.backing_device
+            .is_some()
+            .then(|| format!("{DATASTORE_MOUNT_DIR}/{}", self.name))
     }
 
     pub fn get_maintenance_mode(&self) -> Option<MaintenanceMode> {
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 04/26] maintenance: add 'Unmount' maintenance type
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
                   ` (2 preceding siblings ...)
  2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 03/26] pbs-api-types: add backing-device to DataStoreConfig Hannes Laimer
@ 2024-04-25  6:52 ` Hannes Laimer
  2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 05/26] disks: add UUID to partition info Hannes Laimer
                   ` (23 subsequent siblings)
  27 siblings, 0 replies; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:52 UTC (permalink / raw)
  To: pbs-devel

From: Dietmar Maurer <dietmar@proxmox.com>

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
---
 pbs-api-types/src/datastore.rs   | 3 +++
 pbs-api-types/src/maintenance.rs | 5 ++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index e5c5cfcf..af914fa4 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -404,6 +404,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 1e3413dc..fd4d3416 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);
@@ -94,6 +95,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.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 05/26] disks: add UUID to partition info
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
                   ` (3 preceding siblings ...)
  2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 04/26] maintenance: add 'Unmount' maintenance type Hannes Laimer
@ 2024-04-25  6:52 ` Hannes Laimer
  2024-05-03 11:34   ` Max Carrara
  2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 06/26] datastore: add helper for checking if a removable datastore is available Hannes Laimer
                   ` (22 subsequent siblings)
  27 siblings, 1 reply; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:52 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/tools/disks/mod.rs | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/tools/disks/mod.rs b/src/tools/disks/mod.rs
index 303fe4f1..6227948d 100644
--- a/src/tools/disks/mod.rs
+++ b/src/tools/disks/mod.rs
@@ -59,6 +59,8 @@ pub struct LsblkInfo {
     /// File system label.
     #[serde(rename = "fstype")]
     file_system_type: Option<String>,
+    /// File system UUID.
+    uuid: Option<String>,
 }
 
 impl DiskManage {
@@ -617,7 +619,7 @@ pub struct BlockDevStat {
 /// Use lsblk to read partition type uuids and file system types.
 pub fn get_lsblk_info() -> Result<Vec<LsblkInfo>, Error> {
     let mut command = std::process::Command::new("lsblk");
-    command.args(["--json", "-o", "path,parttype,fstype"]);
+    command.args(["--json", "-o", "path,parttype,fstype,uuid"]);
 
     let output = proxmox_sys::command::run_command(command, None)?;
 
@@ -701,6 +703,8 @@ pub struct PartitionInfo {
     pub size: Option<u64>,
     /// GPT partition
     pub gpt: bool,
+    /// UUID
+    pub uuid: Option<String>,
 }
 
 #[api(
@@ -891,8 +895,10 @@ fn get_partitions_info(
 
             let mounted = disk.is_mounted().unwrap_or(false);
             let mut filesystem = None;
+            let mut uuid = None;
             if let (Some(devpath), Some(infos)) = (devpath.as_ref(), lsblk_infos.as_ref()) {
                 for info in infos.iter().filter(|i| i.path.eq(devpath)) {
+                    uuid = info.uuid.clone();
                     used = match info.partition_type.as_deref() {
                         Some("21686148-6449-6e6f-744e-656564454649") => PartitionUsageType::BIOS,
                         Some("c12a7328-f81f-11d2-ba4b-00a0c93ec93b") => PartitionUsageType::EFI,
@@ -915,6 +921,7 @@ fn get_partitions_info(
                 filesystem,
                 size: disk.size().ok(),
                 gpt: disk.has_gpt(),
+                uuid,
             }
         })
         .collect()
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 06/26] datastore: add helper for checking if a removable datastore is available
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
                   ` (4 preceding siblings ...)
  2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 05/26] disks: add UUID to partition info Hannes Laimer
@ 2024-04-25  6:52 ` Hannes Laimer
  2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 07/26] api: admin: add (un)mount endpoint for removable datastores Hannes Laimer
                   ` (21 subsequent siblings)
  27 siblings, 0 replies; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:52 UTC (permalink / raw)
  To: pbs-devel

Co-authored-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 pbs-api-types/src/maintenance.rs |  2 +-
 pbs-datastore/src/datastore.rs   | 58 ++++++++++++++++++++++++++++++++
 pbs-datastore/src/lib.rs         |  2 +-
 src/bin/proxmox-backup-proxy.rs  |  5 ++-
 4 files changed, 64 insertions(+), 3 deletions(-)

diff --git a/pbs-api-types/src/maintenance.rs b/pbs-api-types/src/maintenance.rs
index fd4d3416..4f653ec7 100644
--- a/pbs-api-types/src/maintenance.rs
+++ b/pbs-api-types/src/maintenance.rs
@@ -81,7 +81,7 @@ 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
+        self.ty == MaintenanceType::Offline || self.ty == MaintenanceType::Unmount
     }
 
     pub fn check(&self, operation: Option<Operation>) -> Result<(), Error> {
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index f9b986c1..4f7865fb 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, 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_sys::WorkerTaskContext;
 use proxmox_sys::{task_log, task_warn};
@@ -49,6 +51,52 @@ pub fn check_backup_owner(owner: &Authid, auth_id: &Authid) -> Result<(), Error>
     Ok(())
 }
 
+/// check if a removable datastore is currently available/mounted by
+/// comparing the `st_rdev` values of `/dev/disk/by-uuid/<uuid>` and the source device in
+/// /proc/self/mountinfo
+pub fn is_datastore_available(config: &DataStoreConfig) -> bool {
+    use nix::sys::stat::SFlag;
+
+    let uuid = match config.backing_device.as_deref() {
+        Some(dev) => dev,
+        None => return true,
+    };
+
+    let Some(store_mount_point) = config.get_mount_point() else {
+        return false;
+    };
+    let store_mount_point = Path::new(&store_mount_point);
+
+    let dev_node = match nix::sys::stat::stat(format!("/dev/disk/by-uuid/{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
+}
+
 /// Datastore Management
 ///
 /// A Datastore can store severals backups, and provides the
@@ -158,6 +206,12 @@ impl DataStore {
             }
         }
 
+        if config.backing_device.is_some() && !is_datastore_available(&config) {
+            let mut datastore_cache = DATASTORE_MAP.lock().unwrap();
+            datastore_cache.remove(&config.name);
+            bail!("Removable Datastore is not mounted");
+        }
+
         if let Some(operation) = operation {
             update_active_operations(name, operation, 1)?;
         }
@@ -261,6 +315,10 @@ impl DataStore {
     ) -> Result<Arc<Self>, Error> {
         let name = config.name.clone();
 
+        if !is_datastore_available(&config) {
+            bail!("Datastore is not available")
+        }
+
         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 43050162..458f93d9 100644
--- a/pbs-datastore/src/lib.rs
+++ b/pbs-datastore/src/lib.rs
@@ -206,7 +206,7 @@ pub use manifest::BackupManifest;
 pub use store_progress::StoreProgress;
 
 mod datastore;
-pub use datastore::{check_backup_owner, DataStore};
+pub use datastore::{check_backup_owner, is_datastore_available, DataStore};
 
 mod hierarchy;
 pub use hierarchy::{
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index 3a05b1f3..9c7a472d 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -20,7 +20,7 @@ use proxmox_sys::linux::procfs::{Loadavg, ProcFsMemInfo, ProcFsNetDev, ProcFsSta
 use proxmox_sys::logrotate::LogRotate;
 use proxmox_sys::{task_log, task_warn};
 
-use pbs_datastore::DataStore;
+use pbs_datastore::{is_datastore_available, DataStore};
 
 use proxmox_rest_server::{
     cleanup_old_tasks, cookie_from_header, rotate_task_log_archive, ApiConfig, Redirector,
@@ -1143,6 +1143,9 @@ fn collect_disk_stats_sync() -> (DiskStat, Vec<DiskStat>) {
                 {
                     continue;
                 }
+                if !is_datastore_available(&config) {
+                    continue;
+                }
                 let path = std::path::PathBuf::from(config.absolute_path());
                 datastores.push(gather_disk_stats(disk_manager.clone(), &path, &config.name));
             }
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 07/26] api: admin: add (un)mount endpoint for removable datastores
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
                   ` (5 preceding siblings ...)
  2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 06/26] datastore: add helper for checking if a removable datastore is available Hannes Laimer
@ 2024-04-25  6:52 ` Hannes Laimer
  2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 08/26] api: removable datastore creation Hannes Laimer
                   ` (20 subsequent siblings)
  27 siblings, 0 replies; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:52 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 pbs-api-types/src/maintenance.rs |   4 +
 src/api2/admin/datastore.rs      | 186 +++++++++++++++++++++++++++++--
 2 files changed, 180 insertions(+), 10 deletions(-)

diff --git a/pbs-api-types/src/maintenance.rs b/pbs-api-types/src/maintenance.rs
index 4f653ec7..469dc25d 100644
--- a/pbs-api-types/src/maintenance.rs
+++ b/pbs-api-types/src/maintenance.rs
@@ -78,6 +78,10 @@ pub struct MaintenanceMode {
 }
 
 impl MaintenanceMode {
+    pub fn new(ty: MaintenanceType, message: Option<String>) -> Self {
+        Self { ty, message }
+    }
+
     /// 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 {
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 3ea17499..327ffffc 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -26,7 +26,7 @@ use proxmox_sortable_macro::sortable;
 use proxmox_sys::fs::{
     file_read_firstline, file_read_optional_string, replace_file, CreateOptions,
 };
-use proxmox_sys::{task_log, task_warn};
+use proxmox_sys::{task_log, task_warn, WorkerTaskContext};
 use proxmox_time::CalendarEvent;
 
 use pxar::accessor::aio::Accessor;
@@ -35,13 +35,13 @@ use pxar::EntryKind;
 use pbs_api_types::{
     print_ns_and_snapshot, print_store_and_ns, Authid, BackupContent, BackupNamespace, BackupType,
     Counts, CryptMode, DataStoreConfig, DataStoreListItem, DataStoreStatus,
-    GarbageCollectionJobStatus, GroupListItem, JobScheduleStatus, KeepOptions, Operation,
-    PruneJobOptions, RRDMode, RRDTimeFrame, SnapshotListItem, SnapshotVerifyState,
-    BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA, BACKUP_TIME_SCHEMA,
-    BACKUP_TYPE_SCHEMA, DATASTORE_SCHEMA, IGNORE_VERIFIED_BACKUPS_SCHEMA, 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,
+    GarbageCollectionJobStatus, GroupListItem, JobScheduleStatus, KeepOptions, MaintenanceMode,
+    MaintenanceType, Operation, PruneJobOptions, RRDMode, RRDTimeFrame, SnapshotListItem,
+    SnapshotVerifyState, BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA,
+    BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA, DATASTORE_SCHEMA, IGNORE_VERIFIED_BACKUPS_SCHEMA,
+    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;
@@ -56,8 +56,8 @@ use pbs_datastore::index::IndexFile;
 use pbs_datastore::manifest::{BackupManifest, CLIENT_LOG_BLOB_NAME, MANIFEST_BLOB_NAME};
 use pbs_datastore::prune::compute_prune_info;
 use pbs_datastore::{
-    check_backup_owner, task_tracking, BackupDir, BackupGroup, DataStore, LocalChunkReader,
-    StoreProgress, CATALOG_NAME,
+    check_backup_owner, is_datastore_available, task_tracking, BackupDir, BackupGroup, DataStore,
+    LocalChunkReader, StoreProgress, CATALOG_NAME,
 };
 use pbs_tools::json::required_string_param;
 use proxmox_rest_server::{formatter, WorkerTask};
@@ -2330,6 +2330,170 @@ pub async fn set_backup_owner(
     .await?
 }
 
+pub fn do_mount_device(
+    datastore: DataStoreConfig,
+    worker: Option<&dyn WorkerTaskContext>,
+) -> Result<(), Error> {
+    if let (Some(uuid), Some(mount_point)) = (
+        datastore.backing_device.as_ref(),
+        datastore.get_mount_point(),
+    ) {
+        if pbs_datastore::is_datastore_available(&datastore) {
+            bail!("device '{uuid}' is already mounted");
+        }
+        let mount_point_path = std::path::Path::new(&mount_point);
+        if let Some(worker) = worker {
+            task_log!(
+                worker,
+                "mounting '{uuid}' for store {} to '{mount_point}'",
+                datastore.name,
+            );
+        }
+        crate::tools::disks::mount_by_uuid(uuid, mount_point_path)?;
+
+        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, Some(&worker)),
+    )?;
+
+    Ok(json!(upid))
+}
+
+fn do_unmount_device(
+    datastore: DataStoreConfig,
+    worker: Option<&dyn WorkerTaskContext>,
+) -> Result<(), Error> {
+    let mut active_operations = task_tracking::get_active_operations(&datastore.name)?;
+    let mut old_status = String::new();
+    while active_operations.read + active_operations.write > 0 {
+        if let Some(worker) = worker {
+            if worker.abort_requested() {
+                bail!("aborted, due to user request");
+            }
+            let status = format!(
+                "cannot unmount yet, still {} read and {} write operations active",
+                active_operations.read, active_operations.write
+            );
+            if status != old_status {
+                task_log!(worker, "{status}");
+                old_status = status;
+            }
+        }
+        std::thread::sleep(std::time::Duration::from_millis(250));
+        active_operations = task_tracking::get_active_operations(&datastore.name)?;
+    }
+    if let Some(mount_point) = datastore.get_mount_point() {
+        crate::tools::disks::unmount_by_mountpoint(&mount_point)?;
+
+        let _lock = pbs_config::datastore::lock_config()?;
+        let (mut section_config, _digest) = pbs_config::datastore::config()?;
+        let mut store_config: DataStoreConfig =
+            section_config.lookup("datastore", &datastore.name)?;
+        store_config.maintenance_mode = None;
+        section_config.set_data(&datastore.name, "datastore", &store_config)?;
+        pbs_config::datastore::save_config(&section_config)?;
+    }
+    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");
+    }
+
+    if !is_datastore_available(&datastore) {
+        bail!("datastore '{store}' is not mounted");
+    }
+
+    datastore.set_maintenance_mode(Some(MaintenanceMode::new(MaintenanceType::Unmount, None)))?;
+    section_config.set_data(&store, "datastore", &datastore)?;
+    pbs_config::datastore::save_config(&section_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_rest_server::ctrl_sock_from_pid(proxy_pid);
+        let _ = proxmox_rest_server::send_raw_command(
+            sock,
+            &format!("{{\"command\":\"update-datastore-cache\",\"args\":\"{store}\"}}\n"),
+        )
+        .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 = &[
     (
@@ -2368,6 +2532,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?!
@@ -2402,6 +2567,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.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 08/26] api: removable datastore creation
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
                   ` (6 preceding siblings ...)
  2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 07/26] api: admin: add (un)mount endpoint for removable datastores Hannes Laimer
@ 2024-04-25  6:52 ` Hannes Laimer
  2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 09/26] api: disks list: add exclude-used flag Hannes Laimer
                   ` (19 subsequent siblings)
  27 siblings, 0 replies; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:52 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/config/datastore.rs | 53 +++++++++++++++++++++++++++++++++---
 1 file changed, 49 insertions(+), 4 deletions(-)

diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 6b742acb..401fbccf 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -8,7 +8,7 @@ use serde_json::Value;
 use proxmox_router::{http_bail, Permission, Router, RpcEnvironment, RpcEnvironmentType};
 use proxmox_schema::{api, param_bail, ApiType};
 use proxmox_section_config::SectionConfigData;
-use proxmox_sys::{task_warn, WorkerTaskContext};
+use proxmox_sys::{task_log, task_warn, WorkerTaskContext};
 use proxmox_uuid::Uuid;
 
 use pbs_api_types::{
@@ -20,7 +20,8 @@ use pbs_config::BackupLockGuard;
 use pbs_datastore::chunk_store::ChunkStore;
 
 use crate::api2::admin::{
-    prune::list_prune_jobs, sync::list_sync_jobs, verify::list_verification_jobs,
+    datastore::do_mount_device, prune::list_prune_jobs, sync::list_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;
@@ -72,13 +73,35 @@ pub(crate) fn do_create_datastore(
     datastore: DataStoreConfig,
     worker: Option<&dyn WorkerTaskContext>,
 ) -> Result<(), Error> {
-    let path: PathBuf = datastore.path.clone().into();
+    let path: PathBuf = datastore.absolute_path().into();
+    let backup_user = pbs_config::backup_user()?;
+    if let Some(store_mount_point) = datastore.get_mount_point() {
+        let default_options = proxmox_sys::fs::CreateOptions::new();
+        proxmox_sys::fs::create_path(
+            store_mount_point,
+            Some(default_options.clone()),
+            Some(default_options.clone()),
+        )?;
+        do_mount_device(datastore.clone(), worker)?;
+
+        if path.join(".chunks").is_dir() {
+            config.set_data(&datastore.name, "datastore", &datastore)?;
+            pbs_config::datastore::save_config(&config)?;
+            jobstate::create_state_file("garbage_collection", &datastore.name)?;
+            if let Some(worker) = worker {
+                task_log!(
+                    worker,
+                    "created removable datastore, chunkstore already exists"
+                );
+            }
+            return Ok(());
+        }
+    }
 
     let tuning: DatastoreTuning = serde_json::from_value(
         DatastoreTuning::API_SCHEMA
             .parse_property_string(datastore.tuning.as_deref().unwrap_or(""))?,
     )?;
-    let backup_user = pbs_config::backup_user()?;
     let _store = ChunkStore::create(
         &datastore.name,
         path,
@@ -122,6 +145,28 @@ pub fn create_datastore(
         param_bail!("name", "datastore '{}' already exists.", config.name);
     }
 
+    if let Some(uuid) = &config.backing_device {
+        let already_used_by = section_config
+            .sections
+            .iter()
+            .flat_map(|(datastore_name, (_, config))| {
+                config
+                    .as_object()
+                    .and_then(|cfg| cfg.get("backing-device"))
+                    .and_then(|backing_device| backing_device.as_str())
+                    .filter(|&device_uuid| device_uuid == uuid)
+                    .map(|_| datastore_name)
+            })
+            .next();
+
+        if let Some(datastore_name) = already_used_by {
+            param_bail!(
+                "backing-device",
+                "device already in use by datastore '{datastore_name}'",
+            );
+        }
+    }
+
     let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
     let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
 
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 09/26] api: disks list: add exclude-used flag
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
                   ` (7 preceding siblings ...)
  2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 08/26] api: removable datastore creation Hannes Laimer
@ 2024-04-25  6:52 ` Hannes Laimer
  2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 10/26] pbs-api-types: add removable/is-available flag to DataStoreListItem Hannes Laimer
                   ` (18 subsequent siblings)
  27 siblings, 0 replies; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:52 UTC (permalink / raw)
  To: pbs-devel

... used by the partition selector for removable datastore creation.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/node/disks/mod.rs |  8 +++++
 src/tools/disks/mod.rs     | 61 ++++++++++++++++++++++++++++++++------
 2 files changed, 60 insertions(+), 9 deletions(-)

diff --git a/src/api2/node/disks/mod.rs b/src/api2/node/disks/mod.rs
index 711dae7b..f63bbd9b 100644
--- a/src/api2/node/disks/mod.rs
+++ b/src/api2/node/disks/mod.rs
@@ -41,6 +41,12 @@ pub mod zfs;
                 optional: true,
                 default: false,
             },
+            "exclude-used": {
+                description: "Exclude partitions already used for removable datastores or mounted directories.",
+                type: bool,
+                optional: true,
+                default: false,
+            },
             "usage-type": {
                 type: DiskUsageType,
                 optional: true,
@@ -62,6 +68,7 @@ pub mod zfs;
 pub fn list_disks(
     skipsmart: bool,
     include_partitions: bool,
+    exclude_used: bool,
     usage_type: Option<DiskUsageType>,
 ) -> Result<Vec<DiskUsageInfo>, Error> {
     let mut list = Vec::new();
@@ -69,6 +76,7 @@ pub fn list_disks(
     for (_, info) in DiskUsageQuery::new()
         .smart(!skipsmart)
         .partitions(include_partitions)
+        .exclude_used(exclude_used)
         .query()?
     {
         if let Some(ref usage_type) = usage_type {
diff --git a/src/tools/disks/mod.rs b/src/tools/disks/mod.rs
index 6227948d..a8018834 100644
--- a/src/tools/disks/mod.rs
+++ b/src/tools/disks/mod.rs
@@ -20,6 +20,7 @@ use proxmox_rest_server::WorkerTask;
 use proxmox_schema::api;
 use proxmox_sys::linux::procfs::{mountinfo::Device, MountInfo};
 use proxmox_sys::task_log;
+use serde_json::Value;
 
 use pbs_api_types::{BLOCKDEVICE_DISK_AND_PARTITION_NAME_REGEX, BLOCKDEVICE_NAME_REGEX};
 
@@ -32,6 +33,7 @@ pub use zpool_list::*;
 mod lvm;
 pub use lvm::*;
 mod smart;
+use crate::api2::node::disks::directory::list_datastore_mounts;
 pub use smart::*;
 
 lazy_static::lazy_static! {
@@ -828,6 +830,7 @@ fn scan_partitions(
 pub struct DiskUsageQuery {
     smart: bool,
     partitions: bool,
+    exclude_used: bool,
 }
 
 impl DiskUsageQuery {
@@ -835,6 +838,7 @@ impl DiskUsageQuery {
         Self {
             smart: true,
             partitions: false,
+            exclude_used: false,
         }
     }
 
@@ -848,12 +852,22 @@ impl DiskUsageQuery {
         self
     }
 
+    pub fn exclude_used(&mut self, exclude_used: bool) -> &mut Self {
+        self.exclude_used = exclude_used;
+        self
+    }
+
     pub fn query(&self) -> Result<HashMap<String, DiskUsageInfo>, Error> {
-        get_disks(None, !self.smart, self.partitions)
+        get_disks(None, !self.smart, self.partitions, self.exclude_used)
     }
 
     pub fn find(&self, disk: &str) -> Result<DiskUsageInfo, Error> {
-        let mut map = get_disks(Some(vec![disk.to_string()]), !self.smart, self.partitions)?;
+        let mut map = get_disks(
+            Some(vec![disk.to_string()]),
+            !self.smart,
+            self.partitions,
+            self.exclude_used,
+        )?;
         if let Some(info) = map.remove(disk) {
             Ok(info)
         } else {
@@ -862,7 +876,7 @@ impl DiskUsageQuery {
     }
 
     pub fn find_all(&self, disks: Vec<String>) -> Result<HashMap<String, DiskUsageInfo>, Error> {
-        get_disks(Some(disks), !self.smart, self.partitions)
+        get_disks(Some(disks), !self.smart, self.partitions, self.exclude_used)
     }
 }
 
@@ -935,6 +949,8 @@ fn get_disks(
     no_smart: bool,
     // include partitions
     include_partitions: bool,
+    // skip partitions which are in use
+    exclude_used: bool,
 ) -> Result<HashMap<String, DiskUsageInfo>, Error> {
     let disk_manager = DiskManage::new();
 
@@ -952,6 +968,31 @@ fn get_disks(
 
     // fixme: ceph journals/volumes
 
+    let uuids_in_use = if exclude_used && include_partitions {
+        let (config, _digest) = pbs_config::datastore::config()?;
+
+        let uuids_from_datastores: Vec<String> = config
+            .sections
+            .iter()
+            .filter_map(|(_, (_, data))| {
+                data.as_object()
+                    .and_then(|cfg| cfg.get("backing-device"))
+                    .and_then(Value::as_str)
+                    .map(String::from)
+            })
+            .collect();
+
+        let uuids_from_mounts: Vec<String> = list_datastore_mounts()?
+            .into_iter()
+            // FIXME: include UUID in DatastoreMountInfo
+            .filter_map(|mount| mount.device.split('/').last().map(String::from))
+            .collect();
+
+        [&uuids_from_datastores[..], &uuids_from_mounts[..]].concat()
+    } else {
+        Vec::new()
+    };
+
     let mut result = HashMap::new();
 
     for item in proxmox_sys::fs::scan_subdir(libc::AT_FDCWD, "/sys/block", &BLOCKDEVICE_NAME_REGEX)?
@@ -1024,12 +1065,14 @@ fn get_disks(
 
         let partitions: Option<Vec<PartitionInfo>> = if include_partitions {
             disk.partitions().map_or(None, |parts| {
-                Some(get_partitions_info(
-                    parts,
-                    &lvm_devices,
-                    &zfs_devices,
-                    &file_system_devices,
-                ))
+                Some(
+                    get_partitions_info(parts, &lvm_devices, &zfs_devices, &file_system_devices)
+                        .into_iter()
+                        .filter(|part| {
+                            !part.uuid.as_ref().is_some_and(|u| uuids_in_use.contains(u))
+                        })
+                        .collect(),
+                )
             })
         } else {
             None
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 10/26] pbs-api-types: add removable/is-available flag to DataStoreListItem
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
                   ` (8 preceding siblings ...)
  2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 09/26] api: disks list: add exclude-used flag Hannes Laimer
@ 2024-04-25  6:52 ` Hannes Laimer
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 11/26] bin: manager: add (un)mount command Hannes Laimer
                   ` (17 subsequent siblings)
  27 siblings, 0 replies; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:52 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 pbs-api-types/src/datastore.rs |  9 ++++++++-
 src/api2/admin/datastore.rs    | 17 +++++++++--------
 src/api2/status.rs             | 18 +++++++++++++++---
 3 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index af914fa4..0ad59b15 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -454,6 +454,10 @@ impl DataStoreConfig {
 pub struct DataStoreListItem {
     pub store: String,
     pub comment: Option<String>,
+    /// Datastore is removable
+    pub removable: bool,
+    /// Datastore is available
+    pub available: bool,
     /// If the datastore is in maintenance mode, information about it
     #[serde(skip_serializing_if = "Option::is_none")]
     pub maintenance: Option<String>,
@@ -1453,6 +1457,8 @@ pub struct DataStoreStatusListItem {
     /// The available bytes of the underlying storage. (-1 on error)
     #[serde(skip_serializing_if = "Option::is_none")]
     pub avail: Option<u64>,
+    /// The datastore is available, relevant if removable
+    pub is_available: bool,
     /// A list of usages of the past (last Month).
     #[serde(skip_serializing_if = "Option::is_none")]
     pub history: Option<Vec<Option<f64>>>,
@@ -1477,12 +1483,13 @@ pub struct DataStoreStatusListItem {
 }
 
 impl DataStoreStatusListItem {
-    pub fn empty(store: &str, err: Option<String>) -> Self {
+    pub fn empty(store: &str, err: Option<String>, is_available: bool) -> Self {
         DataStoreStatusListItem {
             store: store.to_owned(),
             total: None,
             used: None,
             avail: None,
+            is_available,
             history: None,
             history_start: None,
             history_delta: None,
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 327ffffc..df181ead 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -1313,8 +1313,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;
 
@@ -1325,15 +1325,16 @@ pub fn get_datastore_list(
             }
         }
 
+        let store_config: DataStoreConfig = serde_json::from_value(data)?;
+        let is_available = pbs_datastore::is_datastore_available(&store_config);
+
         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),
+                removable: store_config.backing_device.is_some(),
+                available: is_available,
+                maintenance: store_config.maintenance_mode,
             });
         }
     }
diff --git a/src/api2/status.rs b/src/api2/status.rs
index 78bc06b5..e1d33ccf 100644
--- a/src/api2/status.rs
+++ b/src/api2/status.rs
@@ -13,7 +13,7 @@ use pbs_api_types::{
 };
 
 use pbs_config::CachedUserInfo;
-use pbs_datastore::DataStore;
+use pbs_datastore::{is_datastore_available, DataStore};
 
 use crate::rrd_cache::extract_rrd_data;
 use crate::tools::statistics::linear_regression;
@@ -48,10 +48,17 @@ 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("datastore", store)?;
+        if !is_datastore_available(&store_config) {
+            list.push(DataStoreStatusListItem::empty(store, None, false));
+            continue;
+        }
+
         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, true));
                 }
             }
             continue;
@@ -60,7 +67,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()),
+                    true,
+                ));
                 continue;
             }
         };
@@ -71,6 +82,7 @@ pub async fn datastore_status(
             total: Some(status.total),
             used: Some(status.used),
             avail: Some(status.available),
+            is_available: true,
             history: None,
             history_start: None,
             history_delta: None,
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 11/26] bin: manager: add (un)mount command
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
                   ` (9 preceding siblings ...)
  2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 10/26] pbs-api-types: add removable/is-available flag to DataStoreListItem Hannes Laimer
@ 2024-04-25  6:53 ` Hannes Laimer
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 12/26] add auto-mounting for removable datastores Hannes Laimer
                   ` (16 subsequent siblings)
  27 siblings, 0 replies; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:53 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 pbs-config/src/datastore.rs                 | 14 ++++
 src/bin/proxmox_backup_manager/datastore.rs | 76 ++++++++++++++++++++-
 2 files changed, 89 insertions(+), 1 deletion(-)

diff --git a/pbs-config/src/datastore.rs b/pbs-config/src/datastore.rs
index 5844a174..a540788f 100644
--- a/pbs-config/src/datastore.rs
+++ b/pbs-config/src/datastore.rs
@@ -63,6 +63,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 383bcd24..f2795b39 100644
--- a/src/bin/proxmox_backup_manager/datastore.rs
+++ b/src/bin/proxmox_backup_manager/datastore.rs
@@ -1,4 +1,4 @@
-use anyhow::Error;
+use anyhow::{format_err, Error};
 use serde_json::Value;
 
 use proxmox_router::{cli::*, ApiHandler, RpcEnvironment};
@@ -40,6 +40,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: {
@@ -99,6 +127,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: {
@@ -142,6 +198,15 @@ async fn delete_datastore(mut param: Value, rpcenv: &mut dyn RpcEnvironment) ->
 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)
@@ -152,6 +217,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(&api2::config::datastore::API_METHOD_UPDATE_DATASTORE)
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 12/26] add auto-mounting for removable datastores
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
                   ` (10 preceding siblings ...)
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 11/26] bin: manager: add (un)mount command Hannes Laimer
@ 2024-04-25  6:53 ` Hannes Laimer
  2024-05-03 11:34   ` Max Carrara
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 13/26] datastore: handle deletion of removable datastore properly Hannes Laimer
                   ` (15 subsequent siblings)
  27 siblings, 1 reply; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:53 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 debian/proxmox-backup-server.install        |  1 +
 debian/proxmox-backup-server.udev           |  3 ++
 etc/Makefile                                |  3 +-
 etc/removable-device-attach@.service.in     |  8 +++
 src/bin/proxmox_backup_manager/datastore.rs | 55 ++++++++++++++++++++-
 5 files changed, 68 insertions(+), 2 deletions(-)
 create mode 100644 etc/removable-device-attach@.service.in

diff --git a/debian/proxmox-backup-server.install b/debian/proxmox-backup-server.install
index ef1e9ba1..34bfc454 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 afdfb2bc..e21b8bc7 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 42f639f6..730de4f8 100644
--- a/etc/Makefile
+++ b/etc/Makefile
@@ -7,7 +7,8 @@ DYNAMIC_UNITS := \
 	proxmox-backup-banner.service \
 	proxmox-backup-daily-update.service \
 	proxmox-backup.service \
-	proxmox-backup-proxy.service
+	proxmox-backup-proxy.service \
+	removable-device-attach@.service
 
 all: $(UNITS) $(DYNAMIC_UNITS) pbs-enterprise.list
 
diff --git a/etc/removable-device-attach@.service.in b/etc/removable-device-attach@.service.in
new file mode 100644
index 00000000..e10d1ea3
--- /dev/null
+++ b/etc/removable-device-attach@.service.in
@@ -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 f2795b39..e74c8537 100644
--- a/src/bin/proxmox_backup_manager/datastore.rs
+++ b/src/bin/proxmox_backup_manager/datastore.rs
@@ -1,4 +1,4 @@
-use anyhow::{format_err, Error};
+use anyhow::{bail, format_err, Error};
 use serde_json::Value;
 
 use proxmox_router::{cli::*, ApiHandler, RpcEnvironment};
@@ -195,6 +195,55 @@ async fn delete_datastore(mut param: Value, rpcenv: &mut dyn RpcEnvironment) ->
     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 info = &api2::config::datastore::API_METHOD_LIST_DATASTORES;
+    let data: Value = match info.handler {
+        ApiHandler::Sync(handler) => (handler)(serde_json::json!({}), info, rpcenv)?,
+        _ => unreachable!(),
+    };
+
+    let store_name = data.as_array().and_then(|list| {
+        list.iter()
+            .filter_map(Value::as_object)
+            .find(|store| store.get("backing-device").map_or(false, |d| d.eq(&uuid)))
+            .and_then(|s| s.get("name").and_then(Value::as_str))
+    });
+
+    if let Some(store_name) = store_name {
+        let info = &api2::admin::datastore::API_METHOD_MOUNT;
+        let mount_param = serde_json::json!({
+            "store": store_name,
+        });
+        let result = match info.handler {
+            ApiHandler::Sync(handler) => (handler)(mount_param, info, rpcenv)?,
+            _ => unreachable!(),
+        };
+        crate::wait_for_local_worker(result.as_str().unwrap()).await?;
+        return Ok(Value::Null);
+    }
+    bail!("'{uuid}' is not associated with any datastore")
+}
+
 pub fn datastore_commands() -> CommandLineInterface {
     let cmd_def = CliCommandMap::new()
         .insert("list", CliCommand::new(&API_METHOD_LIST_DATASTORES))
@@ -240,6 +289,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.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 13/26] datastore: handle deletion of removable datastore properly
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
                   ` (11 preceding siblings ...)
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 12/26] add auto-mounting for removable datastores Hannes Laimer
@ 2024-04-25  6:53 ` Hannes Laimer
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 14/26] docs: add removable datastores section Hannes Laimer
                   ` (14 subsequent siblings)
  27 siblings, 0 replies; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:53 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 pbs-datastore/src/datastore.rs | 10 ++++++----
 src/api2/config/datastore.rs   | 16 ++++++++++++++++
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 4f7865fb..3b8af833 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -1528,10 +1528,12 @@ impl DataStore {
                         // weird, but ok
                     }
                     Err(err) if err.is_errno(nix::errno::Errno::EBUSY) => {
-                        task_warn!(
-                            worker,
-                            "Cannot delete datastore directory (is it a mount point?)."
-                        )
+                        if datastore_config.backing_device.is_none() {
+                            task_warn!(
+                                worker,
+                                "Cannot delete datastore directory (is it a mount point?)."
+                            )
+                        }
                     }
                     Err(err) if err.is_errno(nix::errno::Errno::ENOTEMPTY) => {
                         task_warn!(worker, "Datastore directory not empty, not deleting.")
diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 401fbccf..fae44a75 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -29,9 +29,11 @@ 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::is_datastore_available;
 use proxmox_rest_server::WorkerTask;
 
 use crate::server::jobstate;
+use crate::tools::disks::unmount_by_mountpoint;
 
 #[api(
     input: {
@@ -541,6 +543,14 @@ 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 && is_datastore_available(&store_config) {
+        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)?
@@ -579,6 +589,12 @@ pub async fn delete_datastore(
             // ignore errors
             let _ = jobstate::remove_state_file("prune", &name);
             let _ = jobstate::remove_state_file("garbage_collection", &name);
+            if destroy_data {
+                if let Some(mount_point) = store_config.get_mount_point() {
+                    let _ = unmount_by_mountpoint(&mount_point);
+                    let _ = std::fs::remove_dir(&mount_point);
+                }
+            }
 
             if let Err(err) =
                 proxmox_async::runtime::block_on(crate::server::notify_datastore_removed())
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 14/26] docs: add removable datastores section
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
                   ` (12 preceding siblings ...)
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 13/26] datastore: handle deletion of removable datastore properly Hannes Laimer
@ 2024-04-25  6:53 ` Hannes Laimer
  2024-05-03 11:34   ` Max Carrara
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 15/26] ui: add partition selector form Hannes Laimer
                   ` (13 subsequent siblings)
  27 siblings, 1 reply; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:53 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 docs/storage.rst | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/docs/storage.rst b/docs/storage.rst
index 4444c423..0a2f8f8a 100644
--- a/docs/storage.rst
+++ b/docs/storage.rst
@@ -165,6 +165,35 @@ 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 "Admnistartion" > "Disks / Storage" > "Directory",
+using this method the disk will be partitioned and formatted automatically for the datastore.
+
+They are mounted automatically when plugged in. It is possible to create a
+removable datastore on one PBS and use it on multiple, the device just has to be added
+on all as a removable datastore. This can be done using the normal `Create Datastore`
+form, by checking **Removable datastore**, selecting the device and setting the correct path.
+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
+
+.. 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.
+
+
 Managing Datastores
 ^^^^^^^^^^^^^^^^^^^
 
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 15/26] ui: add partition selector form
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
                   ` (13 preceding siblings ...)
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 14/26] docs: add removable datastores section Hannes Laimer
@ 2024-04-25  6:53 ` Hannes Laimer
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 16/26] ui: add removable datastore creation support Hannes Laimer
                   ` (12 subsequent siblings)
  27 siblings, 0 replies; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:53 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 609a0ba6..45adfc54 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 00000000..3d6eadd5
--- /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&exclude-used=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.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 16/26] ui: add removable datastore creation support
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
                   ` (14 preceding siblings ...)
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 15/26] ui: add partition selector form Hannes Laimer
@ 2024-04-25  6:53 ` Hannes Laimer
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 17/26] ui: add (un)mount button to summary Hannes Laimer
                   ` (11 subsequent siblings)
  27 siblings, 0 replies; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:53 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 b6115460..54929382 100644
--- a/www/window/DataStoreEdit.js
+++ b/www/window/DataStoreEdit.js
@@ -62,6 +62,20 @@ Ext.define('PBS.DataStoreEdit', {
 			fieldLabel: gettext('Backing Path'),
 			emptyText: gettext('An absolute path'),
 		    },
+		    {
+			xtype: 'pmxDisplayEditField',
+			fieldLabel: gettext('Device'),
+			name: 'backing-device',
+			disabled: true,
+			cbind: {
+			    editable: '{isCreate}',
+			},
+			editConfig: {
+			    xtype: 'pbsPartitionSelector',
+			    allowBlank: true,
+			},
+			emptyText: gettext('Device path'),
+		    },
 		],
 		column2: [
 		    {
@@ -87,6 +101,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.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 17/26] ui: add (un)mount button to summary
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
                   ` (15 preceding siblings ...)
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 16/26] ui: add removable datastore creation support Hannes Laimer
@ 2024-04-25  6:53 ` Hannes Laimer
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 18/26] ui: tree: render unmounted datastores correctly Hannes Laimer
                   ` (10 subsequent siblings)
  27 siblings, 0 replies; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:53 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>
---
 www/datastore/Summary.js | 92 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 90 insertions(+), 2 deletions(-)

diff --git a/www/datastore/Summary.js b/www/datastore/Summary.js
index a932b4e0..49aa3b3c 100644
--- a/www/datastore/Summary.js
+++ b/www/datastore/Summary.js
@@ -309,7 +309,82 @@ 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) {
+			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.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 +397,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 +425,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.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 18/26] ui: tree: render unmounted datastores correctly
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
                   ` (16 preceding siblings ...)
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 17/26] ui: add (un)mount button to summary Hannes Laimer
@ 2024-04-25  6:53 ` Hannes Laimer
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 19/26] ui: utils: make parseMaintenanceMode more robust Hannes Laimer
                   ` (9 subsequent siblings)
  27 siblings, 0 replies; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:53 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 www/NavigationTree.js                 | 15 ++++++++++++---
 www/css/ext6-pbs.css                  |  8 ++++++++
 www/datastore/DataStoreListSummary.js |  1 +
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/www/NavigationTree.js b/www/NavigationTree.js
index a5ea390f..4f1a8fd5 100644
--- a/www/NavigationTree.js
+++ b/www/NavigationTree.js
@@ -267,13 +267,22 @@ Ext.define('PBS.view.main.NavigationTree', {
 		    j++;
 		}
 
-		let [qtip, iconCls] = ['', 'fa fa-database'];
+		let mainIcon = `fa fa-${records[i].data.removable ? 'plug' : 'database'}`;
+		let [qtip, iconCls] = ['', mainIcon];
 		const maintenance = records[i].data.maintenance;
+		const removable_not_available = records[i].data.removable && !records[i].data.available;
+		if (removable_not_available) {
+		    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 mainenanceTypeCls = type === 'delete' ? 'destroying' : 'maintenance';
-		    iconCls = `fa fa-database pmx-tree-icon-custom ${mainenanceTypeCls}`;
+		    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 c33ce684..706e681e 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 b908034d..f7ea83e7 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.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 19/26] ui: utils: make parseMaintenanceMode more robust
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
                   ` (17 preceding siblings ...)
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 18/26] ui: tree: render unmounted datastores correctly Hannes Laimer
@ 2024-04-25  6:53 ` Hannes Laimer
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 20/26] ui: add datastore status mask for unmounted removable datastores Hannes Laimer
                   ` (8 subsequent siblings)
  27 siblings, 0 replies; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:53 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 40e7cbff..5b27b480 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -708,14 +708,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.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 20/26] ui: add datastore status mask for unmounted removable datastores
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
                   ` (18 preceding siblings ...)
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 19/26] ui: utils: make parseMaintenanceMode more robust Hannes Laimer
@ 2024-04-25  6:53 ` Hannes Laimer
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 21/26] ui: maintenance: fix disable msg field if no type is selected Hannes Laimer
                   ` (7 subsequent siblings)
  27 siblings, 0 replies; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:53 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 706e681e..891189ae 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 49aa3b3c..33b4e18a 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.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 21/26] ui: maintenance: fix disable msg field if no type is selected
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
                   ` (19 preceding siblings ...)
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 20/26] ui: add datastore status mask for unmounted removable datastores Hannes Laimer
@ 2024-04-25  6:53 ` Hannes Laimer
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 22/26] ui: render 'unmount' maintenance mode correctly Hannes Laimer
                   ` (6 subsequent siblings)
  27 siblings, 0 replies; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:53 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 1ee92542..527c3698 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.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 22/26] ui: render 'unmount' maintenance mode correctly
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
                   ` (20 preceding siblings ...)
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 21/26] ui: maintenance: fix disable msg field if no type is selected Hannes Laimer
@ 2024-04-25  6:53 ` Hannes Laimer
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 23/26] api: node: allow creation of removable datastore through directory endpoint Hannes Laimer
                   ` (5 subsequent siblings)
  27 siblings, 0 replies; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:53 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 5b27b480..029f690d 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -743,7 +743,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> ';
@@ -763,6 +763,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 527c3698..d7348cb4 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.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 23/26] api: node: allow creation of removable datastore through directory endpoint
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
                   ` (21 preceding siblings ...)
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 22/26] ui: render 'unmount' maintenance mode correctly Hannes Laimer
@ 2024-04-25  6:53 ` Hannes Laimer
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 24/26] api: node: include removable datastores in directory list Hannes Laimer
                   ` (4 subsequent siblings)
  27 siblings, 0 replies; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:53 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/node/disks/directory.rs | 67 +++++++++++++++++++++++++++++---
 1 file changed, 62 insertions(+), 5 deletions(-)

diff --git a/src/api2/node/disks/directory.rs b/src/api2/node/disks/directory.rs
index 73af92cc..25d43c79 100644
--- a/src/api2/node/disks/directory.rs
+++ b/src/api2/node/disks/directory.rs
@@ -122,6 +122,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,
@@ -140,7 +145,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> {
@@ -154,8 +160,59 @@ pub fn create_datastore_disk(
         bail!("disk '{}' is already in use.", disk);
     }
 
-    let mount_point = format!("{}{}", BASE_MOUNT_DIR, &name);
+    if add_datastore && removable_datastore {
+        let upid_str = WorkerTask::new_thread(
+            "dircreate",
+            Some(name.clone()),
+            auth_id,
+            to_stdout,
+            move |worker| {
+                task_log!(
+                    worker,
+                    "create removable datastore '{}' on disk {}",
+                    name,
+                    disk
+                );
+
+                let filesystem = filesystem.unwrap_or(FileSystemType::Ext4);
+
+                let manager = DiskManage::new();
+
+                let disk = manager.disk_by_name(&disk)?;
+
+                let partition = create_single_linux_partition(&disk)?;
+                create_file_system(&partition, filesystem)?;
+
+                let uuid = get_fs_uuid(&partition)?;
 
+                let lock = pbs_config::datastore::lock_config()?;
+                let datastore: DataStoreConfig = serde_json::from_value(
+                    json!({ "name": name, "path": name, "backing-device": uuid }),
+                )?;
+
+                let (config, _digest) = pbs_config::datastore::config()?;
+
+                if config.sections.get(&datastore.name).is_some() {
+                    bail!("datastore '{}' already exists.", datastore.name);
+                }
+
+                // we don't have to check if the UUID is already in use since we just created the
+                // fs ourself
+
+                crate::api2::config::datastore::do_create_datastore(
+                    lock,
+                    config,
+                    datastore,
+                    Some(&worker),
+                )?;
+
+                Ok(())
+            },
+        )?;
+        return Ok(upid_str);
+    };
+
+    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);
@@ -182,7 +239,6 @@ pub fn create_datastore_disk(
         move |worker| {
             task_log!(worker, "create datastore '{}' on disk {}", name, disk);
 
-            let add_datastore = add_datastore.unwrap_or(false);
             let filesystem = filesystem.unwrap_or(FileSystemType::Ext4);
 
             let manager = DiskManage::new();
@@ -250,8 +306,9 @@ pub fn delete_datastore_disk(name: String) -> Result<(), Error> {
     // path of datastore cannot be changed
     let (config, _) = pbs_config::datastore::config()?;
     let datastores: Vec<DataStoreConfig> = config.convert_to_typed_array("datastore")?;
-    let conflicting_datastore: Option<DataStoreConfig> =
-        datastores.into_iter().find(|ds| ds.absolute_path() == path);
+    let conflicting_datastore: Option<DataStoreConfig> = datastores.into_iter().find(|ds| {
+        ds.absolute_path() == path || ds.get_mount_point().map_or(false, |mp| mp == path)
+    });
 
     if let Some(conflicting_datastore) = conflicting_datastore {
         bail!(
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 24/26] api: node: include removable datastores in directory list
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
                   ` (22 preceding siblings ...)
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 23/26] api: node: allow creation of removable datastore through directory endpoint Hannes Laimer
@ 2024-04-25  6:53 ` Hannes Laimer
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 25/26] node: disks: replace BASE_MOUNT_DIR with DATASTORE_MOUNT_DIR Hannes Laimer
                   ` (3 subsequent siblings)
  27 siblings, 0 replies; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:53 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/node/disks/directory.rs | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/src/api2/node/disks/directory.rs b/src/api2/node/disks/directory.rs
index 25d43c79..55bf6be0 100644
--- a/src/api2/node/disks/directory.rs
+++ b/src/api2/node/disks/directory.rs
@@ -43,6 +43,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
@@ -59,7 +61,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,
@@ -99,6 +101,31 @@ 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;
+        };
+        let Some(mount_point) = item.get_mount_point() else {
+            continue;
+        };
+        list.push(DatastoreMountInfo {
+            unitfile: "datastore config".to_string(),
+            name: item.name.clone(),
+            device: format!("/dev/disk/by-uuid/{backing_device}"),
+            path: mount_point,
+            filesystem: None,
+            options: None,
+            removable: true,
         });
     }
 
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 25/26] node: disks: replace BASE_MOUNT_DIR with DATASTORE_MOUNT_DIR
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
                   ` (23 preceding siblings ...)
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 24/26] api: node: include removable datastores in directory list Hannes Laimer
@ 2024-04-25  6:53 ` Hannes Laimer
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 26/26] ui: support create removable datastore through directory creation Hannes Laimer
                   ` (2 subsequent siblings)
  27 siblings, 0 replies; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:53 UTC (permalink / raw)
  To: pbs-devel

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 55bf6be0..94d271ff 100644
--- a/src/api2/node/disks/directory.rs
+++ b/src/api2/node/disks/directory.rs
@@ -9,8 +9,8 @@ use proxmox_section_config::SectionConfigData;
 use proxmox_sys::task_log;
 
 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::{
@@ -21,8 +21,6 @@ use crate::tools::systemd::{self, types::*};
 
 use proxmox_rest_server::WorkerTask;
 
-const BASE_MOUNT_DIR: &str = "/mnt/datastore/";
-
 #[api(
     properties: {
         "filesystem": {
@@ -90,7 +88,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();
 
@@ -239,7 +237,7 @@ pub fn create_datastore_disk(
         return Ok(upid_str);
     };
 
-    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);
@@ -247,7 +245,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");
             }
@@ -329,7 +327,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.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v10 26/26] ui: support create removable datastore through directory creation
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
                   ` (24 preceding siblings ...)
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 25/26] node: disks: replace BASE_MOUNT_DIR with DATASTORE_MOUNT_DIR Hannes Laimer
@ 2024-04-25  6:53 ` Hannes Laimer
  2024-05-03 11:34 ` [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Max Carrara
  2024-08-20 15:47 ` Hannes Laimer
  27 siblings, 0 replies; 34+ messages in thread
From: Hannes Laimer @ 2024-04-25  6:53 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 94d271ff..3a6cf8c3 100644
--- a/src/api2/node/disks/directory.rs
+++ b/src/api2/node/disks/directory.rs
@@ -152,6 +152,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 adefa9ab..25921a62 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 6aabe21a..38d6979d 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.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup v10 01/26] tools: add disks utility functions
  2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 01/26] tools: add disks utility functions Hannes Laimer
@ 2024-05-03 11:33   ` Max Carrara
  0 siblings, 0 replies; 34+ messages in thread
From: Max Carrara @ 2024-05-03 11:33 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion

On Thu Apr 25, 2024 at 8:52 AM CEST, Hannes Laimer wrote:
> ... for mounting and unmounting
>
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  src/tools/disks/mod.rs | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/src/tools/disks/mod.rs b/src/tools/disks/mod.rs
> index 94f89e0a..303fe4f1 100644
> --- a/src/tools/disks/mod.rs
> +++ b/src/tools/disks/mod.rs
> @@ -1323,3 +1323,22 @@ pub fn get_fs_uuid(disk: &Disk) -> Result<String, Error> {
>  
>      bail!("get_fs_uuid failed - missing UUID");
>  }
> +
> +/// Mount a disk by its UUID and the mount point.
> +pub fn mount_by_uuid(uuid: &str, mount_point: &Path) -> Result<(), Error> {

Wouldn't it make sense to use our own UUID type from `proxmox_uuid`
here?

> +    let mut command = std::process::Command::new("mount");
> +    command.arg(&format!("UUID={uuid}"));
> +    command.arg(mount_point);
> +
> +    proxmox_sys::command::run_command(command, None)?;
> +    Ok(())
> +}
> +
> +/// Unmount a disk by its mount point.
> +pub fn unmount_by_mountpoint(path: &str) -> Result<(), Error> {
> +    let mut command = std::process::Command::new("umount");
> +    command.arg(path);
> +
> +    proxmox_sys::command::run_command(command, None)?;
> +    Ok(())
> +}



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup v10 03/26] pbs-api-types: add backing-device to DataStoreConfig
  2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 03/26] pbs-api-types: add backing-device to DataStoreConfig Hannes Laimer
@ 2024-05-03 11:34   ` Max Carrara
  0 siblings, 0 replies; 34+ messages in thread
From: Max Carrara @ 2024-05-03 11:34 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion

On Thu Apr 25, 2024 at 8:52 AM CEST, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  pbs-api-types/src/datastore.rs | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
> index a5704c93..e5c5cfcf 100644
> --- a/pbs-api-types/src/datastore.rs
> +++ b/pbs-api-types/src/datastore.rs
> @@ -160,6 +160,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")]
> @@ -273,6 +276,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)]
> @@ -320,6 +329,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>,

Here as well, maybe better to use our own UUID type. It even has serde
support :)

>  }
>  
>  #[api]
> @@ -354,12 +368,23 @@ 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 let Some(mount_point) = self.get_mount_point() {
> +            format!("{mount_point}/{}", self.path.trim_matches('/'))
> +        } else {
> +            self.path.clone()
> +        }
> +    }
> +
> +    pub fn get_mount_point(&self) -> Option<String> {
> +        self.backing_device
> +            .is_some()
> +            .then(|| format!("{DATASTORE_MOUNT_DIR}/{}", self.name))
>      }
>  
>      pub fn get_maintenance_mode(&self) -> Option<MaintenanceMode> {



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup v10 05/26] disks: add UUID to partition info
  2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 05/26] disks: add UUID to partition info Hannes Laimer
@ 2024-05-03 11:34   ` Max Carrara
  0 siblings, 0 replies; 34+ messages in thread
From: Max Carrara @ 2024-05-03 11:34 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion

On Thu Apr 25, 2024 at 8:52 AM CEST, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  src/tools/disks/mod.rs | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/src/tools/disks/mod.rs b/src/tools/disks/mod.rs
> index 303fe4f1..6227948d 100644
> --- a/src/tools/disks/mod.rs
> +++ b/src/tools/disks/mod.rs
> @@ -59,6 +59,8 @@ pub struct LsblkInfo {
>      /// File system label.
>      #[serde(rename = "fstype")]
>      file_system_type: Option<String>,
> +    /// File system UUID.
> +    uuid: Option<String>,

Maybe use `proxmox_uuid` here as well.

>  }
>  
>  impl DiskManage {
> @@ -617,7 +619,7 @@ pub struct BlockDevStat {
>  /// Use lsblk to read partition type uuids and file system types.
>  pub fn get_lsblk_info() -> Result<Vec<LsblkInfo>, Error> {
>      let mut command = std::process::Command::new("lsblk");
> -    command.args(["--json", "-o", "path,parttype,fstype"]);
> +    command.args(["--json", "-o", "path,parttype,fstype,uuid"]);
>  
>      let output = proxmox_sys::command::run_command(command, None)?;
>  
> @@ -701,6 +703,8 @@ pub struct PartitionInfo {
>      pub size: Option<u64>,
>      /// GPT partition
>      pub gpt: bool,
> +    /// UUID
> +    pub uuid: Option<String>,

.. and here too.

>  }
>  
>  #[api(
> @@ -891,8 +895,10 @@ fn get_partitions_info(
>  
>              let mounted = disk.is_mounted().unwrap_or(false);
>              let mut filesystem = None;
> +            let mut uuid = None;
>              if let (Some(devpath), Some(infos)) = (devpath.as_ref(), lsblk_infos.as_ref()) {
>                  for info in infos.iter().filter(|i| i.path.eq(devpath)) {
> +                    uuid = info.uuid.clone();
>                      used = match info.partition_type.as_deref() {
>                          Some("21686148-6449-6e6f-744e-656564454649") => PartitionUsageType::BIOS,
>                          Some("c12a7328-f81f-11d2-ba4b-00a0c93ec93b") => PartitionUsageType::EFI,
> @@ -915,6 +921,7 @@ fn get_partitions_info(
>                  filesystem,
>                  size: disk.size().ok(),
>                  gpt: disk.has_gpt(),
> +                uuid,
>              }
>          })
>          .collect()



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup v10 12/26] add auto-mounting for removable datastores
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 12/26] add auto-mounting for removable datastores Hannes Laimer
@ 2024-05-03 11:34   ` Max Carrara
  0 siblings, 0 replies; 34+ messages in thread
From: Max Carrara @ 2024-05-03 11:34 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion

On Thu Apr 25, 2024 at 8:53 AM CEST, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  debian/proxmox-backup-server.install        |  1 +
>  debian/proxmox-backup-server.udev           |  3 ++
>  etc/Makefile                                |  3 +-
>  etc/removable-device-attach@.service.in     |  8 +++
>  src/bin/proxmox_backup_manager/datastore.rs | 55 ++++++++++++++++++++-
>  5 files changed, 68 insertions(+), 2 deletions(-)
>  create mode 100644 etc/removable-device-attach@.service.in
>
> diff --git a/debian/proxmox-backup-server.install b/debian/proxmox-backup-server.install
> index ef1e9ba1..34bfc454 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 afdfb2bc..e21b8bc7 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 42f639f6..730de4f8 100644
> --- a/etc/Makefile
> +++ b/etc/Makefile
> @@ -7,7 +7,8 @@ DYNAMIC_UNITS := \
>  	proxmox-backup-banner.service \
>  	proxmox-backup-daily-update.service \
>  	proxmox-backup.service \
> -	proxmox-backup-proxy.service
> +	proxmox-backup-proxy.service \
> +	removable-device-attach@.service
>  
>  all: $(UNITS) $(DYNAMIC_UNITS) pbs-enterprise.list
>  
> diff --git a/etc/removable-device-attach@.service.in b/etc/removable-device-attach@.service.in
> new file mode 100644
> index 00000000..e10d1ea3
> --- /dev/null
> +++ b/etc/removable-device-attach@.service.in
> @@ -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 f2795b39..e74c8537 100644
> --- a/src/bin/proxmox_backup_manager/datastore.rs
> +++ b/src/bin/proxmox_backup_manager/datastore.rs
> @@ -1,4 +1,4 @@
> -use anyhow::{format_err, Error};
> +use anyhow::{bail, format_err, Error};
>  use serde_json::Value;
>  
>  use proxmox_router::{cli::*, ApiHandler, RpcEnvironment};
> @@ -195,6 +195,55 @@ async fn delete_datastore(mut param: Value, rpcenv: &mut dyn RpcEnvironment) ->
>      Ok(())
>  }
>  
> +#[api(
> +    protected: true,
> +    input: {
> +        properties: {
> +            uuid: {
> +                type: String,
> +                description: "The UUID of the device that should be mounted",

Not sure if we can parse the UUID with our own type from `proxmox_uuid`
here, but would be cool if that worked.

> +            },
> +            "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"))?;

Otherwise, I'd use our own UUID type here too.

> +
> +    let info = &api2::config::datastore::API_METHOD_LIST_DATASTORES;
> +    let data: Value = match info.handler {
> +        ApiHandler::Sync(handler) => (handler)(serde_json::json!({}), info, rpcenv)?,
> +        _ => unreachable!(),
> +    };
> +
> +    let store_name = data.as_array().and_then(|list| {
> +        list.iter()
> +            .filter_map(Value::as_object)
> +            .find(|store| store.get("backing-device").map_or(false, |d| d.eq(&uuid)))
> +            .and_then(|s| s.get("name").and_then(Value::as_str))
> +    });
> +
> +    if let Some(store_name) = store_name {
> +        let info = &api2::admin::datastore::API_METHOD_MOUNT;
> +        let mount_param = serde_json::json!({
> +            "store": store_name,
> +        });
> +        let result = match info.handler {
> +            ApiHandler::Sync(handler) => (handler)(mount_param, info, rpcenv)?,
> +            _ => unreachable!(),
> +        };
> +        crate::wait_for_local_worker(result.as_str().unwrap()).await?;
> +        return Ok(Value::Null);
> +    }
> +    bail!("'{uuid}' is not associated with any datastore")
> +}
> +
>  pub fn datastore_commands() -> CommandLineInterface {
>      let cmd_def = CliCommandMap::new()
>          .insert("list", CliCommand::new(&API_METHOD_LIST_DATASTORES))
> @@ -240,6 +289,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)



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup v10 14/26] docs: add removable datastores section
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 14/26] docs: add removable datastores section Hannes Laimer
@ 2024-05-03 11:34   ` Max Carrara
  0 siblings, 0 replies; 34+ messages in thread
From: Max Carrara @ 2024-05-03 11:34 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion

On Thu Apr 25, 2024 at 8:53 AM CEST, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  docs/storage.rst | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/docs/storage.rst b/docs/storage.rst
> index 4444c423..0a2f8f8a 100644
> --- a/docs/storage.rst
> +++ b/docs/storage.rst
> @@ -165,6 +165,35 @@ 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 "Admnistartion" > "Disks / Storage" > "Directory",

Typo here and wrong name here - should be:
  "Administration" > "Storage / Disks"

> +using this method the disk will be partitioned and formatted automatically for the datastore.
> +
> +They are mounted automatically when plugged in. It is possible to create a
> +removable datastore on one PBS and use it on multiple, the device just has to be added
> +on all as a removable datastore. This can be done using the normal `Create Datastore`
> +form, by checking **Removable datastore**, selecting the device and setting the correct path.
> +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
> +
> +.. 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.
> +
> +
>  Managing Datastores
>  ^^^^^^^^^^^^^^^^^^^
>  



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
                   ` (25 preceding siblings ...)
  2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 26/26] ui: support create removable datastore through directory creation Hannes Laimer
@ 2024-05-03 11:34 ` Max Carrara
  2024-08-20 15:47 ` Hannes Laimer
  27 siblings, 0 replies; 34+ messages in thread
From: Max Carrara @ 2024-05-03 11:34 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion

On Thu Apr 25, 2024 at 8:52 AM CEST, 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.

Looks pretty solid! Applied the patches and gave it a spin on my test
instance. There are some more comments inline, mostly suggesting to use
our own UUID type from `proxmox_uuid` instead of plain strings.
Otherwise, see the sections below.

Testing
=======

* Attached a new disk to my VM
* Created a new removable datastore via the Storage / Disks menu
* Configured the new datastore in my host's storage
* Ran a couple backups
* Set up Prune, GC and Verify jobs
* Set up Sync job to pull from other local datastore
* Manually unmounted and mounted the datastore, UI reacted smoothly
  (very dank actually)

Now to the more fun part: I decided to ignore the docs and detach the
(SCSI) disk I set the datastore up on in the GUI of my host. Lo and
behold, the datastore was still unmounted correctly; at least the UI
didn't complain. As soon as I attached the disk again, the datastore was
almost immediately found. Very robust, I like it!

Code Review
===========

I've got nothing to complain - apart from the comments inline regarding
UUIDs. The vast majority of patches are very easy to follow, each change
is kept comprehensible. The context of the commits is very clear - no
commit tries to do too many things at once. Very good work!

Regarding Failed Jobs
=====================

One thing I noticed is that jobs will still run and consequently fail if
the datastore is unmounted. I guess that's behaving as expected, but if
I put myself in the user's shoes, I'd prefer those jobs to not run at
all instead of failing on every run.

If I (for example) have a job that runs every hour, that would amount to
24 errors a day, which might be a lot of noise - if I have 168 failed
jobs in a week and some other e.g. *very important* job happens to fail
some time inbetween, that very important job now gets lost between those
(IMO) unnecessary fails.

That's IMHO the only thing that requires some more attention
(disclaimer: I'm not sure if there's been some discussion about this
already); I've got no complaints otherwise. Pretty solid work I gotta
say!

Bikeshedding
============

The lesser important thing I wanted to bring up is: Is "Removable
Datastore" really the right name? I would personally prefer something
like "Pluggable Datastore" or something similar, because all datastores
can be removed - it's just that the features added in this series allows
*some* of them to be re-attached again.
  
Specifically, there's a "Remove Datastore" button already in the options
section of datastores; I think the name would conflict with the meaning
of this button. (Sure, you could argue that it's context-dependent, but
I still don't think that those should clash.)

>
> The datastore path specified is relative to `/mnt/removable_datastore/<name>`.
>
> When a removable datastore is deleted and 'destroy-data' is set, the
> device has to be mounted in. 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. 
>
>  
> 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 (2):
>   config: factor out method to get the absolute datastore path
>   maintenance: add 'Unmount' maintenance type
>
> Hannes Laimer (24):
>   tools: add disks utility functions
>   pbs-api-types: add backing-device to DataStoreConfig
>   disks: add UUID to partition info
>   datastore: add helper for checking if a removable datastore is
>     available
>   api: admin: add (un)mount endpoint for removable datastores
>   api: removable datastore creation
>   api: disks list: add exclude-used flag
>   pbs-api-types: add removable/is-available flag 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
>
>  debian/proxmox-backup-server.install        |   1 +
>  debian/proxmox-backup-server.udev           |   3 +
>  docs/storage.rst                            |  29 +++
>  etc/Makefile                                |   3 +-
>  etc/removable-device-attach@.service.in     |   8 +
>  pbs-api-types/src/datastore.rs              |  42 +++-
>  pbs-api-types/src/maintenance.rs            |  11 +-
>  pbs-config/src/datastore.rs                 |  14 ++
>  pbs-datastore/src/datastore.rs              |  79 +++++++-
>  pbs-datastore/src/lib.rs                    |   2 +-
>  src/api2/admin/datastore.rs                 | 203 ++++++++++++++++++--
>  src/api2/config/datastore.rs                |  69 ++++++-
>  src/api2/node/disks/directory.rs            | 112 +++++++++--
>  src/api2/node/disks/mod.rs                  |   8 +
>  src/api2/status.rs                          |  18 +-
>  src/bin/proxmox-backup-proxy.rs             |   9 +-
>  src/bin/proxmox_backup_manager/datastore.rs | 129 ++++++++++++-
>  src/tools/disks/mod.rs                      |  89 ++++++++-
>  www/DirectoryList.js                        |  13 ++
>  www/Makefile                                |   1 +
>  www/NavigationTree.js                       |  15 +-
>  www/Utils.js                                |  33 +++-
>  www/css/ext6-pbs.css                        |  20 ++
>  www/datastore/DataStoreListSummary.js       |   1 +
>  www/datastore/Summary.js                    | 113 ++++++++++-
>  www/form/PartitionSelector.js               |  81 ++++++++
>  www/window/CreateDirectory.js               |  14 ++
>  www/window/DataStoreEdit.js                 |  37 ++++
>  www/window/MaintenanceOptions.js            |  17 +-
>  29 files changed, 1086 insertions(+), 88 deletions(-)
>  create mode 100644 etc/removable-device-attach@.service.in
>  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] 34+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores
  2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
                   ` (26 preceding siblings ...)
  2024-05-03 11:34 ` [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Max Carrara
@ 2024-08-20 15:47 ` Hannes Laimer
  27 siblings, 0 replies; 34+ messages in thread
From: Hannes Laimer @ 2024-08-20 15:47 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion

outdated, v11 -> [1]

[1] https://lists.proxmox.com/pipermail/pbs-devel/2024-August/010620.html


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2024-08-20 15:47 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-25  6:52 [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Hannes Laimer
2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 01/26] tools: add disks utility functions Hannes Laimer
2024-05-03 11:33   ` Max Carrara
2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 02/26] config: factor out method to get the absolute datastore path Hannes Laimer
2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 03/26] pbs-api-types: add backing-device to DataStoreConfig Hannes Laimer
2024-05-03 11:34   ` Max Carrara
2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 04/26] maintenance: add 'Unmount' maintenance type Hannes Laimer
2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 05/26] disks: add UUID to partition info Hannes Laimer
2024-05-03 11:34   ` Max Carrara
2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 06/26] datastore: add helper for checking if a removable datastore is available Hannes Laimer
2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 07/26] api: admin: add (un)mount endpoint for removable datastores Hannes Laimer
2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 08/26] api: removable datastore creation Hannes Laimer
2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 09/26] api: disks list: add exclude-used flag Hannes Laimer
2024-04-25  6:52 ` [pbs-devel] [PATCH proxmox-backup v10 10/26] pbs-api-types: add removable/is-available flag to DataStoreListItem Hannes Laimer
2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 11/26] bin: manager: add (un)mount command Hannes Laimer
2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 12/26] add auto-mounting for removable datastores Hannes Laimer
2024-05-03 11:34   ` Max Carrara
2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 13/26] datastore: handle deletion of removable datastore properly Hannes Laimer
2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 14/26] docs: add removable datastores section Hannes Laimer
2024-05-03 11:34   ` Max Carrara
2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 15/26] ui: add partition selector form Hannes Laimer
2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 16/26] ui: add removable datastore creation support Hannes Laimer
2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 17/26] ui: add (un)mount button to summary Hannes Laimer
2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 18/26] ui: tree: render unmounted datastores correctly Hannes Laimer
2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 19/26] ui: utils: make parseMaintenanceMode more robust Hannes Laimer
2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 20/26] ui: add datastore status mask for unmounted removable datastores Hannes Laimer
2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 21/26] ui: maintenance: fix disable msg field if no type is selected Hannes Laimer
2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 22/26] ui: render 'unmount' maintenance mode correctly Hannes Laimer
2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 23/26] api: node: allow creation of removable datastore through directory endpoint Hannes Laimer
2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 24/26] api: node: include removable datastores in directory list Hannes Laimer
2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 25/26] node: disks: replace BASE_MOUNT_DIR with DATASTORE_MOUNT_DIR Hannes Laimer
2024-04-25  6:53 ` [pbs-devel] [PATCH proxmox-backup v10 26/26] ui: support create removable datastore through directory creation Hannes Laimer
2024-05-03 11:34 ` [pbs-devel] [PATCH proxmox-backup v10 00/26] add removable datastores Max Carrara
2024-08-20 15:47 ` Hannes Laimer

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal