On Sat, 16 Mar 2024 at 01:51, Abhiram Tilak <atp.exp@gmail.com> wrote:

The current way of updating a network configuration uses `virsh
net-update` to add, delete or modify entries. But with such a mechansim
one should know if an entry with current info already exists. Adding
modify-or-add option automatically performs either modify or add
depending on the current state.

Fixes: https://gitlab.com/libvirt/libvirt/-/issues/363
Signed-off-by: Abhiram Tilak <atp.exp@gmail.com>
---
docs/manpages/virsh.rst | 5 +-
include/libvirt/libvirt-network.h | 2 +
src/conf/network_conf.c | 148 ++++++++++++++++++++++++------
tools/virsh-network.c | 4 +-
4 files changed, 126 insertions(+), 33 deletions(-)

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 115b802c45..dc91ba895c 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -5908,7 +5908,10 @@ changes optionally taking effect immediately, without needing to
destroy and re-start the network.

*command* is one of "add-first", "add-last", "add" (a synonym for
-add-last), "delete", or "modify".
+add-last), "delete", "modify", "modify-or-add" (modify + add-last),
+"modify-or-add-first". The 'modify-or-add' commands perform modify or
+add operation depending on the given state, and can be useful for
+scripting.

*section* is one of "bridge", "domain", "ip", "ip-dhcp-host",
"ip-dhcp-range", "forward", "forward-interface", "forward-pf",
diff --git a/include/libvirt/libvirt-network.h b/include/libvirt/libvirt-network.h
index 58591be7ac..a6e132f407 100644
--- a/include/libvirt/libvirt-network.h
+++ b/include/libvirt/libvirt-network.h
@@ -181,6 +181,8 @@ typedef enum {
VIR_NETWORK_UPDATE_COMMAND_DELETE = 2, /* delete an existing element (Since: 0.10.2) */
VIR_NETWORK_UPDATE_COMMAND_ADD_LAST = 3, /* add an element at end of list (Since: 0.10.2) */
VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST = 4, /* add an element at start of list (Since: 0.10.2) */
+ VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_LAST = 5, /* if exists modify or add an element at end of list (Since: 0.10.2) */
+ VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST = 6, /* if exists modify or add an element at start of list (Since: 0.10.2) */
# ifdef VIR_ENUM_SENTINELS
VIR_NETWORK_UPDATE_COMMAND_LAST /* (Since: 0.10.2) */
# endif
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index cc92ed0b03..2835395385 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -2721,6 +2721,9 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDef *def,
virNetworkDHCPHostDef host = { 0 };
bool partialOkay = (command == VIR_NETWORK_UPDATE_COMMAND_DELETE);

+ /* added for modify-or-add feature */
+ bool modified = false;
+
if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "host") < 0)
goto cleanup;

@@ -2826,7 +2829,34 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDef *def,
virNetworkDHCPHostDefClear(&ipdef->hosts[i]);
VIR_DELETE_ELEMENT(ipdef->hosts, i, ipdef->nhosts);

- } else {
+ } else if ((command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_LAST) ||
+ (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST)) {
+
+ /* find entries with matching name/address/ip */
+ for (i = 0; i < ipdef->nhosts; i++) {
+ if ((host.mac && ipdef->hosts[i].mac &&
+ !virMacAddrCompare(host.mac, ipdef->hosts[i].mac)) ||
+ (host.name &&
+ STREQ_NULLABLE(host.name, ipdef->hosts[i].name)) ||
+ (VIR_SOCKET_ADDR_VALID(&host.ip) &&
+ virSocketAddrEqual(&host.ip, &ipdef->hosts[i].ip))) {
+
+ modified = true;
+ break;
+ }
+ }
+
+ /* if element is found then modify, or else add to beginning/end of list */
+ if (modified) {
+ virNetworkDHCPHostDefClear(&ipdef->hosts[i]);
+ ipdef->hosts[i] = host;
+ memset(&host, 0, sizeof(host));
+ } else if (VIR_INSERT_ELEMENT(ipdef->hosts,
+ command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST
+ ? 0 : ipdef->nhosts,
+ ipdef->nhosts, host) < 0)
+ goto cleanup;
+ } else {
virNetworkDefUpdateUnknownCommand(command);
goto cleanup;
}
@@ -2885,7 +2915,9 @@ virNetworkDefUpdateIPDHCPRange(virNetworkDef *def,
}

if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) ||
- (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST)) {
+ (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST) ||
+ (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_LAST) ||
+ (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST)) {

if (virNetworkDefUpdateCheckMultiDHCP(def, ipdef) < 0)
return -1;
@@ -2894,17 +2926,24 @@ virNetworkDefUpdateIPDHCPRange(virNetworkDef *def,
g_autofree char *startip = virSocketAddrFormat(&range.addr.start);
g_autofree char *endip = virSocketAddrFormat(&range.addr.end);

- virReportError(VIR_ERR_OPERATION_INVALID,
- _("there is an existing dhcp range entry in network '%1$s' that matches \"<range start='%2$s' end='%3$s'/>\""),
- def->name,
- startip ? startip : "unknown",
- endip ? endip : "unknown");
+
+ if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) ||
+ (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST))
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ _("there is an existing dhcp range entry in network '%1$s' that matches \"<range start='%2$s' end='%3$s'/>\""),
+ def->name,
+ startip ? startip : "unknown",
+ endip ? endip : "unknown");
+ else
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("dhcp ranges cannot be modified, only added or deleted"));
return -1;
}

/* add to beginning/end of list */
if (VIR_INSERT_ELEMENT(ipdef->ranges,
- command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST
+ (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST ||
+ command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST)
? 0 : ipdef->nranges,
ipdef->nranges, range) < 0)
return -1;
@@ -2981,18 +3020,26 @@ virNetworkDefUpdateForwardInterface(virNetworkDef *def,
}

if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) ||
- (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST)) {
+ (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST) ||
+ (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_LAST) ||
+ (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST)) {

if (i < def->forward.nifs) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- _("there is an existing interface entry in network '%1$s' that matches \"<interface dev='%2$s'>\""),
- def->name, iface.device.dev);
+ if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST) ||
+ command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST)
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ _("there is an existing interface entry in network '%1$s' that matches \"<interface dev='%2$s'>\""),
+ def->name, iface.device.dev);
+ else
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("forward interface entries cannot be modified, only added or deleted"));
goto cleanup;
}

/* add to beginning/end of list */
if (VIR_INSERT_ELEMENT(def->forward.ifs,
- command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST
+ (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST ||
+ command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST)
? 0 : def->forward.nifs,
def->forward.nifs, iface) < 0)
goto cleanup;
@@ -3056,6 +3103,9 @@ virNetworkDefUpdatePortGroup(virNetworkDef *def,
int ret = -1;
virPortGroupDef portgroup = { 0 };

+ /* added for modify-or-add feature */
+ bool modified = false;
+
if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "portgroup") < 0)
goto cleanup;

@@ -3097,6 +3147,17 @@ virNetworkDefUpdatePortGroup(virNetworkDef *def,
goto cleanup;
}

+ /* modify found entries for modify-or-add command */
+ if (foundName >= 0 && (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_LAST ||
+ command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST)) {
+
+ /* replace existing entry */
+ virPortGroupDefClear(&def->portGroups[foundName]);
+ def->portGroups[foundName] = portgroup;
+ memset(&portgroup, 0, sizeof(portgroup));
+ modified = true;
+ }
+
if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) {

/* replace existing entry */
@@ -3105,11 +3166,14 @@ virNetworkDefUpdatePortGroup(virNetworkDef *def,
memset(&portgroup, 0, sizeof(portgroup));

} else if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) ||
- (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST)) {
+ (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST) ||
+ (!modified && command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_LAST) ||
+ (!modified && command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST)) {

/* add to beginning/end of list */
if (VIR_INSERT_ELEMENT(def->portGroups,
- command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST
+ (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST ||
+ command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST)
? 0 : def->nPortGroups,
def->nPortGroups, portgroup) < 0)
goto cleanup;
@@ -3144,7 +3208,9 @@ virNetworkDefUpdateDNSHost(virNetworkDef *def,
virNetworkDNSDef *dns = &def->dns;
virNetworkDNSHostDef host = { 0 };
bool isAdd = (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST ||
- command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST);
+ command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST ||
+ command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST ||
+ command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_LAST);
int foundCt = 0;

if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) {
@@ -3185,15 +3251,21 @@ virNetworkDefUpdateDNSHost(virNetworkDef *def,
if (isAdd) {

if (foundCt > 0) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- _("there is already at least one DNS HOST record with a matching field in network %1$s"),
- def->name);
+ if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) ||
+ command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST)
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ _("there is already at least one DNS HOST record with a matching field in network %1$s"),
+ def->name);
+ else
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("DNS HOST records cannot be modified, only added or deleted"));
goto cleanup;
}

/* add to beginning/end of list */
if (VIR_INSERT_ELEMENT(dns->hosts,
- command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST
+ (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST ||
+ command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST)
? 0 : dns->nhosts, dns->nhosts, host) < 0)
goto cleanup;
} else if (command == VIR_NETWORK_UPDATE_COMMAND_DELETE) {
@@ -3240,7 +3312,9 @@ virNetworkDefUpdateDNSSrv(virNetworkDef *def,
virNetworkDNSDef *dns = &def->dns;
virNetworkDNSSrvDef srv = { 0 };
bool isAdd = (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST ||
- command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST);
+ command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST ||
+ command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST ||
+ command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_LAST);
int foundCt = 0;

if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) {
@@ -3268,15 +3342,21 @@ virNetworkDefUpdateDNSSrv(virNetworkDef *def,
if (isAdd) {

if (foundCt > 0) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- _("there is already at least one DNS SRV record matching all specified fields in network %1$s"),
- def->name);
+ if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) ||
+ (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST))
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ _("there is already at least one DNS SRV record matching all specified fields in network %1$s"),
+ def->name);
+ else
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("DNS SRV records cannot be modified, only added or deleted"));
goto cleanup;
}

/* add to beginning/end of list */
if (VIR_INSERT_ELEMENT(dns->srvs,
- command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST
+ (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST ||
+ command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST)
? 0 : dns->nsrvs, dns->nsrvs, srv) < 0)
goto cleanup;
} else if (command == VIR_NETWORK_UPDATE_COMMAND_DELETE) {
@@ -3322,7 +3402,9 @@ virNetworkDefUpdateDNSTxt(virNetworkDef *def,
virNetworkDNSDef *dns = &def->dns;
virNetworkDNSTxtDef txt = { 0 };
bool isAdd = (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST ||
- command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST);
+ command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST ||
+ command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST ||
+ command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_LAST);

if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
@@ -3344,15 +3426,21 @@ virNetworkDefUpdateDNSTxt(virNetworkDef *def,
if (isAdd) {

if (foundIdx < dns->ntxts) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- _("there is already a DNS TXT record with name '%1$s' in network %2$s"),
- txt.name, def->name);
+ if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST) ||
+ (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST))
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ _("there is already a DNS TXT record with name '%1$s' in network %2$s"),
+ txt.name, def->name);
+ else
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("DNS TXT records cannot be modified, only added or deleted"));
goto cleanup;
}

/* add to beginning/end of list */
if (VIR_INSERT_ELEMENT(dns->txts,
- command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST
+ (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST ||
+ command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST)
? 0 : dns->ntxts, dns->ntxts, txt) < 0)
goto cleanup;
} else if (command == VIR_NETWORK_UPDATE_COMMAND_DELETE) {
diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index 597e3d4530..c30305ac50 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -1231,7 +1231,7 @@ static const vshCmdOptDef opts_network_update[] = {
.positional = true,
.required = true,
.completer = virshNetworkUpdateCommandCompleter,
- .help = N_("type of update (add-first, add-last (add), delete, or modify)")
+ .help = N_("type of update (add-first, add-last (add), delete, modify, modify-or-add, or modify-or-add-first)")
},
{.name = "section",
.type = VSH_OT_STRING,
@@ -1260,7 +1260,7 @@ static const vshCmdOptDef opts_network_update[] = {

VIR_ENUM_IMPL(virshNetworkUpdateCommand,
VIR_NETWORK_UPDATE_COMMAND_LAST,
- "none", "modify", "delete", "add-last", "add-first");
+ "none", "modify", "delete", "add-last", "add-first", "modify-or-add", "modify-or-add-first");

VIR_ENUM_IMPL(virshNetworkSection,
VIR_NETWORK_SECTION_LAST,
--
2.44.0