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. Resolves: https://gitlab.com/libvirt/libvirt/-/issues/363 Signed-off-by: Abhiram Tilak <atp.exp(a)gmail.com&gt; --- Changes in v4: - Fix the variable name mistake `foundEntry` to `foundMatchedEntry` - Fix the `ip`, not being declared or assigned in required location. Changes in v3: - Changed the indexes used to delete or modify instead of `i`. Changes in v2: - Removed the modify-or-add functionality for sections where modify is not applicable. - Changed the existing implementation of `UpdateIPDHCPHost`, to avoid code duplication. - Changed the implementation of modify-or-delete to reassign the `command` variable instead of using multiple nested conditions. docs/manpages/virsh.rst | 5 +- include/libvirt/libvirt-network.h | 12 +-- src/conf/network_conf.c | 143 +++++++++++++++++++++--------- tools/virsh-network.c | 6 +- 4 files changed, 115 insertions(+), 51 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index fa038e4547..918e6db30c 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -5906,7 +5906,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), or +"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..bb4468b160 100644 --- a/include/libvirt/libvirt-network.h +++ b/include/libvirt/libvirt-network.h @@ -176,11 +176,13 @@ int virNetworkUndefine (virNetworkPtr network); * Since: 0.10.2 */ typedef enum { - VIR_NETWORK_UPDATE_COMMAND_NONE = 0, /* invalid (Since: 0.10.2) */ - VIR_NETWORK_UPDATE_COMMAND_MODIFY = 1, /* modify an existing element (Since: 0.10.2) */ - 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_NONE = 0, /* invalid (Since: 0.10.2) */ + VIR_NETWORK_UPDATE_COMMAND_MODIFY = 1, /* modify an existing element (Since: 0.10.2) */ + 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_ADD_LAST = 5, /* conditionally modify or add an element at end (Since: 10.4.0) */ + VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST = 6, /* conditionally modify or add an element at start (Since: 10.4.0) */ # 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..16fa26ab86 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -2720,6 +2720,7 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDef *def, virNetworkIPDef *ipdef = virNetworkIPDefByIndex(def, parentIndex); virNetworkDHCPHostDef host = { 0 }; bool partialOkay = (command == VIR_NETWORK_UPDATE_COMMAND_DELETE); + int foundMatchedEntry = -1, foundExactEntry = -1; if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "host") < 0) goto cleanup; @@ -2740,22 +2741,47 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDef *def, goto cleanup; } - if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) { + /* check if the entry already exsits */ + for (i = 0; i < ipdef->nhosts; i++) { - /* search for the entry with this (ip|mac|name), - * and update the IP+(mac|name) */ - for (i = 0; i < ipdef->nhosts; i++) { - if ((host.mac && ipdef->hosts[i].mac && - !virMacAddrCompare(host.mac, ipdef->hosts[i].mac)) || - (VIR_SOCKET_ADDR_VALID(&host.ip) && - virSocketAddrEqual(&host.ip, &ipdef->hosts[i].ip)) || - (host.name && - STREQ_NULLABLE(host.name, ipdef->hosts[i].name))) { - break; - } + /* try to match any of (ip|mac|name) attributes */ + if ((host.mac && ipdef->hosts[i].mac && + !virMacAddrCompare(host.mac, ipdef->hosts[i].mac)) || + (VIR_SOCKET_ADDR_VALID(&host.ip) && + virSocketAddrEqual(&host.ip, &ipdef->hosts[i].ip)) || + (host.name && + STREQ_NULLABLE(host.name, ipdef->hosts[i].name))) { + foundMatchedEntry = i; + } + + /* find exact entry - all specified attributes must match */ + 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))) { + foundExactEntry = i; + break; } + } + + /* modify-or-add: convert command to add or modify based on foundEntry */ + if (foundMatchedEntry == -1) { + if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST) + command = VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST; + else if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST) + command = VIR_NETWORK_UPDATE_COMMAND_ADD_LAST; + } else if (foundMatchedEntry >= 0) { + if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST || + command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST) + command = VIR_NETWORK_UPDATE_COMMAND_MODIFY; + } + + if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) { - if (i == ipdef->nhosts) { + /* log error if no such entry exists to be modified */ + if (foundMatchedEntry == -1) { g_autofree char *ip = virSocketAddrFormat(&host.ip); virReportError(VIR_ERR_OPERATION_INVALID, _("couldn't locate an existing dhcp host entry with \"mac='%1$s'\" \"name='%2$s'\" \"ip='%3$s'\" in network '%4$s'"), @@ -2768,8 +2794,8 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDef *def, * then clear out the extra copy to get rid of the duplicate pointers * to its data (mac and name strings). */ - virNetworkDHCPHostDefClear(&ipdef->hosts[i]); - ipdef->hosts[i] = host; + virNetworkDHCPHostDefClear(&ipdef->hosts[foundMatchedEntry]); + ipdef->hosts[foundMatchedEntry] = host; memset(&host, 0, sizeof(host)); } else if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) || @@ -2779,21 +2805,14 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDef *def, goto cleanup; /* log error if an entry with same name/address/ip already exists */ - 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))) { - g_autofree char *ip = virSocketAddrFormat(&host.ip); + if (foundMatchedEntry >= 0) { + g_autofree char *ip = virSocketAddrFormat(&host.ip); + virReportError(VIR_ERR_OPERATION_INVALID, + _("there is an existing dhcp host entry in network '%1$s' that matches \"<host mac='%2$s' name='%3$s' ip='%4$s'/>\""), + def->name, host.mac ? host.mac : _("unknown"), + host.name, ip ? ip : _("unknown")); + goto cleanup; - virReportError(VIR_ERR_OPERATION_INVALID, - _("there is an existing dhcp host entry in network '%1$s' that matches \"<host mac='%2$s' name='%3$s' ip='%4$s'/>\""), - def->name, host.mac ? host.mac : _("unknown"), - host.name, ip ? ip : _("unknown")); - goto cleanup; - } } /* add to beginning/end of list */ @@ -2804,18 +2823,8 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDef *def, goto cleanup; } else if (command == VIR_NETWORK_UPDATE_COMMAND_DELETE) { - /* find matching entry - all specified attributes must match */ - 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))) { - break; - } - } - if (i == ipdef->nhosts) { + /* log error if there is no entry with exact match*/ + if (foundExactEntry == -1) { virReportError(VIR_ERR_OPERATION_INVALID, _("couldn't locate a matching dhcp host entry in network '%1$s'"), def->name); @@ -2823,8 +2832,8 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDef *def, } /* remove it */ - virNetworkDHCPHostDefClear(&ipdef->hosts[i]); - VIR_DELETE_ELEMENT(ipdef->hosts, i, ipdef->nhosts); + virNetworkDHCPHostDefClear(&ipdef->hosts[foundExactEntry]); + VIR_DELETE_ELEMENT(ipdef->hosts, foundExactEntry, ipdef->nhosts); } else { virNetworkDefUpdateUnknownCommand(command); @@ -2865,6 +2874,14 @@ virNetworkDefUpdateIPDHCPRange(virNetworkDef *def, return -1; } + if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST || + command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST) { + + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("dhcp ranges cannot be modified, use add command instead of modify-or-add")); + return -1; + } + if (virNetworkDHCPRangeDefParseXML(def->name, ipdef, ctxt->node, &range) < 0) return -1; @@ -2964,6 +2981,13 @@ virNetworkDefUpdateForwardInterface(virNetworkDef *def, goto cleanup; } + if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST || + command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("forward interface entries cannot be modified, use add command instead of modify-or-add")); + goto cleanup; + } + /* parsing this is so simple that it doesn't have its own function */ iface.type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV; if (!(iface.device.dev = virXMLPropString(ctxt->node, "dev"))) { @@ -3085,6 +3109,18 @@ virNetworkDefUpdatePortGroup(virNetworkDef *def, goto cleanup; } + /* modify-or-add: convert command to add or modify based on foundName */ + if (foundName == -1) { + if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST) + command = VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST; + else if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST) + command = VIR_NETWORK_UPDATE_COMMAND_ADD_LAST; + } else if (foundName >= 0) { + if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST || + command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST) + command = VIR_NETWORK_UPDATE_COMMAND_MODIFY; + } + /* if there is already a different default, we can't make this * one the default. */ @@ -3153,6 +3189,13 @@ virNetworkDefUpdateDNSHost(virNetworkDef *def, goto cleanup; } + if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST || + command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("DNS HOST records cannot be modified, use add instead of modify-or-add")); + goto cleanup; + } + if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "host") < 0) goto cleanup; @@ -3249,6 +3292,13 @@ virNetworkDefUpdateDNSSrv(virNetworkDef *def, goto cleanup; } + if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST || + command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("DNS SRV records cannot be modified, use add command instead of modify-or-add")); + goto cleanup; + } + if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "srv") < 0) goto cleanup; @@ -3330,6 +3380,13 @@ virNetworkDefUpdateDNSTxt(virNetworkDef *def, goto cleanup; } + if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST || + command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("DNS TXT records cannot be modified, use add command instead of modify-or-add")); + goto cleanup; + } + if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "txt") < 0) goto cleanup; diff --git a/tools/virsh-network.c b/tools/virsh-network.c index 6fcc7fd8ee..88eb673482 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -1215,7 +1215,8 @@ 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, @@ -1245,7 +1246,8 @@ 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.39.2