all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH qemu-server 1/2] config: net: avoid duplicate ipam entries on nic update
@ 2024-07-17 13:06 Stefan Hanreich
  2024-07-17 13:06 ` [pve-devel] [PATCH pve-manager 2/2] sdn: ipam: fix editing custom mappings Stefan Hanreich
  2024-07-22 16:39 ` [pve-devel] applied: [PATCH qemu-server 1/2] config: net: avoid duplicate ipam entries on nic update Thomas Lamprecht
  0 siblings, 2 replies; 4+ messages in thread
From: Stefan Hanreich @ 2024-07-17 13:06 UTC (permalink / raw)
  To: pve-devel

Updating the NIC of a VM when the following conditions were met:
* VM is turned off
* NIC is on a bridge that uses automatic dhcp
* Leave bridge unchanged

led to duplicate IPAM entries for the same network device.

This is due to the fact that the add_next_free_cidr always ran on
applying pending network changes.

Now we only add a new ipam entry if either:
* the value of the bridge or mac address changed
* the network device has been newly added

This way no duplicate IPAM entries should get created.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 PVE/QemuServer.pm | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 88c274d..bf59b09 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5254,10 +5254,11 @@ sub vmconfig_apply_pending {
 			safe_string_ne($old_net->{macaddr}, $new_net->{macaddr})
 		    )) {
 			PVE::Network::SDN::Vnets::del_ips_from_mac($old_net->{bridge}, $old_net->{macaddr}, $conf->{name});
+			PVE::Network::SDN::Vnets::add_next_free_cidr($new_net->{bridge}, $conf->{name}, $new_net->{macaddr}, $vmid, undef, 1);
 		    }
+		} else {
+		    PVE::Network::SDN::Vnets::add_next_free_cidr($new_net->{bridge}, $conf->{name}, $new_net->{macaddr}, $vmid, undef, 1);
 		}
-		#fixme: reuse ip if mac change && same bridge
-		PVE::Network::SDN::Vnets::add_next_free_cidr($new_net->{bridge}, $conf->{name}, $new_net->{macaddr}, $vmid, undef, 1);
 	    }
 	};
 	if (my $err = $@) {
-- 
2.39.2


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [pve-devel] [PATCH pve-manager 2/2] sdn: ipam: fix editing custom mappings
  2024-07-17 13:06 [pve-devel] [PATCH qemu-server 1/2] config: net: avoid duplicate ipam entries on nic update Stefan Hanreich
@ 2024-07-17 13:06 ` Stefan Hanreich
  2024-07-22 16:39   ` [pve-devel] applied: " Thomas Lamprecht
  2024-07-22 16:39 ` [pve-devel] applied: [PATCH qemu-server 1/2] config: net: avoid duplicate ipam entries on nic update Thomas Lamprecht
  1 sibling, 1 reply; 4+ messages in thread
From: Stefan Hanreich @ 2024-07-17 13:06 UTC (permalink / raw)
  To: pve-devel

Currently custom mappings cannot be edited, due to them having no VMID
value. The VMID parameter was always sent by the frontend to the
update call - even if it was empty - leading to validation failure on
the backend. Fix this by only sending the vmid parameter when it is
actually set.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 www/manager6/tree/DhcpTree.js | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/www/manager6/tree/DhcpTree.js b/www/manager6/tree/DhcpTree.js
index d0b80803..6868bb60 100644
--- a/www/manager6/tree/DhcpTree.js
+++ b/www/manager6/tree/DhcpTree.js
@@ -156,15 +156,20 @@ Ext.define('PVE.sdn.DhcpTree', {
 	openEditWindow: function(data) {
 	    let me = this;
 
+	    let extraRequestParams = {
+		mac: data.mac,
+		zone: data.zone,
+		vnet: data.vnet,
+	    };
+
+	    if (data.vmid) {
+		extraRequestParams.vmid = data.vmid;
+	    }
+
 	    Ext.create('PVE.sdn.IpamEdit', {
 		autoShow: true,
 		mapping: data,
-		extraRequestParams: {
-		    vmid: data.vmid,
-		    mac: data.mac,
-		    zone: data.zone,
-		    vnet: data.vnet,
-		},
+		extraRequestParams,
 		listeners: {
 		    destroy: () => me.reload(),
 		},
-- 
2.39.2


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [pve-devel] applied: [PATCH qemu-server 1/2] config: net: avoid duplicate ipam entries on nic update
  2024-07-17 13:06 [pve-devel] [PATCH qemu-server 1/2] config: net: avoid duplicate ipam entries on nic update Stefan Hanreich
  2024-07-17 13:06 ` [pve-devel] [PATCH pve-manager 2/2] sdn: ipam: fix editing custom mappings Stefan Hanreich
@ 2024-07-22 16:39 ` Thomas Lamprecht
  1 sibling, 0 replies; 4+ messages in thread
From: Thomas Lamprecht @ 2024-07-22 16:39 UTC (permalink / raw)
  To: Proxmox VE development discussion, Stefan Hanreich

Am 17/07/2024 um 15:06 schrieb Stefan Hanreich:
> Updating the NIC of a VM when the following conditions were met:
> * VM is turned off
> * NIC is on a bridge that uses automatic dhcp
> * Leave bridge unchanged
> 
> led to duplicate IPAM entries for the same network device.
> 
> This is due to the fact that the add_next_free_cidr always ran on
> applying pending network changes.
> 
> Now we only add a new ipam entry if either:
> * the value of the bridge or mac address changed
> * the network device has been newly added
> 
> This way no duplicate IPAM entries should get created.
> 
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>  PVE/QemuServer.pm | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
>

applied, thanks!


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [pve-devel] applied: [PATCH pve-manager 2/2] sdn: ipam: fix editing custom mappings
  2024-07-17 13:06 ` [pve-devel] [PATCH pve-manager 2/2] sdn: ipam: fix editing custom mappings Stefan Hanreich
@ 2024-07-22 16:39   ` Thomas Lamprecht
  0 siblings, 0 replies; 4+ messages in thread
From: Thomas Lamprecht @ 2024-07-22 16:39 UTC (permalink / raw)
  To: Proxmox VE development discussion, Stefan Hanreich

Am 17/07/2024 um 15:06 schrieb Stefan Hanreich:
> Currently custom mappings cannot be edited, due to them having no VMID
> value. The VMID parameter was always sent by the frontend to the
> update call - even if it was empty - leading to validation failure on
> the backend. Fix this by only sending the vmid parameter when it is
> actually set.
> 
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>  www/manager6/tree/DhcpTree.js | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
>

applied, thanks!


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-07-22 16:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-17 13:06 [pve-devel] [PATCH qemu-server 1/2] config: net: avoid duplicate ipam entries on nic update Stefan Hanreich
2024-07-17 13:06 ` [pve-devel] [PATCH pve-manager 2/2] sdn: ipam: fix editing custom mappings Stefan Hanreich
2024-07-22 16:39   ` [pve-devel] applied: " Thomas Lamprecht
2024-07-22 16:39 ` [pve-devel] applied: [PATCH qemu-server 1/2] config: net: avoid duplicate ipam entries on nic update Thomas Lamprecht

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal