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 5EDDD7105E for ; Fri, 1 Oct 2021 10:18:04 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 549C928B2F for ; Fri, 1 Oct 2021 10:18:04 +0200 (CEST) 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 81EDD28B20 for ; Fri, 1 Oct 2021 10:18:02 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 4D1A3453C0; Fri, 1 Oct 2021 10:18:02 +0200 (CEST) Message-ID: <0a1f9d39-b658-5265-e59c-ca86376a61a5@proxmox.com> Date: Fri, 1 Oct 2021 10:18:01 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:93.0) Gecko/20100101 Thunderbird/93.0 Content-Language: en-US To: Proxmox Backup Server development discussion , Hannes Laimer References: <20210928100548.4873-1-h.laimer@proxmox.com> From: Dominik Csapak In-Reply-To: <20210928100548.4873-1-h.laimer@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 2.084 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.499 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 Subject: Re: [pbs-devel] [PATCH proxmox-backup 0/5] maintenance mode for datastore 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: Fri, 01 Oct 2021 08:18:04 -0000 high level comment here, rest in the individual patches following i like the idea, but i find some things a bit clunky: * what gets saved is the maintenance mode (which is okay) but the interface to check it is confusing since the user has to give the mode that is *not* enough. IMHO it would be better if the check would require the least mode it requires instead. this could either be a "none" (or "writable") mode of the enum, or making the parameter an Option where None represents needing write access. * the check requiring an addition call to the storage seems like a thing one can easily forget. instead we could make our intent clear on the lookup part, which could do the check for us. if we then need elevated access, we can still have the check function. this way we cannot even request a datastore if its in offline mode anywhere in the code. i know this would require more code changes possibly, but i think this would provide a safer interface for this feature