[pve-devel] [PATCH pve-container/qemu-server/pve-guest-common/pve-docs 0/1] Add pre/post-migrate hooks

[Stefan Hanreich]

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 <target> <source>' ? 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.