Re: [pve-devel] [PATCH qemu-server v2] copy conntrack information on migration

[Mira Limbeck <m.limbeck@proxmox.com>]

On 2/5/21 9:11 AM, Fabian Ebner wrote:
> Am 03.02.21 um 15:25 schrieb Mira Limbeck:
>> Requires the pve-conntrack-tool. On migration the conntrack information
>> from the source node is dumped and sent to the target node where it is
>> then inserted.
>> This helps with open connections during migration when the firewall 
>> is active.
>>
>> A new 'migrate-conntracks' option is added to the migrate_vm API call.
>>
>> Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
>> ---
>> v2:
>>   - added the migrate-conntracks option so that it only copies conntrack
>>     information when requested
>>
>>   PVE/API2/Qemu.pm   | 5 +++++
>>   PVE/QemuMigrate.pm | 5 +++++
>>   2 files changed, 10 insertions(+)
>>
>> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
>> index 3571f5e..8c4336b 100644
>> --- a/PVE/API2/Qemu.pm
>> +++ b/PVE/API2/Qemu.pm
>> @@ -3556,6 +3556,11 @@ __PACKAGE__->register_method({
>>           minimum => '0',
>>           default => 'migrate limit from datacenter or storage config',
>>           },
>> +        'migrate-conntracks' => {
>> +        description => "Migrate connection tracking info.",
>> +        type => 'boolean',
>> +        optional => 1,
>> +        }
>>       },
>>       },
>>       returns => {
>> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
>> index 5c019fc..2ccef2a 100644
>> --- a/PVE/QemuMigrate.pm
>> +++ b/PVE/QemuMigrate.pm
>> @@ -1087,6 +1087,11 @@ sub phase2 {
>>           die "unable to parse migration status '$stat->{status}' - 
>> aborting\n";
>>       }
>>       }
>> +
>> +    if ($self->{opts}->{'migrate-conntracks'}) {
>> +    $self->log('info', 'copy conntrack information');
>> +    PVE::Tools::run_command([['/usr/bin/pve-conntrack-tool', 
>> 'dump'], [@{$self->{rem_ssh}}, '/usr/bin/pve-conntrack-tool', 
>> 'insert']]);
>
> Just wondering if having an eval here and still continue with the 
> migration if there's a problem would make sense? OTOH one can argue 
> that if the option is set and the connection information cannot be 
> copied, it's better to abort.
>
I'm in favor of aborting, but perhaps eval and die on error with a 
custom error message?
>> +    }
>>   }
>>     sub phase2_cleanup {
>>