From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 3E7B77D63B for ; Tue, 9 Nov 2021 09:06:12 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 2B3A82EF15 for ; Tue, 9 Nov 2021 09:05:42 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 3F52F2EF06 for ; Tue, 9 Nov 2021 09:05:41 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 0B2E842326 for ; Tue, 9 Nov 2021 09:05:41 +0100 (CET) Message-ID: Date: Tue, 9 Nov 2021 09:05:40 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:95.0) Gecko/20100101 Thunderbird/95.0 Content-Language: en-US To: Proxmox Backup Server development discussion , Hannes Laimer References: <20211108164655.25913-1-h.laimer@proxmox.com> <20211108164655.25913-5-h.laimer@proxmox.com> From: Thomas Lamprecht In-Reply-To: <20211108164655.25913-5-h.laimer@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 1.681 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -3.06 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [datastore.rs] Subject: Re: [pbs-devel] [PATCH v3 proxmox-backup 4/5] api2: make maintenance_type updatable X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 09 Nov 2021 08:06:12 -0000 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::>(); > + ( > + tmp[0].parse::().unwrap_or(0), > + tmp[1].parse::().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)?; >