public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support
@ 2022-07-05 13:08 Hannes Laimer
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-widget-toolkit 1/1] proxy: allow setting of reader config Hannes Laimer
                   ` (27 more replies)
  0 siblings, 28 replies; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

Adds removable-device which has 
 * _unique_ 'name'
 * _unique_ 'uuid'
 * 'initialized': true if the chunkstore is already created on the
    device, if false, it will be created the next time it is mounted
 * 'store': the datastore the device is associated with, the path of the
    datastore will be used as a mountpoint for this removable-device

When a new datastore is created it has to be makred as 'removable', if
it is marked as removable the creation of the chunkstore is skipped.
After the creation a removable datastore has no remoavbale-devices
associated with with.

When creating a new removable-device a name, uuid and store have to be
provided, by default a removable device is not initialized. If a device
already contains a chunkstore (possibly created on a different PBS)
initialized has to be set to true.

Unmounting a removable-device puts the associated datastore in
'unplugged' maintenance mode, but it is only really unplugged after all
already started reading and writing operations to the datastore are
finished. The 'unplugged' maintenance mode prevents the start of reading
and writing operations to the datastore, the only way to change the
maintenance mode from 'unplugged' to something else is by mounting(not
mount, but through PBS) a removable-device associated with the
datastore.

Mounting a removable-device device
 1. if nothing else is already mounted there
  -> mount
  otherwise bail!
 2. if removable device has to be initialized
  -> create chunckstore
 3. maintenance mode is changed from 'unplugged' to none.

UI-wise this series adds icons specific to datastores that are marked as
removable, removable-devices behave very similar to sync/prune jobs in
the UI. One grid with all the removable-devices and a grid per datastore
only containing the removable-devices associated to the current
datastore.

Since the maintenance mode has to be changed 'in code' and not just
through the API, the print_property_string function is add to schema.

WIP:
 * list of jobs(per removable-device) that should be triggert when the removable-device
   is mounted mounted.

The patches for proxmox-backup depends on the changes in proxmox and in
proxmox-widget-toolkit. Version bump needed.
(patch 19 needs the changes in proxmox-widget-toolkit)
(patches >=6 need the changes in proxmox)

Since the maintenance mode has to be changed 'in code' and not just
through the API, the print_property_string function is add to schema.

* proxmox
Hannes Laimer (1):
  schema: add print_property_string function

 proxmox-schema/src/schema.rs | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)


* proxmox-widget-toolkit
Hannes Laimer (1):
  proxy: allow setting of reader config

 src/data/ProxmoxProxy.js | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)


* proxmox-backup
Hannes Laimer (26):
  api-types: add RemovableDeviceConfig
  config: make RemovableDeviceConfig savable to config file
  api-types: add "removable" field to DataStoreConfig
  api2: add config endpoints for RemovableDeviceConfig
  api-types: add "unplugged" maintenance type
  api-types: add set_maintenance_mode function to DataStoreConfig
  api2: datastore create: don't init chunkstore if removable
  tools: disks: add mount and unmount helper
  api2: admin: add unmount-device endpoint to datastore
  api2: admin: add mount-device and list endpoint for RemavableDevices
  tools: disks: add uuid to PrtitionInfo
  api-types: add "removable" to DataStoreListItem
  ui: utils: remove parseMaintenanceMode function
  ui: maintenance edit: disable description in maintenance edit if no
    type is selected
  ui: config: add RemovableDeviceView
  ui: add "no removable device present" mask to summary
  ui: add removable datastore specific icons to list
  ui: window: add RemovableDeviceEdit
  ui: form: add PartitionSelector
  ui: add "removable" checkbox to datastore edit
  ui: disable maintenance update while removable datastore is unplugged
  ui: datstore selector: add icon for removable datastores
  api2: admin: add mount-device endpoint that just takes an UUID
  cli: manager: add removable-device commands
  debian/etc: add udev rules and simple service for automounting
  api: mark all removable datastores as 'unplugged' after restart

 debian/proxmox-backup-server.install          |   1 +
 debian/proxmox-backup-server.udev             |   3 +
 etc/Makefile                                  |   3 +-
 etc/removable-device-attach@.service.in       |   6 +
 pbs-api-types/src/datastore.rs                |  26 ++
 pbs-api-types/src/lib.rs                      |   3 +
 pbs-api-types/src/maintenance.rs              |  16 +-
 pbs-api-types/src/removable_device.rs         |  79 +++++
 pbs-config/src/lib.rs                         |   1 +
 pbs-config/src/removable_device.rs            |  63 ++++
 pbs-datastore/src/datastore.rs                |   2 +-
 src/api2/admin/datastore.rs                   |  98 +++++-
 src/api2/admin/mod.rs                         |   6 +
 src/api2/admin/removable_device.rs            | 283 +++++++++++++++++
 src/api2/config/datastore.rs                  |  52 +++-
 src/api2/config/mod.rs                        |   2 +
 src/api2/config/removable_device.rs           | 284 ++++++++++++++++++
 src/bin/proxmox-backup-api.rs                 |  15 +
 src/bin/proxmox-backup-manager.rs             |   1 +
 src/bin/proxmox_backup_manager/mod.rs         |   2 +
 .../removable_device.rs                       | 209 +++++++++++++
 src/tools/disks/mod.rs                        |  27 +-
 www/Makefile                                  |   3 +
 www/NavigationTree.js                         |  19 +-
 www/Utils.js                                  |  27 +-
 www/config/RemovableDeviceView.js             | 211 +++++++++++++
 www/css/ext6-pbs.css                          |  29 ++
 www/datastore/DataStoreList.js                |   5 +
 www/datastore/Panel.js                        |   9 +
 www/datastore/Summary.js                      |  17 +-
 www/form/DataStoreSelector.js                 |   3 +
 www/form/PartitionSelector.js                 |  61 ++++
 www/window/DataStoreEdit.js                   |  10 +
 www/window/MaintenanceOptions.js              |  23 +-
 www/window/RemovableDeviceEdit.js             |  76 +++++
 35 files changed, 1614 insertions(+), 61 deletions(-)
 create mode 100644 etc/removable-device-attach@.service.in
 create mode 100644 pbs-api-types/src/removable_device.rs
 create mode 100644 pbs-config/src/removable_device.rs
 create mode 100644 src/api2/admin/removable_device.rs
 create mode 100644 src/api2/config/removable_device.rs
 create mode 100644 src/bin/proxmox_backup_manager/removable_device.rs
 create mode 100644 www/config/RemovableDeviceView.js
 create mode 100644 www/form/PartitionSelector.js
 create mode 100644 www/window/RemovableDeviceEdit.js

-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-widget-toolkit 1/1] proxy: allow setting of reader config
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox 1/1] schema: add print_property_string function Hannes Laimer
                   ` (26 subsequent siblings)
  27 siblings, 0 replies; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

needed for modifying the raw data that comes from the API, not just
single items

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/data/ProxmoxProxy.js | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/src/data/ProxmoxProxy.js b/src/data/ProxmoxProxy.js
index d4dfe22..9d0cfbf 100644
--- a/src/data/ProxmoxProxy.js
+++ b/src/data/ProxmoxProxy.js
@@ -15,12 +15,11 @@ Ext.define('Proxmox.RestProxy', {
     },
 
     constructor: function(config) {
-	Ext.applyIf(config, {
-	    reader: {
-		responseType: undefined,
-		type: 'json',
-		rootProperty: config.root || 'data',
-	    },
+	config.reader = config.reader ?? {};
+	Ext.applyIf(config.reader, {
+	    responseType: undefined,
+	    type: 'json',
+	    rootProperty: config.root || 'data',
 	});
 
 	this.callParent([config]);
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox 1/1] schema: add print_property_string function
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-widget-toolkit 1/1] proxy: allow setting of reader config Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-06 11:31   ` Wolfgang Bumiller
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 01/26] api-types: add RemovableDeviceConfig Hannes Laimer
                   ` (25 subsequent siblings)
  27 siblings, 1 reply; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

helpful when property string field are not updated through the API

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

diff --git a/proxmox-schema/src/schema.rs b/proxmox-schema/src/schema.rs
index a2b165c..8c93452 100644
--- a/proxmox-schema/src/schema.rs
+++ b/proxmox-schema/src/schema.rs
@@ -7,6 +7,7 @@
 use std::fmt;
 
 use anyhow::{bail, format_err, Error};
+use serde::Serialize;
 use serde_json::{json, Value};
 
 use crate::ConstRegexPattern;
@@ -932,6 +933,33 @@ impl Schema {
         Ok(value)
     }
 
+    /// Generate property string of structs with only simple fields
+    pub fn print_property_string<T: Serialize>(&'static self, data: &T) -> Result<String, Error> {
+        fn print_simple_value(value: Value) -> Result<String, Error> {
+            let string = match value {
+                Value::Bool(b) if b => "1".to_string(),
+                Value::Bool(_) => "0".to_string(),
+                Value::Number(n) => n.to_string(),
+                Value::String(s) => s,
+                _ => {
+                    bail!("Only structs with only simple fields can be formatted as a property string.")
+                }
+            };
+            Ok(string)
+        }
+
+        match serde_json::to_value(data)? {
+            Value::Object(obj) => {
+                let mut props: Vec<String> = Vec::new();
+                for (key, value) in obj.into_iter() {
+                    props.push(format!("{}=\"{}\"", key, print_simple_value(value)?));
+                }
+                Ok(props.join(","))
+            }
+            value => print_simple_value(value),
+        }
+    }
+
     /// Parse a complex property string (`ApiStringFormat::PropertyString`)
     pub fn parse_property_string(&'static self, value_str: &str) -> Result<Value, Error> {
         // helper for object/allof schemas:
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 01/26] api-types: add RemovableDeviceConfig
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-widget-toolkit 1/1] proxy: allow setting of reader config Hannes Laimer
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox 1/1] schema: add print_property_string function Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 02/26] config: make RemovableDeviceConfig savable to config file Hannes Laimer
                   ` (24 subsequent siblings)
  27 siblings, 0 replies; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 pbs-api-types/src/lib.rs              |  3 ++
 pbs-api-types/src/removable_device.rs | 58 +++++++++++++++++++++++++++
 2 files changed, 61 insertions(+)
 create mode 100644 pbs-api-types/src/removable_device.rs

diff --git a/pbs-api-types/src/lib.rs b/pbs-api-types/src/lib.rs
index 70c9ec45..43f68e33 100644
--- a/pbs-api-types/src/lib.rs
+++ b/pbs-api-types/src/lib.rs
@@ -111,6 +111,9 @@ pub use openid::*;
 mod remote;
 pub use remote::*;
 
+mod removable_device;
+pub use removable_device::*;
+
 mod tape;
 pub use tape::*;
 
diff --git a/pbs-api-types/src/removable_device.rs b/pbs-api-types/src/removable_device.rs
new file mode 100644
index 00000000..cb1dca11
--- /dev/null
+++ b/pbs-api-types/src/removable_device.rs
@@ -0,0 +1,58 @@
+use proxmox_schema::{api, Schema, StringSchema, Updater};
+use serde::{Deserialize, Serialize};
+
+use crate::{DATASTORE_SCHEMA, PROXMOX_SAFE_ID_FORMAT, UUID_FORMAT};
+
+pub const DEVICE_NAME_SCHEMA: Schema = StringSchema::new("Removable device name.")
+    .format(&PROXMOX_SAFE_ID_FORMAT)
+    .min_length(3)
+    .max_length(32)
+    .schema();
+
+pub const DEVICE_UUID_SCHEMA: Schema = StringSchema::new("Removable device UUID.")
+    .format(&UUID_FORMAT)
+    .schema();
+
+#[api(
+    properties: {
+        name: {
+            schema: DEVICE_NAME_SCHEMA,
+        },
+        store: {
+            schema: DATASTORE_SCHEMA,
+        },
+        uuid: {
+            schema: DEVICE_UUID_SCHEMA,
+        },
+        initialized: {
+            default: false,
+            optional: true,
+            type: bool,
+        },
+    },
+    default_key: "name",
+)]
+#[derive(Serialize, Deserialize, Clone, Updater)]
+#[serde(rename_all = "kebab-case")]
+/// Information about a removable device
+pub struct RemovableDeviceConfig {
+    /// Name of the removable device
+    #[updater(skip)]
+    pub name: String,
+
+    /// The datastore the device is associated with
+    pub store: String,
+
+    /// UUID of the removeable device
+    pub uuid: String,
+
+    /// this device is initialized
+    #[serde(default)]
+    pub initialized: bool,
+}
+
+impl RemovableDeviceConfig {
+    pub fn acl_path<'a>(&'a self) -> Vec<&'a str> {
+        vec!["datastore", &self.store]
+    }
+}
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 02/26] config: make RemovableDeviceConfig savable to config file
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
                   ` (2 preceding siblings ...)
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 01/26] api-types: add RemovableDeviceConfig Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-06 11:33   ` Wolfgang Bumiller
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 03/26] api-types: add "removable" field to DataStoreConfig Hannes Laimer
                   ` (23 subsequent siblings)
  27 siblings, 1 reply; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 pbs-config/src/lib.rs              |  1 +
 pbs-config/src/removable_device.rs | 63 ++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+)
 create mode 100644 pbs-config/src/removable_device.rs

diff --git a/pbs-config/src/lib.rs b/pbs-config/src/lib.rs
index a83db4e1..5ea67652 100644
--- a/pbs-config/src/lib.rs
+++ b/pbs-config/src/lib.rs
@@ -10,6 +10,7 @@ pub mod metrics;
 pub mod network;
 pub mod prune;
 pub mod remote;
+pub mod removable_device;
 pub mod sync;
 pub mod tape_encryption_keys;
 pub mod tape_job;
diff --git a/pbs-config/src/removable_device.rs b/pbs-config/src/removable_device.rs
new file mode 100644
index 00000000..4587d72b
--- /dev/null
+++ b/pbs-config/src/removable_device.rs
@@ -0,0 +1,63 @@
+use std::collections::HashMap;
+
+use anyhow::Error;
+use lazy_static::lazy_static;
+
+use pbs_api_types::{RemovableDeviceConfig, DEVICE_NAME_SCHEMA};
+use proxmox_schema::*;
+use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
+
+use crate::{open_backup_lockfile, replace_backup_config, BackupLockGuard};
+
+lazy_static! {
+    pub static ref CONFIG: SectionConfig = init();
+}
+
+fn init() -> SectionConfig {
+    const OBJ_SCHEMA: &ObjectSchema = RemovableDeviceConfig::API_SCHEMA.unwrap_object_schema();
+
+    let plugin = SectionConfigPlugin::new(
+        "removable-device".to_string(),
+        Some(String::from("name")),
+        OBJ_SCHEMA,
+    );
+    let mut config = SectionConfig::new(&DEVICE_NAME_SCHEMA);
+    config.register_plugin(plugin);
+
+    config
+}
+
+pub const REMOVABLE_DEVIVE_CFG_FILENAME: &str = "/etc/proxmox-backup/removable-device.cfg";
+pub const REMOVABLE_DEVIVE_CFG_LOCKFILE: &str = "/etc/proxmox-backup/.removable-device.lck";
+
+/// Get exclusive lock
+pub fn lock_config() -> Result<BackupLockGuard, Error> {
+    open_backup_lockfile(REMOVABLE_DEVIVE_CFG_LOCKFILE, None, true)
+}
+
+pub fn config() -> Result<(SectionConfigData, [u8; 32]), Error> {
+    let content = proxmox_sys::fs::file_read_optional_string(REMOVABLE_DEVIVE_CFG_FILENAME)?;
+    let content = content.unwrap_or_default();
+
+    let digest = openssl::sha::sha256(content.as_bytes());
+    let data = CONFIG.parse(REMOVABLE_DEVIVE_CFG_FILENAME, &content)?;
+
+    Ok((data, digest))
+}
+
+pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
+    let raw = CONFIG.write(REMOVABLE_DEVIVE_CFG_FILENAME, config)?;
+    replace_backup_config(REMOVABLE_DEVIVE_CFG_FILENAME, raw.as_bytes())
+}
+
+// shell completion helper
+pub fn complete_removable_device_name(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> {
+    match config() {
+        Ok((data, _digest)) => data
+            .sections
+            .iter()
+            .map(|(name, _)| name.to_string())
+            .collect(),
+        Err(_) => return vec![],
+    }
+}
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 03/26] api-types: add "removable" field to DataStoreConfig
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
                   ` (3 preceding siblings ...)
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 02/26] config: make RemovableDeviceConfig savable to config file Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 04/26] api2: add config endpoints for RemovableDeviceConfig Hannes Laimer
                   ` (22 subsequent siblings)
  27 siblings, 0 replies; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

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

diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index 70b639ea..965e3795 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -235,6 +235,11 @@ pub const DATASTORE_TUNING_STRING_SCHEMA: Schema = StringSchema::new("Datastore
             format: &ApiStringFormat::PropertyString(&MaintenanceMode::API_SCHEMA),
             type: String,
         },
+        removable: {
+            optional: true,
+            default: false,
+            type: bool,
+        }
     }
 )]
 #[derive(Serialize, Deserialize, Updater)]
@@ -278,6 +283,10 @@ 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 datastore is removable
+    #[updater(skip)]
+    pub removable: bool,
 }
 
 impl DataStoreConfig {
@@ -294,6 +303,7 @@ impl DataStoreConfig {
             notify: None,
             tuning: None,
             maintenance_mode: None,
+            removable: false,
         }
     }
 
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 04/26] api2: add config endpoints for RemovableDeviceConfig
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
                   ` (4 preceding siblings ...)
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 03/26] api-types: add "removable" field to DataStoreConfig Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 05/26] api-types: add "unplugged" maintenance type Hannes Laimer
                   ` (21 subsequent siblings)
  27 siblings, 0 replies; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

list, create, read, update and delete

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/config/mod.rs              |   2 +
 src/api2/config/removable_device.rs | 284 ++++++++++++++++++++++++++++
 2 files changed, 286 insertions(+)
 create mode 100644 src/api2/config/removable_device.rs

diff --git a/src/api2/config/mod.rs b/src/api2/config/mod.rs
index 265b6fc8..67b3099c 100644
--- a/src/api2/config/mod.rs
+++ b/src/api2/config/mod.rs
@@ -13,6 +13,7 @@ pub mod media_pool;
 pub mod metrics;
 pub mod prune;
 pub mod remote;
+pub mod removable_device;
 pub mod sync;
 pub mod tape_backup_job;
 pub mod tape_encryption_keys;
@@ -30,6 +31,7 @@ const SUBDIRS: SubdirMap = &sorted!([
     ("metrics", &metrics::ROUTER),
     ("prune", &prune::ROUTER),
     ("remote", &remote::ROUTER),
+    ("removable-device", &removable_device::ROUTER),
     ("sync", &sync::ROUTER),
     ("tape-backup-job", &tape_backup_job::ROUTER),
     ("tape-encryption-keys", &tape_encryption_keys::ROUTER),
diff --git a/src/api2/config/removable_device.rs b/src/api2/config/removable_device.rs
new file mode 100644
index 00000000..c3dc5bad
--- /dev/null
+++ b/src/api2/config/removable_device.rs
@@ -0,0 +1,284 @@
+use anyhow::Error;
+use hex::FromHex;
+use pbs_api_types::{
+    Authid, DataStoreConfig, RemovableDeviceConfig, RemovableDeviceConfigUpdater,
+    DEVICE_NAME_SCHEMA, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_MODIFY, PROXMOX_CONFIG_DIGEST_SCHEMA,
+};
+use pbs_config::{datastore, removable_device, CachedUserInfo};
+use proxmox_router::{http_bail, Permission, Router, RpcEnvironment};
+use proxmox_schema::{api, param_bail};
+use serde::{Deserialize, Serialize};
+use serde_json::Value;
+
+fn check_store(store: &str) -> Result<(), Error> {
+    let (datastore_section_config, _digest) = datastore::config()?;
+    match datastore_section_config.lookup::<DataStoreConfig>("datastore", store) {
+        Ok(store) if store.removable => Ok(()),
+        Ok(_) => param_bail!("store", "datastore '{}' is not marked as removable.", store),
+        Err(_) => param_bail!("store", "datastore '{}' does not exist.", store),
+    }
+}
+
+#[api(
+    input: {
+        properties: {},
+    },
+    returns: {
+        description: "List configured removable devices.",
+        type: Array,
+        items: { type: RemovableDeviceConfig },
+    },
+    access: {
+        permission: &Permission::Anybody,
+        description: "Requires Datastore.Audit.",
+    },
+)]
+/// List all removable devices.
+pub fn list_removable_device(
+    _param: Value,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<Vec<RemovableDeviceConfig>, Error> {
+    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let user_info = CachedUserInfo::new()?;
+
+    let required_privs = PRIV_DATASTORE_AUDIT | PRIV_DATASTORE_MODIFY;
+
+    let (config, digest) = removable_device::config()?;
+
+    let list = config
+        .convert_to_typed_array("removable-device")?
+        .into_iter()
+        .filter(|device: &RemovableDeviceConfig| {
+            let privs = user_info.lookup_privs(&auth_id, &device.acl_path());
+            privs & required_privs != 00
+        })
+        .collect();
+
+    rpcenv["digest"] = hex::encode(&digest).into();
+
+    Ok(list)
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            config: {
+                type: RemovableDeviceConfig,
+                flatten: true,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Anybody,
+        description: "Requires Datastore.Modify on removable devices's datastore.",
+    },
+)]
+/// Create a new removable device.
+pub fn create_removable_device(
+    config: RemovableDeviceConfig,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let user_info = CachedUserInfo::new()?;
+
+    user_info.check_privs(&auth_id, &config.acl_path(), PRIV_DATASTORE_MODIFY, true)?;
+
+    let _lock = removable_device::lock_config()?;
+
+    let (mut section_config, _digest) = removable_device::config()?;
+    if section_config.sections.get(&config.name).is_some() {
+        param_bail!("name", "device '{}' already exists.", config.name);
+    }
+    if section_config
+        .convert_to_typed_array::<RemovableDeviceConfig>("removable-device")?
+        .iter()
+        .any(|device| device.uuid.eq(&config.uuid))
+    {
+        param_bail!("uuid", "device with uuid '{}' already exists.", config.uuid);
+    }
+
+    check_store(&config.store)?;
+
+    section_config.set_data(&config.name, "removable-device", &config)?;
+
+    removable_device::save_config(&section_config)?;
+
+    Ok(())
+}
+
+#[api]
+#[derive(Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
+/// Deletable property name
+pub enum DeletableProperty {}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            name: {
+                schema: DEVICE_NAME_SCHEMA,
+            },
+            update: {
+                type: RemovableDeviceConfigUpdater,
+                flatten: true,
+            },
+            delete: {
+                description: "List of properties to delete.",
+                type: Array,
+                optional: true,
+                items: {
+                    type: DeletableProperty,
+                }
+            },
+            digest: {
+                optional: true,
+                schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Anybody,
+        description: "Requires Datastore.Modify on removable devices's datastore.",
+    },
+)]
+/// Update removable device config.
+pub fn update_removable_device(
+    name: String,
+    update: RemovableDeviceConfigUpdater,
+    delete: Option<Vec<DeletableProperty>>,
+    digest: Option<String>,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let user_info = CachedUserInfo::new()?;
+
+    let _lock = removable_device::lock_config()?;
+
+    // pass/compare digest
+    let (mut config, expected_digest) = removable_device::config()?;
+
+    if let Some(ref digest) = digest {
+        let digest = <[u8; 32]>::from_hex(digest)?;
+        crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
+    }
+
+    let mut data: RemovableDeviceConfig = config.lookup("removable-device", &name)?;
+
+    user_info.check_privs(&auth_id, &data.acl_path(), PRIV_DATASTORE_MODIFY, true)?;
+
+    if let Some(_delete) = delete {}
+
+    if let Some(initialized) = update.initialized {
+        data.initialized = initialized;
+    }
+
+    if let Some(store) = update.store {
+        check_store(&store)?;
+        data.store = store;
+    }
+
+    config.set_data(&name, "removable-device", &data)?;
+
+    removable_device::save_config(&config)?;
+
+    Ok(())
+}
+
+#[api(
+   input: {
+        properties: {
+            name: {
+                schema: DEVICE_NAME_SCHEMA,
+            },
+        },
+    },
+    returns: { type: RemovableDeviceConfig },
+    access: {
+        permission: &Permission::Anybody,
+        description: "Requires Datastore.Audit removable devices's datastore.",
+    },
+)]
+/// Read a removable device configuration.
+pub fn read_removable_device(
+    name: String,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<RemovableDeviceConfig, Error> {
+    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let user_info = CachedUserInfo::new()?;
+
+    let (config, digest) = removable_device::config()?;
+
+    let device_config: RemovableDeviceConfig = config.lookup("removable-device", &name)?;
+
+    user_info.check_privs(
+        &auth_id,
+        &device_config.acl_path(),
+        PRIV_DATASTORE_AUDIT,
+        true,
+    )?;
+
+    rpcenv["digest"] = hex::encode(&digest).into();
+
+    Ok(device_config)
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            name: {
+                schema: DEVICE_NAME_SCHEMA,
+            },
+            digest: {
+                optional: true,
+                schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Anybody,
+        description: "Requires Datastore.Modify on removable devices's datastore.",
+    },
+)]
+/// Remove a removable device configuration
+pub fn delete_removable_device(
+    name: String,
+    digest: Option<String>,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let user_info = CachedUserInfo::new()?;
+
+    let _lock = removable_device::lock_config()?;
+
+    let (mut config, expected_digest) = removable_device::config()?;
+
+    let device: RemovableDeviceConfig = config.lookup("removable-device", &name)?;
+
+    user_info.check_privs(&auth_id, &device.acl_path(), PRIV_DATASTORE_MODIFY, true)?;
+
+    if let Some(ref digest) = digest {
+        let digest = <[u8; 32]>::from_hex(digest)?;
+        crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
+    }
+
+    if config.sections.remove(&name).is_none() {
+        http_bail!(NOT_FOUND, "removable device '{}' does not exist.", name);
+    }
+
+    removable_device::save_config(&config)?;
+
+    Ok(())
+}
+
+const ITEM_ROUTER: Router = Router::new()
+    .get(&API_METHOD_READ_REMOVABLE_DEVICE)
+    .put(&API_METHOD_UPDATE_REMOVABLE_DEVICE)
+    .delete(&API_METHOD_DELETE_REMOVABLE_DEVICE);
+
+pub const ROUTER: Router = Router::new()
+    .get(&API_METHOD_LIST_REMOVABLE_DEVICE)
+    .post(&API_METHOD_CREATE_REMOVABLE_DEVICE)
+    .match_all("name", &ITEM_ROUTER);
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 05/26] api-types: add "unplugged" maintenance type
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
                   ` (5 preceding siblings ...)
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 04/26] api2: add config endpoints for RemovableDeviceConfig Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 06/26] api-types: add set_maintenance_mode function to DataStoreConfig Hannes Laimer
                   ` (20 subsequent siblings)
  27 siblings, 0 replies; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

... and adjust maintenance error message formatting

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

diff --git a/pbs-api-types/src/maintenance.rs b/pbs-api-types/src/maintenance.rs
index 5bbba043..4f0dfc7f 100644
--- a/pbs-api-types/src/maintenance.rs
+++ b/pbs-api-types/src/maintenance.rs
@@ -46,6 +46,8 @@ pub enum MaintenanceType {
     ReadOnly,
     /// Neither read nor write operations are allowed on the datastore.
     Offline,
+    /// No removable device is mounted
+    Unplugged,
 }
 
 #[api(
@@ -63,7 +65,7 @@ pub enum MaintenanceType {
 #[derive(Deserialize, Serialize)]
 /// Maintenance mode
 pub struct MaintenanceMode {
-    /// Type of maintenance ("read-only" or "offline").
+    /// Type of maintenance ("read-only", "offline" or "unplugged").
     #[serde(rename = "type")]
     ty: MaintenanceType,
 
@@ -80,11 +82,13 @@ impl MaintenanceMode {
 
         if let Some(Operation::Lookup) = operation {
             return Ok(());
+        } else if self.ty == MaintenanceType::Unplugged {
+            bail!(": no removable device associated with the datastore is present");
         } else if self.ty == MaintenanceType::Offline {
-            bail!("offline maintenance mode: {}", message);
+            bail!(" is in offline maintenance mode: {}", message);
         } else if self.ty == MaintenanceType::ReadOnly {
             if let Some(Operation::Write) = operation {
-                bail!("read-only maintenance mode: {}", message);
+                bail!(" is in read-only maintenance mode: {}", message);
             }
         }
         Ok(())
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 1ae5bcb6..51dbdb23 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -129,7 +129,7 @@ impl DataStore {
 
         if let Some(maintenance_mode) = config.get_maintenance_mode() {
             if let Err(error) = maintenance_mode.check(operation) {
-                bail!("datastore '{name}' is in {error}");
+                bail!("datastore '{name}'{error}");
             }
         }
 
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 06/26] api-types: add set_maintenance_mode function to DataStoreConfig
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
                   ` (6 preceding siblings ...)
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 05/26] api-types: add "unplugged" maintenance type Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-06 11:40   ` Wolfgang Bumiller
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 07/26] api2: datastore create: don't init chunkstore if removable Hannes Laimer
                   ` (19 subsequent siblings)
  27 siblings, 1 reply; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

helper for updating the MaintenanceMode of a Datastore not through the
API

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 pbs-api-types/src/datastore.rs   | 9 +++++++++
 pbs-api-types/src/maintenance.rs | 6 +++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index 965e3795..ce77f47d 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -313,6 +313,15 @@ impl DataStoreConfig {
             .and_then(|str| MaintenanceMode::API_SCHEMA.parse_property_string(str).ok())
             .and_then(|value| MaintenanceMode::deserialize(value).ok())
     }
+
+    pub fn set_maintenance_mode(&mut self, mode: MaintenanceMode) {
+        if let Some(property_string) = MaintenanceMode::API_SCHEMA
+            .print_property_string(&mode)
+            .ok()
+        {
+            self.maintenance_mode = Some(property_string);
+        }
+    }
 }
 
 #[api(
diff --git a/pbs-api-types/src/maintenance.rs b/pbs-api-types/src/maintenance.rs
index 4f0dfc7f..ec34397f 100644
--- a/pbs-api-types/src/maintenance.rs
+++ b/pbs-api-types/src/maintenance.rs
@@ -67,7 +67,7 @@ pub enum MaintenanceType {
 pub struct MaintenanceMode {
     /// Type of maintenance ("read-only", "offline" or "unplugged").
     #[serde(rename = "type")]
-    ty: MaintenanceType,
+    pub ty: MaintenanceType,
 
     /// Reason for maintenance.
     #[serde(skip_serializing_if = "Option::is_none")]
@@ -75,6 +75,10 @@ pub struct MaintenanceMode {
 }
 
 impl MaintenanceMode {
+    pub fn new(ty: MaintenanceType, message: Option<String>) -> Self {
+        MaintenanceMode { ty, message }
+    }
+
     pub fn check(&self, operation: Option<Operation>) -> Result<(), Error> {
         let message = percent_encoding::percent_decode_str(self.message.as_deref().unwrap_or(""))
             .decode_utf8()
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 07/26] api2: datastore create: don't init chunkstore if removable
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
                   ` (7 preceding siblings ...)
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 06/26] api-types: add set_maintenance_mode function to DataStoreConfig Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-06 11:35   ` Wolfgang Bumiller
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 08/26] tools: disks: add mount and unmount helper Hannes Laimer
                   ` (18 subsequent siblings)
  27 siblings, 1 reply; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

.. instead set the maintenance mode to unplugged. So on creation every
removable datastore is unplugged until a removable device is associated
with it.

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

diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 2d769722..24d7a37a 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -1,19 +1,20 @@
 use std::path::PathBuf;
 
 use ::serde::{Deserialize, Serialize};
-use anyhow::Error;
+use anyhow::{bail, Error};
 use hex::FromHex;
 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::fs::CreateOptions;
 use proxmox_sys::WorkerTaskContext;
 
 use pbs_api_types::{
-    Authid, DataStoreConfig, DataStoreConfigUpdater, DatastoreNotify, DATASTORE_SCHEMA,
-    PRIV_DATASTORE_ALLOCATE, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_MODIFY,
-    PROXMOX_CONFIG_DIGEST_SCHEMA,
+    Authid, DataStoreConfig, DataStoreConfigUpdater, DatastoreNotify, MaintenanceMode,
+    MaintenanceType, DATASTORE_SCHEMA, PRIV_DATASTORE_ALLOCATE, PRIV_DATASTORE_AUDIT,
+    PRIV_DATASTORE_MODIFY, PROXMOX_CONFIG_DIGEST_SCHEMA,
 };
 use pbs_config::BackupLockGuard;
 use pbs_datastore::chunk_store::ChunkStore;
@@ -65,19 +66,37 @@ pub fn list_datastores(
 pub(crate) fn do_create_datastore(
     _lock: BackupLockGuard,
     mut config: SectionConfigData,
-    datastore: DataStoreConfig,
+    mut datastore: DataStoreConfig,
     worker: Option<&dyn WorkerTaskContext>,
 ) -> Result<(), Error> {
     let path: PathBuf = datastore.path.clone().into();
-
     let backup_user = pbs_config::backup_user()?;
-    let _store = ChunkStore::create(
-        &datastore.name,
-        path,
-        backup_user.uid,
-        backup_user.gid,
-        worker,
-    )?;
+    let _store;
+
+    if !datastore.removable {
+        _store = ChunkStore::create(
+            &datastore.name,
+            path,
+            backup_user.uid,
+            backup_user.gid,
+            worker,
+        )?;
+    } else {
+        let options = CreateOptions::new()
+            .owner(backup_user.uid)
+            .group(backup_user.gid);
+        let default_options = CreateOptions::new();
+        match proxmox_sys::fs::create_path(&datastore.path, Some(default_options), Some(options)) {
+            Err(err) => bail!("unable to create directory '{}' - {}", &datastore.path, err),
+            Ok(res) => {
+                if !res {
+                    nix::unistd::chown(&path, Some(backup_user.uid), Some(backup_user.gid))?
+                }
+            }
+        }
+
+        datastore.set_maintenance_mode(MaintenanceMode::new(MaintenanceType::Unplugged, None));
+    }
 
     config.set_data(&datastore.name, "datastore", &datastore)?;
 
@@ -351,6 +370,13 @@ pub fn update_datastore(
     }
 
     if update.maintenance_mode.is_some() {
+        if let Some(maintenance) = data.get_maintenance_mode() {
+            if data.removable && maintenance.ty == MaintenanceType::Unplugged {
+                bail!(
+                    "can't change the maintenance mode as long as no removable device is present"
+                );
+            }
+        }
         data.maintenance_mode = update.maintenance_mode;
     }
 
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 08/26] tools: disks: add mount and unmount helper
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
                   ` (8 preceding siblings ...)
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 07/26] api2: datastore create: don't init chunkstore if removable Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-06 11:42   ` Wolfgang Bumiller
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 09/26] api2: admin: add unmount-device endpoint to datastore Hannes Laimer
                   ` (17 subsequent siblings)
  27 siblings, 1 reply; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

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

diff --git a/src/tools/disks/mod.rs b/src/tools/disks/mod.rs
index 35ec9996..e217ac87 100644
--- a/src/tools/disks/mod.rs
+++ b/src/tools/disks/mod.rs
@@ -1136,6 +1136,24 @@ pub fn complete_disk_name(_arg: &str, _param: &HashMap<String, String>) -> Vec<S
         .collect()
 }
 
+pub fn mount_by_uuid(uuid: &str, mountpoint: &str) -> Result<(), Error> {
+    let mut command = std::process::Command::new("mount");
+    command.args(&[format!("UUID={}", uuid)]);
+    command.arg(mountpoint);
+
+    proxmox_sys::command::run_command(command, None)?;
+    Ok(())
+}
+
+pub fn unmount_by_mountpoint(path: &str) -> Result<(), Error> {
+    let mut command = std::process::Command::new("umount");
+    command.arg("-l");
+    command.arg(path);
+
+    proxmox_sys::command::run_command(command, None)?;
+    Ok(())
+}
+
 /// Read the FS UUID (parse blkid output)
 ///
 /// Note: Calling blkid is more reliable than using the udev ID_FS_UUID property.
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 09/26] api2: admin: add unmount-device endpoint to datastore
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
                   ` (9 preceding siblings ...)
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 08/26] tools: disks: add mount and unmount helper Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-06 11:43   ` Wolfgang Bumiller
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 10/26] api2: admin: add mount-device and list endpoint for RemavableDevices Hannes Laimer
                   ` (16 subsequent siblings)
  27 siblings, 1 reply; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

unmounts the removable device mounted at the datstore path and set the
maintenance-mode of the datastore to 'unplugged'. The worker waits for
all running tasks to finish before unmounting.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/admin/datastore.rs | 97 +++++++++++++++++++++++++++++++++----
 1 file changed, 87 insertions(+), 10 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 5448fa10..2be95f92 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -22,27 +22,28 @@ use proxmox_router::{
     Router, RpcEnvironment, RpcEnvironmentType, SubdirMap,
 };
 use proxmox_schema::*;
+use proxmox_section_config::SectionConfigData;
 use proxmox_sys::fs::{
     file_read_firstline, file_read_optional_string, replace_file, CreateOptions,
 };
-use proxmox_sys::sortable;
-use proxmox_sys::{task_log, task_warn};
+use proxmox_sys::{sortable, task_log, task_warn, WorkerTaskContext};
 
 use pxar::accessor::aio::Accessor;
 use pxar::EntryKind;
 
 use pbs_api_types::{
     print_ns_and_snapshot, print_store_and_ns, Authid, BackupContent, BackupNamespace, BackupType,
-    Counts, CryptMode, DataStoreListItem, DataStoreStatus, GarbageCollectionStatus, GroupListItem,
-    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_SCHEMA, VERIFICATION_OUTDATED_AFTER_SCHEMA,
+    Counts, CryptMode, DataStoreConfig, DataStoreListItem, DataStoreStatus,
+    GarbageCollectionStatus, GroupListItem, 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_SCHEMA,
+    VERIFICATION_OUTDATED_AFTER_SCHEMA,
 };
 use pbs_client::pxar::{create_tar, create_zip};
-use pbs_config::CachedUserInfo;
+use pbs_config::{BackupLockGuard, CachedUserInfo};
 use pbs_datastore::backup_info::BackupInfo;
 use pbs_datastore::cached_chunk_reader::CachedChunkReader;
 use pbs_datastore::catalog::{ArchiveEntry, CatalogReader};
@@ -106,6 +107,39 @@ fn check_privs_and_load_store(
     Ok(datastore)
 }
 
+fn do_unmount_device(
+    _lock: BackupLockGuard,
+    mut config: SectionConfigData,
+    mut datastore: DataStoreConfig,
+    worker: Option<&dyn WorkerTaskContext>,
+) -> Result<(), Error> {
+    datastore.set_maintenance_mode(MaintenanceMode::new(MaintenanceType::Unplugged, None));
+    config.set_data(&datastore.name, "datastore", &datastore)?;
+    pbs_config::datastore::save_config(&config)?;
+    drop(_lock);
+
+    let mut active_operations = task_tracking::get_active_operations(&datastore.name)?;
+    while active_operations.read + active_operations.write > 0 {
+        if let Some(worker) = worker {
+            if worker.abort_requested() {
+                bail!("aborted, due to user request");
+            }
+            task_log!(
+                worker,
+                "can't unmount yet, still {} read and {} write operations active",
+                active_operations.read,
+                active_operations.write
+            );
+        }
+
+        std::thread::sleep(std::time::Duration::new(5, 0));
+        active_operations = task_tracking::get_active_operations(&datastore.name)?;
+    }
+
+    crate::tools::disks::unmount_by_mountpoint(&datastore.path)?;
+
+    Ok(())
+}
 fn read_backup_index(
     backup_dir: &BackupDir,
 ) -> Result<(BackupManifest, Vec<BackupContent>), Error> {
@@ -693,6 +727,45 @@ pub fn status(
     })
 }
 
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            store: { schema: DATASTORE_SCHEMA },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_MODIFY, true),
+    }
+)]
+/// Unmount a removable device that is associated with the datastore
+pub fn unmount_device(
+    store: String,
+    _info: &ApiMethod,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<String, Error> {
+    let (section_config, _digest) = pbs_config::datastore::config()?;
+    let datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
+
+    if !datastore.removable {
+        bail!("datastore '{}' is not removable", &store);
+    }
+
+    let lock = pbs_config::datastore::lock_config()?;
+    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
+
+    let upid = WorkerTask::new_thread(
+        "unmount-device",
+        Some(store),
+        auth_id.to_string(),
+        to_stdout,
+        move |worker| do_unmount_device(lock, section_config, datastore, Some(&worker)),
+    )?;
+
+    return Ok(upid);
+}
+
 #[api(
     input: {
         properties: {
@@ -2262,6 +2335,10 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
             .delete(&API_METHOD_DELETE_SNAPSHOT),
     ),
     ("status", &Router::new().get(&API_METHOD_STATUS)),
+    (
+        "unmount-device",
+        &Router::new().post(&API_METHOD_UNMOUNT_DEVICE),
+    ),
     (
         "upload-backup-log",
         &Router::new().upload(&API_METHOD_UPLOAD_BACKUP_LOG),
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 10/26] api2: admin: add mount-device and list endpoint for RemavableDevices
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
                   ` (10 preceding siblings ...)
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 09/26] api2: admin: add unmount-device endpoint to datastore Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 11/26] tools: disks: add uuid to PrtitionInfo Hannes Laimer
                   ` (15 subsequent siblings)
  27 siblings, 0 replies; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

*mount-device: mounts the removable at the path of the associated
datastore, only done if nothing else is already mounted at that path
*list: returns a list of removable devices and their mounting status

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 pbs-api-types/src/removable_device.rs |  21 +++
 src/api2/admin/mod.rs                 |   2 +
 src/api2/admin/removable_device.rs    | 217 ++++++++++++++++++++++++++
 3 files changed, 240 insertions(+)
 create mode 100644 src/api2/admin/removable_device.rs

diff --git a/pbs-api-types/src/removable_device.rs b/pbs-api-types/src/removable_device.rs
index cb1dca11..94f7e977 100644
--- a/pbs-api-types/src/removable_device.rs
+++ b/pbs-api-types/src/removable_device.rs
@@ -56,3 +56,24 @@ impl RemovableDeviceConfig {
         vec!["datastore", &self.store]
     }
 }
+
+#[api(
+    properties: {
+        config: {
+            type: RemovableDeviceConfig,
+        },
+        mounted: {
+            type: bool,
+        },
+    },
+)]
+#[derive(Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
+/// Status of a removable device
+pub struct RemovableDeviceStatus {
+    #[serde(flatten)]
+    pub config: RemovableDeviceConfig,
+
+    /// The device is mounted
+    pub mounted: bool,
+}
diff --git a/src/api2/admin/mod.rs b/src/api2/admin/mod.rs
index 9b6fc9ad..00ff9131 100644
--- a/src/api2/admin/mod.rs
+++ b/src/api2/admin/mod.rs
@@ -8,6 +8,7 @@ pub mod datastore;
 pub mod metrics;
 pub mod namespace;
 pub mod prune;
+pub mod removable_device;
 pub mod sync;
 pub mod traffic_control;
 pub mod verify;
@@ -18,6 +19,7 @@ const SUBDIRS: SubdirMap = &sorted!([
     ("metrics", &metrics::ROUTER),
     ("prune", &prune::ROUTER),
     ("sync", &sync::ROUTER),
+    ("removable-device", &removable_device::ROUTER),
     ("traffic-control", &traffic_control::ROUTER),
     ("verify", &verify::ROUTER),
 ]);
diff --git a/src/api2/admin/removable_device.rs b/src/api2/admin/removable_device.rs
new file mode 100644
index 00000000..53b92ba7
--- /dev/null
+++ b/src/api2/admin/removable_device.rs
@@ -0,0 +1,217 @@
+use std::path::PathBuf;
+
+use anyhow::{bail, Error};
+use proxmox_router::{
+    list_subdirs_api_method, ApiMethod, Permission, Router, RpcEnvironment, RpcEnvironmentType,
+    SubdirMap,
+};
+use proxmox_schema::api;
+use proxmox_section_config::SectionConfigData;
+use proxmox_sys::{sortable, task_log, WorkerTaskContext};
+use serde_json::Value;
+
+use pbs_api_types::{
+    Authid, DataStoreConfig, RemovableDeviceConfig, RemovableDeviceStatus, DATASTORE_SCHEMA,
+    DEVICE_NAME_SCHEMA, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_MODIFY,
+};
+use pbs_config::{datastore, removable_device, BackupLockGuard, CachedUserInfo};
+use pbs_datastore::ChunkStore;
+use proxmox_rest_server::WorkerTask;
+
+use crate::tools::disks::{DiskManage, DiskUsageQuery};
+
+fn do_mount_removable(
+    _lock: BackupLockGuard,
+    mut device_section_config: SectionConfigData,
+    mut datastore_section_config: SectionConfigData,
+    mut datastore: DataStoreConfig,
+    mut device: RemovableDeviceConfig,
+    worker: Option<&dyn WorkerTaskContext>,
+) -> Result<(), Error> {
+    if DiskManage::new()
+        .mount_info()?
+        .iter()
+        .filter_map(|(_id, entry)| entry.mount_point.to_str())
+        .any(|mount_point| datastore.path.eq(mount_point))
+    {
+        bail!("something is already mounted at '{}'", &datastore.path);
+    };
+
+    crate::tools::disks::mount_by_uuid(&device.uuid, &datastore.path)?;
+    if !device.initialized {
+        if let Some(worker) = worker {
+            task_log!(
+                worker,
+                "Initializing '{}' {} for datastore {}",
+                device.name,
+                device.uuid,
+                datastore.name
+            );
+        }
+        let path: PathBuf = datastore.path.clone().into();
+
+        let backup_user = pbs_config::backup_user()?;
+        if let Err(e) = ChunkStore::create(
+            &datastore.name,
+            path,
+            backup_user.uid,
+            backup_user.gid,
+            worker,
+        ) {
+            crate::tools::disks::unmount_by_mountpoint(&datastore.path)?;
+            return Err(e);
+        }
+        device.initialized = true;
+    }
+    datastore.maintenance_mode = None;
+
+    device_section_config.set_data(&device.name, "removable-device", &device)?;
+    datastore_section_config.set_data(&datastore.name, "datastore", &datastore)?;
+
+    pbs_config::removable_device::save_config(&device_section_config)?;
+    pbs_config::datastore::save_config(&datastore_section_config)?;
+
+    Ok(())
+}
+
+#[api(
+    input: {
+        properties: {
+            store: {
+                schema: DATASTORE_SCHEMA,
+                optional: true,
+            },
+        },
+    },
+    returns: {
+        description: "List configured removable devices and their status.",
+        type: Array,
+        items: { type: RemovableDeviceStatus },
+    },
+    access: {
+        permission: &Permission::Anybody,
+        description: "Requires Datastore.Audit on datastore.",
+    },
+)]
+/// List all removable devices with their status
+pub fn list_removable_devices(
+    store: Option<String>,
+    _param: Value,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<Vec<RemovableDeviceStatus>, Error> {
+    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let user_info = CachedUserInfo::new()?;
+
+    let required_privs = PRIV_DATASTORE_AUDIT;
+    let (config, digest) = removable_device::config()?;
+
+    let device_config_iter = config
+        .convert_to_typed_array("removable-device")?
+        .into_iter()
+        .filter(|device: &RemovableDeviceConfig| {
+            let privs = user_info.lookup_privs(&auth_id, &device.acl_path());
+            if privs & required_privs == 0 {
+                return false;
+            }
+            store.as_ref().map_or(true, |store| device.store.eq(store))
+        });
+
+    let mut list = Vec::new();
+
+    for device in device_config_iter {
+        let mounted = DiskUsageQuery::new()
+            .partitions(true)
+            .query()?
+            .iter()
+            .filter_map(|(_path, info)| info.partitions.as_ref())
+            .flatten()
+            .any(|partition| {
+                partition
+                    .uuid
+                    .as_ref()
+                    .map_or(false, |p| p.eq(&device.uuid))
+                    && partition.mounted
+            });
+        list.push(RemovableDeviceStatus {
+            config: device,
+            mounted,
+        });
+    }
+
+    rpcenv["digest"] = hex::encode(&digest).into();
+
+    Ok(list)
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            name: {
+                schema: DEVICE_NAME_SCHEMA,
+            }
+        }
+    },
+    access: {
+        permission: &Permission::Anybody,
+        description: "Requires Datastore.Modify on job's datastore.",
+    },
+)]
+/// Mount removable device.
+pub fn mount_removable_device(
+    name: String,
+    _info: &ApiMethod,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<String, Error> {
+    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let user_info = CachedUserInfo::new()?;
+
+    let lock = pbs_config::datastore::lock_config()?;
+    let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
+
+    let (section_config, _digest) = removable_device::config()?;
+    let device_config: RemovableDeviceConfig = section_config.lookup("removable-device", &name)?;
+
+    user_info.check_privs(
+        &auth_id,
+        &device_config.acl_path(),
+        PRIV_DATASTORE_MODIFY,
+        true,
+    )?;
+
+    let (datastore_section_config, _digest) = datastore::config()?;
+    let store: DataStoreConfig =
+        datastore_section_config.lookup("datastore", &device_config.store)?;
+
+    let upid = WorkerTask::new_thread(
+        "mount-device",
+        Some(store.name.to_string()),
+        auth_id.to_string(),
+        to_stdout,
+        move |worker| {
+            do_mount_removable(
+                lock,
+                section_config,
+                datastore_section_config,
+                store,
+                device_config,
+                Some(&worker),
+            )
+        },
+    )?;
+    Ok(upid)
+}
+
+#[sortable]
+const DEVICE_INFO_SUBDIRS: SubdirMap = &[(
+    "mount",
+    &Router::new().post(&API_METHOD_MOUNT_REMOVABLE_DEVICE),
+)];
+
+const DEVICE_INFO_ROUTER: Router = Router::new()
+    .get(&list_subdirs_api_method!(DEVICE_INFO_SUBDIRS))
+    .subdirs(DEVICE_INFO_SUBDIRS);
+
+pub const ROUTER: Router = Router::new()
+    .get(&API_METHOD_LIST_REMOVABLE_DEVICES)
+    .match_all("name", &DEVICE_INFO_ROUTER);
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 11/26] tools: disks: add uuid to PrtitionInfo
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
                   ` (11 preceding siblings ...)
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 10/26] api2: admin: add mount-device and list endpoint for RemavableDevices Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 12/26] api-types: add "removable" to DataStoreListItem Hannes Laimer
                   ` (14 subsequent siblings)
  27 siblings, 0 replies; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 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 e217ac87..b40c4f4d 100644
--- a/src/tools/disks/mod.rs
+++ b/src/tools/disks/mod.rs
@@ -57,6 +57,8 @@ pub struct LsblkInfo {
     /// File system label.
     #[serde(rename = "fstype")]
     file_system_type: Option<String>,
+    /// File system UUID.
+    uuid: Option<String>,
 }
 
 impl DiskManage {
@@ -559,7 +561,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)?;
 
@@ -633,6 +635,8 @@ pub struct PartitionInfo {
     pub name: String,
     /// What the partition is used for
     pub used: PartitionUsageType,
+    /// UUID of the partition
+    pub uuid: Option<String>,
     /// Is the partition mounted
     pub mounted: bool,
     /// The filesystem of the partition
@@ -833,8 +837,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,
@@ -853,6 +859,7 @@ fn get_partitions_info(
                 name: disk.sysname().to_str().unwrap_or("?").to_string(),
                 devpath,
                 used,
+                uuid,
                 mounted,
                 filesystem,
                 size: disk.size().ok(),
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 12/26] api-types: add "removable" to DataStoreListItem
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
                   ` (12 preceding siblings ...)
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 11/26] tools: disks: add uuid to PrtitionInfo Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 13/26] ui: utils: remove parseMaintenanceMode function Hannes Laimer
                   ` (13 subsequent siblings)
  27 siblings, 0 replies; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 pbs-api-types/src/datastore.rs | 7 +++++++
 src/api2/admin/datastore.rs    | 1 +
 2 files changed, 8 insertions(+)

diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index ce77f47d..60c51c8d 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -337,6 +337,11 @@ impl DataStoreConfig {
             optional: true,
             format: &ApiStringFormat::PropertyString(&MaintenanceMode::API_SCHEMA),
             type: String,
+        },
+        removable: {
+            optional: true,
+            default: false,
+            type: bool,
         }
     },
 )]
@@ -349,6 +354,8 @@ pub struct DataStoreListItem {
     /// If the datastore is in maintenance mode, information about it
     #[serde(skip_serializing_if = "Option::is_none")]
     pub maintenance: Option<String>,
+    /// This datastore is marked as removable
+    pub removable: bool,
 }
 
 #[api(
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 2be95f92..cbdadce2 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -1280,6 +1280,7 @@ pub fn get_datastore_list(
                     data["comment"].as_str().map(String::from)
                 },
                 maintenance: data["maintenance-mode"].as_str().map(String::from),
+                removable: data["removable"].as_bool().unwrap_or(false),
             });
         }
     }
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 13/26] ui: utils: remove parseMaintenanceMode function
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
                   ` (13 preceding siblings ...)
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 12/26] api-types: add "removable" to DataStoreListItem Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 14/26] ui: maintenance edit: disable description in maintenance edit if no type is selected Hannes Laimer
                   ` (12 subsequent siblings)
  27 siblings, 0 replies; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

... and use parsePropertyString instead + remove quotes from quoted
values in parsePropertyString.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 www/NavigationTree.js            |  8 ++++----
 www/Utils.js                     | 24 +++++++-----------------
 www/datastore/Summary.js         |  6 +++---
 www/window/MaintenanceOptions.js |  6 +++---
 4 files changed, 17 insertions(+), 27 deletions(-)

diff --git a/www/NavigationTree.js b/www/NavigationTree.js
index 5f44aace..aae35012 100644
--- a/www/NavigationTree.js
+++ b/www/NavigationTree.js
@@ -255,10 +255,10 @@ Ext.define('PBS.view.main.NavigationTree', {
 		}
 
 		let [qtip, iconCls] = ['', 'fa fa-database'];
-		const maintenance = records[i].data.maintenance;
-		if (maintenance) {
-		    const [type, message] = PBS.Utils.parseMaintenanceMode(maintenance);
-		    qtip = `${type}${message ? ': ' + message : ''}`;
+		const maintenanceString = records[i].data.maintenance;
+		if (maintenanceString) {
+		    const maintenance = PBS.Utils.parsePropertyString(maintenanceString, 'type');
+		    qtip = `${maintenance.type}${maintenance.message ? ': ' + maintenance.message : ''}`;
 		    iconCls = 'fa fa-database pmx-tree-icon-custom maintenance';
 		}
 
diff --git a/www/Utils.js b/www/Utils.js
index ad451c9f..a1f17bdc 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -73,13 +73,13 @@ Ext.define('PBS.Utils', {
 	Ext.Array.each(value.split(','), function(p) {
 	    var kv = p.split('=', 2);
 	    if (Ext.isDefined(kv[1])) {
-		res[kv[0]] = kv[1];
+		res[kv[0]] = kv[1].replace(/^"(.*)"$/, '$1');
 	    } else if (Ext.isDefined(defaultKey)) {
 		if (Ext.isDefined(res[defaultKey])) {
 		    error = 'defaultKey may be only defined once in propertyString';
 		    return false; // break
 		}
-		res[defaultKey] = kv[0];
+		res[defaultKey] = kv[0].replace(/^"(.*)"$/, '$1');
 	    } else {
 		error = 'invalid propertyString, not a key=value pair and no defaultKey defined';
 		return false; // break
@@ -656,27 +656,17 @@ Ext.define('PBS.Utils', {
 	return `${icon} ${value}`;
     },
 
-    // FIXME: this "parser" is brittle and relies on the order the arguments will appear in
-    parseMaintenanceMode: function(mode) {
-	let [type, message] = mode.split(/,(.+)/);
-	type = type.split("=").pop();
-	message = message ? message.split("=")[1]
-	    .replace(/^"(.*)"$/, '$1')
-	    .replaceAll('\\"', '"') : null;
-	return [type, message];
-    },
-
     renderMaintenance: function(mode, activeTasks) {
 	if (!mode) {
 	    return gettext('None');
 	}
 
-	let [type, message] = PBS.Utils.parseMaintenanceMode(mode);
+	const maintenance = PBS.Utils.parsePropertyString(mode, 'type');
 
 	let extra = '';
 
 	if (activeTasks !== undefined) {
-	    const conflictingTasks = activeTasks.write + (type === 'offline' ? activeTasks.read : 0);
+	    const conflictingTasks = activeTasks.write + (maintenance.type === 'offline' ? activeTasks.read : 0);
 
 	    if (conflictingTasks > 0) {
 		extra += '| <i class="fa fa-spinner fa-pulse fa-fw"></i> ';
@@ -686,12 +676,12 @@ Ext.define('PBS.Utils', {
 	    }
 	}
 
-	if (message) {
-	    extra += ` ("${message}")`;
+	if (maintenance.message) {
+	    extra += ` ("${maintenance.message}")`;
 	}
 
 	let modeText = Proxmox.Utils.unknownText;
-	switch (type) {
+	switch (maintenance.type) {
 	    case 'read-only': modeText = gettext("Read-only");
 		break;
 	    case 'offline': modeText = gettext("Offline");
diff --git a/www/datastore/Summary.js b/www/datastore/Summary.js
index 94be9559..6e73e9ad 100644
--- a/www/datastore/Summary.js
+++ b/www/datastore/Summary.js
@@ -49,10 +49,10 @@ Ext.define('PBS.DataStoreInfo', {
 		    success: function(response) {
 			const config = response.result.data;
 			if (config['maintenance-mode']) {
-			    const [_type, msg] = PBS.Utils.parseMaintenanceMode(config['maintenance-mode']);
+			    const maintenance = PBS.Utils.parsePropertyString(config['maintenance-mode'], 'type');
 			    me.view.el.mask(
-				`${gettext('Datastore is in maintenance mode')}${msg ? ': ' + msg : ''}`,
-				'fa pbs-maintenance-mask',
+			        `${gettext('Datastore is in maintenance mode')}${maintenance.message ? ': ' + maintenance.message : ''}`,
+			        'fa pbs-maintenance-mask',
 			    );
 			} else {
 			    me.view.el.mask(gettext('Datastore is not available'));
diff --git a/www/window/MaintenanceOptions.js b/www/window/MaintenanceOptions.js
index 1ee92542..3cd3e2ce 100644
--- a/www/window/MaintenanceOptions.js
+++ b/www/window/MaintenanceOptions.js
@@ -73,10 +73,10 @@ Ext.define('PBS.window.MaintenanceOptions', {
 	    'maintenance-msg': '',
 	};
 	if (values['maintenance-mode']) {
-	    const [type, message] = PBS.Utils.parseMaintenanceMode(values['maintenance-mode']);
+	    const maintenance = PBS.Utils.parsePropertyString(values['maintenance-mode'], 'type');
 	    options = {
-		'maintenance-type': type,
-		'maintenance-msg': message ?? '',
+		'maintenance-type': maintenance.type,
+		'maintenance-msg': maintenance.message ?? '',
 	    };
 	}
 
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 14/26] ui: maintenance edit: disable description in maintenance edit if no type is selected
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
                   ` (14 preceding siblings ...)
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 13/26] ui: utils: remove parseMaintenanceMode function Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 15/26] ui: config: add RemovableDeviceView Hannes Laimer
                   ` (11 subsequent siblings)
  27 siblings, 0 replies; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 www/Utils.js                     |  5 ++++-
 www/window/MaintenanceOptions.js | 11 ++++++++++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/www/Utils.js b/www/Utils.js
index a1f17bdc..977a5255 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -666,7 +666,8 @@ Ext.define('PBS.Utils', {
 	let extra = '';
 
 	if (activeTasks !== undefined) {
-	    const conflictingTasks = activeTasks.write + (maintenance.type === 'offline' ? activeTasks.read : 0);
+	    const conflictingTasks = activeTasks.write +
+		(['offline', 'unplugged'].includes(maintenance.type) ? activeTasks.read : 0);
 
 	    if (conflictingTasks > 0) {
 		extra += '| <i class="fa fa-spinner fa-pulse fa-fw"></i> ';
@@ -682,6 +683,8 @@ Ext.define('PBS.Utils', {
 
 	let modeText = Proxmox.Utils.unknownText;
 	switch (maintenance.type) {
+	    case 'unplugged': modeText = `<i>${gettext("Unplugged")}</i>`;
+		break;
 	    case 'read-only': modeText = gettext("Read-only");
 		break;
 	    case 'offline': modeText = gettext("Offline");
diff --git a/www/window/MaintenanceOptions.js b/www/window/MaintenanceOptions.js
index 3cd3e2ce..8907d75d 100644
--- a/www/window/MaintenanceOptions.js
+++ b/www/window/MaintenanceOptions.js
@@ -54,14 +54,20 @@ Ext.define('PBS.window.MaintenanceOptions', {
 		xtype: 'pbsMaintenanceType',
 		name: 'maintenance-type',
 		fieldLabel: gettext('Maintenance Type'),
+		listeners: {
+		    change: (_picker, newValue, _oldValue) => {
+			Ext.ComponentManager.get('message-field')
+			    .setDisabled(newValue === '__default__');
+		    },
+		},
 		value: '__default__',
 		deleteEmpty: true,
 	    },
 	    {
 		xtype: 'proxmoxtextfield',
+		id: 'message-field',
 		name: 'maintenance-msg',
 		fieldLabel: gettext('Description'),
-		// FIXME: disable if maintenance type is none
 	    },
 	],
     },
@@ -81,5 +87,8 @@ Ext.define('PBS.window.MaintenanceOptions', {
 	}
 
 	me.callParent([options]);
+
+	Ext.ComponentManager.get('message-field')
+	    .setDisabled(options['maintenance-type'] === '__default__');
     },
 });
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 15/26] ui: config: add RemovableDeviceView
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
                   ` (15 preceding siblings ...)
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 14/26] ui: maintenance edit: disable description in maintenance edit if no type is selected Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 16/26] ui: add "no removable device present" mask to summary Hannes Laimer
                   ` (10 subsequent siblings)
  27 siblings, 0 replies; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 www/Makefile                      |   1 +
 www/config/RemovableDeviceView.js | 211 ++++++++++++++++++++++++++++++
 www/datastore/DataStoreList.js    |   5 +
 www/datastore/Panel.js            |   9 ++
 4 files changed, 226 insertions(+)
 create mode 100644 www/config/RemovableDeviceView.js

diff --git a/www/Makefile b/www/Makefile
index 4aa2b026..4d29f1b4 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -55,6 +55,7 @@ JSSRC=							\
 	config/UserView.js				\
 	config/TokenView.js				\
 	config/RemoteView.js				\
+	config/RemovableDeviceView.js			\
 	config/TrafficControlView.js			\
 	config/ACLView.js				\
 	config/SyncView.js				\
diff --git a/www/config/RemovableDeviceView.js b/www/config/RemovableDeviceView.js
new file mode 100644
index 00000000..7c0894b5
--- /dev/null
+++ b/www/config/RemovableDeviceView.js
@@ -0,0 +1,211 @@
+Ext.define('pbs-removable-device-status', {
+    extend: 'Ext.data.Model',
+    fields: ['name', 'uuid', 'store', 'mounted', 'initialized'],
+    idProperty: 'name',
+    proxy: {
+	type: 'proxmox',
+	url: '/api2/json/admin/removable-device',
+    },
+});
+
+Ext.define('PBS.config.RemovableDeviceView', {
+    extend: 'Ext.grid.GridPanel',
+    alias: 'widget.pbsRemovableDeviceView',
+
+    stateful: true,
+    stateId: 'grid-devices',
+
+    title: gettext('Removable Devices'),
+
+    tools: [PBS.Utils.get_help_tool("backup-remote")],
+
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	addRemovableDevice: function() {
+	    let me = this;
+	    let view = me.getView();
+            Ext.create('PBS.window.RemovableDeviceEdit', {
+		datastore: view.datastore,
+		listeners: {
+		    destroy: function() {
+			me.reload();
+		    },
+		},
+            }).show();
+	},
+
+	editRemovableDevice: function() {
+	    let me = this;
+	    let view = me.getView();
+	    let selection = view.getSelection();
+	    if (selection.length < 1) return;
+
+            Ext.create('PBS.window.RemovableDeviceEdit', {
+                name: selection[0].data.name,
+		datastore: view.datastore,
+		listeners: {
+		    destroy: function() {
+			me.reload();
+		    },
+		},
+            }).show();
+	},
+
+	mountDevice: function() {
+	    let me = this;
+	    let view = me.getView();
+	    let selection = view.getSelection();
+	    if (!selection || selection.length < 1) return;
+	    let rec = selection[0];
+	    view.setSelection(null);
+	    Proxmox.Utils.API2Request({
+		url: `/api2/extjs/admin/removable-device/${rec.data.name}/mount`,
+		waitMsgTarget: view,
+		method: 'POST',
+		failure: response => Ext.Msg.alert(gettext('Error'), response.htmlStatus),
+		success: function(response, options) {
+		    Ext.create('Proxmox.window.TaskProgress', {
+		        upid: response.result.data,
+			taskDone: function() {
+			    me.reload();
+			},
+			autoShow: true,
+		    });
+		},
+	    });
+	},
+
+	unmountDevice: function() {
+	    let me = this;
+	    let view = me.getView();
+	    let selection = view.getSelection();
+	    if (!selection || selection.length < 1) return;
+	    let rec = selection[0];
+	    view.setSelection(null);
+	    Proxmox.Utils.API2Request({
+		url: `/api2/extjs/admin/datastore/${rec.data.store}/unmount-device`,
+		waitMsgTarget: view,
+		method: 'POST',
+		failure: response => Ext.Msg.alert(gettext('Error'), response.htmlStatus),
+		success: function(response, options) {
+		    Ext.create('Proxmox.window.TaskProgress', {
+		        upid: response.result.data,
+			taskDone: function() {
+			    me.reload();
+			},
+			autoShow: true,
+		    });
+		},
+	    });
+	},
+
+	startStore: function() { this.getView().getStore().rstore.startUpdate(); },
+	stopStore: function() { this.getView().getStore().rstore.stopUpdate(); },
+	reload: function() { this.getView().getStore().rstore.load(); },
+
+	init: function(view) {
+	    let params = {};
+	    if (view.datastore !== undefined) {
+		params.store = view.datastore;
+	    }
+	    view.getStore().rstore.getProxy().setExtraParams(params);
+	    Proxmox.Utils.monStoreErrors(view, view.getStore().rstore);
+	},
+    },
+
+    listeners: {
+	activate: 'reload',
+	itemdblclick: 'editRemovableDevice',
+    },
+
+    store: {
+	type: 'diff',
+	autoDestroy: true,
+	autoDestroyRstore: true,
+	sorters: 'name',
+	rstore: {
+	    type: 'update',
+	    storeid: 'pbs-removable-device-status',
+	    model: 'pbs-removable-device-status',
+	    autoStart: true,
+	    interval: 5000,
+	},
+    },
+
+    tbar: [
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Add'),
+	    handler: 'addRemovableDevice',
+	    selModel: false,
+	},
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Edit'),
+	    handler: 'editRemovableDevice',
+	    disabled: true,
+	},
+	{
+	    xtype: 'proxmoxStdRemoveButton',
+	    baseurl: '/config/removable-device',
+	    confirmMsg: gettext('Remove device?'),
+	    callback: 'reload',
+	},
+	'-',
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Mount'),
+	    handler: 'mountDevice',
+	    enableFn: (rec) => !rec.data.mounted,
+	    disabled: true,
+	},
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Unmount'),
+	    handler: 'unmountDevice',
+	    enableFn: (rec) => !!rec.data.mounted,
+	    disabled: true,
+	},
+    ],
+
+    viewConfig: {
+	trackOver: false,
+    },
+
+    columns: [
+	{
+	    header: gettext('Name'),
+	    width: 200,
+	    sortable: true,
+	    renderer: Ext.String.htmlEncode,
+	    dataIndex: 'name',
+	},
+	{
+	    header: gettext('Datastore'),
+	    width: 200,
+	    sortable: true,
+	    dataIndex: 'store',
+	},
+	{
+	    header: gettext('UUID'),
+	    width: 200,
+	    sortable: true,
+	    dataIndex: 'uuid',
+	},
+	{
+	    header: gettext('Initialized'),
+	    sortable: false,
+	    renderer: Proxmox.Utils.format_boolean,
+	    dataIndex: 'initialized',
+	    width: 60,
+	},
+	{
+	    header: gettext('Mounted'),
+	    sortable: false,
+	    renderer: Proxmox.Utils.format_boolean,
+	    dataIndex: 'mounted',
+	    width: 60,
+	},
+    ],
+});
diff --git a/www/datastore/DataStoreList.js b/www/datastore/DataStoreList.js
index b496bcbc..20362d6e 100644
--- a/www/datastore/DataStoreList.js
+++ b/www/datastore/DataStoreList.js
@@ -242,6 +242,11 @@ Ext.define('PBS.datastore.DataStores', {
 	    itemId: 'prunejobs',
 	    xtype: 'pbsPruneJobView',
 	},
+	{
+	    iconCls: 'fa fa-hdd-o',
+	    itemId: 'removabledevices',
+	    xtype: 'pbsRemovableDeviceView',
+	},
 	{
 	    iconCls: 'fa fa-check-circle',
 	    itemId: 'verifyjobs',
diff --git a/www/datastore/Panel.js b/www/datastore/Panel.js
index fd1b4611..3c32badd 100644
--- a/www/datastore/Panel.js
+++ b/www/datastore/Panel.js
@@ -82,6 +82,15 @@ Ext.define('PBS.DataStorePanel', {
 		datastore: '{datastore}',
 	    },
 	},
+	{
+	    xtype: 'pbsRemovableDeviceView',
+	    itemId: 'devices',
+	    title: gettext('Removable devices'),
+	    iconCls: 'fa fa-hdd-o',
+	    cbind: {
+		datastore: '{datastore}',
+	    },
+	},
 	{
 	    xtype: 'pbsDatastoreOptionView',
 	    itemId: 'options',
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 16/26] ui: add "no removable device present" mask to summary
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
                   ` (16 preceding siblings ...)
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 15/26] ui: config: add RemovableDeviceView Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 17/26] ui: add removable datastore specific icons to list Hannes Laimer
                   ` (9 subsequent siblings)
  27 siblings, 0 replies; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 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 | 15 +++++++++++----
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/www/css/ext6-pbs.css b/www/css/ext6-pbs.css
index 3c8bed55..045a7376 100644
--- a/www/css/ext6-pbs.css
+++ b/www/css/ext6-pbs.css
@@ -269,6 +269,18 @@ span.snapshot-comment-column {
     content: "\f0ad";
 }
 
+.pbs-removable-device-mask div.x-mask-msg-text {
+    background: None;
+    padding: 12px 0 0;
+}
+
+.pbs-removable-device-mask:before {
+    font-size: 3em;
+    display: flex;
+    justify-content: center;
+    content: "\f0a0";
+}
+
 /* 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 6e73e9ad..7a77b35c 100644
--- a/www/datastore/Summary.js
+++ b/www/datastore/Summary.js
@@ -50,10 +50,17 @@ Ext.define('PBS.DataStoreInfo', {
 			const config = response.result.data;
 			if (config['maintenance-mode']) {
 			    const maintenance = PBS.Utils.parsePropertyString(config['maintenance-mode'], 'type');
-			    me.view.el.mask(
-			        `${gettext('Datastore is in maintenance mode')}${maintenance.message ? ': ' + maintenance.message : ''}`,
-			        'fa pbs-maintenance-mask',
-			    );
+			    if (maintenance.type === 'unplugged' && config.removable) {
+				me.view.el.mask(
+				    `${maintenance.message ?? 'No removable device assiciated with the datastore is present'}`,
+				    'fa pbs-removable-device-mask',
+				);
+			    } else {
+				me.view.el.mask(
+				    `${gettext('Datastore is in maintenance mode')}${maintenance.message ? ': ' + maintenance.message : ''}`,
+				    'fa pbs-maintenance-mask',
+				);
+			    }
 			} else {
 			    me.view.el.mask(gettext('Datastore is not available'));
 			}
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 17/26] ui: add removable datastore specific icons to list
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
                   ` (17 preceding siblings ...)
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 16/26] ui: add "no removable device present" mask to summary Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 18/26] ui: window: add RemovableDeviceEdit Hannes Laimer
                   ` (8 subsequent siblings)
  27 siblings, 0 replies; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 www/NavigationTree.js | 17 +++++++++++++----
 www/css/ext6-pbs.css  | 17 +++++++++++++++++
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/www/NavigationTree.js b/www/NavigationTree.js
index aae35012..f316f678 100644
--- a/www/NavigationTree.js
+++ b/www/NavigationTree.js
@@ -1,6 +1,6 @@
 Ext.define('pbs-datastore-list', {
     extend: 'Ext.data.Model',
-    fields: ['name', 'comment', 'maintenance'],
+    fields: ['name', 'comment', 'maintenance', 'removable'],
     proxy: {
         type: 'proxmox',
         url: "/api2/json/admin/datastore",
@@ -255,9 +255,18 @@ Ext.define('PBS.view.main.NavigationTree', {
 		}
 
 		let [qtip, iconCls] = ['', 'fa fa-database'];
-		const maintenanceString = records[i].data.maintenance;
-		if (maintenanceString) {
-		    const maintenance = PBS.Utils.parsePropertyString(maintenanceString, 'type');
+		const maintenance = records[i].data.maintenance
+		    ? PBS.Utils.parsePropertyString(records[i].data.maintenance, 'type') : null;
+		const removable = records[i].data.removable;
+		if (removable) {
+		    iconCls = `fa fa-database pmx-tree-icon-custom removable-present`;
+		    if (maintenance) {
+			iconCls = `fa fa-database pmx-tree-icon-custom removable-not-present`;
+			qtip = `${maintenance.message ? maintenance.message : 'no device present'}`;
+		    }
+		}
+
+		if (maintenance && maintenance.type !== 'unplugged') {
 		    qtip = `${maintenance.type}${maintenance.message ? ': ' + maintenance.message : ''}`;
 		    iconCls = 'fa fa-database pmx-tree-icon-custom maintenance';
 		}
diff --git a/www/css/ext6-pbs.css b/www/css/ext6-pbs.css
index 045a7376..a7aa36cb 100644
--- a/www/css/ext6-pbs.css
+++ b/www/css/ext6-pbs.css
@@ -301,6 +301,23 @@ span.snapshot-comment-column {
     color: #888;
 }
 
+/* removable datastore */
+.pmx-tree-icon-custom.removable-present:after {
+    content: "\f287";
+    font-size: 0.7em;
+    color: #000;
+}
+
+.pmx-tree-icon-custom.removable-not-present:after {
+    content: "\f287";
+    font-size: 0.7em;
+    color: #000;
+}
+
+.pmx-tree-icon-custom.removable-not-present:before {
+    color: #888;
+}
+
 /*' PBS specific icons */
 
 .pbs-icon-tape {
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 18/26] ui: window: add RemovableDeviceEdit
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
                   ` (18 preceding siblings ...)
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 17/26] ui: add removable datastore specific icons to list Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 19/26] ui: form: add PartitionSelector Hannes Laimer
                   ` (7 subsequent siblings)
  27 siblings, 0 replies; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 www/Makefile                      |  1 +
 www/window/RemovableDeviceEdit.js | 76 +++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+)
 create mode 100644 www/window/RemovableDeviceEdit.js

diff --git a/www/Makefile b/www/Makefile
index 4d29f1b4..12be1ce5 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -74,6 +74,7 @@ JSSRC=							\
 	window/MaintenanceOptions.js			\
 	window/NotesEdit.js				\
 	window/RemoteEdit.js				\
+	window/RemovableDeviceEdit.js			\
 	window/TrafficControlEdit.js			\
 	window/NotifyOptions.js				\
 	window/SyncJobEdit.js				\
diff --git a/www/window/RemovableDeviceEdit.js b/www/window/RemovableDeviceEdit.js
new file mode 100644
index 00000000..6a14e9a6
--- /dev/null
+++ b/www/window/RemovableDeviceEdit.js
@@ -0,0 +1,76 @@
+Ext.define('PBS.window.RemovableDeviceEdit', {
+    extend: 'Proxmox.window.Edit',
+    alias: 'widget.pbsRemovableDeviceEdit',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    isAdd: true,
+
+    subject: gettext('Removable Device'),
+
+    fieldDefaults: { labelWidth: 120 },
+
+    cbindData: function(initialConfig) {
+	let me = this;
+
+	let baseurl = '/api2/extjs/config/removable-device';
+	let name = initialConfig.name;
+
+	me.isCreate = !name;
+	me.url = name ? `${baseurl}/${name}` : baseurl;
+	me.method = name ? 'PUT' : 'POST';
+	me.autoLoad = !!name;
+	me.datastore = initialConfig.datastore;
+	me.editDatastore = me.datastore === undefined;
+    },
+    width: 450,
+
+    items: {
+	xtype: 'inputpanel',
+	items: [
+	    {
+		xtype: 'pmxDisplayEditField',
+		name: 'name',
+		fieldLabel: gettext('Name'),
+		renderer: Ext.htmlEncode,
+		allowBlank: false,
+		minLength: 4,
+		cbind: {
+		    editable: '{isCreate}',
+		},
+	    },
+	    {
+		xtype: 'pmxDisplayEditField',
+		fieldLabel: gettext('UUID'),
+		name: 'uuid',
+		submitValue: true,
+		cbind: {
+		    editable: '{isCreate}',
+		},
+		editConfig: {
+		    xtype: 'pbsPartitionSelector',
+		    allowBlank: false,
+		},
+	    },
+	    {
+		fieldLabel: gettext('Initialized'),
+		xtype: 'proxmoxcheckbox',
+		uncheckedValue: false,
+		name: 'initialized',
+	    },
+	    {
+		xtype: 'pmxDisplayEditField',
+		fieldLabel: gettext('Datastore'),
+		name: 'store',
+		submitValue: true,
+		cbind: {
+		    editable: '{editDatastore}',
+		    value: '{datastore}',
+		},
+		editConfig: {
+		    xtype: 'pbsDataStoreSelector',
+		    allowBlank: false,
+		},
+	    },
+	],
+    },
+});
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 19/26] ui: form: add PartitionSelector
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
                   ` (19 preceding siblings ...)
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 18/26] ui: window: add RemovableDeviceEdit Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 20/26] ui: add "removable" checkbox to datastore edit Hannes Laimer
                   ` (6 subsequent siblings)
  27 siblings, 0 replies; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

... used by the removable device edit window, only shows partitions
that have a filesystem and gives their uuid as value

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 www/Makefile                  |  1 +
 www/form/PartitionSelector.js | 61 +++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+)
 create mode 100644 www/form/PartitionSelector.js

diff --git a/www/Makefile b/www/Makefile
index 12be1ce5..b916ecc8 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -44,6 +44,7 @@ JSSRC=							\
 	form/DataStoreSelector.js			\
 	form/NamespaceSelector.js			\
 	form/NamespaceMaxDepth.js			\
+	form/PartitionSelector.js			\
 	form/CalendarEvent.js				\
 	form/PermissionPathSelector.js			\
 	form/GroupSelector.js				\
diff --git a/www/form/PartitionSelector.js b/www/form/PartitionSelector.js
new file mode 100644
index 00000000..53c0560a
--- /dev/null
+++ b/www/form/PartitionSelector.js
@@ -0,0 +1,61 @@
+Ext.define('pbs-partition-list', {
+    extend: 'Ext.data.Model',
+    fields: ['name', 'uuid', 'filesystem', 'devpath', 'size'],
+    proxy: {
+        type: 'proxmox',
+        url: "/api2/json/nodes/localhost/disks/list?include-partitions=1",
+	reader: {
+	    transform: (rawData) => {
+		let partitions = [];
+		for (let disk of rawData.data) {
+		    const p = (disk.partitions ?? []).filter((i) => i.used === 'filesystem');
+		    partitions.push(...p);
+		}
+		return { data: partitions };
+	    },
+	},
+    },
+    idProperty: 'devpath',
+
+});
+
+Ext.define('PBS.form.PartitionSelector', {
+    extend: 'Proxmox.form.ComboGrid',
+    alias: 'widget.pbsPartitionSelector',
+
+    allowBlank: false,
+    autoSelect: false,
+    valueField: 'uuid',
+    displayField: 'uuid',
+
+    store: {
+	model: 'pbs-partition-list',
+	autoLoad: true,
+	sorters: 'devpath',
+    },
+
+    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,
+	    },
+	],
+    },
+});
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 20/26] ui: add "removable" checkbox to datastore edit
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
                   ` (20 preceding siblings ...)
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 19/26] ui: form: add PartitionSelector Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 21/26] ui: disable maintenance update while removable datastore is unplugged Hannes Laimer
                   ` (5 subsequent siblings)
  27 siblings, 0 replies; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 www/window/DataStoreEdit.js | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/www/window/DataStoreEdit.js b/www/window/DataStoreEdit.js
index aecf6b8d..13ce741b 100644
--- a/www/window/DataStoreEdit.js
+++ b/www/window/DataStoreEdit.js
@@ -59,6 +59,16 @@ Ext.define('PBS.DataStoreEdit', {
 			fieldLabel: gettext('Backing Path'),
 			emptyText: gettext('An absolute path'),
 		    },
+		    {
+			xtype: 'proxmoxcheckbox',
+			cbind: {
+			    editable: '{isCreate}',
+			},
+			name: 'removable',
+			uncheckedValue: 0,
+			defaultValue: 0,
+			fieldLabel: gettext('Removable'),
+		    },
 		],
 		column2: [
 		    {
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 21/26] ui: disable maintenance update while removable datastore is unplugged
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
                   ` (21 preceding siblings ...)
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 20/26] ui: add "removable" checkbox to datastore edit Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 22/26] ui: datstore selector: add icon for removable datastores Hannes Laimer
                   ` (4 subsequent siblings)
  27 siblings, 0 replies; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 www/window/MaintenanceOptions.js | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/www/window/MaintenanceOptions.js b/www/window/MaintenanceOptions.js
index 8907d75d..2d83fbf8 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'),
 		listeners: {
@@ -85,10 +86,15 @@ Ext.define('PBS.window.MaintenanceOptions', {
 		'maintenance-msg': maintenance.message ?? '',
 	    };
 	}
+	const unplugged = options['maintenance-type'] === 'unplugged';
+	const defaultType = options['maintenance-type'] === '__default__';
+	if (unplugged) {
+	    options['maintenance-type'] = '';
+	}
 
 	me.callParent([options]);
 
-	Ext.ComponentManager.get('message-field')
-	    .setDisabled(options['maintenance-type'] === '__default__');
+	Ext.ComponentManager.get('type-field').setDisabled(unplugged);
+	Ext.ComponentManager.get('message-field').setDisabled(unplugged || defaultType);
     },
 });
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 22/26] ui: datstore selector: add icon for removable datastores
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
                   ` (22 preceding siblings ...)
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 21/26] ui: disable maintenance update while removable datastore is unplugged Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 23/26] api2: admin: add mount-device endpoint that just takes an UUID Hannes Laimer
                   ` (3 subsequent siblings)
  27 siblings, 0 replies; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 www/form/DataStoreSelector.js | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/www/form/DataStoreSelector.js b/www/form/DataStoreSelector.js
index eae42bef..e1ee46ad 100644
--- a/www/form/DataStoreSelector.js
+++ b/www/form/DataStoreSelector.js
@@ -25,6 +25,9 @@ Ext.define('PBS.form.DataStoreSelector', {
 			let tip = Ext.String.htmlEncode(PBS.Utils.renderMaintenance(rec.data?.maintenance));
 			icon = ` <i data-qtip="${tip}" class="fa fa-wrench"></i>`;
 		    }
+		    if (rec.data?.removable) {
+			icon = ` <i class="fa fa-usb"></i>`;
+		    }
 		    return Ext.String.htmlEncode(v) + icon;
 		},
 		flex: 1,
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 23/26] api2: admin: add mount-device endpoint that just takes an UUID
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
                   ` (23 preceding siblings ...)
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 22/26] ui: datstore selector: add icon for removable datastores Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 24/26] cli: manager: add removable-device commands Hannes Laimer
                   ` (2 subsequent siblings)
  27 siblings, 0 replies; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

... needed for auto mounting a removable device when it is plugged in

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/admin/mod.rs              |  4 ++
 src/api2/admin/removable_device.rs | 70 +++++++++++++++++++++++++++++-
 2 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/src/api2/admin/mod.rs b/src/api2/admin/mod.rs
index 00ff9131..09da6f2f 100644
--- a/src/api2/admin/mod.rs
+++ b/src/api2/admin/mod.rs
@@ -20,6 +20,10 @@ const SUBDIRS: SubdirMap = &sorted!([
     ("prune", &prune::ROUTER),
     ("sync", &sync::ROUTER),
     ("removable-device", &removable_device::ROUTER),
+    (
+        "mount-device",
+        &Router::new().post(&removable_device::API_METHOD_MOUNT_UUID_DEVICE),
+    ),
     ("traffic-control", &traffic_control::ROUTER),
     ("verify", &verify::ROUTER),
 ]);
diff --git a/src/api2/admin/removable_device.rs b/src/api2/admin/removable_device.rs
index 53b92ba7..54aee88f 100644
--- a/src/api2/admin/removable_device.rs
+++ b/src/api2/admin/removable_device.rs
@@ -5,14 +5,14 @@ use proxmox_router::{
     list_subdirs_api_method, ApiMethod, Permission, Router, RpcEnvironment, RpcEnvironmentType,
     SubdirMap,
 };
-use proxmox_schema::api;
+use proxmox_schema::{api, param_bail};
 use proxmox_section_config::SectionConfigData;
 use proxmox_sys::{sortable, task_log, WorkerTaskContext};
 use serde_json::Value;
 
 use pbs_api_types::{
     Authid, DataStoreConfig, RemovableDeviceConfig, RemovableDeviceStatus, DATASTORE_SCHEMA,
-    DEVICE_NAME_SCHEMA, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_MODIFY,
+    DEVICE_NAME_SCHEMA, DEVICE_UUID_SCHEMA, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_MODIFY,
 };
 use pbs_config::{datastore, removable_device, BackupLockGuard, CachedUserInfo};
 use pbs_datastore::ChunkStore;
@@ -202,6 +202,72 @@ pub fn mount_removable_device(
     Ok(upid)
 }
 
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            uuid: {
+                schema: DEVICE_UUID_SCHEMA,
+            }
+        }
+    },
+    access: {
+        permission: &Permission::Anybody,
+        description: "Requires Datastore.Modify on removable devices's datastore.",
+    },
+)]
+/// Mounts removable device by uuid.
+pub fn mount_uuid_device(
+    uuid: String,
+    _info: &ApiMethod,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<String, Error> {
+    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let user_info = CachedUserInfo::new()?;
+
+    let lock = pbs_config::datastore::lock_config()?;
+    let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
+
+    let (section_config, _digest) = removable_device::config()?;
+    if let Some(device_config) = section_config
+        .convert_to_typed_array::<RemovableDeviceConfig>("removable-device")?
+        .into_iter()
+        .find(|c| c.uuid.eq(&uuid))
+    {
+        user_info.check_privs(
+            &auth_id,
+            &device_config.acl_path(),
+            PRIV_DATASTORE_MODIFY,
+            true,
+        )?;
+
+        let (datastore_section_config, _digest) = datastore::config()?;
+        let store: DataStoreConfig =
+            datastore_section_config.lookup("datastore", &device_config.store)?;
+
+        let upid = WorkerTask::new_thread(
+            "mount-device",
+            Some(store.name.to_string()),
+            auth_id.to_string(),
+            to_stdout,
+            move |worker| {
+                do_mount_removable(
+                    lock,
+                    section_config,
+                    datastore_section_config,
+                    store,
+                    device_config,
+                    Some(&worker),
+                )
+            },
+        )?;
+
+        Ok(upid)
+    } else {
+        param_bail!("uuid", "no removable device with uuid '{}' exists", uuid);
+    }
+}
+
 #[sortable]
 const DEVICE_INFO_SUBDIRS: SubdirMap = &[(
     "mount",
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 24/26] cli: manager: add removable-device commands
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
                   ` (24 preceding siblings ...)
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 23/26] api2: admin: add mount-device endpoint that just takes an UUID Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 25/26] debian/etc: add udev rules and simple service for automounting Hannes Laimer
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 26/26] api: mark all removable datastores as 'unplugged' after restart Hannes Laimer
  27 siblings, 0 replies; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/bin/proxmox-backup-manager.rs             |   1 +
 src/bin/proxmox_backup_manager/mod.rs         |   2 +
 .../removable_device.rs                       | 209 ++++++++++++++++++
 3 files changed, 212 insertions(+)
 create mode 100644 src/bin/proxmox_backup_manager/removable_device.rs

diff --git a/src/bin/proxmox-backup-manager.rs b/src/bin/proxmox-backup-manager.rs
index 0ca3e990..fba02019 100644
--- a/src/bin/proxmox-backup-manager.rs
+++ b/src/bin/proxmox-backup-manager.rs
@@ -425,6 +425,7 @@ async fn run() -> Result<(), Error> {
         .insert("user", user_commands())
         .insert("openid", openid_commands())
         .insert("remote", remote_commands())
+        .insert("removable-device", removable_device_commands())
         .insert("traffic-control", traffic_control_commands())
         .insert("garbage-collection", garbage_collection_commands())
         .insert("acme", acme_mgmt_cli())
diff --git a/src/bin/proxmox_backup_manager/mod.rs b/src/bin/proxmox_backup_manager/mod.rs
index 9788f637..888caf01 100644
--- a/src/bin/proxmox_backup_manager/mod.rs
+++ b/src/bin/proxmox_backup_manager/mod.rs
@@ -14,6 +14,8 @@ mod prune;
 pub use prune::*;
 mod remote;
 pub use remote::*;
+mod removable_device;
+pub use removable_device::*;
 mod sync;
 pub use sync::*;
 mod verify;
diff --git a/src/bin/proxmox_backup_manager/removable_device.rs b/src/bin/proxmox_backup_manager/removable_device.rs
new file mode 100644
index 00000000..383840b2
--- /dev/null
+++ b/src/bin/proxmox_backup_manager/removable_device.rs
@@ -0,0 +1,209 @@
+use anyhow::Error;
+use serde_json::{json, Value};
+
+use proxmox_router::{cli::*, ApiHandler, RpcEnvironment};
+use proxmox_schema::api;
+
+use pbs_api_types::{RemovableDeviceConfig, DEVICE_NAME_SCHEMA, DEVICE_UUID_SCHEMA};
+use pbs_client::view_task_result;
+
+use proxmox_backup::api2;
+use proxmox_backup::client_helpers::connect_to_localhost;
+
+#[api(
+    input: {
+        properties: {
+            "output-format": {
+                schema: OUTPUT_FORMAT,
+                optional: true,
+            },
+        }
+    }
+)]
+/// Removable device list.
+fn list_removable_devices(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
+    let output_format = get_output_format(&param);
+
+    let info = &api2::config::removable_device::API_METHOD_LIST_REMOVABLE_DEVICE;
+    let mut data = match info.handler {
+        ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
+        _ => unreachable!(),
+    };
+
+    let options = default_table_format_options()
+        .column(ColumnConfig::new("name"))
+        .column(ColumnConfig::new("store"))
+        .column(ColumnConfig::new("initialized"))
+        .column(ColumnConfig::new("uuid"));
+
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
+
+    Ok(Value::Null)
+}
+
+#[api(
+    input: {
+        properties: {
+            name: {
+                schema: DEVICE_NAME_SCHEMA,
+            },
+            "output-format": {
+                schema: OUTPUT_FORMAT,
+                optional: true,
+            },
+        }
+    }
+)]
+/// Show removable devive configuration
+fn show_removable_device(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
+    let output_format = get_output_format(&param);
+
+    let info = &api2::config::removable_device::API_METHOD_READ_REMOVABLE_DEVICE;
+    let mut data = match info.handler {
+        ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
+        _ => unreachable!(),
+    };
+
+    let options = default_table_format_options();
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
+
+    Ok(Value::Null)
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            config: {
+                type: RemovableDeviceConfig,
+                flatten: true,
+            },
+            "output-format": {
+                schema: OUTPUT_FORMAT,
+                optional: true,
+            },
+        },
+    },
+)]
+/// Create new removable device config.
+async fn create_removable_device(mut param: Value) -> Result<Value, Error> {
+    let output_format = extract_output_format(&mut param);
+
+    let client = connect_to_localhost()?;
+
+    let result = client
+        .post("api2/json/config/removable-device", Some(param))
+        .await?;
+
+    view_task_result(&client, result, &output_format).await?;
+
+    Ok(Value::Null)
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            name: {
+                schema: DEVICE_NAME_SCHEMA,
+            },
+            "output-format": {
+                schema: OUTPUT_FORMAT,
+                optional: true,
+            },
+        },
+    },
+)]
+/// Mount removable device.
+async fn mount_removable_device(name: String, mut param: Value) -> Result<Value, Error> {
+    let output_format = extract_output_format(&mut param);
+
+    let client = connect_to_localhost()?;
+
+    let path = format!("api2/json/admin/removable-device/{}/mount", name);
+
+    let result = client.post(&path, None).await?;
+
+    view_task_result(&client, result, &output_format).await?;
+
+    Ok(Value::Null)
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            uuid: {
+                schema: DEVICE_UUID_SCHEMA,
+            },
+            "output-format": {
+                schema: OUTPUT_FORMAT,
+                optional: true,
+            },
+        },
+    },
+)]
+/// Mount removable device by uuid, this just triggers the mounting in the backend and ignores the result.
+async fn mount_uuid_device(uuid: String) -> Result<Value, Error> {
+    let client = connect_to_localhost()?;
+
+    let path = "api2/json/admin/mount-device";
+
+    client.post(&path, Some(json!({ "uuid": uuid }))).await?;
+
+    Ok(Value::Null)
+}
+
+pub fn removable_device_commands() -> CommandLineInterface {
+    let cmd_def = CliCommandMap::new()
+        .insert("list", CliCommand::new(&API_METHOD_LIST_REMOVABLE_DEVICES))
+        .insert(
+            "show",
+            CliCommand::new(&API_METHOD_SHOW_REMOVABLE_DEVICE)
+                .arg_param(&["name"])
+                .completion_cb(
+                    "name",
+                    pbs_config::removable_device::complete_removable_device_name,
+                ),
+        )
+        .insert(
+            "create",
+            CliCommand::new(&API_METHOD_CREATE_REMOVABLE_DEVICE)
+                .arg_param(&["name"])
+                .completion_cb("store", pbs_config::datastore::complete_datastore_name),
+        )
+        .insert(
+            "mount",
+            CliCommand::new(&API_METHOD_MOUNT_REMOVABLE_DEVICE)
+                .arg_param(&["name"])
+                .completion_cb(
+                    "name",
+                    pbs_config::removable_device::complete_removable_device_name,
+                ),
+        )
+        .insert(
+            "mount-uuid",
+            CliCommand::new(&API_METHOD_MOUNT_UUID_DEVICE).arg_param(&["uuid"]),
+        )
+        .insert(
+            "update",
+            CliCommand::new(&api2::config::removable_device::API_METHOD_UPDATE_REMOVABLE_DEVICE)
+                .arg_param(&["name"])
+                .completion_cb(
+                    "name",
+                    pbs_config::removable_device::complete_removable_device_name,
+                )
+                .completion_cb("store", pbs_config::datastore::complete_datastore_name),
+        )
+        .insert(
+            "remove",
+            CliCommand::new(&api2::config::removable_device::API_METHOD_DELETE_REMOVABLE_DEVICE)
+                .arg_param(&["name"])
+                .completion_cb(
+                    "name",
+                    pbs_config::removable_device::complete_removable_device_name,
+                ),
+        );
+
+    cmd_def.into()
+}
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 25/26] debian/etc: add udev rules and simple service for automounting
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
                   ` (25 preceding siblings ...)
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 24/26] cli: manager: add removable-device commands Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  2022-07-06 11:49   ` Wolfgang Bumiller
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 26/26] api: mark all removable datastores as 'unplugged' after restart Hannes Laimer
  27 siblings, 1 reply; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 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 | 6 ++++++
 4 files changed, 12 insertions(+), 1 deletion(-)
 create mode 100644 etc/removable-device-attach@.service.in

diff --git a/debian/proxmox-backup-server.install b/debian/proxmox-backup-server.install
index 6e2219b4..3cabd46e 100644
--- a/debian/proxmox-backup-server.install
+++ b/debian/proxmox-backup-server.install
@@ -3,6 +3,7 @@ etc/proxmox-backup.service /lib/systemd/system/
 etc/proxmox-backup-banner.service /lib/systemd/system/
 etc/proxmox-backup-daily-update.service /lib/systemd/system/
 etc/proxmox-backup-daily-update.timer /lib/systemd/system/
+etc/removable-device-attach@.service /lib/systemd/system/
 etc/pbs-enterprise.list /etc/apt/sources.list.d/
 usr/lib/x86_64-linux-gnu/proxmox-backup/proxmox-backup-api
 usr/lib/x86_64-linux-gnu/proxmox-backup/proxmox-backup-proxy
diff --git a/debian/proxmox-backup-server.udev b/debian/proxmox-backup-server.udev
index afdfb2bc..e5aa26c3 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}"
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..12fa103f
--- /dev/null
+++ b/etc/removable-device-attach@.service.in
@@ -0,0 +1,6 @@
+[Unit]
+Description=Try to mount the removable device with uuid '%i'.
+
+[Service]
+Type=simple
+ExecStart=/usr/sbin/proxmox-backup-manager removable-device mount-uuid %i
-- 
2.30.2





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

* [pbs-devel] [PATCH proxmox-backup 26/26] api: mark all removable datastores as 'unplugged' after restart
  2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
                   ` (26 preceding siblings ...)
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 25/26] debian/etc: add udev rules and simple service for automounting Hannes Laimer
@ 2022-07-05 13:08 ` Hannes Laimer
  27 siblings, 0 replies; 42+ messages in thread
From: Hannes Laimer @ 2022-07-05 13:08 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/bin/proxmox-backup-api.rs | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/src/bin/proxmox-backup-api.rs b/src/bin/proxmox-backup-api.rs
index dda4b638..9614a6ea 100644
--- a/src/bin/proxmox-backup-api.rs
+++ b/src/bin/proxmox-backup-api.rs
@@ -8,6 +8,7 @@ use http::HeaderMap;
 use http::Response;
 use hyper::{Body, Method, StatusCode};
 
+use pbs_api_types::{DataStoreConfig, MaintenanceMode, MaintenanceType};
 use proxmox_lang::try_block;
 use proxmox_router::{RpcEnvironmentType, UserInformation};
 use proxmox_sys::fs::CreateOptions;
@@ -159,6 +160,7 @@ async fn run() -> Result<(), Error> {
     proxmox_rest_server::write_pid(pbs_buildcfg::PROXMOX_BACKUP_API_PID_FN)?;
 
     let init_result: Result<(), Error> = try_block!({
+        mark_removable_datastores_unplugged()?;
         proxmox_rest_server::register_task_control_commands(&mut commando_sock)?;
         commando_sock.spawn()?;
         proxmox_rest_server::catch_shutdown_signal()?;
@@ -178,3 +180,16 @@ async fn run() -> Result<(), Error> {
 
     Ok(())
 }
+
+fn mark_removable_datastores_unplugged() -> Result<(), Error> {
+    let (mut config, _digest) = pbs_config::datastore::config()?;
+    let list: Vec<DataStoreConfig> = config.convert_to_typed_array("datastore")?;
+    for mut datastore in list {
+        if datastore.removable {
+            datastore.set_maintenance_mode(MaintenanceMode::new(MaintenanceType::Unplugged, None));
+            config.set_data(&datastore.name, "datastore", &datastore)?;
+        }
+    }
+    pbs_config::datastore::save_config(&config)?;
+    Ok(())
+}
-- 
2.30.2





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

* Re: [pbs-devel] [PATCH proxmox 1/1] schema: add print_property_string function
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox 1/1] schema: add print_property_string function Hannes Laimer
@ 2022-07-06 11:31   ` Wolfgang Bumiller
  0 siblings, 0 replies; 42+ messages in thread
From: Wolfgang Bumiller @ 2022-07-06 11:31 UTC (permalink / raw)
  To: Hannes Laimer; +Cc: pbs-devel

On Tue, Jul 05, 2022 at 01:08:08PM +0000, Hannes Laimer wrote:
> helpful when property string field are not updated through the API
> 
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  proxmox-schema/src/schema.rs | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/proxmox-schema/src/schema.rs b/proxmox-schema/src/schema.rs
> index a2b165c..8c93452 100644
> --- a/proxmox-schema/src/schema.rs
> +++ b/proxmox-schema/src/schema.rs
> @@ -7,6 +7,7 @@
>  use std::fmt;
>  
>  use anyhow::{bail, format_err, Error};
> +use serde::Serialize;
>  use serde_json::{json, Value};
>  
>  use crate::ConstRegexPattern;
> @@ -932,6 +933,33 @@ impl Schema {
>          Ok(value)
>      }
>  
> +    /// Generate property string of structs with only simple fields
> +    pub fn print_property_string<T: Serialize>(&'static self, data: &T) -> Result<String, Error> {

I think this should start right away with `to_value` and then pass it to a
non-generic private helper taking the `Value`, to avoid multiple
instances, since we need the `Value` anyway, at least for now.

> +        fn print_simple_value(value: Value) -> Result<String, Error> {

essentially this, but this should also be handling `Object`, and also
`Array` which you're currently not handling at all, since we now support
quoted values which *technically* allow more nesting (even though we
currently don't make use of that).

I'd also *prefer* to avoid the temporary strings by eg. adding 2 more
parameters: `output: &mut String` to avoid all the temporary string
allocations, and `quoting: u32` to know how many times we need to quote
the contents - which you're also not doing at all currently.

(but if that seems too tedious for you now, you can keep going with the
string values for now)

> +            let string = match value {
> +                Value::Bool(b) if b => "1".to_string(),
> +                Value::Bool(_) => "0".to_string(),
> +                Value::Number(n) => n.to_string(),
> +                Value::String(s) => s,
> +                _ => {
> +                    bail!("Only structs with only simple fields can be formatted as a property string.")
> +                }
> +            };
> +            Ok(string)
> +        }
> +
> +        match serde_json::to_value(data)? {
> +            Value::Object(obj) => {
> +                let mut props: Vec<String> = Vec::new();
> +                for (key, value) in obj.into_iter() {
> +                    props.push(format!("{}=\"{}\"", key, print_simple_value(value)?));

We don't typically put quotes around values unless required by their
contents (if they contain commas, or - if inside an array - semicolons,
so you might need to pass along a list of in-use separators while
recursing through the data).

Note that objects can define a `default_key` which would come first and
not actually write out `key=` before.
This means you also need to walk through the *schema* here, not only the
values.

Also: This definitely needs `#[test]` cases for at least arrays and
objects with simple contents and objects containing simple arrays.

> +                }
> +                Ok(props.join(","))
> +            }
> +            value => print_simple_value(value),
> +        }
> +    }
> +
>      /// Parse a complex property string (`ApiStringFormat::PropertyString`)
>      pub fn parse_property_string(&'static self, value_str: &str) -> Result<Value, Error> {
>          // helper for object/allof schemas:
> -- 
> 2.30.2




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

* Re: [pbs-devel] [PATCH proxmox-backup 02/26] config: make RemovableDeviceConfig savable to config file
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 02/26] config: make RemovableDeviceConfig savable to config file Hannes Laimer
@ 2022-07-06 11:33   ` Wolfgang Bumiller
  2022-07-06 11:44     ` Thomas Lamprecht
  0 siblings, 1 reply; 42+ messages in thread
From: Wolfgang Bumiller @ 2022-07-06 11:33 UTC (permalink / raw)
  To: Hannes Laimer; +Cc: pbs-devel

Design question:

Do we need this to be a separate config file though? Can this not simply
be part of the datastore directly? We already "link" them by having to
define the datastore as `removable`, so can we not just put all the
values in there?

On Tue, Jul 05, 2022 at 01:08:10PM +0000, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  pbs-config/src/lib.rs              |  1 +
>  pbs-config/src/removable_device.rs | 63 ++++++++++++++++++++++++++++++
>  2 files changed, 64 insertions(+)
>  create mode 100644 pbs-config/src/removable_device.rs
> 
> diff --git a/pbs-config/src/lib.rs b/pbs-config/src/lib.rs
> index a83db4e1..5ea67652 100644
> --- a/pbs-config/src/lib.rs
> +++ b/pbs-config/src/lib.rs
> @@ -10,6 +10,7 @@ pub mod metrics;
>  pub mod network;
>  pub mod prune;
>  pub mod remote;
> +pub mod removable_device;
>  pub mod sync;
>  pub mod tape_encryption_keys;
>  pub mod tape_job;
> diff --git a/pbs-config/src/removable_device.rs b/pbs-config/src/removable_device.rs
> new file mode 100644
> index 00000000..4587d72b
> --- /dev/null
> +++ b/pbs-config/src/removable_device.rs
> @@ -0,0 +1,63 @@
> +use std::collections::HashMap;
> +
> +use anyhow::Error;
> +use lazy_static::lazy_static;
> +
> +use pbs_api_types::{RemovableDeviceConfig, DEVICE_NAME_SCHEMA};
> +use proxmox_schema::*;
> +use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
> +
> +use crate::{open_backup_lockfile, replace_backup_config, BackupLockGuard};
> +
> +lazy_static! {
> +    pub static ref CONFIG: SectionConfig = init();
> +}
> +
> +fn init() -> SectionConfig {
> +    const OBJ_SCHEMA: &ObjectSchema = RemovableDeviceConfig::API_SCHEMA.unwrap_object_schema();
> +
> +    let plugin = SectionConfigPlugin::new(
> +        "removable-device".to_string(),
> +        Some(String::from("name")),
> +        OBJ_SCHEMA,
> +    );
> +    let mut config = SectionConfig::new(&DEVICE_NAME_SCHEMA);
> +    config.register_plugin(plugin);
> +
> +    config
> +}
> +
> +pub const REMOVABLE_DEVIVE_CFG_FILENAME: &str = "/etc/proxmox-backup/removable-device.cfg";
> +pub const REMOVABLE_DEVIVE_CFG_LOCKFILE: &str = "/etc/proxmox-backup/.removable-device.lck";
> +
> +/// Get exclusive lock
> +pub fn lock_config() -> Result<BackupLockGuard, Error> {
> +    open_backup_lockfile(REMOVABLE_DEVIVE_CFG_LOCKFILE, None, true)
> +}
> +
> +pub fn config() -> Result<(SectionConfigData, [u8; 32]), Error> {
> +    let content = proxmox_sys::fs::file_read_optional_string(REMOVABLE_DEVIVE_CFG_FILENAME)?;
> +    let content = content.unwrap_or_default();
> +
> +    let digest = openssl::sha::sha256(content.as_bytes());
> +    let data = CONFIG.parse(REMOVABLE_DEVIVE_CFG_FILENAME, &content)?;
> +
> +    Ok((data, digest))
> +}
> +
> +pub fn save_config(config: &SectionConfigData) -> Result<(), Error> {
> +    let raw = CONFIG.write(REMOVABLE_DEVIVE_CFG_FILENAME, config)?;
> +    replace_backup_config(REMOVABLE_DEVIVE_CFG_FILENAME, raw.as_bytes())
> +}
> +
> +// shell completion helper
> +pub fn complete_removable_device_name(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> {
> +    match config() {
> +        Ok((data, _digest)) => data
> +            .sections
> +            .iter()
> +            .map(|(name, _)| name.to_string())
> +            .collect(),
> +        Err(_) => return vec![],
> +    }
> +}
> -- 
> 2.30.2




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

* Re: [pbs-devel] [PATCH proxmox-backup 07/26] api2: datastore create: don't init chunkstore if removable
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 07/26] api2: datastore create: don't init chunkstore if removable Hannes Laimer
@ 2022-07-06 11:35   ` Wolfgang Bumiller
  2022-07-07  9:06     ` Hannes Laimer
  0 siblings, 1 reply; 42+ messages in thread
From: Wolfgang Bumiller @ 2022-07-06 11:35 UTC (permalink / raw)
  To: Hannes Laimer; +Cc: pbs-devel

On Tue, Jul 05, 2022 at 01:08:15PM +0000, Hannes Laimer wrote:
> .. instead set the maintenance mode to unplugged. So on creation every
> removable datastore is unplugged until a removable device is associated
> with it.

As a continuation of my design question: if we were to include the
removable device info right in the datastore, we could just drop the
late initialization code altogether IMO.

Actually, we can drop it regardless IMO. When do you really need to
declare a datastore before adding the devices for it? Is that really
something we need?




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

* Re: [pbs-devel] [PATCH proxmox-backup 06/26] api-types: add set_maintenance_mode function to DataStoreConfig
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 06/26] api-types: add set_maintenance_mode function to DataStoreConfig Hannes Laimer
@ 2022-07-06 11:40   ` Wolfgang Bumiller
  0 siblings, 0 replies; 42+ messages in thread
From: Wolfgang Bumiller @ 2022-07-06 11:40 UTC (permalink / raw)
  To: Hannes Laimer; +Cc: pbs-devel

On Tue, Jul 05, 2022 at 01:08:14PM +0000, Hannes Laimer wrote:
> helper for updating the MaintenanceMode of a Datastore not through the
> API
> 
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  pbs-api-types/src/datastore.rs   | 9 +++++++++
>  pbs-api-types/src/maintenance.rs | 6 +++++-
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
> index 965e3795..ce77f47d 100644
> --- a/pbs-api-types/src/datastore.rs
> +++ b/pbs-api-types/src/datastore.rs
> @@ -313,6 +313,15 @@ impl DataStoreConfig {
>              .and_then(|str| MaintenanceMode::API_SCHEMA.parse_property_string(str).ok())
>              .and_then(|value| MaintenanceMode::deserialize(value).ok())
>      }
> +
> +    pub fn set_maintenance_mode(&mut self, mode: MaintenanceMode) {
> +        if let Some(property_string) = MaintenanceMode::API_SCHEMA

Instead of using the `API_SCHEMA` *here*, this should be a helper method
in `MaintenanceMode`.
And I don't think it should be able to fail. Given that
`MaintenanceMode` currently is just a mode with a reason string, the
stringification (& potential string quoting) could be done there without
a fully generic `print_property_string` helper way more easily ;-)

> +            .print_property_string(&mode)
> +            .ok()
> +        {
> +            self.maintenance_mode = Some(property_string);
> +        }
> +    }
>  }
>  
>  #[api(
> diff --git a/pbs-api-types/src/maintenance.rs b/pbs-api-types/src/maintenance.rs
> index 4f0dfc7f..ec34397f 100644
> --- a/pbs-api-types/src/maintenance.rs
> +++ b/pbs-api-types/src/maintenance.rs
> @@ -67,7 +67,7 @@ pub enum MaintenanceType {
>  pub struct MaintenanceMode {
>      /// Type of maintenance ("read-only", "offline" or "unplugged").
>      #[serde(rename = "type")]
> -    ty: MaintenanceType,
> +    pub ty: MaintenanceType,
>  
>      /// Reason for maintenance.
>      #[serde(skip_serializing_if = "Option::is_none")]
> @@ -75,6 +75,10 @@ pub struct MaintenanceMode {
>  }
>  
>  impl MaintenanceMode {
> +    pub fn new(ty: MaintenanceType, message: Option<String>) -> Self {
> +        MaintenanceMode { ty, message }
> +    }
> +
>      pub fn check(&self, operation: Option<Operation>) -> Result<(), Error> {
>          let message = percent_encoding::percent_decode_str(self.message.as_deref().unwrap_or(""))
>              .decode_utf8()
> -- 
> 2.30.2




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

* Re: [pbs-devel] [PATCH proxmox-backup 08/26] tools: disks: add mount and unmount helper
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 08/26] tools: disks: add mount and unmount helper Hannes Laimer
@ 2022-07-06 11:42   ` Wolfgang Bumiller
  0 siblings, 0 replies; 42+ messages in thread
From: Wolfgang Bumiller @ 2022-07-06 11:42 UTC (permalink / raw)
  To: Hannes Laimer; +Cc: pbs-devel

On Tue, Jul 05, 2022 at 01:08:16PM +0000, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  src/tools/disks/mod.rs | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/src/tools/disks/mod.rs b/src/tools/disks/mod.rs
> index 35ec9996..e217ac87 100644
> --- a/src/tools/disks/mod.rs
> +++ b/src/tools/disks/mod.rs
> @@ -1136,6 +1136,24 @@ pub fn complete_disk_name(_arg: &str, _param: &HashMap<String, String>) -> Vec<S
>          .collect()
>  }
>  
> +pub fn mount_by_uuid(uuid: &str, mountpoint: &str) -> Result<(), Error> {
> +    let mut command = std::process::Command::new("mount");
> +    command.args(&[format!("UUID={}", uuid)]);
> +    command.arg(mountpoint);
> +
> +    proxmox_sys::command::run_command(command, None)?;
> +    Ok(())
> +}
> +
> +pub fn unmount_by_mountpoint(path: &str) -> Result<(), Error> {
> +    let mut command = std::process::Command::new("umount");
> +    command.arg("-l");

I'm really not a fan of using `-l` at all. This would pull the device
out from any current user of it. Anything not exclusively using
`openat()` on an already open directory handle to it will start to
simply keep using the paths without the disk mounted.

I'd rather have it fail when still in use.

> +    command.arg(path);
> +
> +    proxmox_sys::command::run_command(command, None)?;
> +    Ok(())
> +}
> +
>  /// Read the FS UUID (parse blkid output)
>  ///
>  /// Note: Calling blkid is more reliable than using the udev ID_FS_UUID property.
> -- 
> 2.30.2




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

* Re: [pbs-devel] [PATCH proxmox-backup 09/26] api2: admin: add unmount-device endpoint to datastore
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 09/26] api2: admin: add unmount-device endpoint to datastore Hannes Laimer
@ 2022-07-06 11:43   ` Wolfgang Bumiller
  2022-07-07  9:11     ` Hannes Laimer
  0 siblings, 1 reply; 42+ messages in thread
From: Wolfgang Bumiller @ 2022-07-06 11:43 UTC (permalink / raw)
  To: Hannes Laimer; +Cc: pbs-devel

On Tue, Jul 05, 2022 at 01:08:17PM +0000, Hannes Laimer wrote:
> unmounts the removable device mounted at the datstore path and set the
> maintenance-mode of the datastore to 'unplugged'. The worker waits for
> all running tasks to finish before unmounting.
> 
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  src/api2/admin/datastore.rs | 97 +++++++++++++++++++++++++++++++++----
>  1 file changed, 87 insertions(+), 10 deletions(-)
> 
> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
> index 5448fa10..2be95f92 100644
> --- a/src/api2/admin/datastore.rs
> +++ b/src/api2/admin/datastore.rs
> @@ -22,27 +22,28 @@ use proxmox_router::{
>      Router, RpcEnvironment, RpcEnvironmentType, SubdirMap,
>  };
>  use proxmox_schema::*;
> +use proxmox_section_config::SectionConfigData;
>  use proxmox_sys::fs::{
>      file_read_firstline, file_read_optional_string, replace_file, CreateOptions,
>  };
> -use proxmox_sys::sortable;
> -use proxmox_sys::{task_log, task_warn};
> +use proxmox_sys::{sortable, task_log, task_warn, WorkerTaskContext};
>  
>  use pxar::accessor::aio::Accessor;
>  use pxar::EntryKind;
>  
>  use pbs_api_types::{
>      print_ns_and_snapshot, print_store_and_ns, Authid, BackupContent, BackupNamespace, BackupType,
> -    Counts, CryptMode, DataStoreListItem, DataStoreStatus, GarbageCollectionStatus, GroupListItem,
> -    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_SCHEMA, VERIFICATION_OUTDATED_AFTER_SCHEMA,
> +    Counts, CryptMode, DataStoreConfig, DataStoreListItem, DataStoreStatus,
> +    GarbageCollectionStatus, GroupListItem, 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_SCHEMA,
> +    VERIFICATION_OUTDATED_AFTER_SCHEMA,
>  };
>  use pbs_client::pxar::{create_tar, create_zip};
> -use pbs_config::CachedUserInfo;
> +use pbs_config::{BackupLockGuard, CachedUserInfo};
>  use pbs_datastore::backup_info::BackupInfo;
>  use pbs_datastore::cached_chunk_reader::CachedChunkReader;
>  use pbs_datastore::catalog::{ArchiveEntry, CatalogReader};
> @@ -106,6 +107,39 @@ fn check_privs_and_load_store(
>      Ok(datastore)
>  }
>  
> +fn do_unmount_device(
> +    _lock: BackupLockGuard,
> +    mut config: SectionConfigData,
> +    mut datastore: DataStoreConfig,
> +    worker: Option<&dyn WorkerTaskContext>,
> +) -> Result<(), Error> {
> +    datastore.set_maintenance_mode(MaintenanceMode::new(MaintenanceType::Unplugged, None));
> +    config.set_data(&datastore.name, "datastore", &datastore)?;
> +    pbs_config::datastore::save_config(&config)?;
> +    drop(_lock);
> +
> +    let mut active_operations = task_tracking::get_active_operations(&datastore.name)?;
> +    while active_operations.read + active_operations.write > 0 {

I'd again prefer to fail than loop endlessly...

> +        if let Some(worker) = worker {
> +            if worker.abort_requested() {
> +                bail!("aborted, due to user request");
> +            }
> +            task_log!(
> +                worker,
> +                "can't unmount yet, still {} read and {} write operations active",
> +                active_operations.read,
> +                active_operations.write
> +            );
> +        }
> +
> +        std::thread::sleep(std::time::Duration::new(5, 0));
> +        active_operations = task_tracking::get_active_operations(&datastore.name)?;
> +    }
> +
> +    crate::tools::disks::unmount_by_mountpoint(&datastore.path)?;
> +
> +    Ok(())
> +}
>  fn read_backup_index(
>      backup_dir: &BackupDir,
>  ) -> Result<(BackupManifest, Vec<BackupContent>), Error> {




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

* Re: [pbs-devel] [PATCH proxmox-backup 02/26] config: make RemovableDeviceConfig savable to config file
  2022-07-06 11:33   ` Wolfgang Bumiller
@ 2022-07-06 11:44     ` Thomas Lamprecht
  2022-07-07  8:35       ` Hannes Laimer
  0 siblings, 1 reply; 42+ messages in thread
From: Thomas Lamprecht @ 2022-07-06 11:44 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Wolfgang Bumiller,
	Hannes Laimer

On 06/07/2022 13:33, Wolfgang Bumiller wrote:
> Do we need this to be a separate config file though? Can this not simply
> be part of the datastore directly? We already "link" them by having to
> define the datastore as `removable`, so can we not just put all the
> values in there?

IIRC we talked about adding just a "backing-device", or the like (probably
something a bit more explicit w.r.t. to removable), property to existing
datastores, and then pretty much handle them like existing ones.

That way we can reuse most of existing infrastructure and functionality,
what changes is a different (or no) error on sync, GC, etc. (or repeat skipped
jobs when plugged in) and the "auto-mount + activate in PBS" via udev helper.




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

* Re: [pbs-devel] [PATCH proxmox-backup 25/26] debian/etc: add udev rules and simple service for automounting
  2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 25/26] debian/etc: add udev rules and simple service for automounting Hannes Laimer
@ 2022-07-06 11:49   ` Wolfgang Bumiller
  0 siblings, 0 replies; 42+ messages in thread
From: Wolfgang Bumiller @ 2022-07-06 11:49 UTC (permalink / raw)
  To: Hannes Laimer; +Cc: pbs-devel

On Tue, Jul 05, 2022 at 01:08:33PM +0000, 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 | 6 ++++++
>  4 files changed, 12 insertions(+), 1 deletion(-)
>  create mode 100644 etc/removable-device-attach@.service.in
> 
> diff --git a/debian/proxmox-backup-server.install b/debian/proxmox-backup-server.install
> index 6e2219b4..3cabd46e 100644
> --- a/debian/proxmox-backup-server.install
> +++ b/debian/proxmox-backup-server.install
> @@ -3,6 +3,7 @@ etc/proxmox-backup.service /lib/systemd/system/
>  etc/proxmox-backup-banner.service /lib/systemd/system/
>  etc/proxmox-backup-daily-update.service /lib/systemd/system/
>  etc/proxmox-backup-daily-update.timer /lib/systemd/system/
> +etc/removable-device-attach@.service /lib/systemd/system/
>  etc/pbs-enterprise.list /etc/apt/sources.list.d/
>  usr/lib/x86_64-linux-gnu/proxmox-backup/proxmox-backup-api
>  usr/lib/x86_64-linux-gnu/proxmox-backup/proxmox-backup-proxy
> diff --git a/debian/proxmox-backup-server.udev b/debian/proxmox-backup-server.udev
> index afdfb2bc..e5aa26c3 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}"

Have you tried using systemd generators? (man 7 systemd.generator)

Because this adds `Wants=removable-device-attach@...` to *all* blockdevs
with uuids. I don't like this.

I think we could put a binary in `/lib/systemd/system-generators/` which
writes out a unit file for all the removable datastores which depends on
the `.device` unit created by the `TAG+="systemd"` bit of the udev rule.
(We could then remove the ENV bit).

Additionally, further unit files in `/run` would have to be created and
removed by the daemon when they are added/removed via the API/CLI.

> 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..12fa103f
> --- /dev/null
> +++ b/etc/removable-device-attach@.service.in
> @@ -0,0 +1,6 @@
> +[Unit]
> +Description=Try to mount the removable device with uuid '%i'.
> +
> +[Service]
> +Type=simple
> +ExecStart=/usr/sbin/proxmox-backup-manager removable-device mount-uuid %i
> -- 
> 2.30.2




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

* Re: [pbs-devel] [PATCH proxmox-backup 02/26] config: make RemovableDeviceConfig savable to config file
  2022-07-06 11:44     ` Thomas Lamprecht
@ 2022-07-07  8:35       ` Hannes Laimer
  2022-07-29  8:26         ` Thomas Lamprecht
  0 siblings, 1 reply; 42+ messages in thread
From: Hannes Laimer @ 2022-07-07  8:35 UTC (permalink / raw)
  To: Thomas Lamprecht, Proxmox Backup Server development discussion,
	Wolfgang Bumiller

Am 06.07.22 um 13:44 schrieb Thomas Lamprecht:
> On 06/07/2022 13:33, Wolfgang Bumiller wrote:
>> Do we need this to be a separate config file though? Can this not simply
>> be part of the datastore directly? We already "link" them by having to
>> define the datastore as `removable`, so can we not just put all the
>> values in there?
> 
> IIRC we talked about adding just a "backing-device", or the like (probably
> something a bit more explicit w.r.t. to removable), property to existing
> datastores, and then pretty much handle them like existing ones.
> 
> That way we can reuse most of existing infrastructure and functionality,
> what changes is a different (or no) error on sync, GC, etc. (or repeat skipped
> jobs when plugged in) and the "auto-mount + activate in PBS" via udev helper.

Yes, we could put all the removable-device data directly into the
datastore config. But I think this is a cleaner and more flexible
approach, I guess you could argue similarly for why sync and prune jobs
have their own configs. What do you mean with we can't reuse
infrastructure with the own config approach? Sync/Prune/GC work like on
a normal datastore, just with the possibility of failing with "no
device present".

I think by having a removable-device as its own thing we actually end up
reusing more of the already existing API/UI functionality, because
having it directly in the datastore config would mean a lot of "manual"
parsing of property strings and a "custom" update/add implementation.




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

* Re: [pbs-devel] [PATCH proxmox-backup 07/26] api2: datastore create: don't init chunkstore if removable
  2022-07-06 11:35   ` Wolfgang Bumiller
@ 2022-07-07  9:06     ` Hannes Laimer
  0 siblings, 0 replies; 42+ messages in thread
From: Hannes Laimer @ 2022-07-07  9:06 UTC (permalink / raw)
  To: Wolfgang Bumiller; +Cc: pbs-devel

Am 06.07.22 um 13:35 schrieb Wolfgang Bumiller:
> On Tue, Jul 05, 2022 at 01:08:15PM +0000, Hannes Laimer wrote:
>> .. instead set the maintenance mode to unplugged. So on creation every
>> removable datastore is unplugged until a removable device is associated
>> with it.
> 
> As a continuation of my design question: if we were to include the
> removable device info right in the datastore, we could just drop the
> late initialization code altogether IMO.

Yes, but we will have to do it also in a another place as soon as we add
a seconds removable device. So by moving the initialization to the
mounting process we have it in just one place.
> 
> Actually, we can drop it regardless IMO. When do you really need to
> declare a datastore before adding the devices for it? Is that really
> something we need?

If we have a PBS and want to use a removable device that contains data
that was written by another PBS we have to be able to create a datastore
without creating the chunkstore on the device.




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

* Re: [pbs-devel] [PATCH proxmox-backup 09/26] api2: admin: add unmount-device endpoint to datastore
  2022-07-06 11:43   ` Wolfgang Bumiller
@ 2022-07-07  9:11     ` Hannes Laimer
  2022-07-29  9:24       ` Wolfgang Bumiller
  0 siblings, 1 reply; 42+ messages in thread
From: Hannes Laimer @ 2022-07-07  9:11 UTC (permalink / raw)
  To: Wolfgang Bumiller; +Cc: pbs-devel

Am 06.07.22 um 13:43 schrieb Wolfgang Bumiller:
> On Tue, Jul 05, 2022 at 01:08:17PM +0000, Hannes Laimer wrote:
>> unmounts the removable device mounted at the datstore path and set the
>> maintenance-mode of the datastore to 'unplugged'. The worker waits for
>> all running tasks to finish before unmounting.
>>
>> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
>> ---
>>   src/api2/admin/datastore.rs | 97 +++++++++++++++++++++++++++++++++----
>>   1 file changed, 87 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
>> index 5448fa10..2be95f92 100644
>> --- a/src/api2/admin/datastore.rs
>> +++ b/src/api2/admin/datastore.rs
>> @@ -22,27 +22,28 @@ use proxmox_router::{
>>       Router, RpcEnvironment, RpcEnvironmentType, SubdirMap,
>>   };
>>   use proxmox_schema::*;
>> +use proxmox_section_config::SectionConfigData;
>>   use proxmox_sys::fs::{
>>       file_read_firstline, file_read_optional_string, replace_file, CreateOptions,
>>   };
>> -use proxmox_sys::sortable;
>> -use proxmox_sys::{task_log, task_warn};
>> +use proxmox_sys::{sortable, task_log, task_warn, WorkerTaskContext};
>>   
>>   use pxar::accessor::aio::Accessor;
>>   use pxar::EntryKind;
>>   
>>   use pbs_api_types::{
>>       print_ns_and_snapshot, print_store_and_ns, Authid, BackupContent, BackupNamespace, BackupType,
>> -    Counts, CryptMode, DataStoreListItem, DataStoreStatus, GarbageCollectionStatus, GroupListItem,
>> -    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_SCHEMA, VERIFICATION_OUTDATED_AFTER_SCHEMA,
>> +    Counts, CryptMode, DataStoreConfig, DataStoreListItem, DataStoreStatus,
>> +    GarbageCollectionStatus, GroupListItem, 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_SCHEMA,
>> +    VERIFICATION_OUTDATED_AFTER_SCHEMA,
>>   };
>>   use pbs_client::pxar::{create_tar, create_zip};
>> -use pbs_config::CachedUserInfo;
>> +use pbs_config::{BackupLockGuard, CachedUserInfo};
>>   use pbs_datastore::backup_info::BackupInfo;
>>   use pbs_datastore::cached_chunk_reader::CachedChunkReader;
>>   use pbs_datastore::catalog::{ArchiveEntry, CatalogReader};
>> @@ -106,6 +107,39 @@ fn check_privs_and_load_store(
>>       Ok(datastore)
>>   }
>>   
>> +fn do_unmount_device(
>> +    _lock: BackupLockGuard,
>> +    mut config: SectionConfigData,
>> +    mut datastore: DataStoreConfig,
>> +    worker: Option<&dyn WorkerTaskContext>,
>> +) -> Result<(), Error> {
>> +    datastore.set_maintenance_mode(MaintenanceMode::new(MaintenanceType::Unplugged, None));
>> +    config.set_data(&datastore.name, "datastore", &datastore)?;
>> +    pbs_config::datastore::save_config(&config)?;
>> +    drop(_lock);
>> +
>> +    let mut active_operations = task_tracking::get_active_operations(&datastore.name)?;
>> +    while active_operations.read + active_operations.write > 0 {
> 
> I'd again prefer to fail than loop endlessly...
The idea was you start some sync/verify and then go to sleep or smth, so
it would unmount the device asap. But yes, we can also fail.
> 
>> +        if let Some(worker) = worker {
>> +            if worker.abort_requested() {
>> +                bail!("aborted, due to user request");
>> +            }
>> +            task_log!(
>> +                worker,
>> +                "can't unmount yet, still {} read and {} write operations active",
>> +                active_operations.read,
>> +                active_operations.write
>> +            );
>> +        }
>> +
>> +        std::thread::sleep(std::time::Duration::new(5, 0));
>> +        active_operations = task_tracking::get_active_operations(&datastore.name)?;
>> +    }
>> +
>> +    crate::tools::disks::unmount_by_mountpoint(&datastore.path)?;
>> +
>> +    Ok(())
>> +}
>>   fn read_backup_index(
>>       backup_dir: &BackupDir,
>>   ) -> Result<(BackupManifest, Vec<BackupContent>), Error> {




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

* Re: [pbs-devel] [PATCH proxmox-backup 02/26] config: make RemovableDeviceConfig savable to config file
  2022-07-07  8:35       ` Hannes Laimer
@ 2022-07-29  8:26         ` Thomas Lamprecht
  0 siblings, 0 replies; 42+ messages in thread
From: Thomas Lamprecht @ 2022-07-29  8:26 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Hannes Laimer,
	Wolfgang Bumiller

On 07/07/2022 10:35, Hannes Laimer wrote:
> Am 06.07.22 um 13:44 schrieb Thomas Lamprecht:
>> On 06/07/2022 13:33, Wolfgang Bumiller wrote:
>>> Do we need this to be a separate config file though? Can this not simply
>>> be part of the datastore directly? We already "link" them by having to
>>> define the datastore as `removable`, so can we not just put all the
>>> values in there?
>>
>> IIRC we talked about adding just a "backing-device", or the like (probably
>> something a bit more explicit w.r.t. to removable), property to existing
>> datastores, and then pretty much handle them like existing ones.
>>
>> That way we can reuse most of existing infrastructure and functionality,
>> what changes is a different (or no) error on sync, GC, etc. (or repeat skipped
>> jobs when plugged in) and the "auto-mount + activate in PBS" via udev helper.
> 
> Yes, we could put all the removable-device data directly into the
> datastore config. But I think this is a cleaner and more flexible
> approach, I guess you could argue similarly for why sync and prune jobs
> have their own configs.

Having less config files def. reduces complexity, that is for both, us
_and_ the user/admins to maintain.

Having two separate API and possible GUI endpoints makes whole PBS more
complex to manage, especially for new users (IME the perceived complexity
grows over-proportionally with the amount of UI/API knobs exposed), so
reducing that has more weight than the complexity of a potential
implementation, or better the change to one, as most of the time simple
UI/UX can mean simpler end result (even if the way to get there definitively
can be more complex).

> What do you mean with we can't reuse
> infrastructure with the own config approach? Sync/Prune/GC work like on
> a normal datastore, just with the possibility of failing with "no
> device present".

Yeah, exactly, a separate config or section type doesn't provides much value
there, at least fwict with having only a short look now and relying more on
my previous experience with that parts of the code base.

> 
> I think by having a removable-device as its own thing we actually end up
> reusing more of the already existing API/UI functionality, because
> having it directly in the datastore config would mean a lot of "manual"
> parsing of property strings and a "custom" update/add implementation.

hmm, not sure why that should be, albeit  wouldn't it be just a normal,
optional property? And updater support probably isn't required as we don't
want to expose altering this after creation, at least not initially.

W.r.t. "reuse more": is this meant as "we can then factor out some stuff and
use it twice with the special logic for each case upfront"? As while I'm not
having all that code base in my head atm., I'd think that adding some simple
special function call to the respective run job handlers (either in the task
if we want to still log a skipped run as task log, or upfront, in the "should
a job run now" logic, if we don't want a task log entry) would seem fine and
def. less code churn without duplicating config (or section types).




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

* Re: [pbs-devel] [PATCH proxmox-backup 09/26] api2: admin: add unmount-device endpoint to datastore
  2022-07-07  9:11     ` Hannes Laimer
@ 2022-07-29  9:24       ` Wolfgang Bumiller
  0 siblings, 0 replies; 42+ messages in thread
From: Wolfgang Bumiller @ 2022-07-29  9:24 UTC (permalink / raw)
  To: Hannes Laimer; +Cc: pbs-devel

On Thu, Jul 07, 2022 at 11:11:23AM +0200, Hannes Laimer wrote:
> Am 06.07.22 um 13:43 schrieb Wolfgang Bumiller:
> > On Tue, Jul 05, 2022 at 01:08:17PM +0000, Hannes Laimer wrote:
> > > +fn do_unmount_device(
> > > +    _lock: BackupLockGuard,
> > > +    mut config: SectionConfigData,
> > > +    mut datastore: DataStoreConfig,
> > > +    worker: Option<&dyn WorkerTaskContext>,
> > > +) -> Result<(), Error> {
> > > +    datastore.set_maintenance_mode(MaintenanceMode::new(MaintenanceType::Unplugged, None));
> > > +    config.set_data(&datastore.name, "datastore", &datastore)?;
> > > +    pbs_config::datastore::save_config(&config)?;
> > > +    drop(_lock);
> > > +
> > > +    let mut active_operations = task_tracking::get_active_operations(&datastore.name)?;
> > > +    while active_operations.read + active_operations.write > 0 {
> > 
> > I'd again prefer to fail than loop endlessly...
> The idea was you start some sync/verify and then go to sleep or smth, so
> it would unmount the device asap. But yes, we can also fail.

I'd still prefer to fail for now in a way that the API user can actually
distinguish easily.
I'd rather have the API user retry the API call than have them
potentially wait hours for a response, wondering if the connection is
still alive.

Perhaps we could add a "pending unmount" flag somewhere (possibly in
the active operation counter) and make it so the last user then unmounts
it?

The API user would have to poll the status though.

Thomas and I also recently touched on the topic of "events" an API user
could monitor/subscribe to, that might also be something to consider
here.




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

end of thread, other threads:[~2022-07-29  9:24 UTC | newest]

Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-05 13:08 [pbs-devel] [SERIES proxmox-backup/proxmox/widget-toolkit 00/28] add removable datastore support Hannes Laimer
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-widget-toolkit 1/1] proxy: allow setting of reader config Hannes Laimer
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox 1/1] schema: add print_property_string function Hannes Laimer
2022-07-06 11:31   ` Wolfgang Bumiller
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 01/26] api-types: add RemovableDeviceConfig Hannes Laimer
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 02/26] config: make RemovableDeviceConfig savable to config file Hannes Laimer
2022-07-06 11:33   ` Wolfgang Bumiller
2022-07-06 11:44     ` Thomas Lamprecht
2022-07-07  8:35       ` Hannes Laimer
2022-07-29  8:26         ` Thomas Lamprecht
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 03/26] api-types: add "removable" field to DataStoreConfig Hannes Laimer
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 04/26] api2: add config endpoints for RemovableDeviceConfig Hannes Laimer
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 05/26] api-types: add "unplugged" maintenance type Hannes Laimer
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 06/26] api-types: add set_maintenance_mode function to DataStoreConfig Hannes Laimer
2022-07-06 11:40   ` Wolfgang Bumiller
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 07/26] api2: datastore create: don't init chunkstore if removable Hannes Laimer
2022-07-06 11:35   ` Wolfgang Bumiller
2022-07-07  9:06     ` Hannes Laimer
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 08/26] tools: disks: add mount and unmount helper Hannes Laimer
2022-07-06 11:42   ` Wolfgang Bumiller
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 09/26] api2: admin: add unmount-device endpoint to datastore Hannes Laimer
2022-07-06 11:43   ` Wolfgang Bumiller
2022-07-07  9:11     ` Hannes Laimer
2022-07-29  9:24       ` Wolfgang Bumiller
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 10/26] api2: admin: add mount-device and list endpoint for RemavableDevices Hannes Laimer
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 11/26] tools: disks: add uuid to PrtitionInfo Hannes Laimer
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 12/26] api-types: add "removable" to DataStoreListItem Hannes Laimer
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 13/26] ui: utils: remove parseMaintenanceMode function Hannes Laimer
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 14/26] ui: maintenance edit: disable description in maintenance edit if no type is selected Hannes Laimer
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 15/26] ui: config: add RemovableDeviceView Hannes Laimer
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 16/26] ui: add "no removable device present" mask to summary Hannes Laimer
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 17/26] ui: add removable datastore specific icons to list Hannes Laimer
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 18/26] ui: window: add RemovableDeviceEdit Hannes Laimer
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 19/26] ui: form: add PartitionSelector Hannes Laimer
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 20/26] ui: add "removable" checkbox to datastore edit Hannes Laimer
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 21/26] ui: disable maintenance update while removable datastore is unplugged Hannes Laimer
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 22/26] ui: datstore selector: add icon for removable datastores Hannes Laimer
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 23/26] api2: admin: add mount-device endpoint that just takes an UUID Hannes Laimer
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 24/26] cli: manager: add removable-device commands Hannes Laimer
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 25/26] debian/etc: add udev rules and simple service for automounting Hannes Laimer
2022-07-06 11:49   ` Wolfgang Bumiller
2022-07-05 13:08 ` [pbs-devel] [PATCH proxmox-backup 26/26] api: mark all removable datastores as 'unplugged' after restart Hannes Laimer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal