From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <t.lamprecht@proxmox.com> 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) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id E04B269CC9 for <pbs-devel@lists.proxmox.com>; Mon, 14 Mar 2022 15:54:26 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id CF1005BCB for <pbs-devel@lists.proxmox.com>; Mon, 14 Mar 2022 15:53:56 +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 208345BC0 for <pbs-devel@lists.proxmox.com>; Mon, 14 Mar 2022 15:53:56 +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 DFCC9427F1 for <pbs-devel@lists.proxmox.com>; Mon, 14 Mar 2022 15:53:55 +0100 (CET) Message-ID: <5ca8cb1b-f677-67d0-a4c5-052de38472b2@proxmox.com> Date: Mon, 14 Mar 2022 15:53:54 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:99.0) Gecko/20100101 Thunderbird/99.0 Content-Language: en-US To: Stefan Sterz <s.sterz@proxmox.com>, Wolfgang Bumiller <w.bumiller@proxmox.com> Cc: Proxmox Backup Server development discussion <pbs-devel@lists.proxmox.com> References: <20220309135031.1995207-1-s.sterz@proxmox.com> <717c8999-d3f8-a01b-a8f5-da0f5960d23f@proxmox.com> <20220314093617.n2mc2jv4k6ntzroo@wobu-vie.proxmox.com> <e5ddc12e-dc3a-6a5d-f1b4-242e118db85e@proxmox.com> <738d037f-ed3c-db76-287f-5b6d37a3b7f3@proxmox.com> <fc7591f1-e070-a25e-d811-afb438360d17@proxmox.com> <f16d7d3e-3c55-a48e-6c20-564c27d49854@proxmox.com> From: Thomas Lamprecht <t.lamprecht@proxmox.com> In-Reply-To: <f16d7d3e-3c55-a48e-6c20-564c27d49854@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.059 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 -0.001 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 T_SCC_BODY_TEXT_LINE -0.01 - Subject: Re: [pbs-devel] [PATCH proxmox-backup] fix #3336: api: remove backup group if the last snapshot is removed X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion <pbs-devel.lists.proxmox.com> List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe> List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/> List-Post: <mailto:pbs-devel@lists.proxmox.com> List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help> List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe> X-List-Received-Date: Mon, 14 Mar 2022 14:54:26 -0000 On 14.03.22 15:18, Stefan Sterz wrote: > On 14.03.22 12:36, Thomas Lamprecht wrote: >> On the other hand, we also handle creation in a similar implicit matter, >> so maybe I'm overthinking it and just removing it would actually be more >> consistent/expected for users. >> >> So, if you don't see a problem/issue with that approach and agree with >> the last paragraph above feel free to go for deleting the owner file only. > > for the most part i agree with you. i would also like to point out > that when a group is deleted (as in, not the last snapshot, but the > entire group at once) the owner is also implicitly removed (because > the entire group directory is removed). so in a way, we already delete > ownership information implicitly and the proposed solution would just > be consistent with that behavior. I really do not think that's comparable or would count as implicit deletion ;-) A user triggering a whole-group removal explicitly expects that all the associated stuff gets removed too, including owner + group directory, there's nothing to own after that. Iow., the difference would be like: `rm -rf group-dir/` vs. `rm -rf group-dir/snapshot-dir` > > however, i did some more digging and testing and it turns out that we > currently assume the owner file to be present when a group directory > exists. this affects not only sync jobs, but also verification and > more. thus, i would need to do quite a bit of refactoring to get this > to work and even more testing. so while this issue seemed simple > enough, as far as i can tell our current options are: > > 1. re-factor locking and remove the directory > 2. re-factor how an empty group directory and the owner file is > treated meh, not really liking this one as it could conflict with some assumptions. > 3. add "empty" groups to the gui Thinking more of it with past users-behavior in mind, I'd be surprised if we then would get the bug report for not auto-removing this in one step ^^ > > in light of this, taking the gui route is possibly the easiest option. > sorry, for not being aware of this earlier. I mean, the locking problem Wolfgang pointed out already exists currently, meaning that we either could: 1. stay ignorant (for now) and just delete the directory 2. fixing that up-front already as it has its own merits I don't see 1. as _that_ problematic as the deletion of the last snapshot always has to be a manual action, (auto)pruning will never cause that. This would allow the assumption that the user/admin already took care of periodic backup jobs before cleaning up stuff. But yeah, definitively has a slight sour taste. Putting this on hold and see how we can best improve the locking w.r.t. to full backup-dir removals would IMO be the cleanest solution.