Re: [pve-devel] [PATCH V3 qemu-server 2/3] vm_start/vm_resume : add_nets_bridge_fdb

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

Am 24/08/2022 um 18:26 schrieb Alexandre Derumier:
> on vm start (no live migration), we can simply add mac address in fdb.
> In case of a live migration, we add the mac address just before the resume.
> 
> Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> ---
>  PVE/QemuServer.pm | 25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 0114d06..6d71006 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -5797,6 +5797,7 @@ sub vm_start_nolock {
>  	    my $nicconf = parse_net($conf->{$opt});
>  	    qemu_set_link_status($vmid, $opt, 0) if $nicconf->{link_down};
>  	}
> +	add_nets_bridge_fdb($conf, $vmid);
>      }
>  
>      mon_cmd($vmid, 'qom-set',
> @@ -6155,6 +6156,7 @@ sub vm_resume {
>  	my $res = mon_cmd($vmid, 'query-status');
>  	my $resume_cmd = 'cont';
>  	my $reset = 0;
> +	my $conf = PVE::QemuConfig->load_config($vmid);
>  
>  	if ($res->{status}) {
>  	    return if $res->{status} eq 'running'; # job done, go home
> @@ -6164,8 +6166,6 @@ sub vm_resume {
>  
>  	if (!$nocheck) {
>  
> -	    my $conf = PVE::QemuConfig->load_config($vmid);
> -
>  	    PVE::QemuConfig->check_lock($conf)
>  		if !($skiplock || PVE::QemuConfig->has_lock($conf, 'backup'));
>  	}
> @@ -6175,6 +6175,9 @@ sub vm_resume {
>  	    # request before the backup finishes for example
>  	    mon_cmd($vmid, "system_reset");
>  	}
> +
> +	add_nets_bridge_fdb($conf, $vmid) if $resume_cmd eq 'cont';
> +
>  	mon_cmd($vmid, $resume_cmd);
>      });
>  }
> @@ -8237,4 +8240,22 @@ sub check_volume_storage_type {
>      return 1;
>  }
>  
> +sub add_nets_bridge_fdb {
> +    my ($conf, $vmid) = @_;
> +
> +    foreach my $opt (keys %$conf) {
> +	if ($opt =~  m/^net(\d+)$/) {
> +	    my $net = parse_net($conf->{$opt});
> +	    next if !$net;
> +	    next if !$net->{macaddr};

note that parse_net *always* checks for a $net->{macaddr} itself and auto_generates
one if not present:
https://git.proxmox.com/?p=qemu-server.git;a=blob;f=PVE/QemuServer.pm;h=2376bf46439e300ec5e0191f3bd9552510e91467;hb=HEAD#l1943

So this will never call next and register potentially a random mac address that then
isn't used by the VM.

I think it could be better to change this method to not get the full config but the
already parsed network entry, as then we can better ensure that the MAC address is
actually the one we'll use.

We'd then need to keep track of such auto-generated ones so that we got that available
when we actually need to add the entry, or read that out again from the QEMU command
line opts (or maybe its available via QMP?)

> +
> +	    my $iface = "tap${vmid}i$1";
> +	    if ($have_sdn) {
> +		PVE::Network::SDN::Zones::add_bridge_fdb($iface, $net->{macaddr}, $net->{bridge}, $net->{firewall});
> +	    } else {
> +		PVE::Network::add_bridge_fdb($iface, $net->{macaddr}, $net->{firewall});
> +	    }
> +	}
> +    }
> +}
>  1;