* [pbs-devel] [PATCH proxmox-backup v3 01/24] tools: add disks utility functions
2024-04-09 10:59 [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Hannes Laimer
@ 2024-04-09 10:59 ` Hannes Laimer
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 02/24] pbs-api-types: add backing-device to DataStoreConfig Hannes Laimer
` (23 subsequent siblings)
24 siblings, 0 replies; 54+ messages in thread
From: Hannes Laimer @ 2024-04-09 10:59 UTC (permalink / raw)
To: pbs-devel
... for mounting and unmounting
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/tools/disks/mod.rs | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/src/tools/disks/mod.rs b/src/tools/disks/mod.rs
index 94f89e0a..68854918 100644
--- a/src/tools/disks/mod.rs
+++ b/src/tools/disks/mod.rs
@@ -1323,3 +1323,22 @@ pub fn get_fs_uuid(disk: &Disk) -> Result<String, Error> {
bail!("get_fs_uuid failed - missing UUID");
}
+
+/// Mount a disk by its UUID and the mount point.
+pub fn mount_by_uuid(uuid: &str, mount_point: &Path) -> Result<(), Error> {
+ let mut command = std::process::Command::new("mount");
+ command.args([&format!("UUID={uuid}")]);
+ command.arg(mount_point);
+
+ proxmox_sys::command::run_command(command, None)?;
+ Ok(())
+}
+
+/// Unmount a disk by its mount point.
+pub fn unmount_by_mountpoint(path: &str) -> Result<(), Error> {
+ let mut command = std::process::Command::new("umount");
+ command.arg(path);
+
+ proxmox_sys::command::run_command(command, None)?;
+ Ok(())
+}
--
2.39.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pbs-devel] [PATCH proxmox-backup v3 02/24] pbs-api-types: add backing-device to DataStoreConfig
2024-04-09 10:59 [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Hannes Laimer
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 01/24] tools: add disks utility functions Hannes Laimer
@ 2024-04-09 10:59 ` Hannes Laimer
2024-04-10 8:13 ` Dietmar Maurer
` (2 more replies)
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 03/24] maintenance: add 'Unpplugged' maintenance type Hannes Laimer
` (22 subsequent siblings)
24 siblings, 3 replies; 54+ messages in thread
From: Hannes Laimer @ 2024-04-09 10:59 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 5e13c157..f57957d2 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -273,6 +273,11 @@ pub const DATASTORE_TUNING_STRING_SCHEMA: Schema = StringSchema::new("Datastore
format: &ApiStringFormat::PropertyString(&MaintenanceMode::API_SCHEMA),
type: String,
},
+ "backing-device": {
+ description: "The UUID of the device for removable datastores.",
+ optional: true,
+ type: String,
+ }
}
)]
#[derive(Serialize, Deserialize, Updater, Clone, PartialEq)]
@@ -316,6 +321,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 UUID of the device(for removable datastores)
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub backing_device: Option<String>,
}
impl DataStoreConfig {
@@ -332,6 +341,7 @@ impl DataStoreConfig {
notify: None,
tuning: None,
maintenance_mode: None,
+ backing_device: None,
}
}
--
2.39.2
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 02/24] pbs-api-types: add backing-device to DataStoreConfig
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 02/24] pbs-api-types: add backing-device to DataStoreConfig Hannes Laimer
@ 2024-04-10 8:13 ` Dietmar Maurer
2024-04-11 10:04 ` Dietmar Maurer
2024-04-15 15:17 ` Christian Ebner
2 siblings, 0 replies; 54+ messages in thread
From: Dietmar Maurer @ 2024-04-10 8:13 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
> + "backing-device": {
> + description: "The UUID of the device for removable datastores.",
> + optional: true,
> + type: String,
> + }
I guess we can be more specific here (regex or character selection, length limit, ...)
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 02/24] pbs-api-types: add backing-device to DataStoreConfig
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 02/24] pbs-api-types: add backing-device to DataStoreConfig Hannes Laimer
2024-04-10 8:13 ` Dietmar Maurer
@ 2024-04-11 10:04 ` Dietmar Maurer
2024-04-15 15:17 ` Christian Ebner
2 siblings, 0 replies; 54+ messages in thread
From: Dietmar Maurer @ 2024-04-11 10:04 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
> + "backing-device": {
> + description: "The UUID of the device for removable datastores.",
> + optional: true,
> + type: String,
> + }
> }
> )]
> #[derive(Serialize, Deserialize, Updater, Clone, PartialEq)]
> @@ -316,6 +321,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 UUID of the device(for removable datastores)
I guess we also want:
#[updater(skip)]
> + #[serde(skip_serializing_if = "Option::is_none")]
> + pub backing_device: Option<String>,
> }
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 02/24] pbs-api-types: add backing-device to DataStoreConfig
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 02/24] pbs-api-types: add backing-device to DataStoreConfig Hannes Laimer
2024-04-10 8:13 ` Dietmar Maurer
2024-04-11 10:04 ` Dietmar Maurer
@ 2024-04-15 15:17 ` Christian Ebner
2 siblings, 0 replies; 54+ messages in thread
From: Christian Ebner @ 2024-04-15 15:17 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
a comments inline
On 4/9/24 12:59, Hannes Laimer wrote:
> 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 5e13c157..f57957d2 100644
> --- a/pbs-api-types/src/datastore.rs
> +++ b/pbs-api-types/src/datastore.rs
> @@ -273,6 +273,11 @@ pub const DATASTORE_TUNING_STRING_SCHEMA: Schema = StringSchema::new("Datastore
> format: &ApiStringFormat::PropertyString(&MaintenanceMode::API_SCHEMA),
> type: String,
> },
> + "backing-device": {
> + description: "The UUID of the device for removable datastores.",
this is the UUID for the filesystem partition, so maybe this description
can be more specific.
> + optional: true,
> + type: String,
> + }
> }
> )]
> #[derive(Serialize, Deserialize, Updater, Clone, PartialEq)]
> @@ -316,6 +321,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 UUID of the device(for removable datastores)
> + #[serde(skip_serializing_if = "Option::is_none")]
> + pub backing_device: Option<String>,
> }
>
> impl DataStoreConfig {
> @@ -332,6 +341,7 @@ impl DataStoreConfig {
> notify: None,
> tuning: None,
> maintenance_mode: None,
> + backing_device: None,
> }
> }
>
^ permalink raw reply [flat|nested] 54+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v3 03/24] maintenance: add 'Unpplugged' maintenance type
2024-04-09 10:59 [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Hannes Laimer
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 01/24] tools: add disks utility functions Hannes Laimer
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 02/24] pbs-api-types: add backing-device to DataStoreConfig Hannes Laimer
@ 2024-04-09 10:59 ` Hannes Laimer
2024-04-11 7:19 ` Dietmar Maurer
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 04/24] disks: add UUID to partition info Hannes Laimer
` (21 subsequent siblings)
24 siblings, 1 reply; 54+ messages in thread
From: Hannes Laimer @ 2024-04-09 10:59 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
pbs-api-types/src/maintenance.rs | 31 +++++++++++++++++++++++++++++--
1 file changed, 29 insertions(+), 2 deletions(-)
diff --git a/pbs-api-types/src/maintenance.rs b/pbs-api-types/src/maintenance.rs
index a605cc17..e7ffbcd0 100644
--- a/pbs-api-types/src/maintenance.rs
+++ b/pbs-api-types/src/maintenance.rs
@@ -38,7 +38,6 @@ pub enum Operation {
/// Maintenance type.
pub enum MaintenanceType {
// TODO:
- // - Add "unmounting" once we got pluggable datastores
// - Add "GarbageCollection" or "DeleteOnly" as type and track GC (or all deletes) as separate
// operation, so that one can enable a mode where nothing new can be added but stuff can be
// cleaned
@@ -48,6 +47,8 @@ pub enum MaintenanceType {
Offline,
/// The datastore is being deleted.
Delete,
+ /// The removable datastore is unplugged
+ Unplugged,
}
serde_plain::derive_display_from_serialize!(MaintenanceType);
serde_plain::derive_fromstr_from_deserialize!(MaintenanceType);
@@ -80,7 +81,7 @@ impl MaintenanceMode {
/// Used for deciding whether the datastore is cleared from the internal cache after the last
/// task finishes, so all open files are closed.
pub fn is_offline(&self) -> bool {
- self.ty == MaintenanceType::Offline
+ self.ty == MaintenanceType::Offline || self.ty == MaintenanceType::Unplugged
}
pub fn check(&self, operation: Option<Operation>) -> Result<(), Error> {
@@ -96,6 +97,8 @@ impl MaintenanceMode {
return Ok(());
} else if self.ty == MaintenanceType::Offline {
bail!("offline maintenance mode: {}", message);
+ } else if self.ty == MaintenanceType::Unplugged {
+ bail!("datastore is not plugged in");
} else if self.ty == MaintenanceType::ReadOnly {
if let Some(Operation::Write) = operation {
bail!("read-only maintenance mode: {}", message);
@@ -104,3 +107,27 @@ impl MaintenanceMode {
Ok(())
}
}
+
+#[test]
+fn test_check() {
+ let ro_mode = MaintenanceMode::new(MaintenanceType::ReadOnly, None);
+ let offline_mode = MaintenanceMode::new(MaintenanceType::Offline, None);
+ let delete_mode = MaintenanceMode::new(MaintenanceType::Delete, None);
+ let unplugged_mode = MaintenanceMode::new(MaintenanceType::Unplugged, None);
+
+ assert!(ro_mode.check(Some(Operation::Lookup)).is_ok());
+ assert!(ro_mode.check(Some(Operation::Read)).is_ok());
+ assert!(ro_mode.check(Some(Operation::Write)).is_err());
+
+ assert!(offline_mode.check(Some(Operation::Lookup)).is_ok());
+ assert!(offline_mode.check(Some(Operation::Read)).is_err());
+ assert!(offline_mode.check(Some(Operation::Write)).is_err());
+
+ assert!(delete_mode.check(Some(Operation::Lookup)).is_err());
+ assert!(delete_mode.check(Some(Operation::Read)).is_err());
+ assert!(delete_mode.check(Some(Operation::Write)).is_err());
+
+ assert!(unplugged_mode.check(Some(Operation::Lookup)).is_ok());
+ assert!(unplugged_mode.check(Some(Operation::Read)).is_err());
+ assert!(unplugged_mode.check(Some(Operation::Write)).is_err());
+}
--
2.39.2
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 03/24] maintenance: add 'Unpplugged' maintenance type
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 03/24] maintenance: add 'Unpplugged' maintenance type Hannes Laimer
@ 2024-04-11 7:19 ` Dietmar Maurer
2024-04-11 8:16 ` Hannes Laimer
0 siblings, 1 reply; 54+ messages in thread
From: Dietmar Maurer @ 2024-04-11 7:19 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
Maybe a stupid question, but what is the purpose of this
additional maintenance type? I can see that we get a different error message, but else it behaves the same as "Offline"?
> On 9.4.2024 12:59 CEST Hannes Laimer <h.laimer@proxmox.com> wrote:
>
>
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> pbs-api-types/src/maintenance.rs | 31 +++++++++++++++++++++++++++++--
> 1 file changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/pbs-api-types/src/maintenance.rs b/pbs-api-types/src/maintenance.rs
> index a605cc17..e7ffbcd0 100644
> --- a/pbs-api-types/src/maintenance.rs
> +++ b/pbs-api-types/src/maintenance.rs
> @@ -38,7 +38,6 @@ pub enum Operation {
> /// Maintenance type.
> pub enum MaintenanceType {
> // TODO:
> - // - Add "unmounting" once we got pluggable datastores
> // - Add "GarbageCollection" or "DeleteOnly" as type and track GC (or all deletes) as separate
> // operation, so that one can enable a mode where nothing new can be added but stuff can be
> // cleaned
> @@ -48,6 +47,8 @@ pub enum MaintenanceType {
> Offline,
> /// The datastore is being deleted.
> Delete,
> + /// The removable datastore is unplugged
> + Unplugged,
> }
> serde_plain::derive_display_from_serialize!(MaintenanceType);
> serde_plain::derive_fromstr_from_deserialize!(MaintenanceType);
> @@ -80,7 +81,7 @@ impl MaintenanceMode {
> /// Used for deciding whether the datastore is cleared from the internal cache after the last
> /// task finishes, so all open files are closed.
> pub fn is_offline(&self) -> bool {
> - self.ty == MaintenanceType::Offline
> + self.ty == MaintenanceType::Offline || self.ty == MaintenanceType::Unplugged
> }
>
> pub fn check(&self, operation: Option<Operation>) -> Result<(), Error> {
> @@ -96,6 +97,8 @@ impl MaintenanceMode {
> return Ok(());
> } else if self.ty == MaintenanceType::Offline {
> bail!("offline maintenance mode: {}", message);
> + } else if self.ty == MaintenanceType::Unplugged {
> + bail!("datastore is not plugged in");
> } else if self.ty == MaintenanceType::ReadOnly {
> if let Some(Operation::Write) = operation {
> bail!("read-only maintenance mode: {}", message);
> @@ -104,3 +107,27 @@ impl MaintenanceMode {
> Ok(())
> }
> }
> +
> +#[test]
> +fn test_check() {
> + let ro_mode = MaintenanceMode::new(MaintenanceType::ReadOnly, None);
> + let offline_mode = MaintenanceMode::new(MaintenanceType::Offline, None);
> + let delete_mode = MaintenanceMode::new(MaintenanceType::Delete, None);
> + let unplugged_mode = MaintenanceMode::new(MaintenanceType::Unplugged, None);
> +
> + assert!(ro_mode.check(Some(Operation::Lookup)).is_ok());
> + assert!(ro_mode.check(Some(Operation::Read)).is_ok());
> + assert!(ro_mode.check(Some(Operation::Write)).is_err());
> +
> + assert!(offline_mode.check(Some(Operation::Lookup)).is_ok());
> + assert!(offline_mode.check(Some(Operation::Read)).is_err());
> + assert!(offline_mode.check(Some(Operation::Write)).is_err());
> +
> + assert!(delete_mode.check(Some(Operation::Lookup)).is_err());
> + assert!(delete_mode.check(Some(Operation::Read)).is_err());
> + assert!(delete_mode.check(Some(Operation::Write)).is_err());
> +
> + assert!(unplugged_mode.check(Some(Operation::Lookup)).is_ok());
> + assert!(unplugged_mode.check(Some(Operation::Read)).is_err());
> + assert!(unplugged_mode.check(Some(Operation::Write)).is_err());
> +}
> --
> 2.39.2
>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 03/24] maintenance: add 'Unpplugged' maintenance type
2024-04-11 7:19 ` Dietmar Maurer
@ 2024-04-11 8:16 ` Hannes Laimer
0 siblings, 0 replies; 54+ messages in thread
From: Hannes Laimer @ 2024-04-11 8:16 UTC (permalink / raw)
To: Dietmar Maurer, Proxmox Backup Server development discussion
On Thu Apr 11, 2024 at 9:19 AM CEST, Dietmar Maurer wrote:
> Maybe a stupid question, but what is the purpose of this
> additional maintenance type? I can see that we get a different error message, but else it behaves the same as "Offline"?
>
Functionally they are basically the same, but they are represtened
differently in the UI/(error-)messages. As we discussed off-list it
makes sense to not have the current 'state' represented in the ds
config, but rather somewhere under `/var/..` as you suggested. This is
another reason for why that is a good idea. In some way a maintenance
mode is also more of a state than a configuration, but that's different
topic.
> > On 9.4.2024 12:59 CEST Hannes Laimer <h.laimer@proxmox.com> wrote:
> >
> >
> > Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> > ---
> > pbs-api-types/src/maintenance.rs | 31 +++++++++++++++++++++++++++++--
> > 1 file changed, 29 insertions(+), 2 deletions(-)
> >
> > diff --git a/pbs-api-types/src/maintenance.rs b/pbs-api-types/src/maintenance.rs
> > index a605cc17..e7ffbcd0 100644
> > --- a/pbs-api-types/src/maintenance.rs
> > +++ b/pbs-api-types/src/maintenance.rs
> > @@ -38,7 +38,6 @@ pub enum Operation {
> > /// Maintenance type.
> > pub enum MaintenanceType {
> > // TODO:
> > - // - Add "unmounting" once we got pluggable datastores
> > // - Add "GarbageCollection" or "DeleteOnly" as type and track GC (or all deletes) as separate
> > // operation, so that one can enable a mode where nothing new can be added but stuff can be
> > // cleaned
> > @@ -48,6 +47,8 @@ pub enum MaintenanceType {
> > Offline,
> > /// The datastore is being deleted.
> > Delete,
> > + /// The removable datastore is unplugged
> > + Unplugged,
> > }
> > serde_plain::derive_display_from_serialize!(MaintenanceType);
> > serde_plain::derive_fromstr_from_deserialize!(MaintenanceType);
> > @@ -80,7 +81,7 @@ impl MaintenanceMode {
> > /// Used for deciding whether the datastore is cleared from the internal cache after the last
> > /// task finishes, so all open files are closed.
> > pub fn is_offline(&self) -> bool {
> > - self.ty == MaintenanceType::Offline
> > + self.ty == MaintenanceType::Offline || self.ty == MaintenanceType::Unplugged
> > }
> >
> > pub fn check(&self, operation: Option<Operation>) -> Result<(), Error> {
> > @@ -96,6 +97,8 @@ impl MaintenanceMode {
> > return Ok(());
> > } else if self.ty == MaintenanceType::Offline {
> > bail!("offline maintenance mode: {}", message);
> > + } else if self.ty == MaintenanceType::Unplugged {
> > + bail!("datastore is not plugged in");
> > } else if self.ty == MaintenanceType::ReadOnly {
> > if let Some(Operation::Write) = operation {
> > bail!("read-only maintenance mode: {}", message);
> > @@ -104,3 +107,27 @@ impl MaintenanceMode {
> > Ok(())
> > }
> > }
> > +
> > +#[test]
> > +fn test_check() {
> > + let ro_mode = MaintenanceMode::new(MaintenanceType::ReadOnly, None);
> > + let offline_mode = MaintenanceMode::new(MaintenanceType::Offline, None);
> > + let delete_mode = MaintenanceMode::new(MaintenanceType::Delete, None);
> > + let unplugged_mode = MaintenanceMode::new(MaintenanceType::Unplugged, None);
> > +
> > + assert!(ro_mode.check(Some(Operation::Lookup)).is_ok());
> > + assert!(ro_mode.check(Some(Operation::Read)).is_ok());
> > + assert!(ro_mode.check(Some(Operation::Write)).is_err());
> > +
> > + assert!(offline_mode.check(Some(Operation::Lookup)).is_ok());
> > + assert!(offline_mode.check(Some(Operation::Read)).is_err());
> > + assert!(offline_mode.check(Some(Operation::Write)).is_err());
> > +
> > + assert!(delete_mode.check(Some(Operation::Lookup)).is_err());
> > + assert!(delete_mode.check(Some(Operation::Read)).is_err());
> > + assert!(delete_mode.check(Some(Operation::Write)).is_err());
> > +
> > + assert!(unplugged_mode.check(Some(Operation::Lookup)).is_ok());
> > + assert!(unplugged_mode.check(Some(Operation::Read)).is_err());
> > + assert!(unplugged_mode.check(Some(Operation::Write)).is_err());
> > +}
> > --
> > 2.39.2
> >
> >
> >
> > _______________________________________________
> > pbs-devel mailing list
> > pbs-devel@lists.proxmox.com
> > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 54+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v3 04/24] disks: add UUID to partition info
2024-04-09 10:59 [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Hannes Laimer
` (2 preceding siblings ...)
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 03/24] maintenance: add 'Unpplugged' maintenance type Hannes Laimer
@ 2024-04-09 10:59 ` Hannes Laimer
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 05/24] add helper for checking if a removable datastore is available Hannes Laimer
` (20 subsequent siblings)
24 siblings, 0 replies; 54+ messages in thread
From: Hannes Laimer @ 2024-04-09 10:59 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 68854918..eaf016df 100644
--- a/src/tools/disks/mod.rs
+++ b/src/tools/disks/mod.rs
@@ -59,6 +59,8 @@ pub struct LsblkInfo {
/// File system label.
#[serde(rename = "fstype")]
file_system_type: Option<String>,
+ /// File system UUID.
+ uuid: Option<String>,
}
impl DiskManage {
@@ -617,7 +619,7 @@ pub struct BlockDevStat {
/// Use lsblk to read partition type uuids and file system types.
pub fn get_lsblk_info() -> Result<Vec<LsblkInfo>, Error> {
let mut command = std::process::Command::new("lsblk");
- command.args(["--json", "-o", "path,parttype,fstype"]);
+ command.args(["--json", "-o", "path,parttype,fstype,uuid"]);
let output = proxmox_sys::command::run_command(command, None)?;
@@ -701,6 +703,8 @@ pub struct PartitionInfo {
pub size: Option<u64>,
/// GPT partition
pub gpt: bool,
+ /// UUID
+ pub uuid: Option<String>,
}
#[api(
@@ -891,8 +895,10 @@ fn get_partitions_info(
let mounted = disk.is_mounted().unwrap_or(false);
let mut filesystem = None;
+ let mut uuid = None;
if let (Some(devpath), Some(infos)) = (devpath.as_ref(), lsblk_infos.as_ref()) {
for info in infos.iter().filter(|i| i.path.eq(devpath)) {
+ uuid = info.uuid.clone();
used = match info.partition_type.as_deref() {
Some("21686148-6449-6e6f-744e-656564454649") => PartitionUsageType::BIOS,
Some("c12a7328-f81f-11d2-ba4b-00a0c93ec93b") => PartitionUsageType::EFI,
@@ -915,6 +921,7 @@ fn get_partitions_info(
filesystem,
size: disk.size().ok(),
gpt: disk.has_gpt(),
+ uuid,
}
})
.collect()
--
2.39.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pbs-devel] [PATCH proxmox-backup v3 05/24] add helper for checking if a removable datastore is available
2024-04-09 10:59 [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Hannes Laimer
` (3 preceding siblings ...)
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 04/24] disks: add UUID to partition info Hannes Laimer
@ 2024-04-09 10:59 ` Hannes Laimer
2024-04-10 8:40 ` Dietmar Maurer
2024-04-15 15:09 ` Christian Ebner
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 06/24] api2: admin: add (un)mount endpoint for removable datastores Hannes Laimer
` (19 subsequent siblings)
24 siblings, 2 replies; 54+ messages in thread
From: Hannes Laimer @ 2024-04-09 10:59 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
pbs-datastore/src/datastore.rs | 18 ++++++++++++++++++
pbs-datastore/src/lib.rs | 2 +-
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 0685cc84..db47205c 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -49,6 +49,22 @@ pub fn check_backup_owner(owner: &Authid, auth_id: &Authid) -> Result<(), Error>
Ok(())
}
+pub fn check_if_available(config: &DataStoreConfig) -> Result<(), Error> {
+ config.backing_device.as_ref().map_or(Ok(()), |uuid| {
+ let mut command = std::process::Command::new("findmnt");
+ command.args(["-n", "-o", "TARGET", "--source", &format!("UUID={uuid}")]);
+
+ match proxmox_sys::command::run_command(command, None) {
+ Ok(mount_point) if mount_point.trim_end() == config.path => Ok(()),
+ _ => Err(format_err!(
+ "device for datastore '{}' has to be mounted at '{}'",
+ config.name,
+ config.path
+ )),
+ }
+ })
+}
+
/// Datastore Management
///
/// A Datastore can store severals backups, and provides the
@@ -261,6 +277,8 @@ impl DataStore {
) -> Result<Arc<Self>, Error> {
let name = config.name.clone();
+ check_if_available(&config)?;
+
let tuning: DatastoreTuning = serde_json::from_value(
DatastoreTuning::API_SCHEMA
.parse_property_string(config.tuning.as_deref().unwrap_or(""))?,
diff --git a/pbs-datastore/src/lib.rs b/pbs-datastore/src/lib.rs
index 43050162..f5e93e92 100644
--- a/pbs-datastore/src/lib.rs
+++ b/pbs-datastore/src/lib.rs
@@ -206,7 +206,7 @@ pub use manifest::BackupManifest;
pub use store_progress::StoreProgress;
mod datastore;
-pub use datastore::{check_backup_owner, DataStore};
+pub use datastore::{check_backup_owner, check_if_available, DataStore};
mod hierarchy;
pub use hierarchy::{
--
2.39.2
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 05/24] add helper for checking if a removable datastore is available
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 05/24] add helper for checking if a removable datastore is available Hannes Laimer
@ 2024-04-10 8:40 ` Dietmar Maurer
2024-04-15 15:09 ` Christian Ebner
1 sibling, 0 replies; 54+ messages in thread
From: Dietmar Maurer @ 2024-04-10 8:40 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
All public functions should have docs!
> +pub fn check_if_available(config: &DataStoreConfig) -> Result<(), Error> {
> + config.backing_device.as_ref().map_or(Ok(()), |uuid| {
> + let mut command = std::process::Command::new("findmnt");
> + command.args(["-n", "-o", "TARGET", "--source", &format!("UUID={uuid}")]);
> +
> + match proxmox_sys::command::run_command(command, None) {
> + Ok(mount_point) if mount_point.trim_end() == config.path => Ok(()),
> + _ => Err(format_err!(
> + "device for datastore '{}' has to be mounted at '{}'",
> + config.name,
> + config.path
> + )),
> + }
> + })
> +}
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 05/24] add helper for checking if a removable datastore is available
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 05/24] add helper for checking if a removable datastore is available Hannes Laimer
2024-04-10 8:40 ` Dietmar Maurer
@ 2024-04-15 15:09 ` Christian Ebner
2024-04-16 14:17 ` Hannes Laimer
1 sibling, 1 reply; 54+ messages in thread
From: Christian Ebner @ 2024-04-15 15:09 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
some comments inline
On 4/9/24 12:59, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> pbs-datastore/src/datastore.rs | 18 ++++++++++++++++++
> pbs-datastore/src/lib.rs | 2 +-
> 2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
> index 0685cc84..db47205c 100644
> --- a/pbs-datastore/src/datastore.rs
> +++ b/pbs-datastore/src/datastore.rs
> @@ -49,6 +49,22 @@ pub fn check_backup_owner(owner: &Authid, auth_id: &Authid) -> Result<(), Error>
> Ok(())
> }
>
> +pub fn check_if_available(config: &DataStoreConfig) -> Result<(), Error> {
the function signature suggests that this checks if the datastore is
available, so maybe call this `is_datastore_available` and let it return
a boolean instead, further distinguishing between error states because
of runtime errors and the datastore simply not being available.
> + config.backing_device.as_ref().map_or(Ok(()), |uuid| {
> + let mut command = std::process::Command::new("findmnt");
> + command.args(["-n", "-o", "TARGET", "--source", &format!("UUID={uuid}")]);
> +
> + match proxmox_sys::command::run_command(command, None) {
> + Ok(mount_point) if mount_point.trim_end() == config.path => Ok(()),
> + _ => Err(format_err!(
this should distinguish between an error because of the `run_command`
failing because of e.g. outdated/incompatible command line arguments
passed to `findmnt` and the filesystem not being mounted.
I see however that `findmnt` only has 2 error codes, so maybe this could
be done by using `lsblk` and filtering the output based on the `uuid`
instead, but there might be better solutions that I am not aware of.
> + "device for datastore '{}' has to be mounted at '{}'",
should actually be the filesystem on the device that needs to be
mounted, not the device?
> + config.name,
> + config.path
> + )),
> + }
> + })
> +}
> +
> /// Datastore Management
> ///
> /// A Datastore can store severals backups, and provides the
> @@ -261,6 +277,8 @@ impl DataStore {
> ) -> Result<Arc<Self>, Error> {
> let name = config.name.clone();
>
> + check_if_available(&config)?;
> +
> let tuning: DatastoreTuning = serde_json::from_value(
> DatastoreTuning::API_SCHEMA
> .parse_property_string(config.tuning.as_deref().unwrap_or(""))?,
> diff --git a/pbs-datastore/src/lib.rs b/pbs-datastore/src/lib.rs
> index 43050162..f5e93e92 100644
> --- a/pbs-datastore/src/lib.rs
> +++ b/pbs-datastore/src/lib.rs
> @@ -206,7 +206,7 @@ pub use manifest::BackupManifest;
> pub use store_progress::StoreProgress;
>
> mod datastore;
> -pub use datastore::{check_backup_owner, DataStore};
> +pub use datastore::{check_backup_owner, check_if_available, DataStore};
>
> mod hierarchy;
> pub use hierarchy::{
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 05/24] add helper for checking if a removable datastore is available
2024-04-15 15:09 ` Christian Ebner
@ 2024-04-16 14:17 ` Hannes Laimer
0 siblings, 0 replies; 54+ messages in thread
From: Hannes Laimer @ 2024-04-16 14:17 UTC (permalink / raw)
To: Christian Ebner, Proxmox Backup Server development discussion
On Mon Apr 15, 2024 at 5:09 PM CEST, Christian Ebner wrote:
> some comments inline
>
> On 4/9/24 12:59, Hannes Laimer wrote:
> > Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> > ---
> > pbs-datastore/src/datastore.rs | 18 ++++++++++++++++++
> > pbs-datastore/src/lib.rs | 2 +-
> > 2 files changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
> > index 0685cc84..db47205c 100644
> > --- a/pbs-datastore/src/datastore.rs
> > +++ b/pbs-datastore/src/datastore.rs
> > @@ -49,6 +49,22 @@ pub fn check_backup_owner(owner: &Authid, auth_id: &Authid) -> Result<(), Error>
> > Ok(())
> > }
> >
> > +pub fn check_if_available(config: &DataStoreConfig) -> Result<(), Error> {
>
> the function signature suggests that this checks if the datastore is
> available, so maybe call this `is_datastore_available` and let it return
> a boolean instead, further distinguishing between error states because
> of runtime errors and the datastore simply not being available.
>
makes sense, will do in v4
> > + config.backing_device.as_ref().map_or(Ok(()), |uuid| {
> > + let mut command = std::process::Command::new("findmnt");
> > + command.args(["-n", "-o", "TARGET", "--source", &format!("UUID={uuid}")]);
> > +
> > + match proxmox_sys::command::run_command(command, None) {
> > + Ok(mount_point) if mount_point.trim_end() == config.path => Ok(()),
> > + _ => Err(format_err!(
>
> this should distinguish between an error because of the `run_command`
> failing because of e.g. outdated/incompatible command line arguments
> passed to `findmnt` and the filesystem not being mounted.
> I see however that `findmnt` only has 2 error codes, so maybe this could
> be done by using `lsblk` and filtering the output based on the `uuid`
> instead, but there might be better solutions that I am not aware of.
>
yes, but I won't use an external tool in v4
> > + "device for datastore '{}' has to be mounted at '{}'",
>
> should actually be the filesystem on the device that needs to be
> mounted, not the device?
>
> > + config.name,
> > + config.path
> > + )),
> > + }
> > + })
> > +}
> > +
> > /// Datastore Management
> > ///
> > /// A Datastore can store severals backups, and provides the
> > @@ -261,6 +277,8 @@ impl DataStore {
> > ) -> Result<Arc<Self>, Error> {
> > let name = config.name.clone();
> >
> > + check_if_available(&config)?;
> > +
> > let tuning: DatastoreTuning = serde_json::from_value(
> > DatastoreTuning::API_SCHEMA
> > .parse_property_string(config.tuning.as_deref().unwrap_or(""))?,
> > diff --git a/pbs-datastore/src/lib.rs b/pbs-datastore/src/lib.rs
> > index 43050162..f5e93e92 100644
> > --- a/pbs-datastore/src/lib.rs
> > +++ b/pbs-datastore/src/lib.rs
> > @@ -206,7 +206,7 @@ pub use manifest::BackupManifest;
> > pub use store_progress::StoreProgress;
> >
> > mod datastore;
> > -pub use datastore::{check_backup_owner, DataStore};
> > +pub use datastore::{check_backup_owner, check_if_available, DataStore};
> >
> > mod hierarchy;
> > pub use hierarchy::{
^ permalink raw reply [flat|nested] 54+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v3 06/24] api2: admin: add (un)mount endpoint for removable datastores
2024-04-09 10:59 [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Hannes Laimer
` (4 preceding siblings ...)
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 05/24] add helper for checking if a removable datastore is available Hannes Laimer
@ 2024-04-09 10:59 ` Hannes Laimer
2024-04-10 9:02 ` Dietmar Maurer
` (2 more replies)
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 07/24] api2: removable datastore creation Hannes Laimer
` (18 subsequent siblings)
24 siblings, 3 replies; 54+ messages in thread
From: Hannes Laimer @ 2024-04-09 10:59 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
pbs-api-types/src/datastore.rs | 12 ++
pbs-api-types/src/maintenance.rs | 4 +
src/api2/admin/datastore.rs | 189 +++++++++++++++++++++++++++++--
3 files changed, 196 insertions(+), 9 deletions(-)
diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index f57957d2..b6d238ed 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -351,6 +351,18 @@ 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: Option<MaintenanceMode>) {
+ self.maintenance_mode = mode.and_then(|mode| {
+ let mut writer = String::new();
+ mode.serialize(proxmox_schema::ser::PropertyStringSerializer::new(
+ &mut writer,
+ &MaintenanceMode::API_SCHEMA,
+ ))
+ .ok()?;
+ Some(writer)
+ });
+ }
}
#[api(
diff --git a/pbs-api-types/src/maintenance.rs b/pbs-api-types/src/maintenance.rs
index e7ffbcd0..3868688c 100644
--- a/pbs-api-types/src/maintenance.rs
+++ b/pbs-api-types/src/maintenance.rs
@@ -78,6 +78,10 @@ pub struct MaintenanceMode {
}
impl MaintenanceMode {
+ pub fn new(ty: MaintenanceType, message: Option<String>) -> Self {
+ Self { ty, message }
+ }
+
/// Used for deciding whether the datastore is cleared from the internal cache after the last
/// task finishes, so all open files are closed.
pub fn is_offline(&self) -> bool {
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index f7164b87..92f6adc2 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -26,23 +26,24 @@ use proxmox_sortable_macro::sortable;
use proxmox_sys::fs::{
file_read_firstline, file_read_optional_string, replace_file, CreateOptions,
};
-use proxmox_sys::{task_log, task_warn};
+use proxmox_sys::{task_log, task_warn, WorkerTaskContext};
use 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};
@@ -59,6 +60,7 @@ use pbs_datastore::{
};
use pbs_tools::json::required_string_param;
use proxmox_rest_server::{formatter, WorkerTask};
+use proxmox_section_config::SectionConfigData;
use crate::api2::backup::optional_ns_param;
use crate::api2::node::rrd::create_value_from_rrd;
@@ -2240,6 +2242,173 @@ pub async fn set_backup_owner(
.await?
}
+pub fn do_mount_device(
+ _lock: Option<BackupLockGuard>,
+ mut config: SectionConfigData,
+ mut datastore: DataStoreConfig,
+ worker: Option<&dyn WorkerTaskContext>,
+) -> Result<(), Error> {
+ if let Some(uuid) = datastore.backing_device.as_ref() {
+ if pbs_datastore::check_if_available(&datastore).is_ok() {
+ return Err(format_err!("device '{}' is already mounted", &uuid));
+ }
+ let mount_point_path = std::path::Path::new(&datastore.path);
+ if let Some(worker) = worker {
+ task_log!(worker, "mounting '{}' to '{}'", uuid, datastore.path);
+ }
+ crate::tools::disks::mount_by_uuid(uuid, mount_point_path)?;
+
+ datastore.set_maintenance_mode(None);
+ config.set_data(&datastore.name, "datastore", &datastore)?;
+ pbs_config::datastore::save_config(&config)?;
+
+ Ok(())
+ } else {
+ Err(format_err!(
+ "Datastore '{}' can't be mounted because it is not removable.",
+ &datastore.name
+ ))
+ }
+}
+
+#[api(
+ protected: true,
+ input: {
+ properties: {
+ store: {
+ schema: DATASTORE_SCHEMA,
+ },
+ }
+ },
+ returns: {
+ schema: UPID_SCHEMA,
+ },
+ access: {
+ permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT, false),
+ },
+)]
+/// Mount removable datastore.
+pub fn mount(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
+ let (section_config, _digest) = pbs_config::datastore::config()?;
+ let datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
+
+ if datastore.backing_device.is_none() {
+ bail!("datastore '{}' 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(
+ "mount-device",
+ Some(store),
+ auth_id.to_string(),
+ to_stdout,
+ move |worker| do_mount_device(Some(lock), section_config, datastore, Some(&worker)),
+ )?;
+
+ Ok(json!(upid))
+}
+
+fn do_unmount_device(
+ force: bool,
+ datastore: DataStoreConfig,
+ worker: Option<&dyn WorkerTaskContext>,
+) -> Result<(), Error> {
+ if force {
+ let _ = crate::tools::disks::unmount_by_mountpoint(&datastore.path);
+ return Ok(());
+ }
+
+ 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(())
+}
+
+#[api(
+ protected: true,
+ input: {
+ properties: {
+ store: { schema: DATASTORE_SCHEMA },
+ force: {
+ type: Boolean,
+ description: "Force unmount even if there are active operations.",
+ optional: true,
+ default: false,
+ },
+ },
+ },
+ returns: {
+ schema: UPID_SCHEMA,
+ },
+ access: {
+ permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_MODIFY, true),
+ }
+)]
+/// Unmount a removable device that is associated with the datastore
+pub async fn unmount(
+ store: String,
+ force: bool,
+ rpcenv: &mut dyn RpcEnvironment,
+) -> Result<Value, Error> {
+ let _lock = pbs_config::datastore::lock_config()?;
+ let (mut section_config, _digest) = pbs_config::datastore::config()?;
+ let mut datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
+
+ if datastore.backing_device.is_none() {
+ bail!("datastore '{}' is not removable", &store);
+ }
+
+ let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+ let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
+
+ datastore.set_maintenance_mode(Some(MaintenanceMode::new(MaintenanceType::Unplugged, None)));
+ section_config.set_data(&datastore.name, "datastore", &datastore)?;
+ pbs_config::datastore::save_config(§ion_config)?;
+ drop(_lock);
+
+ if let Ok(proxy_pid) = proxmox_rest_server::read_pid(pbs_buildcfg::PROXMOX_BACKUP_PROXY_PID_FN)
+ {
+ let sock = proxmox_rest_server::ctrl_sock_from_pid(proxy_pid);
+ let _ = proxmox_rest_server::send_raw_command(
+ sock,
+ &format!(
+ "{{\"command\":\"update-datastore-cache\",\"args\":\"{}\"}}\n",
+ &store
+ ),
+ )
+ .await;
+ }
+
+ let upid = WorkerTask::new_thread(
+ "unmount-device",
+ Some(store),
+ auth_id.to_string(),
+ to_stdout,
+ move |worker| do_unmount_device(force, datastore, Some(&worker)),
+ )?;
+
+ Ok(json!(upid))
+}
+
#[sortable]
const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
(
@@ -2278,6 +2447,7 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
.get(&API_METHOD_LIST_GROUPS)
.delete(&API_METHOD_DELETE_GROUP),
),
+ ("mount", &Router::new().post(&API_METHOD_MOUNT)),
(
"namespace",
// FIXME: move into datastore:: sub-module?!
@@ -2312,6 +2482,7 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
.delete(&API_METHOD_DELETE_SNAPSHOT),
),
("status", &Router::new().get(&API_METHOD_STATUS)),
+ ("unmount", &Router::new().post(&API_METHOD_UNMOUNT)),
(
"upload-backup-log",
&Router::new().upload(&API_METHOD_UPLOAD_BACKUP_LOG),
--
2.39.2
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 06/24] api2: admin: add (un)mount endpoint for removable datastores
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 06/24] api2: admin: add (un)mount endpoint for removable datastores Hannes Laimer
@ 2024-04-10 9:02 ` Dietmar Maurer
2024-04-10 9:08 ` Hannes Laimer
2024-04-10 9:12 ` Dietmar Maurer
2024-04-15 15:50 ` Christian Ebner
2 siblings, 1 reply; 54+ messages in thread
From: Dietmar Maurer @ 2024-04-10 9:02 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
comments inline
> On 9.4.2024 12:59 CEST Hannes Laimer <h.laimer@proxmox.com> wrote:
>
>
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> pbs-api-types/src/datastore.rs | 12 ++
> pbs-api-types/src/maintenance.rs | 4 +
> src/api2/admin/datastore.rs | 189 +++++++++++++++++++++++++++++--
> 3 files changed, 196 insertions(+), 9 deletions(-)
>
> diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
> index f57957d2..b6d238ed 100644
> --- a/pbs-api-types/src/datastore.rs
> +++ b/pbs-api-types/src/datastore.rs
> @@ -351,6 +351,18 @@ 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: Option<MaintenanceMode>) {
> + self.maintenance_mode = mode.and_then(|mode| {
> + let mut writer = String::new();
> + mode.serialize(proxmox_schema::ser::PropertyStringSerializer::new(
> + &mut writer,
> + &MaintenanceMode::API_SCHEMA,
> + ))
> + .ok()?;
> + Some(writer)
> + });
> + }
> }
>
> #[api(
> diff --git a/pbs-api-types/src/maintenance.rs b/pbs-api-types/src/maintenance.rs
> index e7ffbcd0..3868688c 100644
> --- a/pbs-api-types/src/maintenance.rs
> +++ b/pbs-api-types/src/maintenance.rs
> @@ -78,6 +78,10 @@ pub struct MaintenanceMode {
> }
>
> impl MaintenanceMode {
> + pub fn new(ty: MaintenanceType, message: Option<String>) -> Self {
> + Self { ty, message }
> + }
> +
> /// Used for deciding whether the datastore is cleared from the internal cache after the last
> /// task finishes, so all open files are closed.
> pub fn is_offline(&self) -> bool {
> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
> index f7164b87..92f6adc2 100644
> --- a/src/api2/admin/datastore.rs
> +++ b/src/api2/admin/datastore.rs
> @@ -26,23 +26,24 @@ use proxmox_sortable_macro::sortable;
> use proxmox_sys::fs::{
> file_read_firstline, file_read_optional_string, replace_file, CreateOptions,
> };
> -use proxmox_sys::{task_log, task_warn};
> +use proxmox_sys::{task_log, task_warn, WorkerTaskContext};
>
> use 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};
> @@ -59,6 +60,7 @@ use pbs_datastore::{
> };
> use pbs_tools::json::required_string_param;
> use proxmox_rest_server::{formatter, WorkerTask};
> +use proxmox_section_config::SectionConfigData;
>
> use crate::api2::backup::optional_ns_param;
> use crate::api2::node::rrd::create_value_from_rrd;
> @@ -2240,6 +2242,173 @@ pub async fn set_backup_owner(
> .await?
> }
>
> +pub fn do_mount_device(
why is this public?
> + _lock: Option<BackupLockGuard>,
> + mut config: SectionConfigData,
> + mut datastore: DataStoreConfig,
> + worker: Option<&dyn WorkerTaskContext>,
> +) -> Result<(), Error> {
> + if let Some(uuid) = datastore.backing_device.as_ref() {
> + if pbs_datastore::check_if_available(&datastore).is_ok() {
> + return Err(format_err!("device '{}' is already mounted", &uuid));
Why not use "bail"?
> + }
> + let mount_point_path = std::path::Path::new(&datastore.path);
> + if let Some(worker) = worker {
> + task_log!(worker, "mounting '{}' to '{}'", uuid, datastore.path);
> + }
> + crate::tools::disks::mount_by_uuid(uuid, mount_point_path)?;
> +
> + datastore.set_maintenance_mode(None);
> + config.set_data(&datastore.name, "datastore", &datastore)?;
> + pbs_config::datastore::save_config(&config)?;
> +
> + Ok(())
> + } else {
> + Err(format_err!(
> + "Datastore '{}' can't be mounted because it is not removable.",
> + &datastore.name
> + ))
same here: bail?
> + }
> +}
> +
> +#[api(
> + protected: true,
> + input: {
> + properties: {
> + store: {
> + schema: DATASTORE_SCHEMA,
> + },
> + }
> + },
> + returns: {
> + schema: UPID_SCHEMA,
> + },
> + access: {
> + permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT, false),
> + },
> +)]
> +/// Mount removable datastore.
> +pub fn mount(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
> + let (section_config, _digest) = pbs_config::datastore::config()?;
> + let datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
> +
> + if datastore.backing_device.is_none() {
> + bail!("datastore '{}' 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(
> + "mount-device",
> + Some(store),
> + auth_id.to_string(),
> + to_stdout,
> + move |worker| do_mount_device(Some(lock), section_config, datastore, Some(&worker)),
> + )?;
> +
> + Ok(json!(upid))
> +}
> +
> +fn do_unmount_device(
> + force: bool,
> + datastore: DataStoreConfig,
> + worker: Option<&dyn WorkerTaskContext>,
> +) -> Result<(), Error> {
> + if force {
> + let _ = crate::tools::disks::unmount_by_mountpoint(&datastore.path);
> + return Ok(());
> + }
> +
> + 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(())
> +}
> +
> +#[api(
> + protected: true,
> + input: {
> + properties: {
> + store: { schema: DATASTORE_SCHEMA },
> + force: {
> + type: Boolean,
> + description: "Force unmount even if there are active operations.",
> + optional: true,
> + default: false,
> + },
> + },
> + },
> + returns: {
> + schema: UPID_SCHEMA,
> + },
> + access: {
> + permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_MODIFY, true),
> + }
> +)]
> +/// Unmount a removable device that is associated with the datastore
> +pub async fn unmount(
> + store: String,
> + force: bool,
> + rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<Value, Error> {
> + let _lock = pbs_config::datastore::lock_config()?;
> + let (mut section_config, _digest) = pbs_config::datastore::config()?;
> + let mut datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
> +
> + if datastore.backing_device.is_none() {
> + bail!("datastore '{}' is not removable", &store);
> + }
> +
> + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> + let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
> +
> + datastore.set_maintenance_mode(Some(MaintenanceMode::new(MaintenanceType::Unplugged, None)));
> + section_config.set_data(&datastore.name, "datastore", &datastore)?;
> + pbs_config::datastore::save_config(§ion_config)?;
> + drop(_lock);
> +
> + if let Ok(proxy_pid) = proxmox_rest_server::read_pid(pbs_buildcfg::PROXMOX_BACKUP_PROXY_PID_FN)
> + {
> + let sock = proxmox_rest_server::ctrl_sock_from_pid(proxy_pid);
> + let _ = proxmox_rest_server::send_raw_command(
> + sock,
> + &format!(
> + "{{\"command\":\"update-datastore-cache\",\"args\":\"{}\"}}\n",
> + &store
> + ),
> + )
> + .await;
> + }
> +
> + let upid = WorkerTask::new_thread(
> + "unmount-device",
> + Some(store),
> + auth_id.to_string(),
> + to_stdout,
> + move |worker| do_unmount_device(force, datastore, Some(&worker)),
> + )?;
> +
> + Ok(json!(upid))
> +}
> +
> #[sortable]
> const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
> (
> @@ -2278,6 +2447,7 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
> .get(&API_METHOD_LIST_GROUPS)
> .delete(&API_METHOD_DELETE_GROUP),
> ),
> + ("mount", &Router::new().post(&API_METHOD_MOUNT)),
> (
> "namespace",
> // FIXME: move into datastore:: sub-module?!
> @@ -2312,6 +2482,7 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
> .delete(&API_METHOD_DELETE_SNAPSHOT),
> ),
> ("status", &Router::new().get(&API_METHOD_STATUS)),
> + ("unmount", &Router::new().post(&API_METHOD_UNMOUNT)),
> (
> "upload-backup-log",
> &Router::new().upload(&API_METHOD_UPLOAD_BACKUP_LOG),
> --
> 2.39.2
>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 06/24] api2: admin: add (un)mount endpoint for removable datastores
2024-04-10 9:02 ` Dietmar Maurer
@ 2024-04-10 9:08 ` Hannes Laimer
2024-04-10 10:41 ` Dietmar Maurer
0 siblings, 1 reply; 54+ messages in thread
From: Hannes Laimer @ 2024-04-10 9:08 UTC (permalink / raw)
To: Dietmar Maurer, Proxmox Backup Server development discussion
On Wed Apr 10, 2024 at 11:02 AM CEST, Dietmar Maurer wrote:
> comments inline
>
> > On 9.4.2024 12:59 CEST Hannes Laimer <h.laimer@proxmox.com> wrote:
> >
> >
> > Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> > ---
> > pbs-api-types/src/datastore.rs | 12 ++
> > pbs-api-types/src/maintenance.rs | 4 +
> > src/api2/admin/datastore.rs | 189 +++++++++++++++++++++++++++++--
> > 3 files changed, 196 insertions(+), 9 deletions(-)
> >
> > diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
> > index f57957d2..b6d238ed 100644
> > --- a/pbs-api-types/src/datastore.rs
> > +++ b/pbs-api-types/src/datastore.rs
> > @@ -351,6 +351,18 @@ 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: Option<MaintenanceMode>) {
> > + self.maintenance_mode = mode.and_then(|mode| {
> > + let mut writer = String::new();
> > + mode.serialize(proxmox_schema::ser::PropertyStringSerializer::new(
> > + &mut writer,
> > + &MaintenanceMode::API_SCHEMA,
> > + ))
> > + .ok()?;
> > + Some(writer)
> > + });
> > + }
> > }
> >
> > #[api(
> > diff --git a/pbs-api-types/src/maintenance.rs b/pbs-api-types/src/maintenance.rs
> > index e7ffbcd0..3868688c 100644
> > --- a/pbs-api-types/src/maintenance.rs
> > +++ b/pbs-api-types/src/maintenance.rs
> > @@ -78,6 +78,10 @@ pub struct MaintenanceMode {
> > }
> >
> > impl MaintenanceMode {
> > + pub fn new(ty: MaintenanceType, message: Option<String>) -> Self {
> > + Self { ty, message }
> > + }
> > +
> > /// Used for deciding whether the datastore is cleared from the internal cache after the last
> > /// task finishes, so all open files are closed.
> > pub fn is_offline(&self) -> bool {
> > diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
> > index f7164b87..92f6adc2 100644
> > --- a/src/api2/admin/datastore.rs
> > +++ b/src/api2/admin/datastore.rs
> > @@ -26,23 +26,24 @@ use proxmox_sortable_macro::sortable;
> > use proxmox_sys::fs::{
> > file_read_firstline, file_read_optional_string, replace_file, CreateOptions,
> > };
> > -use proxmox_sys::{task_log, task_warn};
> > +use proxmox_sys::{task_log, task_warn, WorkerTaskContext};
> >
> > use 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};
> > @@ -59,6 +60,7 @@ use pbs_datastore::{
> > };
> > use pbs_tools::json::required_string_param;
> > use proxmox_rest_server::{formatter, WorkerTask};
> > +use proxmox_section_config::SectionConfigData;
> >
> > use crate::api2::backup::optional_ns_param;
> > use crate::api2::node::rrd::create_value_from_rrd;
> > @@ -2240,6 +2242,173 @@ pub async fn set_backup_owner(
> > .await?
> > }
> >
> > +pub fn do_mount_device(
>
> why is this public?
>
we need it on datastore creation which is in `/config/`
> > + _lock: Option<BackupLockGuard>,
> > + mut config: SectionConfigData,
> > + mut datastore: DataStoreConfig,
> > + worker: Option<&dyn WorkerTaskContext>,
> > +) -> Result<(), Error> {
> > + if let Some(uuid) = datastore.backing_device.as_ref() {
> > + if pbs_datastore::check_if_available(&datastore).is_ok() {
> > + return Err(format_err!("device '{}' is already mounted", &uuid));
>
> Why not use "bail"?
>
> > + }
> > + let mount_point_path = std::path::Path::new(&datastore.path);
> > + if let Some(worker) = worker {
> > + task_log!(worker, "mounting '{}' to '{}'", uuid, datastore.path);
> > + }
> > + crate::tools::disks::mount_by_uuid(uuid, mount_point_path)?;
> > +
> > + datastore.set_maintenance_mode(None);
> > + config.set_data(&datastore.name, "datastore", &datastore)?;
> > + pbs_config::datastore::save_config(&config)?;
> > +
> > + Ok(())
> > + } else {
> > + Err(format_err!(
> > + "Datastore '{}' can't be mounted because it is not removable.",
> > + &datastore.name
> > + ))
>
> same here: bail?
>
makes sense
> > + }
> > +}
> > +
> > +#[api(
> > + protected: true,
> > + input: {
> > + properties: {
> > + store: {
> > + schema: DATASTORE_SCHEMA,
> > + },
> > + }
> > + },
> > + returns: {
> > + schema: UPID_SCHEMA,
> > + },
> > + access: {
> > + permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT, false),
> > + },
> > +)]
> > +/// Mount removable datastore.
> > +pub fn mount(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
> > + let (section_config, _digest) = pbs_config::datastore::config()?;
> > + let datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
> > +
> > + if datastore.backing_device.is_none() {
> > + bail!("datastore '{}' 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(
> > + "mount-device",
> > + Some(store),
> > + auth_id.to_string(),
> > + to_stdout,
> > + move |worker| do_mount_device(Some(lock), section_config, datastore, Some(&worker)),
> > + )?;
> > +
> > + Ok(json!(upid))
> > +}
> > +
> > +fn do_unmount_device(
> > + force: bool,
> > + datastore: DataStoreConfig,
> > + worker: Option<&dyn WorkerTaskContext>,
> > +) -> Result<(), Error> {
> > + if force {
> > + let _ = crate::tools::disks::unmount_by_mountpoint(&datastore.path);
> > + return Ok(());
> > + }
> > +
> > + 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(())
> > +}
> > +
> > +#[api(
> > + protected: true,
> > + input: {
> > + properties: {
> > + store: { schema: DATASTORE_SCHEMA },
> > + force: {
> > + type: Boolean,
> > + description: "Force unmount even if there are active operations.",
> > + optional: true,
> > + default: false,
> > + },
> > + },
> > + },
> > + returns: {
> > + schema: UPID_SCHEMA,
> > + },
> > + access: {
> > + permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_MODIFY, true),
> > + }
> > +)]
> > +/// Unmount a removable device that is associated with the datastore
> > +pub async fn unmount(
> > + store: String,
> > + force: bool,
> > + rpcenv: &mut dyn RpcEnvironment,
> > +) -> Result<Value, Error> {
> > + let _lock = pbs_config::datastore::lock_config()?;
> > + let (mut section_config, _digest) = pbs_config::datastore::config()?;
> > + let mut datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
> > +
> > + if datastore.backing_device.is_none() {
> > + bail!("datastore '{}' is not removable", &store);
> > + }
> > +
> > + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> > + let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
> > +
> > + datastore.set_maintenance_mode(Some(MaintenanceMode::new(MaintenanceType::Unplugged, None)));
> > + section_config.set_data(&datastore.name, "datastore", &datastore)?;
> > + pbs_config::datastore::save_config(§ion_config)?;
> > + drop(_lock);
> > +
> > + if let Ok(proxy_pid) = proxmox_rest_server::read_pid(pbs_buildcfg::PROXMOX_BACKUP_PROXY_PID_FN)
> > + {
> > + let sock = proxmox_rest_server::ctrl_sock_from_pid(proxy_pid);
> > + let _ = proxmox_rest_server::send_raw_command(
> > + sock,
> > + &format!(
> > + "{{\"command\":\"update-datastore-cache\",\"args\":\"{}\"}}\n",
> > + &store
> > + ),
> > + )
> > + .await;
> > + }
> > +
> > + let upid = WorkerTask::new_thread(
> > + "unmount-device",
> > + Some(store),
> > + auth_id.to_string(),
> > + to_stdout,
> > + move |worker| do_unmount_device(force, datastore, Some(&worker)),
> > + )?;
> > +
> > + Ok(json!(upid))
> > +}
> > +
> > #[sortable]
> > const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
> > (
> > @@ -2278,6 +2447,7 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
> > .get(&API_METHOD_LIST_GROUPS)
> > .delete(&API_METHOD_DELETE_GROUP),
> > ),
> > + ("mount", &Router::new().post(&API_METHOD_MOUNT)),
> > (
> > "namespace",
> > // FIXME: move into datastore:: sub-module?!
> > @@ -2312,6 +2482,7 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
> > .delete(&API_METHOD_DELETE_SNAPSHOT),
> > ),
> > ("status", &Router::new().get(&API_METHOD_STATUS)),
> > + ("unmount", &Router::new().post(&API_METHOD_UNMOUNT)),
> > (
> > "upload-backup-log",
> > &Router::new().upload(&API_METHOD_UPLOAD_BACKUP_LOG),
> > --
> > 2.39.2
> >
> >
> >
> > _______________________________________________
> > pbs-devel mailing list
> > pbs-devel@lists.proxmox.com
> > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 06/24] api2: admin: add (un)mount endpoint for removable datastores
2024-04-10 9:08 ` Hannes Laimer
@ 2024-04-10 10:41 ` Dietmar Maurer
0 siblings, 0 replies; 54+ messages in thread
From: Dietmar Maurer @ 2024-04-10 10:41 UTC (permalink / raw)
To: Hannes Laimer, Proxmox Backup Server development discussion
> On 10.4.2024 11:08 CEST Hannes Laimer <h.laimer@proxmox.com> wrote:
>
>
> On Wed Apr 10, 2024 at 11:02 AM CEST, Dietmar Maurer wrote:
> > comments inline
> >
> > > On 9.4.2024 12:59 CEST Hannes Laimer <h.laimer@proxmox.com> wrote:
> > >
> > >
> > > +pub fn do_mount_device(
> >
> > why is this public?
> >
>
> we need it on datastore creation which is in `/config/`
I think "pub(crate)" would be good enough?
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup v3 06/24] api2: admin: add (un)mount endpoint for removable datastores
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 06/24] api2: admin: add (un)mount endpoint for removable datastores Hannes Laimer
2024-04-10 9:02 ` Dietmar Maurer
@ 2024-04-10 9:12 ` Dietmar Maurer
2024-04-15 15:50 ` Christian Ebner
2 siblings, 0 replies; 54+ messages in thread
From: Dietmar Maurer @ 2024-04-10 9:12 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
more comments inline:
> +pub fn do_mount_device(
> + _lock: Option<BackupLockGuard>,
> + mut config: SectionConfigData,
> + mut datastore: DataStoreConfig,
> + worker: Option<&dyn WorkerTaskContext>,
> +) -> Result<(), Error> {
> + if let Some(uuid) = datastore.backing_device.as_ref() {
> + if pbs_datastore::check_if_available(&datastore).is_ok() {
> + return Err(format_err!("device '{}' is already mounted", &uuid));
> + }
> + let mount_point_path = std::path::Path::new(&datastore.path);
> + if let Some(worker) = worker {
> + task_log!(worker, "mounting '{}' to '{}'", uuid, datastore.path);
> + }
> + crate::tools::disks::mount_by_uuid(uuid, mount_point_path)?;
> +
> + datastore.set_maintenance_mode(None);
You change the maintenance mode, but do not notify the proxy?
> + config.set_data(&datastore.name, "datastore", &datastore)?;
> + pbs_config::datastore::save_config(&config)?;
> +
> + Ok(())
> + } else {
> + Err(format_err!(
> + "Datastore '{}' can't be mounted because it is not removable.",
> + &datastore.name
> + ))
> + }
> +}
> +
> +#[api(
> + protected: true,
> + input: {
> + properties: {
> + store: {
> + schema: DATASTORE_SCHEMA,
> + },
> + }
> + },
> + returns: {
> + schema: UPID_SCHEMA,
> + },
> + access: {
> + permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT, false),
> + },
> +)]
> +/// Mount removable datastore.
> +pub fn mount(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
> + let (section_config, _digest) = pbs_config::datastore::config()?;
> + let datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
> +
> + if datastore.backing_device.is_none() {
> + bail!("datastore '{}' 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(
> + "mount-device",
> + Some(store),
> + auth_id.to_string(),
> + to_stdout,
> + move |worker| do_mount_device(Some(lock), section_config, datastore, Some(&worker)),
> + )?;
> +
> + Ok(json!(upid))
> +}
> +
> +fn do_unmount_device(
> + force: bool,
> + datastore: DataStoreConfig,
> + worker: Option<&dyn WorkerTaskContext>,
> +) -> Result<(), Error> {
> + if force {
> + let _ = crate::tools::disks::unmount_by_mountpoint(&datastore.path);
> + return Ok(());
> + }
> +
> + 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(())
> +}
> +
> +#[api(
> + protected: true,
> + input: {
> + properties: {
> + store: { schema: DATASTORE_SCHEMA },
> + force: {
> + type: Boolean,
> + description: "Force unmount even if there are active operations.",
> + optional: true,
> + default: false,
> + },
> + },
> + },
> + returns: {
> + schema: UPID_SCHEMA,
> + },
> + access: {
> + permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_MODIFY, true),
> + }
> +)]
> +/// Unmount a removable device that is associated with the datastore
> +pub async fn unmount(
> + store: String,
> + force: bool,
> + rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<Value, Error> {
> + let _lock = pbs_config::datastore::lock_config()?;
> + let (mut section_config, _digest) = pbs_config::datastore::config()?;
> + let mut datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
> +
> + if datastore.backing_device.is_none() {
> + bail!("datastore '{}' is not removable", &store);
> + }
> +
> + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> + let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
> +
> + datastore.set_maintenance_mode(Some(MaintenanceMode::new(MaintenanceType::Unplugged, None)));
> + section_config.set_data(&datastore.name, "datastore", &datastore)?;
> + pbs_config::datastore::save_config(§ion_config)?;
> + drop(_lock);
> +
> + if let Ok(proxy_pid) = proxmox_rest_server::read_pid(pbs_buildcfg::PROXMOX_BACKUP_PROXY_PID_FN)
> + {
> + let sock = proxmox_rest_server::ctrl_sock_from_pid(proxy_pid);
> + let _ = proxmox_rest_server::send_raw_command(
> + sock,
> + &format!(
> + "{{\"command\":\"update-datastore-cache\",\"args\":\"{}\"}}\n",
> + &store
> + ),
> + )
> + .await;
> + }
I found the same code in strc/api2/config/datastore.rs. I guess its is worth to factor this out.
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 06/24] api2: admin: add (un)mount endpoint for removable datastores
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 06/24] api2: admin: add (un)mount endpoint for removable datastores Hannes Laimer
2024-04-10 9:02 ` Dietmar Maurer
2024-04-10 9:12 ` Dietmar Maurer
@ 2024-04-15 15:50 ` Christian Ebner
2024-04-16 14:25 ` Hannes Laimer
2 siblings, 1 reply; 54+ messages in thread
From: Christian Ebner @ 2024-04-15 15:50 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
a few comments inline
On 4/9/24 12:59, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> pbs-api-types/src/datastore.rs | 12 ++
> pbs-api-types/src/maintenance.rs | 4 +
> src/api2/admin/datastore.rs | 189 +++++++++++++++++++++++++++++--
> 3 files changed, 196 insertions(+), 9 deletions(-)
>
> diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
> index f57957d2..b6d238ed 100644
> --- a/pbs-api-types/src/datastore.rs
> +++ b/pbs-api-types/src/datastore.rs
> @@ -351,6 +351,18 @@ 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: Option<MaintenanceMode>) {
> + self.maintenance_mode = mode.and_then(|mode| {
> + let mut writer = String::new();
> + mode.serialize(proxmox_schema::ser::PropertyStringSerializer::new(
> + &mut writer,
> + &MaintenanceMode::API_SCHEMA,
> + ))
> + .ok()?;
> + Some(writer)
> + });
> + }
> }
>
> #[api(
> diff --git a/pbs-api-types/src/maintenance.rs b/pbs-api-types/src/maintenance.rs
> index e7ffbcd0..3868688c 100644
> --- a/pbs-api-types/src/maintenance.rs
> +++ b/pbs-api-types/src/maintenance.rs
> @@ -78,6 +78,10 @@ pub struct MaintenanceMode {
> }
>
> impl MaintenanceMode {
> + pub fn new(ty: MaintenanceType, message: Option<String>) -> Self {
> + Self { ty, message }
> + }
> +
> /// Used for deciding whether the datastore is cleared from the internal cache after the last
> /// task finishes, so all open files are closed.
> pub fn is_offline(&self) -> bool {
> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
> index f7164b87..92f6adc2 100644
> --- a/src/api2/admin/datastore.rs
> +++ b/src/api2/admin/datastore.rs
> @@ -26,23 +26,24 @@ use proxmox_sortable_macro::sortable;
> use proxmox_sys::fs::{
> file_read_firstline, file_read_optional_string, replace_file, CreateOptions,
> };
> -use proxmox_sys::{task_log, task_warn};
> +use proxmox_sys::{task_log, task_warn, WorkerTaskContext};
>
> use 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};
> @@ -59,6 +60,7 @@ use pbs_datastore::{
> };
> use pbs_tools::json::required_string_param;
> use proxmox_rest_server::{formatter, WorkerTask};
> +use proxmox_section_config::SectionConfigData;
>
> use crate::api2::backup::optional_ns_param;
> use crate::api2::node::rrd::create_value_from_rrd;
> @@ -2240,6 +2242,173 @@ pub async fn set_backup_owner(
> .await?
> }
>
> +pub fn do_mount_device(
> + _lock: Option<BackupLockGuard>,
> + mut config: SectionConfigData,
> + mut datastore: DataStoreConfig,
> + worker: Option<&dyn WorkerTaskContext>,
> +) -> Result<(), Error> {
> + if let Some(uuid) = datastore.backing_device.as_ref() {
> + if pbs_datastore::check_if_available(&datastore).is_ok() {
> + return Err(format_err!("device '{}' is already mounted", &uuid));
> + }
use `bail` here and inline the `uuid`:
bail!("device '{uuid}' is already mounted");
> + let mount_point_path = std::path::Path::new(&datastore.path);
> + if let Some(worker) = worker {
> + task_log!(worker, "mounting '{}' to '{}'", uuid, datastore.path);
inline `uuid`
> + }
> + crate::tools::disks::mount_by_uuid(uuid, mount_point_path)?;
> +
> + datastore.set_maintenance_mode(None);
> + config.set_data(&datastore.name, "datastore", &datastore)?;
> + pbs_config::datastore::save_config(&config)?;
> +
> + Ok(())
> + } else {
> + Err(format_err!(
> + "Datastore '{}' can't be mounted because it is not removable.",
> + &datastore.name
this does not need to be a reference, you can pass in `datastore.name`
> + ))
> + }
> +}
> +
> +#[api(
> + protected: true,
> + input: {
> + properties: {
> + store: {
> + schema: DATASTORE_SCHEMA,
> + },
> + }
> + },
> + returns: {
> + schema: UPID_SCHEMA,
> + },
> + access: {
> + permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT, false),
> + },
> +)]
> +/// Mount removable datastore.
> +pub fn mount(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
> + let (section_config, _digest) = pbs_config::datastore::config()?;
> + let datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
> +
> + if datastore.backing_device.is_none() {
> + bail!("datastore '{}' 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(
> + "mount-device",
> + Some(store),
> + auth_id.to_string(),
> + to_stdout,
> + move |worker| do_mount_device(Some(lock), section_config, datastore, Some(&worker)),
> + )?;
What is the motivation for doing this inside a dedicated worker task? Is
this expected to take a considerable amount of time in some cases?
> +
> + Ok(json!(upid))
> +}
> +
> +fn do_unmount_device(
> + force: bool,
> + datastore: DataStoreConfig,
> + worker: Option<&dyn WorkerTaskContext>,
> +) -> Result<(), Error> {
> + if force {
> + let _ = crate::tools::disks::unmount_by_mountpoint(&datastore.path);
> + return Ok(());
> + }
> +
> + 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(())
> +}
> +
> +#[api(
> + protected: true,
> + input: {
> + properties: {
> + store: { schema: DATASTORE_SCHEMA },
> + force: {
> + type: Boolean,
> + description: "Force unmount even if there are active operations.",
> + optional: true,
> + default: false,
> + },
> + },
> + },
> + returns: {
> + schema: UPID_SCHEMA,
> + },
> + access: {
> + permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_MODIFY, true),
> + }
> +)]
> +/// Unmount a removable device that is associated with the datastore
> +pub async fn unmount(
> + store: String,
> + force: bool,
> + rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<Value, Error> {
> + let _lock = pbs_config::datastore::lock_config()?;
> + let (mut section_config, _digest) = pbs_config::datastore::config()?;
> + let mut datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
> +
> + if datastore.backing_device.is_none() {
> + bail!("datastore '{}' is not removable", &store);
> + }
> +
> + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> + let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
> +
> + datastore.set_maintenance_mode(Some(MaintenanceMode::new(MaintenanceType::Unplugged, None)));
> + section_config.set_data(&datastore.name, "datastore", &datastore)?;
> + pbs_config::datastore::save_config(§ion_config)?;
> + drop(_lock);
> +
> + if let Ok(proxy_pid) = proxmox_rest_server::read_pid(pbs_buildcfg::PROXMOX_BACKUP_PROXY_PID_FN)
> + {
> + let sock = proxmox_rest_server::ctrl_sock_from_pid(proxy_pid);
> + let _ = proxmox_rest_server::send_raw_command(
> + sock,
> + &format!(
> + "{{\"command\":\"update-datastore-cache\",\"args\":\"{}\"}}\n",
> + &store
`store` can be inlined as well here, also no need for taking it by reference
> + ),
> + )
> + .await;
> + }
> +
> + let upid = WorkerTask::new_thread(
> + "unmount-device",
> + Some(store),
> + auth_id.to_string(),
> + to_stdout,
> + move |worker| do_unmount_device(force, datastore, Some(&worker)),
> + )?;
> +
> + Ok(json!(upid))
> +}
> +
> #[sortable]
> const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
> (
> @@ -2278,6 +2447,7 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
> .get(&API_METHOD_LIST_GROUPS)
> .delete(&API_METHOD_DELETE_GROUP),
> ),
> + ("mount", &Router::new().post(&API_METHOD_MOUNT)),
> (
> "namespace",
> // FIXME: move into datastore:: sub-module?!
> @@ -2312,6 +2482,7 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
> .delete(&API_METHOD_DELETE_SNAPSHOT),
> ),
> ("status", &Router::new().get(&API_METHOD_STATUS)),
> + ("unmount", &Router::new().post(&API_METHOD_UNMOUNT)),
> (
> "upload-backup-log",
> &Router::new().upload(&API_METHOD_UPLOAD_BACKUP_LOG),
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 06/24] api2: admin: add (un)mount endpoint for removable datastores
2024-04-15 15:50 ` Christian Ebner
@ 2024-04-16 14:25 ` Hannes Laimer
0 siblings, 0 replies; 54+ messages in thread
From: Hannes Laimer @ 2024-04-16 14:25 UTC (permalink / raw)
To: Christian Ebner, Proxmox Backup Server development discussion
On Mon Apr 15, 2024 at 5:50 PM CEST, Christian Ebner wrote:
> a few comments inline
>
> On 4/9/24 12:59, Hannes Laimer wrote:
> > Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> > ---
> > pbs-api-types/src/datastore.rs | 12 ++
> > pbs-api-types/src/maintenance.rs | 4 +
> > src/api2/admin/datastore.rs | 189 +++++++++++++++++++++++++++++--
> > 3 files changed, 196 insertions(+), 9 deletions(-)
> >
> > diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
> > index f57957d2..b6d238ed 100644
> > --- a/pbs-api-types/src/datastore.rs
> > +++ b/pbs-api-types/src/datastore.rs
> > @@ -351,6 +351,18 @@ 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: Option<MaintenanceMode>) {
> > + self.maintenance_mode = mode.and_then(|mode| {
> > + let mut writer = String::new();
> > + mode.serialize(proxmox_schema::ser::PropertyStringSerializer::new(
> > + &mut writer,
> > + &MaintenanceMode::API_SCHEMA,
> > + ))
> > + .ok()?;
> > + Some(writer)
> > + });
> > + }
> > }
> >
> > #[api(
> > diff --git a/pbs-api-types/src/maintenance.rs b/pbs-api-types/src/maintenance.rs
> > index e7ffbcd0..3868688c 100644
> > --- a/pbs-api-types/src/maintenance.rs
> > +++ b/pbs-api-types/src/maintenance.rs
> > @@ -78,6 +78,10 @@ pub struct MaintenanceMode {
> > }
> >
> > impl MaintenanceMode {
> > + pub fn new(ty: MaintenanceType, message: Option<String>) -> Self {
> > + Self { ty, message }
> > + }
> > +
> > /// Used for deciding whether the datastore is cleared from the internal cache after the last
> > /// task finishes, so all open files are closed.
> > pub fn is_offline(&self) -> bool {
> > diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
> > index f7164b87..92f6adc2 100644
> > --- a/src/api2/admin/datastore.rs
> > +++ b/src/api2/admin/datastore.rs
> > @@ -26,23 +26,24 @@ use proxmox_sortable_macro::sortable;
> > use proxmox_sys::fs::{
> > file_read_firstline, file_read_optional_string, replace_file, CreateOptions,
> > };
> > -use proxmox_sys::{task_log, task_warn};
> > +use proxmox_sys::{task_log, task_warn, WorkerTaskContext};
> >
> > use 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};
> > @@ -59,6 +60,7 @@ use pbs_datastore::{
> > };
> > use pbs_tools::json::required_string_param;
> > use proxmox_rest_server::{formatter, WorkerTask};
> > +use proxmox_section_config::SectionConfigData;
> >
> > use crate::api2::backup::optional_ns_param;
> > use crate::api2::node::rrd::create_value_from_rrd;
> > @@ -2240,6 +2242,173 @@ pub async fn set_backup_owner(
> > .await?
> > }
> >
> > +pub fn do_mount_device(
> > + _lock: Option<BackupLockGuard>,
> > + mut config: SectionConfigData,
> > + mut datastore: DataStoreConfig,
> > + worker: Option<&dyn WorkerTaskContext>,
> > +) -> Result<(), Error> {
> > + if let Some(uuid) = datastore.backing_device.as_ref() {
> > + if pbs_datastore::check_if_available(&datastore).is_ok() {
> > + return Err(format_err!("device '{}' is already mounted", &uuid));
> > + }
>
> use `bail` here and inline the `uuid`:
>
> bail!("device '{uuid}' is already mounted");
>
> > + let mount_point_path = std::path::Path::new(&datastore.path);
> > + if let Some(worker) = worker {
> > + task_log!(worker, "mounting '{}' to '{}'", uuid, datastore.path);
>
> inline `uuid`
>
> > + }
> > + crate::tools::disks::mount_by_uuid(uuid, mount_point_path)?;
> > +
> > + datastore.set_maintenance_mode(None);
> > + config.set_data(&datastore.name, "datastore", &datastore)?;
> > + pbs_config::datastore::save_config(&config)?;
> > +
> > + Ok(())
> > + } else {
> > + Err(format_err!(
> > + "Datastore '{}' can't be mounted because it is not removable.",
> > + &datastore.name
>
> this does not need to be a reference, you can pass in `datastore.name`
>
> > + ))
> > + }
> > +}
> > +
> > +#[api(
> > + protected: true,
> > + input: {
> > + properties: {
> > + store: {
> > + schema: DATASTORE_SCHEMA,
> > + },
> > + }
> > + },
> > + returns: {
> > + schema: UPID_SCHEMA,
> > + },
> > + access: {
> > + permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT, false),
> > + },
> > +)]
> > +/// Mount removable datastore.
> > +pub fn mount(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
> > + let (section_config, _digest) = pbs_config::datastore::config()?;
> > + let datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
> > +
> > + if datastore.backing_device.is_none() {
> > + bail!("datastore '{}' 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(
> > + "mount-device",
> > + Some(store),
> > + auth_id.to_string(),
> > + to_stdout,
> > + move |worker| do_mount_device(Some(lock), section_config, datastore, Some(&worker)),
> > + )?;
>
> What is the motivation for doing this inside a dedicated worker task? Is
> this expected to take a considerable amount of time in some cases?
>
mostly so auto mounts appear in the task log, and sometimes it takes a
few seconds
> > +
> > + Ok(json!(upid))
> > +}
> > +
> > +fn do_unmount_device(
> > + force: bool,
> > + datastore: DataStoreConfig,
> > + worker: Option<&dyn WorkerTaskContext>,
> > +) -> Result<(), Error> {
> > + if force {
> > + let _ = crate::tools::disks::unmount_by_mountpoint(&datastore.path);
> > + return Ok(());
> > + }
> > +
> > + 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(())
> > +}
> > +
> > +#[api(
> > + protected: true,
> > + input: {
> > + properties: {
> > + store: { schema: DATASTORE_SCHEMA },
> > + force: {
> > + type: Boolean,
> > + description: "Force unmount even if there are active operations.",
> > + optional: true,
> > + default: false,
> > + },
> > + },
> > + },
> > + returns: {
> > + schema: UPID_SCHEMA,
> > + },
> > + access: {
> > + permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_MODIFY, true),
> > + }
> > +)]
> > +/// Unmount a removable device that is associated with the datastore
> > +pub async fn unmount(
> > + store: String,
> > + force: bool,
> > + rpcenv: &mut dyn RpcEnvironment,
> > +) -> Result<Value, Error> {
> > + let _lock = pbs_config::datastore::lock_config()?;
> > + let (mut section_config, _digest) = pbs_config::datastore::config()?;
> > + let mut datastore: DataStoreConfig = section_config.lookup("datastore", &store)?;
> > +
> > + if datastore.backing_device.is_none() {
> > + bail!("datastore '{}' is not removable", &store);
> > + }
> > +
> > + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> > + let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
> > +
> > + datastore.set_maintenance_mode(Some(MaintenanceMode::new(MaintenanceType::Unplugged, None)));
> > + section_config.set_data(&datastore.name, "datastore", &datastore)?;
> > + pbs_config::datastore::save_config(§ion_config)?;
> > + drop(_lock);
> > +
> > + if let Ok(proxy_pid) = proxmox_rest_server::read_pid(pbs_buildcfg::PROXMOX_BACKUP_PROXY_PID_FN)
> > + {
> > + let sock = proxmox_rest_server::ctrl_sock_from_pid(proxy_pid);
> > + let _ = proxmox_rest_server::send_raw_command(
> > + sock,
> > + &format!(
> > + "{{\"command\":\"update-datastore-cache\",\"args\":\"{}\"}}\n",
> > + &store
>
> `store` can be inlined as well here, also no need for taking it by reference
>
> > + ),
> > + )
> > + .await;
> > + }
> > +
> > + let upid = WorkerTask::new_thread(
> > + "unmount-device",
> > + Some(store),
> > + auth_id.to_string(),
> > + to_stdout,
> > + move |worker| do_unmount_device(force, datastore, Some(&worker)),
> > + )?;
> > +
> > + Ok(json!(upid))
> > +}
> > +
> > #[sortable]
> > const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
> > (
> > @@ -2278,6 +2447,7 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
> > .get(&API_METHOD_LIST_GROUPS)
> > .delete(&API_METHOD_DELETE_GROUP),
> > ),
> > + ("mount", &Router::new().post(&API_METHOD_MOUNT)),
> > (
> > "namespace",
> > // FIXME: move into datastore:: sub-module?!
> > @@ -2312,6 +2482,7 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
> > .delete(&API_METHOD_DELETE_SNAPSHOT),
> > ),
> > ("status", &Router::new().get(&API_METHOD_STATUS)),
> > + ("unmount", &Router::new().post(&API_METHOD_UNMOUNT)),
> > (
> > "upload-backup-log",
> > &Router::new().upload(&API_METHOD_UPLOAD_BACKUP_LOG),
^ permalink raw reply [flat|nested] 54+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v3 07/24] api2: removable datastore creation
2024-04-09 10:59 [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Hannes Laimer
` (5 preceding siblings ...)
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 06/24] api2: admin: add (un)mount endpoint for removable datastores Hannes Laimer
@ 2024-04-09 10:59 ` Hannes Laimer
2024-04-10 9:33 ` Dietmar Maurer
2024-04-15 16:02 ` Christian Ebner
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 08/24] api2: disks list: add only-unused flag Hannes Laimer
` (17 subsequent siblings)
24 siblings, 2 replies; 54+ messages in thread
From: Hannes Laimer @ 2024-04-09 10:59 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/api2/config/datastore.rs | 52 ++++++++++++++++++++++++++++++++++--
1 file changed, 50 insertions(+), 2 deletions(-)
diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 3081e1f4..0194d7d4 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -8,7 +8,7 @@ use serde_json::Value;
use proxmox_router::{http_bail, Permission, Router, RpcEnvironment, RpcEnvironmentType};
use proxmox_schema::{api, param_bail, ApiType};
use proxmox_section_config::SectionConfigData;
-use proxmox_sys::{task_warn, WorkerTaskContext};
+use proxmox_sys::{task_log, task_warn, WorkerTaskContext};
use proxmox_uuid::Uuid;
use pbs_api_types::{
@@ -20,7 +20,8 @@ use pbs_config::BackupLockGuard;
use pbs_datastore::chunk_store::ChunkStore;
use crate::api2::admin::{
- prune::list_prune_jobs, sync::list_sync_jobs, verify::list_verification_jobs,
+ datastore::do_mount_device, prune::list_prune_jobs, sync::list_sync_jobs,
+ verify::list_verification_jobs,
};
use crate::api2::config::prune::{delete_prune_job, do_create_prune_job};
use crate::api2::config::sync::delete_sync_job;
@@ -72,6 +73,31 @@ pub(crate) fn do_create_datastore(
datastore: DataStoreConfig,
worker: Option<&dyn WorkerTaskContext>,
) -> Result<(), Error> {
+ if datastore.backing_device.is_some() {
+ let mut mount_point: PathBuf = PathBuf::from(&datastore.path);
+
+ let default_options = proxmox_sys::fs::CreateOptions::new();
+ proxmox_sys::fs::create_path(
+ &mount_point,
+ Some(default_options.clone()),
+ Some(default_options),
+ )?;
+ do_mount_device(None, config.clone(), datastore.clone(), worker)?;
+
+ mount_point.push(".chunks");
+ if mount_point.is_dir() {
+ config.set_data(&datastore.name, "datastore", &datastore)?;
+ pbs_config::datastore::save_config(&config)?;
+ if let Some(worker) = worker {
+ task_log!(
+ worker,
+ "created removable datastore, chunkstore already exists"
+ );
+ return Ok(());
+ }
+ }
+ }
+
let path: PathBuf = datastore.path.clone().into();
let tuning: DatastoreTuning = serde_json::from_value(
@@ -122,6 +148,28 @@ pub fn create_datastore(
param_bail!("name", "datastore '{}' already exists.", config.name);
}
+ if let Some(uuid) = &config.backing_device {
+ let already_used_by = section_config
+ .sections
+ .iter()
+ .flat_map(|(datastore_name, (_, config))| {
+ config
+ .as_object()
+ .and_then(|cfg| cfg.get("backing-device"))
+ .and_then(|backing_device| backing_device.as_str())
+ .filter(|&device_uuid| device_uuid == uuid)
+ .map(|_| datastore_name)
+ })
+ .next();
+
+ if let Some(datastore_name) = already_used_by {
+ param_bail!(
+ "backing-device",
+ "device already in use by datastore '{datastore_name}'",
+ );
+ }
+ }
+
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
--
2.39.2
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 07/24] api2: removable datastore creation
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 07/24] api2: removable datastore creation Hannes Laimer
@ 2024-04-10 9:33 ` Dietmar Maurer
2024-04-15 16:02 ` Christian Ebner
1 sibling, 0 replies; 54+ messages in thread
From: Dietmar Maurer @ 2024-04-10 9:33 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
> + mount_point.push(".chunks");
> + if mount_point.is_dir() {
> + config.set_data(&datastore.name, "datastore", &datastore)?;
> + pbs_config::datastore::save_config(&config)?;
> + if let Some(worker) = worker {
> + task_log!(
> + worker,
> + "created removable datastore, chunkstore already exists"
> + );
> + return Ok(());
I guess you should also do:
jobstate::create_state_file("garbage_collection", &datastore.name)
> + }
> + }
> + }
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 07/24] api2: removable datastore creation
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 07/24] api2: removable datastore creation Hannes Laimer
2024-04-10 9:33 ` Dietmar Maurer
@ 2024-04-15 16:02 ` Christian Ebner
2024-04-16 14:27 ` Hannes Laimer
1 sibling, 1 reply; 54+ messages in thread
From: Christian Ebner @ 2024-04-15 16:02 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
On 4/9/24 12:59, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> src/api2/config/datastore.rs | 52 ++++++++++++++++++++++++++++++++++--
> 1 file changed, 50 insertions(+), 2 deletions(-)
>
> diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
> index 3081e1f4..0194d7d4 100644
> --- a/src/api2/config/datastore.rs
> +++ b/src/api2/config/datastore.rs
> @@ -8,7 +8,7 @@ use serde_json::Value;
> use proxmox_router::{http_bail, Permission, Router, RpcEnvironment, RpcEnvironmentType};
> use proxmox_schema::{api, param_bail, ApiType};
> use proxmox_section_config::SectionConfigData;
> -use proxmox_sys::{task_warn, WorkerTaskContext};
> +use proxmox_sys::{task_log, task_warn, WorkerTaskContext};
> use proxmox_uuid::Uuid;
>
> use pbs_api_types::{
> @@ -20,7 +20,8 @@ use pbs_config::BackupLockGuard;
> use pbs_datastore::chunk_store::ChunkStore;
>
> use crate::api2::admin::{
> - prune::list_prune_jobs, sync::list_sync_jobs, verify::list_verification_jobs,
> + datastore::do_mount_device, prune::list_prune_jobs, sync::list_sync_jobs,
> + verify::list_verification_jobs,
> };
> use crate::api2::config::prune::{delete_prune_job, do_create_prune_job};
> use crate::api2::config::sync::delete_sync_job;
> @@ -72,6 +73,31 @@ pub(crate) fn do_create_datastore(
> datastore: DataStoreConfig,
> worker: Option<&dyn WorkerTaskContext>,
> ) -> Result<(), Error> {
> + if datastore.backing_device.is_some() {
> + let mut mount_point: PathBuf = PathBuf::from(&datastore.path);
There is no need for the additional type annotation, the type is already
given by the return type of the `PathBuf::from()`.
> +
> + let default_options = proxmox_sys::fs::CreateOptions::new();
> + proxmox_sys::fs::create_path(
> + &mount_point,
> + Some(default_options.clone()),
> + Some(default_options),
> + )?;
> + do_mount_device(None, config.clone(), datastore.clone(), worker)?;
> +
> + mount_point.push(".chunks");
> + if mount_point.is_dir() {
> + config.set_data(&datastore.name, "datastore", &datastore)?;
> + pbs_config::datastore::save_config(&config)?;
> + if let Some(worker) = worker {
> + task_log!(
> + worker,
> + "created removable datastore, chunkstore already exists"
> + );
> + return Ok(());
> + }
> + }
If `.chunks` already exists on the mounted datastore, but it is not a
directory, then I guess something is off. Can and should this be handled
early, or is this already fully covered by the `ChunkStore::create` call
later on?
> + }
> +
> let path: PathBuf = datastore.path.clone().into();
>
> let tuning: DatastoreTuning = serde_json::from_value(
> @@ -122,6 +148,28 @@ pub fn create_datastore(
> param_bail!("name", "datastore '{}' already exists.", config.name);
> }
>
> + if let Some(uuid) = &config.backing_device {
> + let already_used_by = section_config
> + .sections
> + .iter()
> + .flat_map(|(datastore_name, (_, config))| {
> + config
> + .as_object()
> + .and_then(|cfg| cfg.get("backing-device"))
> + .and_then(|backing_device| backing_device.as_str())
> + .filter(|&device_uuid| device_uuid == uuid)
> + .map(|_| datastore_name)
> + })
> + .next();
> +
> + if let Some(datastore_name) = already_used_by {
> + param_bail!(
> + "backing-device",
> + "device already in use by datastore '{datastore_name}'",
> + );
> + }
> + }
> +
> let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
>
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 07/24] api2: removable datastore creation
2024-04-15 16:02 ` Christian Ebner
@ 2024-04-16 14:27 ` Hannes Laimer
0 siblings, 0 replies; 54+ messages in thread
From: Hannes Laimer @ 2024-04-16 14:27 UTC (permalink / raw)
To: Christian Ebner, Proxmox Backup Server development discussion
On Mon Apr 15, 2024 at 6:02 PM CEST, Christian Ebner wrote:
> On 4/9/24 12:59, Hannes Laimer wrote:
> > Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> > ---
> > src/api2/config/datastore.rs | 52 ++++++++++++++++++++++++++++++++++--
> > 1 file changed, 50 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
> > index 3081e1f4..0194d7d4 100644
> > --- a/src/api2/config/datastore.rs
> > +++ b/src/api2/config/datastore.rs
> > @@ -8,7 +8,7 @@ use serde_json::Value;
> > use proxmox_router::{http_bail, Permission, Router, RpcEnvironment, RpcEnvironmentType};
> > use proxmox_schema::{api, param_bail, ApiType};
> > use proxmox_section_config::SectionConfigData;
> > -use proxmox_sys::{task_warn, WorkerTaskContext};
> > +use proxmox_sys::{task_log, task_warn, WorkerTaskContext};
> > use proxmox_uuid::Uuid;
> >
> > use pbs_api_types::{
> > @@ -20,7 +20,8 @@ use pbs_config::BackupLockGuard;
> > use pbs_datastore::chunk_store::ChunkStore;
> >
> > use crate::api2::admin::{
> > - prune::list_prune_jobs, sync::list_sync_jobs, verify::list_verification_jobs,
> > + datastore::do_mount_device, prune::list_prune_jobs, sync::list_sync_jobs,
> > + verify::list_verification_jobs,
> > };
> > use crate::api2::config::prune::{delete_prune_job, do_create_prune_job};
> > use crate::api2::config::sync::delete_sync_job;
> > @@ -72,6 +73,31 @@ pub(crate) fn do_create_datastore(
> > datastore: DataStoreConfig,
> > worker: Option<&dyn WorkerTaskContext>,
> > ) -> Result<(), Error> {
> > + if datastore.backing_device.is_some() {
> > + let mut mount_point: PathBuf = PathBuf::from(&datastore.path);
>
> There is no need for the additional type annotation, the type is already
> given by the return type of the `PathBuf::from()`.
>
> > +
> > + let default_options = proxmox_sys::fs::CreateOptions::new();
> > + proxmox_sys::fs::create_path(
> > + &mount_point,
> > + Some(default_options.clone()),
> > + Some(default_options),
> > + )?;
> > + do_mount_device(None, config.clone(), datastore.clone(), worker)?;
> > +
> > + mount_point.push(".chunks");
> > + if mount_point.is_dir() {
> > + config.set_data(&datastore.name, "datastore", &datastore)?;
> > + pbs_config::datastore::save_config(&config)?;
> > + if let Some(worker) = worker {
> > + task_log!(
> > + worker,
> > + "created removable datastore, chunkstore already exists"
> > + );
> > + return Ok(());
> > + }
> > + }
>
> If `.chunks` already exists on the mounted datastore, but it is not a
> directory, then I guess something is off. Can and should this be handled
> early, or is this already fully covered by the `ChunkStore::create` call
> later on?
>
ChunkStore::create will fail in that case
> > + }
> > +
> > let path: PathBuf = datastore.path.clone().into();
> >
> > let tuning: DatastoreTuning = serde_json::from_value(
> > @@ -122,6 +148,28 @@ pub fn create_datastore(
> > param_bail!("name", "datastore '{}' already exists.", config.name);
> > }
> >
> > + if let Some(uuid) = &config.backing_device {
> > + let already_used_by = section_config
> > + .sections
> > + .iter()
> > + .flat_map(|(datastore_name, (_, config))| {
> > + config
> > + .as_object()
> > + .and_then(|cfg| cfg.get("backing-device"))
> > + .and_then(|backing_device| backing_device.as_str())
> > + .filter(|&device_uuid| device_uuid == uuid)
> > + .map(|_| datastore_name)
> > + })
> > + .next();
> > +
> > + if let Some(datastore_name) = already_used_by {
> > + param_bail!(
> > + "backing-device",
> > + "device already in use by datastore '{datastore_name}'",
> > + );
> > + }
> > + }
> > +
> > let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> > let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
> >
^ permalink raw reply [flat|nested] 54+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v3 08/24] api2: disks list: add only-unused flag
2024-04-09 10:59 [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Hannes Laimer
` (6 preceding siblings ...)
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 07/24] api2: removable datastore creation Hannes Laimer
@ 2024-04-09 10:59 ` Hannes Laimer
2024-04-15 16:27 ` Christian Ebner
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 09/24] pbs-api-types: datastore: use new proxmox_scheme::de for deserialization Hannes Laimer
` (16 subsequent siblings)
24 siblings, 1 reply; 54+ messages in thread
From: Hannes Laimer @ 2024-04-09 10:59 UTC (permalink / raw)
To: pbs-devel
... used by the partition selector for removable datastore creation.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/api2/node/disks/mod.rs | 8 +++++
src/tools/disks/mod.rs | 67 +++++++++++++++++++++++++++++++++-----
2 files changed, 66 insertions(+), 9 deletions(-)
diff --git a/src/api2/node/disks/mod.rs b/src/api2/node/disks/mod.rs
index 711dae7b..bd251dab 100644
--- a/src/api2/node/disks/mod.rs
+++ b/src/api2/node/disks/mod.rs
@@ -41,6 +41,12 @@ pub mod zfs;
optional: true,
default: false,
},
+ "only-unused": {
+ description: "Only list partitions not used for removable datastores or mounted directories.",
+ type: bool,
+ optional: true,
+ default: false,
+ },
"usage-type": {
type: DiskUsageType,
optional: true,
@@ -62,6 +68,7 @@ pub mod zfs;
pub fn list_disks(
skipsmart: bool,
include_partitions: bool,
+ only_unused: bool,
usage_type: Option<DiskUsageType>,
) -> Result<Vec<DiskUsageInfo>, Error> {
let mut list = Vec::new();
@@ -69,6 +76,7 @@ pub fn list_disks(
for (_, info) in DiskUsageQuery::new()
.smart(!skipsmart)
.partitions(include_partitions)
+ .only_not_in_use(only_unused)
.query()?
{
if let Some(ref usage_type) = usage_type {
diff --git a/src/tools/disks/mod.rs b/src/tools/disks/mod.rs
index eaf016df..81184eb6 100644
--- a/src/tools/disks/mod.rs
+++ b/src/tools/disks/mod.rs
@@ -20,6 +20,7 @@ use proxmox_rest_server::WorkerTask;
use proxmox_schema::api;
use proxmox_sys::linux::procfs::{mountinfo::Device, MountInfo};
use proxmox_sys::task_log;
+use serde_json::Value;
use pbs_api_types::{BLOCKDEVICE_DISK_AND_PARTITION_NAME_REGEX, BLOCKDEVICE_NAME_REGEX};
@@ -32,6 +33,7 @@ pub use zpool_list::*;
mod lvm;
pub use lvm::*;
mod smart;
+use crate::api2::node::disks::directory::list_datastore_mounts;
pub use smart::*;
lazy_static::lazy_static! {
@@ -828,6 +830,7 @@ fn scan_partitions(
pub struct DiskUsageQuery {
smart: bool,
partitions: bool,
+ only_not_in_use: bool,
}
impl DiskUsageQuery {
@@ -835,6 +838,7 @@ impl DiskUsageQuery {
Self {
smart: true,
partitions: false,
+ only_not_in_use: false,
}
}
@@ -848,12 +852,22 @@ impl DiskUsageQuery {
self
}
+ pub fn only_not_in_use(&mut self, only_not_in_use: bool) -> &mut Self {
+ self.only_not_in_use = only_not_in_use;
+ self
+ }
+
pub fn query(&self) -> Result<HashMap<String, DiskUsageInfo>, Error> {
- get_disks(None, !self.smart, self.partitions)
+ get_disks(None, !self.smart, self.partitions, self.only_not_in_use)
}
pub fn find(&self, disk: &str) -> Result<DiskUsageInfo, Error> {
- let mut map = get_disks(Some(vec![disk.to_string()]), !self.smart, self.partitions)?;
+ let mut map = get_disks(
+ Some(vec![disk.to_string()]),
+ !self.smart,
+ self.partitions,
+ self.only_not_in_use,
+ )?;
if let Some(info) = map.remove(disk) {
Ok(info)
} else {
@@ -862,7 +876,12 @@ impl DiskUsageQuery {
}
pub fn find_all(&self, disks: Vec<String>) -> Result<HashMap<String, DiskUsageInfo>, Error> {
- get_disks(Some(disks), !self.smart, self.partitions)
+ get_disks(
+ Some(disks),
+ !self.smart,
+ self.partitions,
+ self.only_not_in_use,
+ )
}
}
@@ -935,6 +954,8 @@ fn get_disks(
no_smart: bool,
// include partitions
include_partitions: bool,
+ // skip partitions which are in use
+ only_not_in_use: bool,
) -> Result<HashMap<String, DiskUsageInfo>, Error> {
let disk_manager = DiskManage::new();
@@ -952,6 +973,30 @@ fn get_disks(
// fixme: ceph journals/volumes
+ let uuids_in_use = if only_not_in_use && include_partitions {
+ let (config, _digest) = pbs_config::datastore::config()?;
+
+ let uuids_from_datastores: Vec<String> = config
+ .sections
+ .iter()
+ .filter_map(|(_, (_, data))| {
+ data.as_object()
+ .and_then(|cfg| cfg.get("backing-device"))
+ .and_then(Value::as_str)
+ .map(String::from)
+ })
+ .collect();
+
+ let uuids_from_mounts: Vec<String> = list_datastore_mounts()?
+ .into_iter()
+ .filter_map(|mount| mount.device.split('/').last().map(String::from))
+ .collect();
+
+ [&uuids_from_datastores[..], &uuids_from_mounts[..]].concat()
+ } else {
+ Vec::new()
+ };
+
let mut result = HashMap::new();
for item in proxmox_sys::fs::scan_subdir(libc::AT_FDCWD, "/sys/block", &BLOCKDEVICE_NAME_REGEX)?
@@ -1024,12 +1069,16 @@ fn get_disks(
let partitions: Option<Vec<PartitionInfo>> = if include_partitions {
disk.partitions().map_or(None, |parts| {
- Some(get_partitions_info(
- parts,
- &lvm_devices,
- &zfs_devices,
- &file_system_devices,
- ))
+ let infos =
+ get_partitions_info(parts, &lvm_devices, &zfs_devices, &file_system_devices)
+ .into_iter()
+ .filter(|part| {
+ part.uuid
+ .as_ref()
+ .map_or(true, |u| !uuids_in_use.contains(u))
+ })
+ .collect();
+ Some(infos)
})
} else {
None
--
2.39.2
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 08/24] api2: disks list: add only-unused flag
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 08/24] api2: disks list: add only-unused flag Hannes Laimer
@ 2024-04-15 16:27 ` Christian Ebner
0 siblings, 0 replies; 54+ messages in thread
From: Christian Ebner @ 2024-04-15 16:27 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
a few comments inline
On 4/9/24 12:59, Hannes Laimer wrote:
> ... used by the partition selector for removable datastore creation.
>
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> src/api2/node/disks/mod.rs | 8 +++++
> src/tools/disks/mod.rs | 67 +++++++++++++++++++++++++++++++++-----
> 2 files changed, 66 insertions(+), 9 deletions(-)
>
> diff --git a/src/api2/node/disks/mod.rs b/src/api2/node/disks/mod.rs
> index 711dae7b..bd251dab 100644
> --- a/src/api2/node/disks/mod.rs
> +++ b/src/api2/node/disks/mod.rs
> @@ -41,6 +41,12 @@ pub mod zfs;
> optional: true,
> default: false,
> },
> + "only-unused": {
> + description: "Only list partitions not used for removable datastores or mounted directories.",
Since the parameter before this one is called `include-partitions`,
might be worth to keep a similar naming scheme and call this
`exclude-used` and as description something like `Exclude partitions
already used for removable datastores or mounted directories.` This
would also help improve the naming of the `only_not_in_use` variables
and methods (imho).
> + type: bool,
> + optional: true,
> + default: false,
> + },
> "usage-type": {
> type: DiskUsageType,
> optional: true,
> @@ -62,6 +68,7 @@ pub mod zfs;
> pub fn list_disks(
> skipsmart: bool,
> include_partitions: bool,
> + only_unused: bool,
with above suggestion, this is renamed to `exclude_used` ...
> usage_type: Option<DiskUsageType>,
> ) -> Result<Vec<DiskUsageInfo>, Error> {
> let mut list = Vec::new();
> @@ -69,6 +76,7 @@ pub fn list_disks(
> for (_, info) in DiskUsageQuery::new()
> .smart(!skipsmart)
> .partitions(include_partitions)
> + .only_not_in_use(only_unused)
... `exclude_used(exclude_used)`
> .query()?
> {
> if let Some(ref usage_type) = usage_type {
> diff --git a/src/tools/disks/mod.rs b/src/tools/disks/mod.rs
> index eaf016df..81184eb6 100644
> --- a/src/tools/disks/mod.rs
> +++ b/src/tools/disks/mod.rs
> @@ -20,6 +20,7 @@ use proxmox_rest_server::WorkerTask;
> use proxmox_schema::api;
> use proxmox_sys::linux::procfs::{mountinfo::Device, MountInfo};
> use proxmox_sys::task_log;
> +use serde_json::Value;
>
> use pbs_api_types::{BLOCKDEVICE_DISK_AND_PARTITION_NAME_REGEX, BLOCKDEVICE_NAME_REGEX};
>
> @@ -32,6 +33,7 @@ pub use zpool_list::*;
> mod lvm;
> pub use lvm::*;
> mod smart;
> +use crate::api2::node::disks::directory::list_datastore_mounts;
> pub use smart::*;
>
> lazy_static::lazy_static! {
> @@ -828,6 +830,7 @@ fn scan_partitions(
> pub struct DiskUsageQuery {
> smart: bool,
> partitions: bool,
> + only_not_in_use: bool,
> }
>
> impl DiskUsageQuery {
> @@ -835,6 +838,7 @@ impl DiskUsageQuery {
> Self {
> smart: true,
> partitions: false,
> + only_not_in_use: false,
... `exclude_used`
> }
> }
>
> @@ -848,12 +852,22 @@ impl DiskUsageQuery {
> self
> }
>
> + pub fn only_not_in_use(&mut self, only_not_in_use: bool) -> &mut Self {
> + self.only_not_in_use = only_not_in_use;
... ditto, not going to comment all of them
> + self
> + }
> +
> pub fn query(&self) -> Result<HashMap<String, DiskUsageInfo>, Error> {
> - get_disks(None, !self.smart, self.partitions)
> + get_disks(None, !self.smart, self.partitions, self.only_not_in_use)
> }
>
> pub fn find(&self, disk: &str) -> Result<DiskUsageInfo, Error> {
> - let mut map = get_disks(Some(vec![disk.to_string()]), !self.smart, self.partitions)?;
> + let mut map = get_disks(
> + Some(vec![disk.to_string()]),
> + !self.smart,
> + self.partitions,
> + self.only_not_in_use,
> + )?;
> if let Some(info) = map.remove(disk) {
> Ok(info)
> } else {
> @@ -862,7 +876,12 @@ impl DiskUsageQuery {
> }
>
> pub fn find_all(&self, disks: Vec<String>) -> Result<HashMap<String, DiskUsageInfo>, Error> {
> - get_disks(Some(disks), !self.smart, self.partitions)
> + get_disks(
> + Some(disks),
> + !self.smart,
> + self.partitions,
> + self.only_not_in_use,
> + )
> }
> }
>
> @@ -935,6 +954,8 @@ fn get_disks(
> no_smart: bool,
> // include partitions
> include_partitions: bool,
> + // skip partitions which are in use
> + only_not_in_use: bool,
> ) -> Result<HashMap<String, DiskUsageInfo>, Error> {
> let disk_manager = DiskManage::new();
>
> @@ -952,6 +973,30 @@ fn get_disks(
>
> // fixme: ceph journals/volumes
>
> + let uuids_in_use = if only_not_in_use && include_partitions {
> + let (config, _digest) = pbs_config::datastore::config()?;
> +
> + let uuids_from_datastores: Vec<String> = config
> + .sections
> + .iter()
> + .filter_map(|(_, (_, data))| {
> + data.as_object()
> + .and_then(|cfg| cfg.get("backing-device"))
> + .and_then(Value::as_str)
> + .map(String::from)
> + })
> + .collect();
> +
> + let uuids_from_mounts: Vec<String> = list_datastore_mounts()?
> + .into_iter()
> + .filter_map(|mount| mount.device.split('/').last().map(String::from))
> + .collect();
> +
> + [&uuids_from_datastores[..], &uuids_from_mounts[..]].concat()
> + } else {
> + Vec::new()
> + };
> +
> let mut result = HashMap::new();
>
> for item in proxmox_sys::fs::scan_subdir(libc::AT_FDCWD, "/sys/block", &BLOCKDEVICE_NAME_REGEX)?
> @@ -1024,12 +1069,16 @@ fn get_disks(
>
> let partitions: Option<Vec<PartitionInfo>> = if include_partitions {
> disk.partitions().map_or(None, |parts| {
> - Some(get_partitions_info(
> - parts,
> - &lvm_devices,
> - &zfs_devices,
> - &file_system_devices,
> - ))
> + let infos =
> + get_partitions_info(parts, &lvm_devices, &zfs_devices, &file_system_devices)
> + .into_iter()
> + .filter(|part| {
> + part.uuid
> + .as_ref()
> + .map_or(true, |u| !uuids_in_use.contains(u))
> + })
> + .collect();
> + Some(infos)
> })
> } else {
> None
^ permalink raw reply [flat|nested] 54+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v3 09/24] pbs-api-types: datastore: use new proxmox_scheme::de for deserialization
2024-04-09 10:59 [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Hannes Laimer
` (7 preceding siblings ...)
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 08/24] api2: disks list: add only-unused flag Hannes Laimer
@ 2024-04-09 10:59 ` Hannes Laimer
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 10/24] pbs-api-types: add removable/is-available flag to DataStoreListItem Hannes Laimer
` (15 subsequent siblings)
24 siblings, 0 replies; 54+ messages in thread
From: Hannes Laimer @ 2024-04-09 10:59 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
pbs-api-types/src/datastore.rs | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index b6d238ed..738ba96f 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -346,10 +346,13 @@ impl DataStoreConfig {
}
pub fn get_maintenance_mode(&self) -> Option<MaintenanceMode> {
- self.maintenance_mode
- .as_ref()
- .and_then(|str| MaintenanceMode::API_SCHEMA.parse_property_string(str).ok())
- .and_then(|value| MaintenanceMode::deserialize(value).ok())
+ self.maintenance_mode.as_ref().and_then(|str| {
+ MaintenanceMode::deserialize(proxmox_schema::de::SchemaDeserializer::new(
+ str,
+ &MaintenanceMode::API_SCHEMA,
+ ))
+ .ok()
+ })
}
pub fn set_maintenance_mode(&mut self, mode: Option<MaintenanceMode>) {
--
2.39.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pbs-devel] [PATCH proxmox-backup v3 10/24] pbs-api-types: add removable/is-available flag to DataStoreListItem
2024-04-09 10:59 [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Hannes Laimer
` (8 preceding siblings ...)
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 09/24] pbs-api-types: datastore: use new proxmox_scheme::de for deserialization Hannes Laimer
@ 2024-04-09 10:59 ` Hannes Laimer
2024-04-16 7:37 ` Christian Ebner
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 11/24] pb-manager: add (un)mount command Hannes Laimer
` (14 subsequent siblings)
24 siblings, 1 reply; 54+ messages in thread
From: Hannes Laimer @ 2024-04-09 10:59 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 +
src/api2/status.rs | 18 +++++++++++++++---
3 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index 738ba96f..ce53c375 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -390,6 +390,8 @@ impl DataStoreConfig {
pub struct DataStoreListItem {
pub store: String,
pub comment: Option<String>,
+ /// Datastore is removable
+ pub removable: bool,
/// If the datastore is in maintenance mode, information about it
#[serde(skip_serializing_if = "Option::is_none")]
pub maintenance: Option<String>,
@@ -1357,6 +1359,8 @@ pub struct DataStoreStatusListItem {
/// The available bytes of the underlying storage. (-1 on error)
#[serde(skip_serializing_if = "Option::is_none")]
pub avail: Option<u64>,
+ /// The datastore is available, relevant if removable
+ pub is_available: bool,
/// A list of usages of the past (last Month).
#[serde(skip_serializing_if = "Option::is_none")]
pub history: Option<Vec<Option<f64>>>,
@@ -1381,12 +1385,13 @@ pub struct DataStoreStatusListItem {
}
impl DataStoreStatusListItem {
- pub fn empty(store: &str, err: Option<String>) -> Self {
+ pub fn empty(store: &str, err: Option<String>, is_available: bool) -> Self {
DataStoreStatusListItem {
store: store.to_owned(),
total: None,
used: None,
avail: None,
+ is_available,
history: None,
history_start: None,
history_delta: None,
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 92f6adc2..acf19fa2 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -1245,6 +1245,7 @@ pub fn get_datastore_list(
} else {
data["comment"].as_str().map(String::from)
},
+ removable: data["backing-device"].as_str().is_some(),
maintenance: data["maintenance-mode"].as_str().map(String::from),
});
}
diff --git a/src/api2/status.rs b/src/api2/status.rs
index 78bc06b5..43f0b55d 100644
--- a/src/api2/status.rs
+++ b/src/api2/status.rs
@@ -13,7 +13,7 @@ use pbs_api_types::{
};
use pbs_config::CachedUserInfo;
-use pbs_datastore::DataStore;
+use pbs_datastore::{check_if_available, DataStore};
use crate::rrd_cache::extract_rrd_data;
use crate::tools::statistics::linear_regression;
@@ -48,10 +48,17 @@ pub async fn datastore_status(
for (store, (_, _)) in &config.sections {
let user_privs = user_info.lookup_privs(&auth_id, &["datastore", store]);
let allowed = (user_privs & (PRIV_DATASTORE_AUDIT | PRIV_DATASTORE_BACKUP)) != 0;
+
+ let store_config = config.lookup("datastore", store)?;
+ if check_if_available(&store_config).is_err() {
+ list.push(DataStoreStatusListItem::empty(store, None, false));
+ continue;
+ }
+
if !allowed {
if let Ok(datastore) = DataStore::lookup_datastore(store, Some(Operation::Lookup)) {
if can_access_any_namespace(datastore, &auth_id, &user_info) {
- list.push(DataStoreStatusListItem::empty(store, None));
+ list.push(DataStoreStatusListItem::empty(store, None, true));
}
}
continue;
@@ -60,7 +67,11 @@ pub async fn datastore_status(
let datastore = match DataStore::lookup_datastore(store, Some(Operation::Read)) {
Ok(datastore) => datastore,
Err(err) => {
- list.push(DataStoreStatusListItem::empty(store, Some(err.to_string())));
+ list.push(DataStoreStatusListItem::empty(
+ store,
+ Some(err.to_string()),
+ true,
+ ));
continue;
}
};
@@ -71,6 +82,7 @@ pub async fn datastore_status(
total: Some(status.total),
used: Some(status.used),
avail: Some(status.available),
+ is_available: true,
history: None,
history_start: None,
history_delta: None,
--
2.39.2
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 10/24] pbs-api-types: add removable/is-available flag to DataStoreListItem
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 10/24] pbs-api-types: add removable/is-available flag to DataStoreListItem Hannes Laimer
@ 2024-04-16 7:37 ` Christian Ebner
0 siblings, 0 replies; 54+ messages in thread
From: Christian Ebner @ 2024-04-16 7:37 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
there is a typo in commit title, s/scheme/schema
also maybe you can explicitly include the usage of `SchemaDeserializer`
by stating something like:
`pbs-api-types: use SchemaDeserializer for maintenance mode`
and a comment inline
On 4/9/24 12:59, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> pbs-api-types/src/datastore.rs | 7 ++++++-
> src/api2/admin/datastore.rs | 1 +
> src/api2/status.rs | 18 +++++++++++++++---
> 3 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
> index 738ba96f..ce53c375 100644
> --- a/pbs-api-types/src/datastore.rs
> +++ b/pbs-api-types/src/datastore.rs
> @@ -390,6 +390,8 @@ impl DataStoreConfig {
> pub struct DataStoreListItem {
> pub store: String,
> pub comment: Option<String>,
> + /// Datastore is removable
> + pub removable: bool,
> /// If the datastore is in maintenance mode, information about it
> #[serde(skip_serializing_if = "Option::is_none")]
> pub maintenance: Option<String>,
> @@ -1357,6 +1359,8 @@ pub struct DataStoreStatusListItem {
> /// The available bytes of the underlying storage. (-1 on error)
> #[serde(skip_serializing_if = "Option::is_none")]
> pub avail: Option<u64>,
> + /// The datastore is available, relevant if removable
> + pub is_available: bool,
> /// A list of usages of the past (last Month).
> #[serde(skip_serializing_if = "Option::is_none")]
> pub history: Option<Vec<Option<f64>>>,
> @@ -1381,12 +1385,13 @@ pub struct DataStoreStatusListItem {
> }
>
> impl DataStoreStatusListItem {
> - pub fn empty(store: &str, err: Option<String>) -> Self {
> + pub fn empty(store: &str, err: Option<String>, is_available: bool) -> Self {
> DataStoreStatusListItem {
> store: store.to_owned(),
> total: None,
> used: None,
> avail: None,
> + is_available,
this could default to true, and instead of passing the `is_available`
flag as argument. That could be set by a dedicates
`fn datastore_unavailable(self) -> Self` method for
`DataStoreStatusListItem`, which takes the list item, sets the flag and
returns it to be pushed to the list, so the opaque argument to `empty`
is avoided.
So you would get:
`list.push(DataStoreStatusListItem::empty(store,
None).datastore_unavailable());`
But not sure about this...
> history: None,
> history_start: None,
> history_delta: None,
> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
> index 92f6adc2..acf19fa2 100644
> --- a/src/api2/admin/datastore.rs
> +++ b/src/api2/admin/datastore.rs
> @@ -1245,6 +1245,7 @@ pub fn get_datastore_list(
> } else {
> data["comment"].as_str().map(String::from)
> },
> + removable: data["backing-device"].as_str().is_some(),
> maintenance: data["maintenance-mode"].as_str().map(String::from),
> });
> }
> diff --git a/src/api2/status.rs b/src/api2/status.rs
> index 78bc06b5..43f0b55d 100644
> --- a/src/api2/status.rs
> +++ b/src/api2/status.rs
> @@ -13,7 +13,7 @@ use pbs_api_types::{
> };
>
> use pbs_config::CachedUserInfo;
> -use pbs_datastore::DataStore;
> +use pbs_datastore::{check_if_available, DataStore};
>
> use crate::rrd_cache::extract_rrd_data;
> use crate::tools::statistics::linear_regression;
> @@ -48,10 +48,17 @@ pub async fn datastore_status(
> for (store, (_, _)) in &config.sections {
> let user_privs = user_info.lookup_privs(&auth_id, &["datastore", store]);
> let allowed = (user_privs & (PRIV_DATASTORE_AUDIT | PRIV_DATASTORE_BACKUP)) != 0;
> +
> + let store_config = config.lookup("datastore", store)?;
> + if check_if_available(&store_config).is_err() {
> + list.push(DataStoreStatusListItem::empty(store, None, false));
> + continue;
> + }
> +
> if !allowed {
> if let Ok(datastore) = DataStore::lookup_datastore(store, Some(Operation::Lookup)) {
> if can_access_any_namespace(datastore, &auth_id, &user_info) {
> - list.push(DataStoreStatusListItem::empty(store, None));
> + list.push(DataStoreStatusListItem::empty(store, None, true));
> }
> }
> continue;
> @@ -60,7 +67,11 @@ pub async fn datastore_status(
> let datastore = match DataStore::lookup_datastore(store, Some(Operation::Read)) {
> Ok(datastore) => datastore,
> Err(err) => {
> - list.push(DataStoreStatusListItem::empty(store, Some(err.to_string())));
> + list.push(DataStoreStatusListItem::empty(
> + store,
> + Some(err.to_string()),
> + true,
> + ));
> continue;
> }
> };
> @@ -71,6 +82,7 @@ pub async fn datastore_status(
> total: Some(status.total),
> used: Some(status.used),
> avail: Some(status.available),
> + is_available: true,
> history: None,
> history_start: None,
> history_delta: None,
^ permalink raw reply [flat|nested] 54+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v3 11/24] pb-manager: add (un)mount command
2024-04-09 10:59 [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Hannes Laimer
` (9 preceding siblings ...)
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 10/24] pbs-api-types: add removable/is-available flag to DataStoreListItem Hannes Laimer
@ 2024-04-09 10:59 ` Hannes Laimer
2024-04-10 10:08 ` Dietmar Maurer
2024-04-16 7:50 ` Christian Ebner
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 12/24] add auto-mounting for removable datastores Hannes Laimer
` (13 subsequent siblings)
24 siblings, 2 replies; 54+ messages in thread
From: Hannes Laimer @ 2024-04-09 10:59 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
pbs-config/src/datastore.rs | 14 ++++
src/bin/proxmox_backup_manager/datastore.rs | 76 ++++++++++++++++++++-
2 files changed, 89 insertions(+), 1 deletion(-)
diff --git a/pbs-config/src/datastore.rs b/pbs-config/src/datastore.rs
index 5844a174..a540788f 100644
--- a/pbs-config/src/datastore.rs
+++ b/pbs-config/src/datastore.rs
@@ -63,6 +63,20 @@ pub fn complete_datastore_name(_arg: &str, _param: &HashMap<String, String>) ->
}
}
+pub fn complete_removable_datastore_name(
+ _arg: &str,
+ _param: &HashMap<String, String>,
+) -> Vec<String> {
+ match config() {
+ Ok((data, _digest)) => data
+ .sections
+ .into_iter()
+ .filter_map(|(name, (_, c))| c.get("backing-device").map(|_| name))
+ .collect(),
+ Err(_) => Vec::new(),
+ }
+}
+
pub fn complete_acl_path(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> {
let mut list = vec![
String::from("/"),
diff --git a/src/bin/proxmox_backup_manager/datastore.rs b/src/bin/proxmox_backup_manager/datastore.rs
index 383bcd24..f8462459 100644
--- a/src/bin/proxmox_backup_manager/datastore.rs
+++ b/src/bin/proxmox_backup_manager/datastore.rs
@@ -1,4 +1,4 @@
-use anyhow::Error;
+use anyhow::{format_err, Error};
use serde_json::Value;
use proxmox_router::{cli::*, ApiHandler, RpcEnvironment};
@@ -40,6 +40,34 @@ fn list_datastores(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Valu
Ok(Value::Null)
}
+#[api(
+ protected: true,
+ input: {
+ properties: {
+ store: {
+ schema: DATASTORE_SCHEMA,
+ },
+ digest: {
+ optional: true,
+ schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
+ },
+ },
+ },
+)]
+/// Mount a removable a datastore configuration.
+async fn mount_datastore(mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
+ param["node"] = "localhost".into();
+
+ let info = &api2::admin::datastore::API_METHOD_MOUNT;
+ let result = match info.handler {
+ ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
+ _ => unreachable!(),
+ };
+
+ crate::wait_for_local_worker(result.as_str().unwrap()).await?;
+ Ok(())
+}
+
#[api(
input: {
properties: {
@@ -99,6 +127,34 @@ async fn create_datastore(mut param: Value) -> Result<Value, Error> {
Ok(Value::Null)
}
+#[api(
+ protected: true,
+ input: {
+ properties: {
+ store: {
+ schema: DATASTORE_SCHEMA,
+ },
+ digest: {
+ optional: true,
+ schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
+ },
+ },
+ },
+)]
+/// Unmunt a removable a datastore configuration.
+async fn unmount_datastore(mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
+ param["node"] = "localhost".into();
+
+ let info = &api2::admin::datastore::API_METHOD_UNMOUNT;
+ let result = match info.handler {
+ ApiHandler::Async(handler) => (handler)(param, info, rpcenv).await?,
+ _ => unreachable!(),
+ };
+
+ crate::wait_for_local_worker(result.as_str().unwrap()).await?;
+ Ok(())
+}
+
#[api(
protected: true,
input: {
@@ -142,6 +198,15 @@ async fn delete_datastore(mut param: Value, rpcenv: &mut dyn RpcEnvironment) ->
pub fn datastore_commands() -> CommandLineInterface {
let cmd_def = CliCommandMap::new()
.insert("list", CliCommand::new(&API_METHOD_LIST_DATASTORES))
+ .insert(
+ "mount",
+ CliCommand::new(&API_METHOD_MOUNT_DATASTORE)
+ .arg_param(&["store"])
+ .completion_cb(
+ "store",
+ pbs_config::datastore::complete_removable_datastore_name,
+ ),
+ )
.insert(
"show",
CliCommand::new(&API_METHOD_SHOW_DATASTORE)
@@ -152,6 +217,15 @@ pub fn datastore_commands() -> CommandLineInterface {
"create",
CliCommand::new(&API_METHOD_CREATE_DATASTORE).arg_param(&["name", "path"]),
)
+ .insert(
+ "unmount",
+ CliCommand::new(&API_METHOD_UNMOUNT_DATASTORE)
+ .arg_param(&["store"])
+ .completion_cb(
+ "store",
+ pbs_config::datastore::complete_removable_datastore_name,
+ ),
+ )
.insert(
"update",
CliCommand::new(&api2::config::datastore::API_METHOD_UPDATE_DATASTORE)
--
2.39.2
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 11/24] pb-manager: add (un)mount command
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 11/24] pb-manager: add (un)mount command Hannes Laimer
@ 2024-04-10 10:08 ` Dietmar Maurer
2024-04-16 7:50 ` Christian Ebner
1 sibling, 0 replies; 54+ messages in thread
From: Dietmar Maurer @ 2024-04-10 10:08 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
The binary is called "proxmox-backup-manager" (not "pb-manager")
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup v3 11/24] pb-manager: add (un)mount command
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 11/24] pb-manager: add (un)mount command Hannes Laimer
2024-04-10 10:08 ` Dietmar Maurer
@ 2024-04-16 7:50 ` Christian Ebner
1 sibling, 0 replies; 54+ messages in thread
From: Christian Ebner @ 2024-04-16 7:50 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
one question inline
On 4/9/24 12:59, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> pbs-config/src/datastore.rs | 14 ++++
> src/bin/proxmox_backup_manager/datastore.rs | 76 ++++++++++++++++++++-
> 2 files changed, 89 insertions(+), 1 deletion(-)
>
> diff --git a/pbs-config/src/datastore.rs b/pbs-config/src/datastore.rs
> index 5844a174..a540788f 100644
> --- a/pbs-config/src/datastore.rs
> +++ b/pbs-config/src/datastore.rs
> @@ -63,6 +63,20 @@ pub fn complete_datastore_name(_arg: &str, _param: &HashMap<String, String>) ->
> }
> }
>
> +pub fn complete_removable_datastore_name(
> + _arg: &str,
> + _param: &HashMap<String, String>,
> +) -> Vec<String> {
> + match config() {
> + Ok((data, _digest)) => data
> + .sections
> + .into_iter()
> + .filter_map(|(name, (_, c))| c.get("backing-device").map(|_| name))
> + .collect(),
> + Err(_) => Vec::new(),
> + }
> +}
> +
> pub fn complete_acl_path(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> {
> let mut list = vec![
> String::from("/"),
> diff --git a/src/bin/proxmox_backup_manager/datastore.rs b/src/bin/proxmox_backup_manager/datastore.rs
> index 383bcd24..f8462459 100644
> --- a/src/bin/proxmox_backup_manager/datastore.rs
> +++ b/src/bin/proxmox_backup_manager/datastore.rs
> @@ -1,4 +1,4 @@
> -use anyhow::Error;
> +use anyhow::{format_err, Error};
> use serde_json::Value;
>
> use proxmox_router::{cli::*, ApiHandler, RpcEnvironment};
> @@ -40,6 +40,34 @@ fn list_datastores(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Valu
> Ok(Value::Null)
> }
>
> +#[api(
> + protected: true,
> + input: {
> + properties: {
> + store: {
> + schema: DATASTORE_SCHEMA,
> + },
> + digest: {
> + optional: true,
> + schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
> + },
> + },
> + },
> +)]
> +/// Mount a removable a datastore configuration.
> +async fn mount_datastore(mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
> + param["node"] = "localhost".into();
> +
> + let info = &api2::admin::datastore::API_METHOD_MOUNT;
> + let result = match info.handler {
> + ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
> + _ => unreachable!(),
> + };
> +
> + crate::wait_for_local_worker(result.as_str().unwrap()).await?;
as already mentioned in the patch introducing the mount api method, is
the dedicated worker task needed for the mount? Is this expected to take
some prolonged time in some cases?
> + Ok(())
> +}
> +
> #[api(
> input: {
> properties: {
> @@ -99,6 +127,34 @@ async fn create_datastore(mut param: Value) -> Result<Value, Error> {
> Ok(Value::Null)
> }
>
> +#[api(
> + protected: true,
> + input: {
> + properties: {
> + store: {
> + schema: DATASTORE_SCHEMA,
> + },
> + digest: {
> + optional: true,
> + schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
> + },
> + },
> + },
> +)]
> +/// Unmunt a removable a datastore configuration.
> +async fn unmount_datastore(mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
> + param["node"] = "localhost".into();
> +
> + let info = &api2::admin::datastore::API_METHOD_UNMOUNT;
> + let result = match info.handler {
> + ApiHandler::Async(handler) => (handler)(param, info, rpcenv).await?,
> + _ => unreachable!(),
> + };
> +
> + crate::wait_for_local_worker(result.as_str().unwrap()).await?;
> + Ok(())
> +}
> +
> #[api(
> protected: true,
> input: {
> @@ -142,6 +198,15 @@ async fn delete_datastore(mut param: Value, rpcenv: &mut dyn RpcEnvironment) ->
> pub fn datastore_commands() -> CommandLineInterface {
> let cmd_def = CliCommandMap::new()
> .insert("list", CliCommand::new(&API_METHOD_LIST_DATASTORES))
> + .insert(
> + "mount",
> + CliCommand::new(&API_METHOD_MOUNT_DATASTORE)
> + .arg_param(&["store"])
> + .completion_cb(
> + "store",
> + pbs_config::datastore::complete_removable_datastore_name,
> + ),
> + )
> .insert(
> "show",
> CliCommand::new(&API_METHOD_SHOW_DATASTORE)
> @@ -152,6 +217,15 @@ pub fn datastore_commands() -> CommandLineInterface {
> "create",
> CliCommand::new(&API_METHOD_CREATE_DATASTORE).arg_param(&["name", "path"]),
> )
> + .insert(
> + "unmount",
> + CliCommand::new(&API_METHOD_UNMOUNT_DATASTORE)
> + .arg_param(&["store"])
> + .completion_cb(
> + "store",
> + pbs_config::datastore::complete_removable_datastore_name,
> + ),
> + )
> .insert(
> "update",
> CliCommand::new(&api2::config::datastore::API_METHOD_UPDATE_DATASTORE)
^ permalink raw reply [flat|nested] 54+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v3 12/24] add auto-mounting for removable datastores
2024-04-09 10:59 [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Hannes Laimer
` (10 preceding siblings ...)
2024-04-09 10:59 ` [pbs-devel] [PATCH proxmox-backup v3 11/24] pb-manager: add (un)mount command Hannes Laimer
@ 2024-04-09 11:00 ` Hannes Laimer
2024-04-16 8:05 ` Christian Ebner
2024-04-16 8:45 ` Christian Ebner
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 13/24] api: mark removable datastores as unplugged after restart Hannes Laimer
` (12 subsequent siblings)
24 siblings, 2 replies; 54+ messages in thread
From: Hannes Laimer @ 2024-04-09 11:00 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
debian/proxmox-backup-server.install | 1 +
debian/proxmox-backup-server.udev | 3 ++
etc/Makefile | 3 +-
etc/removable-device-attach@.service.in | 8 +++
src/bin/proxmox_backup_manager/datastore.rs | 55 +++++++++++++++++++++
5 files changed, 69 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 ee114ea3..cda01069 100644
--- a/debian/proxmox-backup-server.install
+++ b/debian/proxmox-backup-server.install
@@ -4,6 +4,7 @@ etc/proxmox-backup-daily-update.service /lib/systemd/system/
etc/proxmox-backup-daily-update.timer /lib/systemd/system/
etc/proxmox-backup-proxy.service /lib/systemd/system/
etc/proxmox-backup.service /lib/systemd/system/
+etc/removable-device-attach@.service /lib/systemd/system/
usr/bin/pmt
usr/bin/pmtx
usr/bin/proxmox-tape
diff --git a/debian/proxmox-backup-server.udev b/debian/proxmox-backup-server.udev
index afdfb2bc..e21b8bc7 100644
--- a/debian/proxmox-backup-server.udev
+++ b/debian/proxmox-backup-server.udev
@@ -16,3 +16,6 @@ SUBSYSTEM=="scsi_generic", SUBSYSTEMS=="scsi", ATTRS{type}=="1", ENV{ID_SCSI_SER
SYMLINK+="tape/by-id/scsi-$env{ID_SCSI_SERIAL}-sg"
LABEL="persistent_storage_tape_end"
+
+# triggers the mounting of a removable device
+ACTION=="add", SUBSYSTEM=="block", ENV{ID_FS_UUID}!="", TAG+="systemd", ENV{SYSTEMD_WANTS}="removable-device-attach@$env{ID_FS_UUID}"
\ No newline at end of file
diff --git a/etc/Makefile b/etc/Makefile
index 42f639f6..730de4f8 100644
--- a/etc/Makefile
+++ b/etc/Makefile
@@ -7,7 +7,8 @@ DYNAMIC_UNITS := \
proxmox-backup-banner.service \
proxmox-backup-daily-update.service \
proxmox-backup.service \
- proxmox-backup-proxy.service
+ proxmox-backup-proxy.service \
+ removable-device-attach@.service
all: $(UNITS) $(DYNAMIC_UNITS) pbs-enterprise.list
diff --git a/etc/removable-device-attach@.service.in b/etc/removable-device-attach@.service.in
new file mode 100644
index 00000000..fe256548
--- /dev/null
+++ b/etc/removable-device-attach@.service.in
@@ -0,0 +1,8 @@
+[Unit]
+Description=Try to mount the removable device of a datastore with uuid '%i'.
+After=proxmox-backup-proxy.service
+Requires=proxmox-backup-proxy.service
+
+[Service]
+Type=simple
+ExecStart=/usr/sbin/proxmox-backup-manager datastore uuid-mount --uuid %i
\ No newline at end of file
diff --git a/src/bin/proxmox_backup_manager/datastore.rs b/src/bin/proxmox_backup_manager/datastore.rs
index f8462459..319d5eb0 100644
--- a/src/bin/proxmox_backup_manager/datastore.rs
+++ b/src/bin/proxmox_backup_manager/datastore.rs
@@ -195,6 +195,60 @@ async fn delete_datastore(mut param: Value, rpcenv: &mut dyn RpcEnvironment) ->
Ok(())
}
+#[api(
+ protected: true,
+ input: {
+ properties: {
+ uuid: {
+ type: String,
+ description: "The UUID of the device that should be mounted",
+ },
+ "output-format": {
+ schema: OUTPUT_FORMAT,
+ optional: true,
+ },
+ },
+ },
+)]
+/// Try mounting a removable datastore given the UUID.
+async fn uuid_mount(mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
+ let output_format = extract_output_format(&mut param);
+ let uuid = param["uuid"]
+ .as_str()
+ .ok_or_else(|| format_err!("uuid has to be specified"))?;
+
+ let info = &api2::config::datastore::API_METHOD_LIST_DATASTORES;
+ let data: Value = match info.handler {
+ ApiHandler::Sync(handler) => (handler)(serde_json::json!({}), info, rpcenv)?,
+ _ => unreachable!(),
+ };
+
+ let store_name = data.as_array().and_then(|list| {
+ list.iter()
+ .filter_map(Value::as_object)
+ .find(|store| store.get("backing-device").map_or(false, |d| d.eq(&uuid)))
+ .and_then(|s| s.get("name").and_then(Value::as_str))
+ });
+
+ if let Some(store_name) = store_name {
+ let client = connect_to_localhost()?;
+ let result = client
+ .post(
+ &format!("api2/json/admin/datastore/{}/mount", store_name),
+ None,
+ )
+ .await?;
+
+ view_task_result(&client, result, &output_format).await?;
+ Ok(Value::Null)
+ } else {
+ Err(format_err!(
+ "'{}' is not associated with any datastore",
+ &uuid
+ ))
+ }
+}
+
pub fn datastore_commands() -> CommandLineInterface {
let cmd_def = CliCommandMap::new()
.insert("list", CliCommand::new(&API_METHOD_LIST_DATASTORES))
@@ -240,6 +294,7 @@ pub fn datastore_commands() -> CommandLineInterface {
pbs_config::datastore::complete_calendar_event,
),
)
+ .insert("uuid-mount", CliCommand::new(&API_METHOD_UUID_MOUNT))
.insert(
"remove",
CliCommand::new(&API_METHOD_DELETE_DATASTORE)
--
2.39.2
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 12/24] add auto-mounting for removable datastores
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 12/24] add auto-mounting for removable datastores Hannes Laimer
@ 2024-04-16 8:05 ` Christian Ebner
2024-04-16 14:39 ` Hannes Laimer
2024-04-16 8:45 ` Christian Ebner
1 sibling, 1 reply; 54+ messages in thread
From: Christian Ebner @ 2024-04-16 8:05 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
comments inline
On 4/9/24 13:00, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> debian/proxmox-backup-server.install | 1 +
> debian/proxmox-backup-server.udev | 3 ++
> etc/Makefile | 3 +-
> etc/removable-device-attach@.service.in | 8 +++
> src/bin/proxmox_backup_manager/datastore.rs | 55 +++++++++++++++++++++
> 5 files changed, 69 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 ee114ea3..cda01069 100644
> --- a/debian/proxmox-backup-server.install
> +++ b/debian/proxmox-backup-server.install
> @@ -4,6 +4,7 @@ etc/proxmox-backup-daily-update.service /lib/systemd/system/
> etc/proxmox-backup-daily-update.timer /lib/systemd/system/
> etc/proxmox-backup-proxy.service /lib/systemd/system/
> etc/proxmox-backup.service /lib/systemd/system/
> +etc/removable-device-attach@.service /lib/systemd/system/
> usr/bin/pmt
> usr/bin/pmtx
> usr/bin/proxmox-tape
> diff --git a/debian/proxmox-backup-server.udev b/debian/proxmox-backup-server.udev
> index afdfb2bc..e21b8bc7 100644
> --- a/debian/proxmox-backup-server.udev
> +++ b/debian/proxmox-backup-server.udev
> @@ -16,3 +16,6 @@ SUBSYSTEM=="scsi_generic", SUBSYSTEMS=="scsi", ATTRS{type}=="1", ENV{ID_SCSI_SER
> SYMLINK+="tape/by-id/scsi-$env{ID_SCSI_SERIAL}-sg"
>
> LABEL="persistent_storage_tape_end"
> +
> +# triggers the mounting of a removable device
> +ACTION=="add", SUBSYSTEM=="block", ENV{ID_FS_UUID}!="", TAG+="systemd", ENV{SYSTEMD_WANTS}="removable-device-attach@$env{ID_FS_UUID}"
> \ No newline at end of file
> diff --git a/etc/Makefile b/etc/Makefile
> index 42f639f6..730de4f8 100644
> --- a/etc/Makefile
> +++ b/etc/Makefile
> @@ -7,7 +7,8 @@ DYNAMIC_UNITS := \
> proxmox-backup-banner.service \
> proxmox-backup-daily-update.service \
> proxmox-backup.service \
> - proxmox-backup-proxy.service
> + proxmox-backup-proxy.service \
> + removable-device-attach@.service
>
> all: $(UNITS) $(DYNAMIC_UNITS) pbs-enterprise.list
>
> diff --git a/etc/removable-device-attach@.service.in b/etc/removable-device-attach@.service.in
> new file mode 100644
> index 00000000..fe256548
> --- /dev/null
> +++ b/etc/removable-device-attach@.service.in
> @@ -0,0 +1,8 @@
> +[Unit]
> +Description=Try to mount the removable device of a datastore with uuid '%i'.
> +After=proxmox-backup-proxy.service
> +Requires=proxmox-backup-proxy.service
> +
> +[Service]
> +Type=simple
> +ExecStart=/usr/sbin/proxmox-backup-manager datastore uuid-mount --uuid %i
> \ No newline at end of file
> diff --git a/src/bin/proxmox_backup_manager/datastore.rs b/src/bin/proxmox_backup_manager/datastore.rs
> index f8462459..319d5eb0 100644
> --- a/src/bin/proxmox_backup_manager/datastore.rs
> +++ b/src/bin/proxmox_backup_manager/datastore.rs
> @@ -195,6 +195,60 @@ async fn delete_datastore(mut param: Value, rpcenv: &mut dyn RpcEnvironment) ->
> Ok(())
> }
>
> +#[api(
> + protected: true,
> + input: {
> + properties: {
> + uuid: {
> + type: String,
> + description: "The UUID of the device that should be mounted",
> + },
> + "output-format": {
> + schema: OUTPUT_FORMAT,
> + optional: true,
> + },
> + },
> + },
> +)]
> +/// Try mounting a removable datastore given the UUID.
> +async fn uuid_mount(mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
> + let output_format = extract_output_format(&mut param);
> + let uuid = param["uuid"]
> + .as_str()
> + .ok_or_else(|| format_err!("uuid has to be specified"))?;
> +
> + let info = &api2::config::datastore::API_METHOD_LIST_DATASTORES;
> + let data: Value = match info.handler {
> + ApiHandler::Sync(handler) => (handler)(serde_json::json!({}), info, rpcenv)?,
> + _ => unreachable!(),
> + };
> +
> + let store_name = data.as_array().and_then(|list| {
> + list.iter()
> + .filter_map(Value::as_object)
> + .find(|store| store.get("backing-device").map_or(false, |d| d.eq(&uuid)))
> + .and_then(|s| s.get("name").and_then(Value::as_str))
> + });
> +
> + if let Some(store_name) = store_name {
> + let client = connect_to_localhost()?;
> + let result = client
> + .post(
> + &format!("api2/json/admin/datastore/{}/mount", store_name),
> + None,
> + )
> + .await?;
Did not test this, but could this also be done by creating a new `param`
instance containing the `store` and `node` and call it via the code below?
```
let info = &api2::admin::datastore::API_METHOD_MOUNT;
let result = match info.handler {
ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
_ => unreachable!(),
};
```
> +
> + view_task_result(&client, result, &output_format).await?;
> + Ok(Value::Null)
> + } else {
> + Err(format_err!(
> + "'{}' is not associated with any datastore",
> + &uuid
> + ))
use bail and inline uuid, removing unneeded reference
> + }
> +}
> +
> pub fn datastore_commands() -> CommandLineInterface {
> let cmd_def = CliCommandMap::new()
> .insert("list", CliCommand::new(&API_METHOD_LIST_DATASTORES))
> @@ -240,6 +294,7 @@ pub fn datastore_commands() -> CommandLineInterface {
> pbs_config::datastore::complete_calendar_event,
> ),
> )
> + .insert("uuid-mount", CliCommand::new(&API_METHOD_UUID_MOUNT))
> .insert(
> "remove",
> CliCommand::new(&API_METHOD_DELETE_DATASTORE)
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 12/24] add auto-mounting for removable datastores
2024-04-16 8:05 ` Christian Ebner
@ 2024-04-16 14:39 ` Hannes Laimer
0 siblings, 0 replies; 54+ messages in thread
From: Hannes Laimer @ 2024-04-16 14:39 UTC (permalink / raw)
To: Christian Ebner, Proxmox Backup Server development discussion
On Tue Apr 16, 2024 at 10:05 AM CEST, Christian Ebner wrote:
> comments inline
>
> On 4/9/24 13:00, Hannes Laimer wrote:
> > Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> > ---
> > debian/proxmox-backup-server.install | 1 +
> > debian/proxmox-backup-server.udev | 3 ++
> > etc/Makefile | 3 +-
> > etc/removable-device-attach@.service.in | 8 +++
> > src/bin/proxmox_backup_manager/datastore.rs | 55 +++++++++++++++++++++
> > 5 files changed, 69 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 ee114ea3..cda01069 100644
> > --- a/debian/proxmox-backup-server.install
> > +++ b/debian/proxmox-backup-server.install
> > @@ -4,6 +4,7 @@ etc/proxmox-backup-daily-update.service /lib/systemd/system/
> > etc/proxmox-backup-daily-update.timer /lib/systemd/system/
> > etc/proxmox-backup-proxy.service /lib/systemd/system/
> > etc/proxmox-backup.service /lib/systemd/system/
> > +etc/removable-device-attach@.service /lib/systemd/system/
> > usr/bin/pmt
> > usr/bin/pmtx
> > usr/bin/proxmox-tape
> > diff --git a/debian/proxmox-backup-server.udev b/debian/proxmox-backup-server.udev
> > index afdfb2bc..e21b8bc7 100644
> > --- a/debian/proxmox-backup-server.udev
> > +++ b/debian/proxmox-backup-server.udev
> > @@ -16,3 +16,6 @@ SUBSYSTEM=="scsi_generic", SUBSYSTEMS=="scsi", ATTRS{type}=="1", ENV{ID_SCSI_SER
> > SYMLINK+="tape/by-id/scsi-$env{ID_SCSI_SERIAL}-sg"
> >
> > LABEL="persistent_storage_tape_end"
> > +
> > +# triggers the mounting of a removable device
> > +ACTION=="add", SUBSYSTEM=="block", ENV{ID_FS_UUID}!="", TAG+="systemd", ENV{SYSTEMD_WANTS}="removable-device-attach@$env{ID_FS_UUID}"
> > \ No newline at end of file
> > diff --git a/etc/Makefile b/etc/Makefile
> > index 42f639f6..730de4f8 100644
> > --- a/etc/Makefile
> > +++ b/etc/Makefile
> > @@ -7,7 +7,8 @@ DYNAMIC_UNITS := \
> > proxmox-backup-banner.service \
> > proxmox-backup-daily-update.service \
> > proxmox-backup.service \
> > - proxmox-backup-proxy.service
> > + proxmox-backup-proxy.service \
> > + removable-device-attach@.service
> >
> > all: $(UNITS) $(DYNAMIC_UNITS) pbs-enterprise.list
> >
> > diff --git a/etc/removable-device-attach@.service.in b/etc/removable-device-attach@.service.in
> > new file mode 100644
> > index 00000000..fe256548
> > --- /dev/null
> > +++ b/etc/removable-device-attach@.service.in
> > @@ -0,0 +1,8 @@
> > +[Unit]
> > +Description=Try to mount the removable device of a datastore with uuid '%i'.
> > +After=proxmox-backup-proxy.service
> > +Requires=proxmox-backup-proxy.service
> > +
> > +[Service]
> > +Type=simple
> > +ExecStart=/usr/sbin/proxmox-backup-manager datastore uuid-mount --uuid %i
> > \ No newline at end of file
> > diff --git a/src/bin/proxmox_backup_manager/datastore.rs b/src/bin/proxmox_backup_manager/datastore.rs
> > index f8462459..319d5eb0 100644
> > --- a/src/bin/proxmox_backup_manager/datastore.rs
> > +++ b/src/bin/proxmox_backup_manager/datastore.rs
> > @@ -195,6 +195,60 @@ async fn delete_datastore(mut param: Value, rpcenv: &mut dyn RpcEnvironment) ->
> > Ok(())
> > }
> >
> > +#[api(
> > + protected: true,
> > + input: {
> > + properties: {
> > + uuid: {
> > + type: String,
> > + description: "The UUID of the device that should be mounted",
> > + },
> > + "output-format": {
> > + schema: OUTPUT_FORMAT,
> > + optional: true,
> > + },
> > + },
> > + },
> > +)]
> > +/// Try mounting a removable datastore given the UUID.
> > +async fn uuid_mount(mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
> > + let output_format = extract_output_format(&mut param);
> > + let uuid = param["uuid"]
> > + .as_str()
> > + .ok_or_else(|| format_err!("uuid has to be specified"))?;
> > +
> > + let info = &api2::config::datastore::API_METHOD_LIST_DATASTORES;
> > + let data: Value = match info.handler {
> > + ApiHandler::Sync(handler) => (handler)(serde_json::json!({}), info, rpcenv)?,
> > + _ => unreachable!(),
> > + };
> > +
> > + let store_name = data.as_array().and_then(|list| {
> > + list.iter()
> > + .filter_map(Value::as_object)
> > + .find(|store| store.get("backing-device").map_or(false, |d| d.eq(&uuid)))
> > + .and_then(|s| s.get("name").and_then(Value::as_str))
> > + });
> > +
> > + if let Some(store_name) = store_name {
> > + let client = connect_to_localhost()?;
> > + let result = client
> > + .post(
> > + &format!("api2/json/admin/datastore/{}/mount", store_name),
> > + None,
> > + )
> > + .await?;
>
> Did not test this, but could this also be done by creating a new `param`
> instance containing the `store` and `node` and call it via the code below?
> ```
> let info = &api2::admin::datastore::API_METHOD_MOUNT;
> let result = match info.handler {
> ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
> _ => unreachable!(),
> };
> ```
>
makes sense, will do in v4
> > +
> > + view_task_result(&client, result, &output_format).await?;
> > + Ok(Value::Null)
> > + } else {
> > + Err(format_err!(
> > + "'{}' is not associated with any datastore",
> > + &uuid
> > + ))
>
> use bail and inline uuid, removing unneeded reference
>
> > + }
> > +}
> > +
> > pub fn datastore_commands() -> CommandLineInterface {
> > let cmd_def = CliCommandMap::new()
> > .insert("list", CliCommand::new(&API_METHOD_LIST_DATASTORES))
> > @@ -240,6 +294,7 @@ pub fn datastore_commands() -> CommandLineInterface {
> > pbs_config::datastore::complete_calendar_event,
> > ),
> > )
> > + .insert("uuid-mount", CliCommand::new(&API_METHOD_UUID_MOUNT))
> > .insert(
> > "remove",
> > CliCommand::new(&API_METHOD_DELETE_DATASTORE)
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup v3 12/24] add auto-mounting for removable datastores
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 12/24] add auto-mounting for removable datastores Hannes Laimer
2024-04-16 8:05 ` Christian Ebner
@ 2024-04-16 8:45 ` Christian Ebner
1 sibling, 0 replies; 54+ messages in thread
From: Christian Ebner @ 2024-04-16 8:45 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
some more comments inline
On 4/9/24 13:00, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> debian/proxmox-backup-server.install | 1 +
> debian/proxmox-backup-server.udev | 3 ++
> etc/Makefile | 3 +-
> etc/removable-device-attach@.service.in | 8 +++
> src/bin/proxmox_backup_manager/datastore.rs | 55 +++++++++++++++++++++
> 5 files changed, 69 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 ee114ea3..cda01069 100644
> --- a/debian/proxmox-backup-server.install
> +++ b/debian/proxmox-backup-server.install
> @@ -4,6 +4,7 @@ etc/proxmox-backup-daily-update.service /lib/systemd/system/
> etc/proxmox-backup-daily-update.timer /lib/systemd/system/
> etc/proxmox-backup-proxy.service /lib/systemd/system/
> etc/proxmox-backup.service /lib/systemd/system/
> +etc/removable-device-attach@.service /lib/systemd/system/
> usr/bin/pmt
> usr/bin/pmtx
> usr/bin/proxmox-tape
> diff --git a/debian/proxmox-backup-server.udev b/debian/proxmox-backup-server.udev
> index afdfb2bc..e21b8bc7 100644
> --- a/debian/proxmox-backup-server.udev
> +++ b/debian/proxmox-backup-server.udev
> @@ -16,3 +16,6 @@ SUBSYSTEM=="scsi_generic", SUBSYSTEMS=="scsi", ATTRS{type}=="1", ENV{ID_SCSI_SER
> SYMLINK+="tape/by-id/scsi-$env{ID_SCSI_SERIAL}-sg"
>
> LABEL="persistent_storage_tape_end"
> +
> +# triggers the mounting of a removable device
> +ACTION=="add", SUBSYSTEM=="block", ENV{ID_FS_UUID}!="", TAG+="systemd", ENV{SYSTEMD_WANTS}="removable-device-attach@$env{ID_FS_UUID}"
> \ No newline at end of file
> diff --git a/etc/Makefile b/etc/Makefile
> index 42f639f6..730de4f8 100644
> --- a/etc/Makefile
> +++ b/etc/Makefile
> @@ -7,7 +7,8 @@ DYNAMIC_UNITS := \
> proxmox-backup-banner.service \
> proxmox-backup-daily-update.service \
> proxmox-backup.service \
> - proxmox-backup-proxy.service
> + proxmox-backup-proxy.service \
> + removable-device-attach@.service
>
> all: $(UNITS) $(DYNAMIC_UNITS) pbs-enterprise.list
>
> diff --git a/etc/removable-device-attach@.service.in b/etc/removable-device-attach@.service.in
> new file mode 100644
> index 00000000..fe256548
> --- /dev/null
> +++ b/etc/removable-device-attach@.service.in
> @@ -0,0 +1,8 @@
> +[Unit]
> +Description=Try to mount the removable device of a datastore with uuid '%i'.
> +After=proxmox-backup-proxy.service
> +Requires=proxmox-backup-proxy.service
> +
> +[Service]
> +Type=simple
> +ExecStart=/usr/sbin/proxmox-backup-manager datastore uuid-mount --uuid %i
> \ No newline at end of file
> diff --git a/src/bin/proxmox_backup_manager/datastore.rs b/src/bin/proxmox_backup_manager/datastore.rs
> index f8462459..319d5eb0 100644
> --- a/src/bin/proxmox_backup_manager/datastore.rs
> +++ b/src/bin/proxmox_backup_manager/datastore.rs
> @@ -195,6 +195,60 @@ async fn delete_datastore(mut param: Value, rpcenv: &mut dyn RpcEnvironment) ->
> Ok(())
> }
>
> +#[api(
> + protected: true,
> + input: {
> + properties: {
> + uuid: {
> + type: String,
> + description: "The UUID of the device that should be mounted",
this should be verified by a schema or regex
> + },
> + "output-format": {
> + schema: OUTPUT_FORMAT,
> + optional: true,
> + },
> + },
> + },
> +)]
> +/// Try mounting a removable datastore given the UUID.
> +async fn uuid_mount(mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
> + let output_format = extract_output_format(&mut param);
> + let uuid = param["uuid"]
> + .as_str()
> + .ok_or_else(|| format_err!("uuid has to be specified"))?;
> +
> + let info = &api2::config::datastore::API_METHOD_LIST_DATASTORES;
> + let data: Value = match info.handler {
> + ApiHandler::Sync(handler) => (handler)(serde_json::json!({}), info, rpcenv)?,
> + _ => unreachable!(),
> + };
> +
> + let store_name = data.as_array().and_then(|list| {
> + list.iter()
> + .filter_map(Value::as_object)
> + .find(|store| store.get("backing-device").map_or(false, |d| d.eq(&uuid)))
> + .and_then(|s| s.get("name").and_then(Value::as_str))
> + });
> +
> + if let Some(store_name) = store_name {
> + let client = connect_to_localhost()?;
> + let result = client
> + .post(
> + &format!("api2/json/admin/datastore/{}/mount", store_name),
> + None,
> + )
> + .await?;
> +
> + view_task_result(&client, result, &output_format).await?;
> + Ok(Value::Null)
> + } else {
> + Err(format_err!(
> + "'{}' is not associated with any datastore",
> + &uuid
> + ))
> + }
> +}
> +
> pub fn datastore_commands() -> CommandLineInterface {
> let cmd_def = CliCommandMap::new()
> .insert("list", CliCommand::new(&API_METHOD_LIST_DATASTORES))
> @@ -240,6 +294,7 @@ pub fn datastore_commands() -> CommandLineInterface {
> pbs_config::datastore::complete_calendar_event,
> ),
> )
> + .insert("uuid-mount", CliCommand::new(&API_METHOD_UUID_MOUNT))
since `uuid` is required, you might want to add it as `arg_param` here,
and maybe this could get some completion callback as well.
> .insert(
> "remove",
> CliCommand::new(&API_METHOD_DELETE_DATASTORE)
^ permalink raw reply [flat|nested] 54+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v3 13/24] api: mark removable datastores as unplugged after restart
2024-04-09 10:59 [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Hannes Laimer
` (11 preceding siblings ...)
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 12/24] add auto-mounting for removable datastores Hannes Laimer
@ 2024-04-09 11:00 ` Hannes Laimer
2024-04-10 11:18 ` Dietmar Maurer
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 14/24] datastore: handle deletion of removable datastore properly Hannes Laimer
` (11 subsequent siblings)
24 siblings, 1 reply; 54+ messages in thread
From: Hannes Laimer @ 2024-04-09 11:00 UTC (permalink / raw)
To: pbs-devel
... if it is not mounted
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/bin/proxmox-backup-api.rs | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/src/bin/proxmox-backup-api.rs b/src/bin/proxmox-backup-api.rs
index e46557a0..50f31d4e 100644
--- a/src/bin/proxmox-backup-api.rs
+++ b/src/bin/proxmox-backup-api.rs
@@ -10,6 +10,8 @@ use proxmox_lang::try_block;
use proxmox_router::RpcEnvironmentType;
use proxmox_sys::fs::CreateOptions;
+use pbs_api_types::{DataStoreConfig, MaintenanceMode, MaintenanceType};
+use pbs_datastore::check_if_available;
use proxmox_rest_server::{daemon, ApiConfig, RestServer};
use proxmox_backup::auth_helpers::*;
@@ -73,6 +75,8 @@ async fn run() -> Result<(), Error> {
proxmox_backup::auth_helpers::setup_auth_context(true);
+ mark_removable_datastores_unplugged()?;
+
let backup_user = pbs_config::backup_user()?;
let mut command_sock = proxmox_rest_server::CommandSocket::new(
proxmox_rest_server::our_ctrl_sock(),
@@ -161,3 +165,17 @@ 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 check_if_available(&datastore).is_err() {
+ datastore
+ .set_maintenance_mode(Some(MaintenanceMode::new(MaintenanceType::Unplugged, None)));
+ config.set_data(&datastore.name, "datastore", &datastore)?;
+ }
+ }
+ pbs_config::datastore::save_config(&config)?;
+ Ok(())
+}
--
2.39.2
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 13/24] api: mark removable datastores as unplugged after restart
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 13/24] api: mark removable datastores as unplugged after restart Hannes Laimer
@ 2024-04-10 11:18 ` Dietmar Maurer
0 siblings, 0 replies; 54+ messages in thread
From: Dietmar Maurer @ 2024-04-10 11:18 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
> diff --git a/src/bin/proxmox-backup-api.rs b/src/bin/proxmox-backup-api.rs
> index e46557a0..50f31d4e 100644
> --- a/src/bin/proxmox-backup-api.rs
> +++ b/src/bin/proxmox-backup-api.rs
> @@ -10,6 +10,8 @@ use proxmox_lang::try_block;
> use proxmox_router::RpcEnvironmentType;
> use proxmox_sys::fs::CreateOptions;
>
> +use pbs_api_types::{DataStoreConfig, MaintenanceMode, MaintenanceType};
> +use pbs_datastore::check_if_available;
> use proxmox_rest_server::{daemon, ApiConfig, RestServer};
>
> use proxmox_backup::auth_helpers::*;
> @@ -73,6 +75,8 @@ async fn run() -> Result<(), Error> {
>
> proxmox_backup::auth_helpers::setup_auth_context(true);
>
> + mark_removable_datastores_unplugged()?;
Do we really want to fail service startup if that fails?
Maybe it is good enough to just log a warning?
Or schedule a task to do it at regular intervals?
^ permalink raw reply [flat|nested] 54+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v3 14/24] datastore: handle deletion of removable datastore properly
2024-04-09 10:59 [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Hannes Laimer
` (12 preceding siblings ...)
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 13/24] api: mark removable datastores as unplugged after restart Hannes Laimer
@ 2024-04-09 11:00 ` Hannes Laimer
2024-04-16 8:10 ` Christian Ebner
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 15/24] docs: mention maintenance mode reset when removable datastore is unplugged Hannes Laimer
` (10 subsequent siblings)
24 siblings, 1 reply; 54+ messages in thread
From: Hannes Laimer @ 2024-04-09 11:00 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
pbs-datastore/src/datastore.rs | 10 ++++++----
src/api2/config/datastore.rs | 15 +++++++++++++++
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index db47205c..1290baee 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -1481,10 +1481,12 @@ impl DataStore {
// weird, but ok
}
Err(err) if err.is_errno(nix::errno::Errno::EBUSY) => {
- task_warn!(
- worker,
- "Cannot delete datastore directory (is it a mount point?)."
- )
+ if datastore_config.backing_device.is_none() {
+ task_warn!(
+ worker,
+ "Cannot delete datastore directory (is it a mount point?)."
+ )
+ }
}
Err(err) if err.is_errno(nix::errno::Errno::ENOTEMPTY) => {
task_warn!(worker, "Datastore directory not empty, not deleting.")
diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 0194d7d4..1b34313f 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -29,9 +29,11 @@ use crate::api2::config::tape_backup_job::{delete_tape_backup_job, list_tape_bac
use crate::api2::config::verify::delete_verification_job;
use pbs_config::CachedUserInfo;
+use pbs_datastore::check_if_available;
use proxmox_rest_server::WorkerTask;
use crate::server::jobstate;
+use crate::tools::disks::unmount_by_mountpoint;
#[api(
input: {
@@ -528,6 +530,15 @@ pub async fn delete_datastore(
http_bail!(NOT_FOUND, "datastore '{}' does not exist.", name);
}
+ let store_config: DataStoreConfig = config.lookup("datastore", &name)?;
+ if destroy_data && check_if_available(&store_config).is_err() {
+ http_bail!(
+ BAD_REQUEST,
+ "can't destroy data on '{}' unless the device is plugged in",
+ name
+ );
+ }
+
if !keep_job_configs {
for job in list_verification_jobs(Some(name.clone()), Value::Null, rpcenv)? {
delete_verification_job(job.config.id, None, rpcenv)?
@@ -566,6 +577,10 @@ pub async fn delete_datastore(
// ignore errors
let _ = jobstate::remove_state_file("prune", &name);
let _ = jobstate::remove_state_file("garbage_collection", &name);
+ if destroy_data && store_config.backing_device.is_some() {
+ let _ = unmount_by_mountpoint(&store_config.path);
+ let _ = std::fs::remove_dir(&store_config.path);
+ }
if let Err(err) =
proxmox_async::runtime::block_on(crate::server::notify_datastore_removed())
--
2.39.2
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 14/24] datastore: handle deletion of removable datastore properly
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 14/24] datastore: handle deletion of removable datastore properly Hannes Laimer
@ 2024-04-16 8:10 ` Christian Ebner
0 siblings, 0 replies; 54+ messages in thread
From: Christian Ebner @ 2024-04-16 8:10 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
comments inline
On 4/9/24 13:00, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> pbs-datastore/src/datastore.rs | 10 ++++++----
> src/api2/config/datastore.rs | 15 +++++++++++++++
> 2 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
> index db47205c..1290baee 100644
> --- a/pbs-datastore/src/datastore.rs
> +++ b/pbs-datastore/src/datastore.rs
> @@ -1481,10 +1481,12 @@ impl DataStore {
> // weird, but ok
> }
> Err(err) if err.is_errno(nix::errno::Errno::EBUSY) => {
> - task_warn!(
> - worker,
> - "Cannot delete datastore directory (is it a mount point?)."
> - )
> + if datastore_config.backing_device.is_none() {
> + task_warn!(
> + worker,
> + "Cannot delete datastore directory (is it a mount point?)."
> + )
> + }
> }
> Err(err) if err.is_errno(nix::errno::Errno::ENOTEMPTY) => {
> task_warn!(worker, "Datastore directory not empty, not deleting.")
> diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
> index 0194d7d4..1b34313f 100644
> --- a/src/api2/config/datastore.rs
> +++ b/src/api2/config/datastore.rs
> @@ -29,9 +29,11 @@ use crate::api2::config::tape_backup_job::{delete_tape_backup_job, list_tape_bac
> use crate::api2::config::verify::delete_verification_job;
> use pbs_config::CachedUserInfo;
>
> +use pbs_datastore::check_if_available;
> use proxmox_rest_server::WorkerTask;
>
> use crate::server::jobstate;
> +use crate::tools::disks::unmount_by_mountpoint;
>
> #[api(
> input: {
> @@ -528,6 +530,15 @@ pub async fn delete_datastore(
> http_bail!(NOT_FOUND, "datastore '{}' does not exist.", name);
> }
>
> + let store_config: DataStoreConfig = config.lookup("datastore", &name)?;
> + if destroy_data && check_if_available(&store_config).is_err() {
> + http_bail!(
> + BAD_REQUEST,
> + "can't destroy data on '{}' unless the device is plugged in",
> + name
> + );
s/can't/cannot and inline name
> + }
> +
> if !keep_job_configs {
> for job in list_verification_jobs(Some(name.clone()), Value::Null, rpcenv)? {
> delete_verification_job(job.config.id, None, rpcenv)?
> @@ -566,6 +577,10 @@ pub async fn delete_datastore(
> // ignore errors
> let _ = jobstate::remove_state_file("prune", &name);
> let _ = jobstate::remove_state_file("garbage_collection", &name);
> + if destroy_data && store_config.backing_device.is_some() {
> + let _ = unmount_by_mountpoint(&store_config.path);
> + let _ = std::fs::remove_dir(&store_config.path);
> + }
>
> if let Err(err) =
> proxmox_async::runtime::block_on(crate::server::notify_datastore_removed())
^ permalink raw reply [flat|nested] 54+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v3 15/24] docs: mention maintenance mode reset when removable datastore is unplugged
2024-04-09 10:59 [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Hannes Laimer
` (13 preceding siblings ...)
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 14/24] datastore: handle deletion of removable datastore properly Hannes Laimer
@ 2024-04-09 11:00 ` Hannes Laimer
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 16/24] ui: add partition selector form Hannes Laimer
` (9 subsequent siblings)
24 siblings, 0 replies; 54+ messages in thread
From: Hannes Laimer @ 2024-04-09 11:00 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
docs/maintenance.rst | 2 ++
1 file changed, 2 insertions(+)
diff --git a/docs/maintenance.rst b/docs/maintenance.rst
index 6dbb6941..9f18cfdb 100644
--- a/docs/maintenance.rst
+++ b/docs/maintenance.rst
@@ -314,3 +314,5 @@ Internally Proxmox Backup Server tracks whether each datastore access is a
write or read operation, so that it can gracefully enter the respective mode,
by allowing conflicting operations that started before enabling the maintenance
mode to finish.
+
+For removable datastores the maintenance mode is reset after they are unplugged.
\ No newline at end of file
--
2.39.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pbs-devel] [PATCH proxmox-backup v3 16/24] ui: add partition selector form
2024-04-09 10:59 [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Hannes Laimer
` (14 preceding siblings ...)
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 15/24] docs: mention maintenance mode reset when removable datastore is unplugged Hannes Laimer
@ 2024-04-09 11:00 ` Hannes Laimer
2024-04-16 8:57 ` Christian Ebner
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 17/24] ui: add removable datastore creation support Hannes Laimer
` (8 subsequent siblings)
24 siblings, 1 reply; 54+ messages in thread
From: Hannes Laimer @ 2024-04-09 11:00 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
www/Makefile | 1 +
www/form/PartitionSelector.js | 59 +++++++++++++++++++++++++++++++++++
2 files changed, 60 insertions(+)
create mode 100644 www/form/PartitionSelector.js
diff --git a/www/Makefile b/www/Makefile
index 79cb4c04..762d90c5 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -49,6 +49,7 @@ JSSRC= \
form/NamespaceMaxDepth.js \
form/CalendarEvent.js \
form/PermissionPathSelector.js \
+ form/PartitionSelector.js \
form/GroupSelector.js \
form/GroupFilter.js \
form/VerifyOutdatedAfter.js \
diff --git a/www/form/PartitionSelector.js b/www/form/PartitionSelector.js
new file mode 100644
index 00000000..64e7990a
--- /dev/null
+++ b/www/form/PartitionSelector.js
@@ -0,0 +1,59 @@
+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&only-unused=1",
+ reader: {
+ transform: (rawData) => rawData.data
+ .flatMap(disk => (disk.partitions ?? [])
+ .filter(partition => partition.used === 'filesystem')),
+ },
+ },
+ idProperty: 'devpath',
+
+});
+
+Ext.define('PBS.form.PartitionSelector', {
+ extend: 'Proxmox.form.ComboGrid',
+ alias: 'widget.pbsPartitionSelector',
+
+ allowBlank: false,
+ autoSelect: false,
+ valueField: 'uuid',
+ displayField: 'devpath',
+
+ 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,
+ },
+ ],
+ viewConfig: {
+ emptyText: 'No partitions present',
+ },
+ },
+});
--
2.39.2
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 16/24] ui: add partition selector form
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 16/24] ui: add partition selector form Hannes Laimer
@ 2024-04-16 8:57 ` Christian Ebner
0 siblings, 0 replies; 54+ messages in thread
From: Christian Ebner @ 2024-04-16 8:57 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
a comment inline
On 4/9/24 13:00, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> www/Makefile | 1 +
> www/form/PartitionSelector.js | 59 +++++++++++++++++++++++++++++++++++
> 2 files changed, 60 insertions(+)
> create mode 100644 www/form/PartitionSelector.js
>
> diff --git a/www/Makefile b/www/Makefile
> index 79cb4c04..762d90c5 100644
> --- a/www/Makefile
> +++ b/www/Makefile
> @@ -49,6 +49,7 @@ JSSRC= \
> form/NamespaceMaxDepth.js \
> form/CalendarEvent.js \
> form/PermissionPathSelector.js \
> + form/PartitionSelector.js \
> form/GroupSelector.js \
> form/GroupFilter.js \
> form/VerifyOutdatedAfter.js \
> diff --git a/www/form/PartitionSelector.js b/www/form/PartitionSelector.js
> new file mode 100644
> index 00000000..64e7990a
> --- /dev/null
> +++ b/www/form/PartitionSelector.js
> @@ -0,0 +1,59 @@
> +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&only-unused=1",
without setting the `skipsmart` flag you fetch also the smart stats, not
used however, so you might set this.
> + reader: {
> + transform: (rawData) => rawData.data
> + .flatMap(disk => (disk.partitions ?? [])
> + .filter(partition => partition.used === 'filesystem')),
> + },
> + },
> + idProperty: 'devpath',
> +
> +});
> +
> +Ext.define('PBS.form.PartitionSelector', {
> + extend: 'Proxmox.form.ComboGrid',
> + alias: 'widget.pbsPartitionSelector',
> +
> + allowBlank: false,
> + autoSelect: false,
> + valueField: 'uuid',
> + displayField: 'devpath',
> +
> + 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,
> + },
> + ],
> + viewConfig: {
> + emptyText: 'No partitions present',
> + },
> + },
> +});
^ permalink raw reply [flat|nested] 54+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v3 17/24] ui: add removable datastore creation support
2024-04-09 10:59 [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Hannes Laimer
` (15 preceding siblings ...)
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 16/24] ui: add partition selector form Hannes Laimer
@ 2024-04-09 11:00 ` Hannes Laimer
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 18/24] ui: add (un)mount button to summary Hannes Laimer
` (7 subsequent siblings)
24 siblings, 0 replies; 54+ messages in thread
From: Hannes Laimer @ 2024-04-09 11:00 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
www/window/DataStoreEdit.js | 51 +++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/www/window/DataStoreEdit.js b/www/window/DataStoreEdit.js
index aecf6b8d..9114cf4f 100644
--- a/www/window/DataStoreEdit.js
+++ b/www/window/DataStoreEdit.js
@@ -59,6 +59,31 @@ Ext.define('PBS.DataStoreEdit', {
fieldLabel: gettext('Backing Path'),
emptyText: gettext('An absolute path'),
},
+ {
+ xtype: 'pmxDisplayEditField',
+ fieldLabel: gettext('Device'),
+ name: 'backing-device',
+ hidden: true,
+ cbind: {
+ editable: '{isCreate}',
+ }, editConfig: {
+ xtype: 'pbsPartitionSelector',
+ allowBlank: true,
+ },
+ emptyText: gettext('Device path'),
+ listeners: {
+ change(field, newValue, oldValue) {
+ const form = field.up('form');
+ const pathField = form.down('[name=path]');
+
+ if (newValue) {
+ pathField.setValue(`/mnt/removable_datastore/${newValue}`);
+ } else if (oldValue) {
+ pathField.setValue('');
+ }
+ },
+ },
+ },
],
column2: [
{
@@ -84,6 +109,32 @@ Ext.define('PBS.DataStoreEdit', {
},
],
columnB: [
+ {
+ xtype: 'checkbox',
+ boxLabel: gettext('Removable datastore'),
+ submitValue: false,
+ listeners: {
+ change: function(checkbox, isRemovable) {
+ let inputPanel = checkbox.up('inputpanel');
+ let pathField = inputPanel.down('[name=path]');
+ let uuidField = inputPanel.down('[name=backing-device]');
+
+ pathField.setValue('');
+ uuidField.setValue('');
+
+ pathField.allowBlank = isRemovable;
+ uuidField.allowBlank = !isRemovable;
+
+ if (isRemovable) {
+ pathField.hide();
+ uuidField.show();
+ } else {
+ pathField.show();
+ uuidField.hide();
+ }
+ },
+ },
+ },
{
xtype: 'textfield',
name: 'comment',
--
2.39.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pbs-devel] [PATCH proxmox-backup v3 18/24] ui: add (un)mount button to summary
2024-04-09 10:59 [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Hannes Laimer
` (16 preceding siblings ...)
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 17/24] ui: add removable datastore creation support Hannes Laimer
@ 2024-04-09 11:00 ` Hannes Laimer
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 19/24] ui: display removable datastores in list Hannes Laimer
` (6 subsequent siblings)
24 siblings, 0 replies; 54+ messages in thread
From: Hannes Laimer @ 2024-04-09 11:00 UTC (permalink / raw)
To: pbs-devel
And only try to load datastore information if the datastore is
available.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
www/datastore/Summary.js | 86 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 84 insertions(+), 2 deletions(-)
diff --git a/www/datastore/Summary.js b/www/datastore/Summary.js
index a932b4e0..0bb59dcd 100644
--- a/www/datastore/Summary.js
+++ b/www/datastore/Summary.js
@@ -309,7 +309,76 @@ Ext.define('PBS.DataStoreSummary', {
model: 'pve-rrd-datastore',
});
- me.callParent();
+ me.statusStore = Ext.create('Proxmox.data.ObjectStore', {
+ url: `/api2/json/admin/datastore/${me.datastore}/status`,
+ interval: 1000,
+ });
+
+ let unmountBtn = Ext.create('Ext.Button', {
+ text: gettext('Unmount'),
+ hidden: true,
+ handler: () => {
+ Proxmox.Utils.API2Request({
+ url: `/admin/datastore/${me.datastore}/unmount`,
+ method: 'POST',
+ failure: function(response) {
+ Ext.Msg.alert(gettext('Error'), response.htmlStatus);
+ },
+ success: function(response, options) {
+ Ext.create('Proxmox.window.TaskViewer', {
+ upid: response.result.data,
+ }).show();
+ },
+ });
+ },
+ });
+
+ let mountBtn = Ext.create('Ext.Button', {
+ text: gettext('Mount'),
+ hidden: true,
+ handler: () => {
+ Proxmox.Utils.API2Request({
+ url: `/admin/datastore/${me.datastore}/mount`,
+ method: 'POST',
+ failure: function(response) {
+ Ext.Msg.alert(gettext('Error'), response.htmlStatus);
+ },
+ success: function(response, options) {
+ Ext.create('Proxmox.window.TaskViewer', {
+ upid: response.result.data,
+ }).show();
+ },
+ });
+ },
+ });
+
+ Ext.apply(me, {
+ tbar: [unmountBtn, mountBtn, '->', { xtype: 'proxmoxRRDTypeSelector' }],
+ });
+
+ me.mon(me.statusStore, 'load', (s, records, success) => {
+ if (!success) {
+ me.down('pbsDataStoreInfo').fireEvent('deactivate');
+ Proxmox.Utils.API2Request({
+ url: `/config/datastore/${me.datastore}`,
+ success: response => {
+ if (!response.result.data['backing-device']) {
+ return;
+ }
+ let maintenanceString = response.result.data['maintenance-mode'];
+ let [type, _msg] = PBS.Utils.parseMaintenanceMode(maintenanceString);
+ let isUnplugged = type === 'unplugged';
+
+ unmountBtn.setDisabled(isUnplugged);
+ mountBtn.setDisabled(!isUnplugged);
+ },
+ });
+ } else {
+ me.down('pbsDataStoreInfo').fireEvent('activate');
+ unmountBtn.setDisabled(false);
+ mountBtn.setDisabled(true);
+ }
+ });
let sp = Ext.state.Manager.getProvider();
me.mon(sp, 'statechange', function(provider, key, value) {
@@ -322,11 +391,17 @@ Ext.define('PBS.DataStoreSummary', {
Proxmox.Utils.updateColumns(me);
});
+ me.callParent();
+
Proxmox.Utils.API2Request({
url: `/config/datastore/${me.datastore}`,
waitMsgTarget: me.down('pbsDataStoreInfo'),
success: function(response) {
- let path = Ext.htmlEncode(response.result.data.path);
+ let data = response.result.data;
+ let path = Ext.htmlEncode(data.path);
+ const removable = Object.prototype.hasOwnProperty.call(data, "backing-device");
+ unmountBtn.setHidden(!removable);
+ mountBtn.setHidden(!removable);
me.down('pbsDataStoreInfo').setTitle(`${me.datastore} (${path})`);
me.down('pbsDataStoreNotes').setNotes(response.result.data.comment);
},
@@ -344,6 +419,13 @@ Ext.define('PBS.DataStoreSummary', {
let hasIoTicks = records?.some((rec) => rec?.data?.io_ticks !== undefined);
me.down('#ioDelayChart').setVisible(!success || hasIoTicks);
}, undefined, { single: true });
+ me.on('afterrender', () => {
+ me.statusStore.startUpdate();
+ });
+
+ me.on('destroy', () => {
+ me.statusStore.stopUpdate();
+ });
me.query('proxmoxRRDChart').forEach((chart) => {
chart.setStore(me.rrdstore);
--
2.39.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pbs-devel] [PATCH proxmox-backup v3 19/24] ui: display removable datastores in list
2024-04-09 10:59 [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Hannes Laimer
` (17 preceding siblings ...)
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 18/24] ui: add (un)mount button to summary Hannes Laimer
@ 2024-04-09 11:00 ` Hannes Laimer
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 20/24] ui: utils: render unplugged maintenance mode correctly Hannes Laimer
` (5 subsequent siblings)
24 siblings, 0 replies; 54+ messages in thread
From: Hannes Laimer @ 2024-04-09 11:00 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
www/NavigationTree.js | 10 +++++++---
www/css/ext6-pbs.css | 8 ++++++++
www/datastore/DataStoreListSummary.js | 1 +
3 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/www/NavigationTree.js b/www/NavigationTree.js
index 3e0040f7..9ce93799 100644
--- a/www/NavigationTree.js
+++ b/www/NavigationTree.js
@@ -261,13 +261,17 @@ Ext.define('PBS.view.main.NavigationTree', {
j++;
}
- let [qtip, iconCls] = ['', 'fa fa-database'];
+ let mainIcon = `fa fa-${records[i].data.removable ? 'plug' : 'database'}`;
+ let [qtip, iconCls] = ['', mainIcon];
const maintenance = records[i].data.maintenance;
if (maintenance) {
const [type, message] = PBS.Utils.parseMaintenanceMode(maintenance);
qtip = `${type}${message ? ': ' + message : ''}`;
- let mainenanceTypeCls = type === 'delete' ? 'destroying' : 'maintenance';
- iconCls = `fa fa-database pmx-tree-icon-custom ${mainenanceTypeCls}`;
+ const maintenanceTypeCls = {
+ 'delete': 'destroying',
+ 'unplugged': 'unplugged',
+ }[type] ?? 'maintenance';
+ iconCls = `${mainIcon} pmx-tree-icon-custom ${maintenanceTypeCls}`;
}
if (getChildTextAt(j).localeCompare(name) !== 0) {
diff --git a/www/css/ext6-pbs.css b/www/css/ext6-pbs.css
index c33ce684..706e681e 100644
--- a/www/css/ext6-pbs.css
+++ b/www/css/ext6-pbs.css
@@ -271,6 +271,10 @@ span.snapshot-comment-column {
content: "\ ";
}
+.x-treelist-item-icon.fa-plug, .pmx-tree-icon-custom.fa-plug {
+ font-size: 12px;
+}
+
/* datastore maintenance */
.pmx-tree-icon-custom.maintenance:after {
content: "\f0ad";
@@ -290,6 +294,10 @@ span.snapshot-comment-column {
color: #888;
}
+.pmx-tree-icon-custom.unplugged:before {
+ color: #888;
+}
+
/*' PBS specific icons */
.pbs-icon-tape {
diff --git a/www/datastore/DataStoreListSummary.js b/www/datastore/DataStoreListSummary.js
index b908034d..f7ea83e7 100644
--- a/www/datastore/DataStoreListSummary.js
+++ b/www/datastore/DataStoreListSummary.js
@@ -22,6 +22,7 @@ Ext.define('PBS.datastore.DataStoreListSummary', {
stillbad: 0,
deduplication: 1.0,
error: "",
+ removable: false,
maintenance: '',
},
},
--
2.39.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pbs-devel] [PATCH proxmox-backup v3 20/24] ui: utils: render unplugged maintenance mode correctly
2024-04-09 10:59 [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Hannes Laimer
` (18 preceding siblings ...)
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 19/24] ui: display removable datastores in list Hannes Laimer
@ 2024-04-09 11:00 ` Hannes Laimer
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 21/24] ui: utils: make parseMaintenanceMode more robust Hannes Laimer
` (4 subsequent siblings)
24 siblings, 0 replies; 54+ messages in thread
From: Hannes Laimer @ 2024-04-09 11:00 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
www/Utils.js | 2 ++
1 file changed, 2 insertions(+)
diff --git a/www/Utils.js b/www/Utils.js
index 5357949b..0ee6609c 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -703,6 +703,8 @@ Ext.define('PBS.Utils', {
break;
case 'offline': modeText = gettext("Offline");
break;
+ case 'unplugged': modeText = gettext("Unplugged");
+ break;
}
return `${modeText} ${extra}`;
},
--
2.39.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pbs-devel] [PATCH proxmox-backup v3 21/24] ui: utils: make parseMaintenanceMode more robust
2024-04-09 10:59 [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Hannes Laimer
` (19 preceding siblings ...)
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 20/24] ui: utils: render unplugged maintenance mode correctly Hannes Laimer
@ 2024-04-09 11:00 ` Hannes Laimer
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 22/24] ui: add datastore status mask for unplugged removable datastores Hannes Laimer
` (3 subsequent siblings)
24 siblings, 0 replies; 54+ messages in thread
From: Hannes Laimer @ 2024-04-09 11:00 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
www/Utils.js | 29 ++++++++++++++++++++++-------
1 file changed, 22 insertions(+), 7 deletions(-)
diff --git a/www/Utils.js b/www/Utils.js
index 0ee6609c..1df46318 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -663,14 +663,29 @@ Ext.define('PBS.Utils', {
return `${icon} ${value}`;
},
- // FIXME: this "parser" is brittle and relies on the order the arguments will appear in
+ /**
+ * Parses maintenance mode property string.
+ * Examples:
+ * "offline,message=foo" -> ["offline", "foo"]
+ * "offline" -> ["offline", null]
+ * "message=foo,offline" -> ["offline", "foo"]
+ * null/undefined -> [null, null]
+ *
+ * @param {string|null} mode - Maintenance mode string to parse.
+ * @return {Array<string|null>} - Parsed maintenance mode values.
+ */
parseMaintenanceMode: function(mode) {
- let [type, message] = mode.split(/,(.+)/);
- type = type.split("=").pop();
- message = message ? message.split("=")[1]
- .replace(/^"(.*)"$/, '$1')
- .replaceAll('\\"', '"') : null;
- return [type, message];
+ if (!mode) {
+ return [null, null];
+ }
+ return mode.split(',').reduce(([m, msg], pair) => {
+ const [key, value] = pair.split('=');
+ if (key === 'message') {
+ return [m, value.replace(/^"(.*)"$/, '$1').replace(/\\"/g, '"')];
+ } else {
+ return [value ?? key, msg];
+ }
+ }, [null, null]);
},
renderMaintenance: function(mode, activeTasks) {
--
2.39.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pbs-devel] [PATCH proxmox-backup v3 22/24] ui: add datastore status mask for unplugged removable datastores
2024-04-09 10:59 [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Hannes Laimer
` (20 preceding siblings ...)
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 21/24] ui: utils: make parseMaintenanceMode more robust Hannes Laimer
@ 2024-04-09 11:00 ` Hannes Laimer
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 23/24] ui: maintenance: fix disable msg field if no type is selected Hannes Laimer
` (2 subsequent siblings)
24 siblings, 0 replies; 54+ messages in thread
From: Hannes Laimer @ 2024-04-09 11:00 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 | 20 ++++++++++++--------
2 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/www/css/ext6-pbs.css b/www/css/ext6-pbs.css
index 706e681e..891189ae 100644
--- a/www/css/ext6-pbs.css
+++ b/www/css/ext6-pbs.css
@@ -261,6 +261,18 @@ span.snapshot-comment-column {
content: "\f0ad";
}
+.pbs-unplugged-mask div.x-mask-msg-text {
+ background: None;
+ padding: 12px 0 0;
+}
+
+.pbs-unplugged-mask:before {
+ font-size: 3em;
+ display: flex;
+ justify-content: center;
+ content: "\f1e6";
+}
+
/* the small icons TODO move to proxmox-widget-toolkit */
.pmx-tree-icon-custom:after {
position: relative;
diff --git a/www/datastore/Summary.js b/www/datastore/Summary.js
index 0bb59dcd..1479ab41 100644
--- a/www/datastore/Summary.js
+++ b/www/datastore/Summary.js
@@ -61,16 +61,20 @@ Ext.define('PBS.DataStoreInfo', {
Proxmox.Utils.API2Request({
url: `/config/datastore/${me.view.datastore}`,
success: function(response) {
- const config = response.result.data;
- if (config['maintenance-mode']) {
- const [_type, msg] = PBS.Utils.parseMaintenanceMode(config['maintenance-mode']);
- me.view.el.mask(
- `${gettext('Datastore is in maintenance mode')}${msg ? ': ' + msg : ''}`,
- 'fa pbs-maintenance-mask',
- );
- } else {
+ let maintenanceString = response.result.data['maintenance-mode'];
+ if (!maintenanceString) {
me.view.el.mask(gettext('Datastore is not available'));
+ return;
}
+
+ let [type, msg] = PBS.Utils.parseMaintenanceMode(maintenanceString);
+ let isUnplugged = type === 'unplugged';
+ let maskMessage = isUnplugged
+ ? gettext('Datastore is unplugged')
+ : `${gettext('Datastore is in maintenance mode')}${msg ? ': ' + msg : ''}`;
+
+ let maskIcon = isUnplugged ? 'fa pbs-unplugged-mask' : 'fa pbs-maintenance-mask';
+ me.view.el.mask(maskMessage, maskIcon);
},
});
return;
--
2.39.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pbs-devel] [PATCH proxmox-backup v3 23/24] ui: maintenance: fix disable msg field if no type is selected
2024-04-09 10:59 [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Hannes Laimer
` (21 preceding siblings ...)
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 22/24] ui: add datastore status mask for unplugged removable datastores Hannes Laimer
@ 2024-04-09 11:00 ` Hannes Laimer
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 24/24] ui: maintenance: disable edit if unplugged Hannes Laimer
2024-04-16 9:37 ` [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Christian Ebner
24 siblings, 0 replies; 54+ messages in thread
From: Hannes Laimer @ 2024-04-09 11:00 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
www/window/MaintenanceOptions.js | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/www/window/MaintenanceOptions.js b/www/window/MaintenanceOptions.js
index 1ee92542..527c3698 100644
--- a/www/window/MaintenanceOptions.js
+++ b/www/window/MaintenanceOptions.js
@@ -56,12 +56,17 @@ Ext.define('PBS.window.MaintenanceOptions', {
fieldLabel: gettext('Maintenance Type'),
value: '__default__',
deleteEmpty: true,
+ listeners: {
+ change: (field, newValue) => {
+ Ext.getCmp('message-field').setDisabled(newValue === '__default__');
+ },
+ },
},
{
xtype: 'proxmoxtextfield',
+ id: 'message-field',
name: 'maintenance-msg',
fieldLabel: gettext('Description'),
- // FIXME: disable if maintenance type is none
},
],
},
--
2.39.2
^ permalink raw reply [flat|nested] 54+ messages in thread* [pbs-devel] [PATCH proxmox-backup v3 24/24] ui: maintenance: disable edit if unplugged
2024-04-09 10:59 [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Hannes Laimer
` (22 preceding siblings ...)
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 23/24] ui: maintenance: fix disable msg field if no type is selected Hannes Laimer
@ 2024-04-09 11:00 ` Hannes Laimer
2024-04-16 9:37 ` [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Christian Ebner
24 siblings, 0 replies; 54+ messages in thread
From: Hannes Laimer @ 2024-04-09 11:00 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
www/window/MaintenanceOptions.js | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/www/window/MaintenanceOptions.js b/www/window/MaintenanceOptions.js
index 527c3698..401fab93 100644
--- a/www/window/MaintenanceOptions.js
+++ b/www/window/MaintenanceOptions.js
@@ -52,6 +52,7 @@ Ext.define('PBS.window.MaintenanceOptions', {
items: [
{
xtype: 'pbsMaintenanceType',
+ id: 'type-field',
name: 'maintenance-type',
fieldLabel: gettext('Maintenance Type'),
value: '__default__',
@@ -84,7 +85,15 @@ Ext.define('PBS.window.MaintenanceOptions', {
'maintenance-msg': message ?? '',
};
}
+ let unplugged = options['maintenance-type'] === 'unplugged';
+ let defaultType = options['maintenance-type'] === '__default__';
+ if (unplugged) {
+ options['maintenance-type'] = '';
+ }
me.callParent([options]);
+
+ Ext.ComponentManager.get('type-field').setDisabled(unplugged);
+ Ext.ComponentManager.get('message-field').setDisabled(unplugged || defaultType);
},
});
--
2.39.2
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores
2024-04-09 10:59 [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Hannes Laimer
` (23 preceding siblings ...)
2024-04-09 11:00 ` [pbs-devel] [PATCH proxmox-backup v3 24/24] ui: maintenance: disable edit if unplugged Hannes Laimer
@ 2024-04-16 9:37 ` Christian Ebner
2024-04-16 15:03 ` Hannes Laimer
24 siblings, 1 reply; 54+ messages in thread
From: Christian Ebner @ 2024-04-16 9:37 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Hannes Laimer
Hi,
had a look a this patch series (mostly at the Rust code, did not feel
competent enough to comment to much on the ExtJS side).
All in all this looks nice and adds a feature requested by quite some
users already.
I did however missed a flag to setup a datastore directly as removable
when setting it up via the disk management interface instead of going
through the datastore create dialog. This would be nice to have if not
to complex to implement.
Also, the current approach with the flag switching from the path field
to the disk selector did not feel very intuitive on first usage. Maybe
both could be present all the time and only enabled/disabled
accordingly. Would also allow to keep the path if I already gave one but
clicked on the checkbox after that.
Also, the error message when navigating to the datastores contents tab
while the store being unplugged needs rephrasing I think. Or even show
the unplugged state as in the summary instead.
If the comments are addressed, please consider this:
Tested-by: Christian Ebner <c.ebner@proxmox.com>
Reviewed-by: Christian Ebner <c.ebner@proxmox.com>
On 4/9/24 12:59, Hannes Laimer wrote:
> These patches add support for removable datastores. All removable
> datastores have a backing-device(a UUID) associated with them. Removable
> datastores work like normal ones, just that they can be unplugged. It is
> possible to create a removable datastore, sync backups onto it, unplug
> it and use it on a different PBS.
>
> The datastore path is also the mountpoint for the removable datastore.
> By default when creating one through the web UI it will be
> `/mnt/removable-datastores/<UUID>`, using the CLI it is possible to
> specify something else. Since a removable datastore is associated with
> the UUID of a partition, it is technically possible to have two
> removable datastores on the same device, but I don't think there is a
> use-case that couldn't also be done using namespaces.
>
> When a removable datastore is deleted and 'destroy-data' is set, the
> device has to be plugged in. If 'destroy-data' is not set the datastore
> can be deleted even if the device is not present. Removable datastores
> are automatically mounted when plugged in. At the API service start all
> removable datastores are marked as 'unplugged', unless they are already
> mounted.
>
> Patches 21/24 and 23/24 are not strictly needed, but they made sense in
> this context, so I kept them in this series.
>
>
> v3:
> * remove lazy unmounting (since 9cba51ac782d04085c0af55128f32178e5132358 is applied)
> * fix CLI (un)mount command, thanks @Gabriel
> * add removable datastore CLI autocomplete helper
> * rebase onto master
> * move ui patches to the end
>
> thanks @Lukas and @Thomas for the feedback
> v2:
> * fix datastore 'add' button in the UI
> * some format!("{}", a) -> format!("{a}")
> * replace `const` with `let` in js code
> * change icon `fa-usb` -> `fa-plug`
> * add some docs
> * add JDoc for parseMaintenanceMode
> * proxmox-schema dep bump
>
> Hannes Laimer (24):
> tools: add disks utility functions
> pbs-api-types: add backing-device to DataStoreConfig
> maintenance: add 'Unpplugged' maintenance type
> disks: add UUID to partition info
> add helper for checking if a removable datastore is available
> api2: admin: add (un)mount endpoint for removable datastores
> api2: removable datastore creation
> api2: disks list: add only-unused flag
> pbs-api-types: datastore: use new proxmox_scheme::de for
> deserialization
> pbs-api-types: add removable/is-available flag to DataStoreListItem
> pb-manager: add (un)mount command
> add auto-mounting for removable datastores
> api: mark removable datastores as unplugged after restart
> datastore: handle deletion of removable datastore properly
> docs: mention maintenance mode reset when removable datastore is
> unplugged
> ui: add partition selector form
> ui: add removable datastore creation support
> ui: add (un)mount button to summary
> ui: display removable datastores in list
> ui: utils: render unplugged maintenance mode correctly
> ui: utils: make parseMaintenanceMode more robust
> ui: add datastore status mask for unplugged removable datastores
> ui: maintenance: fix disable msg field if no type is selected
> ui: maintenance: disable edit if unplugged
>
> debian/proxmox-backup-server.install | 1 +
> debian/proxmox-backup-server.udev | 3 +
> docs/maintenance.rst | 2 +
> etc/Makefile | 3 +-
> etc/removable-device-attach@.service.in | 8 +
> pbs-api-types/src/datastore.rs | 40 ++++-
> pbs-api-types/src/maintenance.rs | 35 +++-
> pbs-config/src/datastore.rs | 14 ++
> pbs-datastore/src/datastore.rs | 28 ++-
> pbs-datastore/src/lib.rs | 2 +-
> src/api2/admin/datastore.rs | 190 +++++++++++++++++++-
> src/api2/config/datastore.rs | 67 ++++++-
> src/api2/node/disks/mod.rs | 8 +
> src/api2/status.rs | 18 +-
> src/bin/proxmox-backup-api.rs | 18 ++
> src/bin/proxmox_backup_manager/datastore.rs | 131 +++++++++++++-
> src/tools/disks/mod.rs | 95 ++++++++--
> www/Makefile | 1 +
> www/NavigationTree.js | 10 +-
> www/Utils.js | 31 +++-
> www/css/ext6-pbs.css | 20 +++
> www/datastore/DataStoreListSummary.js | 1 +
> www/datastore/Summary.js | 106 +++++++++--
> www/form/PartitionSelector.js | 59 ++++++
> www/window/DataStoreEdit.js | 51 ++++++
> www/window/MaintenanceOptions.js | 16 +-
> 26 files changed, 899 insertions(+), 59 deletions(-)
> create mode 100644 etc/removable-device-attach@.service.in
> create mode 100644 www/form/PartitionSelector.js
>
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores
2024-04-16 9:37 ` [pbs-devel] [PATCH proxmox-backup v3 00/24] add removable datastores Christian Ebner
@ 2024-04-16 15:03 ` Hannes Laimer
0 siblings, 0 replies; 54+ messages in thread
From: Hannes Laimer @ 2024-04-16 15:03 UTC (permalink / raw)
To: Christian Ebner, Proxmox Backup Server development discussion
On Tue Apr 16, 2024 at 11:37 AM CEST, Christian Ebner wrote:
> Hi,
>
> had a look a this patch series (mostly at the Rust code, did not feel
> competent enough to comment to much on the ExtJS side).
>
> All in all this looks nice and adds a feature requested by quite some
> users already.
> I did however missed a flag to setup a datastore directly as removable
> when setting it up via the disk management interface instead of going
> through the datastore create dialog. This would be nice to have if not
> to complex to implement.
yes, and shouldn't be too complicated it should mostly be UI stuff, but won't be
in v4.
> Also, the current approach with the flag switching from the path field
> to the disk selector did not feel very intuitive on first usage. Maybe
> both could be present all the time and only enabled/disabled
> accordingly. Would also allow to keep the path if I already gave one but
> clicked on the checkbox after that.
the path is generated based on the selected partition, but sure, could
still make sense to have both fields visible.
> Also, the error message when navigating to the datastores contents tab
> while the store being unplugged needs rephrasing I think. Or even show
> the unplugged state as in the summary instead.
>
> If the comments are addressed, please consider this:
>
> Tested-by: Christian Ebner <c.ebner@proxmox.com>
> Reviewed-by: Christian Ebner <c.ebner@proxmox.com>
>
>
Thanks a lot for looking over the series! v4 will be structured a little
different but I'll include still relevant (which are almost all)
suggeststions.
> On 4/9/24 12:59, Hannes Laimer wrote:
> > These patches add support for removable datastores. All removable
> > datastores have a backing-device(a UUID) associated with them. Removable
> > datastores work like normal ones, just that they can be unplugged. It is
> > possible to create a removable datastore, sync backups onto it, unplug
> > it and use it on a different PBS.
> >
> > The datastore path is also the mountpoint for the removable datastore.
> > By default when creating one through the web UI it will be
> > `/mnt/removable-datastores/<UUID>`, using the CLI it is possible to
> > specify something else. Since a removable datastore is associated with
> > the UUID of a partition, it is technically possible to have two
> > removable datastores on the same device, but I don't think there is a
> > use-case that couldn't also be done using namespaces.
> >
> > When a removable datastore is deleted and 'destroy-data' is set, the
> > device has to be plugged in. If 'destroy-data' is not set the datastore
> > can be deleted even if the device is not present. Removable datastores
> > are automatically mounted when plugged in. At the API service start all
> > removable datastores are marked as 'unplugged', unless they are already
> > mounted.
> >
> > Patches 21/24 and 23/24 are not strictly needed, but they made sense in
> > this context, so I kept them in this series.
> >
> >
> > v3:
> > * remove lazy unmounting (since 9cba51ac782d04085c0af55128f32178e5132358 is applied)
> > * fix CLI (un)mount command, thanks @Gabriel
> > * add removable datastore CLI autocomplete helper
> > * rebase onto master
> > * move ui patches to the end
> >
> > thanks @Lukas and @Thomas for the feedback
> > v2:
> > * fix datastore 'add' button in the UI
> > * some format!("{}", a) -> format!("{a}")
> > * replace `const` with `let` in js code
> > * change icon `fa-usb` -> `fa-plug`
> > * add some docs
> > * add JDoc for parseMaintenanceMode
> > * proxmox-schema dep bump
> >
> > Hannes Laimer (24):
> > tools: add disks utility functions
> > pbs-api-types: add backing-device to DataStoreConfig
> > maintenance: add 'Unpplugged' maintenance type
> > disks: add UUID to partition info
> > add helper for checking if a removable datastore is available
> > api2: admin: add (un)mount endpoint for removable datastores
> > api2: removable datastore creation
> > api2: disks list: add only-unused flag
> > pbs-api-types: datastore: use new proxmox_scheme::de for
> > deserialization
> > pbs-api-types: add removable/is-available flag to DataStoreListItem
> > pb-manager: add (un)mount command
> > add auto-mounting for removable datastores
> > api: mark removable datastores as unplugged after restart
> > datastore: handle deletion of removable datastore properly
> > docs: mention maintenance mode reset when removable datastore is
> > unplugged
> > ui: add partition selector form
> > ui: add removable datastore creation support
> > ui: add (un)mount button to summary
> > ui: display removable datastores in list
> > ui: utils: render unplugged maintenance mode correctly
> > ui: utils: make parseMaintenanceMode more robust
> > ui: add datastore status mask for unplugged removable datastores
> > ui: maintenance: fix disable msg field if no type is selected
> > ui: maintenance: disable edit if unplugged
> >
> > debian/proxmox-backup-server.install | 1 +
> > debian/proxmox-backup-server.udev | 3 +
> > docs/maintenance.rst | 2 +
> > etc/Makefile | 3 +-
> > etc/removable-device-attach@.service.in | 8 +
> > pbs-api-types/src/datastore.rs | 40 ++++-
> > pbs-api-types/src/maintenance.rs | 35 +++-
> > pbs-config/src/datastore.rs | 14 ++
> > pbs-datastore/src/datastore.rs | 28 ++-
> > pbs-datastore/src/lib.rs | 2 +-
> > src/api2/admin/datastore.rs | 190 +++++++++++++++++++-
> > src/api2/config/datastore.rs | 67 ++++++-
> > src/api2/node/disks/mod.rs | 8 +
> > src/api2/status.rs | 18 +-
> > src/bin/proxmox-backup-api.rs | 18 ++
> > src/bin/proxmox_backup_manager/datastore.rs | 131 +++++++++++++-
> > src/tools/disks/mod.rs | 95 ++++++++--
> > www/Makefile | 1 +
> > www/NavigationTree.js | 10 +-
> > www/Utils.js | 31 +++-
> > www/css/ext6-pbs.css | 20 +++
> > www/datastore/DataStoreListSummary.js | 1 +
> > www/datastore/Summary.js | 106 +++++++++--
> > www/form/PartitionSelector.js | 59 ++++++
> > www/window/DataStoreEdit.js | 51 ++++++
> > www/window/MaintenanceOptions.js | 16 +-
> > 26 files changed, 899 insertions(+), 59 deletions(-)
> > create mode 100644 etc/removable-device-attach@.service.in
> > create mode 100644 www/form/PartitionSelector.js
> >
^ permalink raw reply [flat|nested] 54+ messages in thread