Re: [pve-devel] [PATCH v3 qemu-server 09/10] migrate: add remote migration handling

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

On 04.01.22 17:44, Roland wrote:
>>>   +sub phase2_start_remote_cluster {
>>> +    my ($self, $vmid, $params) = @_;
>>> +
>>> +    die "insecure migration to remote cluster not implemented\n"
>>> +    if $params->{migrate_opts}->{type} ne 'websocket';
>>> +
>>> +    my $remote_vmid = $self->{opts}->{remote}->{vmid};
>>> +
>>> +    my $res = PVE::Tunnel::write_tunnel($self->{tunnel}, 10,
>>> "start", $params);
>>
>> 10 seconds feels a bit short to me.

@Fabian(s):

Why not use the same as vm_start_nolock + some buffer? E.g.,

($params->{timeout} // config_aware_timeout($conf, $resume)) + 10;

That should be quite generous, as the worst case time for the start for
incoming migration, which is always paused so not doing much, is normally
way lower than a cold-start.

I mean, we're in an worker task and do not really care much if setup needs
a bit longer.

>>
> Please, administrators like tunables and knobs for changing default values.
> 

@Roland

sure, but exposing all hundreds+ of timeout and other parameters will just
overload most users and produce guides to extend timeouts for issues that
should be actually fixed in the setup, i.e., when the timeout is just a symptom
of a bad config/hw/...

> Not only for being empowered to fix things themselves but also to be
> able to dig into a problem and find the root cause...
> 

With PVE et. al being under AGPLv3 you're already empowered to just change the
code, so lets keep it simple(r) to most users, if a timeout is actually to short
we can always change it to a better fitting value (at least if reports about that
reach us).

> I remember that i had more then one occasion , where i grepped for
> timeout or other limiting values in proxmox or other softwares source, 
> and often gave up in the end, because it was too deeply hidden or i got
> too many hits/findings.
> 

same would happen if we expose every possible parameter as knob, you'd have
hundreds and get many hits on searches..

> Finding such without knowing the code can often be like searching for
> the needle in a haystack and extremely frustrating.

sure and I can imagine that it's frustrating, but you can always ask here, on
pve-user or other channels about the issue at hand and people more used to
the code can often better guide you to the actual parameter location, or give
some other input.

> 
> I would be happy, if such important values would get defined with some
> descriptive variable name at a suitable location, maybe even with some
> comment what's it all about ( even if it's not meant to be changed/tuned)
> 

everybody sees specific knobs as more or less important, so that's not a a clear
cut at all. For comments I'd also suggest using git blame to get the commit
(message) that introduced it, often there's some info encoded in there too.
Also, if you ever got some reasoning into a special timeout that had no info
or comment whatsoever we're naturally happy to accept patches that add one.

> just my 10 cents...

thanks for your input. IMO this specific one can be improved without exposing
a new tunable (see start of my reply), and in general I prefer to avoid to many
tunables, we can still add them if it shows that there's quite some demand for
a specific one, while removing would break compat..

cheers,
Thomas