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 4D06791366 for ; Tue, 27 Sep 2022 10:05:19 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 2C75B1BEC9 for ; Tue, 27 Sep 2022 10:05:19 +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)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Tue, 27 Sep 2022 10:05:18 +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 3B4D6445B9 for ; Tue, 27 Sep 2022 10:05:12 +0200 (CEST) Message-ID: Date: Tue, 27 Sep 2022 10:05:11 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:106.0) Gecko/20100101 Thunderbird/106.0 Content-Language: en-GB To: Stefan Hanreich , Proxmox VE development discussion References: <20220922141321.1510795-1-s.hanreich@proxmox.com> <20220922141321.1510795-2-s.hanreich@proxmox.com> <953a9847-4560-495f-6c2f-62f2b1696793@proxmox.com> <33c05af8-058d-6056-7d9d-9b731b4de58b@proxmox.com> From: Thomas Lamprecht In-Reply-To: <33c05af8-058d-6056-7d9d-9b731b4de58b@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 1.131 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 -2.319 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. [abstractmigrate.pm] Subject: Re: [pve-devel] [PATCH pve-guest-common 1/1] Add abstract methods for pre/post-migrate hooks X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 27 Sep 2022 08:05:19 -0000 Am 27/09/2022 um 09:40 schrieb Stefan Hanreich: > On 9/26/22 17:27, Thomas Lamprecht wrote: >> Am 22/09/2022 um 16:13 schrieb Stefan Hanreich: >>> Signed-off-by: Stefan Hanreich >>> --- >>> src/PVE/AbstractMigrate.pm | 14 ++++++++++++++ >>> 1 file changed, 14 insertions(+) >>> >> >> for the record, if we do it like this (not much rationale given in the >> commit message) this breaks containers and qemu-server without such an >> implementation and needs the respective Breaks/Depends entries in d/control >> (which you cannot add as you cannot predict the exact version this would get >> actually added). >> > > I figured this might pose some problems when releasing. Is there any way I > can work around this (also for future patches)? Or do we have to bump all 3 > packages at once? Anything I should change in particular in this case (if we > stick to having the hooks in the common package..) The only way to avoid that is to add such features in such a way that old dependency consumers (qemu-server and pve-container in this case) can simply ignore their existence (see below for how to do it in this case). Note that this may not be always the thing one actually wants, sometimes it is the cleanest way to break old dependants and and change them and just encode this as breaks of old consumer packages in the provider package's d/control, and as depends on new provider package in the consumer's d/control. This is itself really not a problem, it's just a bit extra work and adds some sort of a "dependency synchronization" boundary, making it much harder for users to downgrade a single package below said boundary after, e.g., getting regressions there with an upgrade; so one needs to weigh each specific feature/bug fix with that in mind. In general, new opt-in features (like here) are more likely to get away without a full break/depends while avoiding much extra code to handle that. > > Do you think it might be smarter to move the hooks into the respective > backend? It shouldn't be too much of a hassle. Maybe it is a smarter idea > after all, since it allows for more fine-grained control of when the hooks > should be run exactly. If you keep them here you could simply drop the die in the base method, then you simply require a one way versioned depenency bump in the consumer packages for the provider one, and even that for a higher level assurance that the hooks are actually called (as code wise nothing would break).