Re: [pve-devel] [PATCH pve-guest-common 1/1] Add abstract methods for pre/post-migrate hooks

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

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 <s.hanreich@proxmox.com>
>>> ---
>>>   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).