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 3C92191348 for ; Tue, 27 Sep 2022 09:40:25 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 266EE1BC25 for ; Tue, 27 Sep 2022 09:40:25 +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 09:40:24 +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 41E26445A9 for ; Tue, 27 Sep 2022 09:40:24 +0200 (CEST) Message-ID: Date: Tue, 27 Sep 2022 09:40:23 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.13.0 Content-Language: en-US To: Thomas Lamprecht , Proxmox VE development discussion References: <20220922141321.1510795-1-s.hanreich@proxmox.com> From: Stefan Hanreich In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 1.813 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 Subject: Re: [pve-devel] [PATCH pve-container/qemu-server/pve-guest-common/pve-docs 0/1] Add 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 07:40:25 -0000 On 9/26/22 17:51, Thomas Lamprecht wrote: > Am 22/09/2022 um 16:13 schrieb Stefan Hanreich: >> I have decided to create distinct event types for source/target nodes, since >> otherwise the same script would run essentially twice on the source/target node. >> With distinct event types, the hooks should be more flexible in their usage. > > just make that a parameter, same flexibility but less cmd explosion and complexity. > > Also, _iff_ (see reply we keep the CLI entries for pct/qm it should just be a single command > there, any difference should be handled in the parameters; it's internal after all > and we want to avoid that there's more internal commands then externals someday ;) > > Target and source should be part of the parameters on either call (pre/post, src/target), > it is relevant info and should be easily available. Some param info like offline/online > migration could be relevant too, but we can always extend on that, so in that regard it > can be fine to stop smaller, to avoid going over board and having to keep all that info > for backward compat. Any parameter would need to be encoded in the example then. > This is also an option I explored. One thing that I wasn't sure about was where the scripts run then? Does the pre event run on the source node and the post event on the target node? Dominik made an interesting point, that it might actually be desirable the other way around since you might want to do some setup code in the pre-hook, which would be nice on the target node. It might also be nice to run some cleanup code on the post-event which would be more suited to running on the source node. Do you think it would be smart to implement it as positional parameters to the script? Like 'qm pre-migrate ' ? Since there are already ideas of adding additional contextual information, might it be smarter to expose all the additional info to the script in a dictionary? Not sure about this, but I could see us ending up with a situation where you have many additional variables only accessible by knowing their indexes. This has other downsides of course.. > Some more general note, the example is better than nothing, but a nice list/table > directly in the docs would be really good to have. This could be done upfront, before > adding new hooks - best for now to duplicate it for both CT and VM chapter (if sensible > it can live in its own guest-hook-list.adoc and just get included twice). Including > the example script as an appendix would be a nice touch too. I looked at the documentation as well and found it a bit lacking, I thought it would be nice to overhaul this in an additional patch series, after all the hooks are merged. I figured it might be okay to silently add these features and document them afterwards in a subsequent patch. I will add a short documentation section for each hook to the documentation in the respective patches as well and then we can maybe overhaul/unify them afterwards.