Re: [pbs-devel] [PATCH v3 proxmox-backup 4/5] api2: make maintenance_type updatable

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

On 08.11.21 17:46, Hannes Laimer wrote:
> And check if conflicting operations are currently active on the
> datastore.

with this approach it will never work on an active server as there always
will be some write or read operation going on..

You need to record the maintenance mode immediately and start blocking new,
conflicting operations also immediately. This can be handled in two ways:

* add a extra "transition" entry to the maintenance mode types, that would
  require active checking every once in a while if the active operation queue
  is empty and update the transition mode-value to the actual, final one.

* keep the transition info an internal state, i.e., if
  + maintenance mode not set in cfg -> no maintenance mode
  + maintenance mode set but active OPs -> transitioning (block new but to not
    allow actions that require a maintenance mode to be actually finished)
  + maintenance mode and no active OPs -> maintenance mode applied

I'd prefer the second, its less work and we do not loose actually functionality..

> ---
>  src/api2/config/datastore.rs | 42 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
> index b9367469..cf16ce02 100644
> --- a/src/api2/config/datastore.rs
> +++ b/src/api2/config/datastore.rs
> @@ -1,3 +1,5 @@
> +use std::fs::File;
> +use std::io::Read;
>  use std::path::PathBuf;
>  
>  use anyhow::{bail, Error};
> @@ -11,7 +13,7 @@ use proxmox_section_config::SectionConfigData;
>  use pbs_datastore::chunk_store::ChunkStore;
>  use pbs_config::BackupLockGuard;
>  use pbs_api_types::{
> -    Authid, DatastoreNotify,
> +    Authid, MaintenanceType, DatastoreNotify,
>      DATASTORE_SCHEMA, PROXMOX_CONFIG_DIGEST_SCHEMA,
>      PRIV_DATASTORE_ALLOCATE, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_MODIFY,
>      DataStoreConfig, DataStoreConfigUpdater,
> @@ -183,6 +185,8 @@ pub enum DeletableProperty {
>      notify_user,
>      /// Delete the notify property
>      notify,
> +    /// Delete the maintenance-type property
> +    maintenance_type,
>  }
>  
>  #[api(
> @@ -249,6 +253,7 @@ pub fn update_datastore(
>                  DeletableProperty::verify_new => { data.verify_new = None; },
>                  DeletableProperty::notify => { data.notify = None; },
>                  DeletableProperty::notify_user => { data.notify_user = None; },
> +                DeletableProperty::maintenance_type => { data.maintenance_type = None; },
>              }
>          }
>      }
> @@ -294,6 +299,41 @@ pub fn update_datastore(
>  
>      if update.notify_user.is_some() { data.notify_user = update.notify_user; }
>  
> +    if let Some(maintenance_type) = update.maintenance_type {
> +        let (reading, writing) = {
> +            let mut path = PathBuf::from(pbs_datastore::ACTIVE_OPERATIONS_DIR);
> +            path.push(&name);
> +            if let Ok(mut file) = File::open(path) {
> +                let timeout = std::time::Duration::new(10, 0);
> +                proxmox::tools::fs::lock_file(&mut file, false, Some(timeout))?;
> +
> +                let mut data = String::new();
> +                file.read_to_string(&mut data)?;
> +                if data.is_empty() {
> +                    (0, 0)
> +                } else {
> +                    let tmp = data.split(" ").collect::<Vec<&str>>();
> +                    (
> +                        tmp[0].parse::<i64>().unwrap_or(0),
> +                        tmp[1].parse::<i64>().unwrap_or(0),
> +                    )
> +                }
> +            } else {
> +                (0, 0)
> +            }
> +        };
> +
> +        match maintenance_type {
> +            MaintenanceType::ReadOnly(_) if writing > 0 => {
> +                bail!("Can't enable maintenance mode because {} conflicting operations are still active.", writing);
> +            }
> +            MaintenanceType::Offline(_) if writing + reading > 0 => {
> +                bail!("Can't enable maintenance mode because {} conflicting operations are still active.", (writing+reading));
> +            }
> +            _ => data.maintenance_type = Some(maintenance_type),
> +        }
> +    }
> +
>      config.set_data(&name, "datastore", &data)?;
>  
>      pbs_config::datastore::save_config(&config)?;
>