On 09/20/2012 12:55 PM, Eric Blake wrote:
On 09/20/2012 09:05 AM, Laine Stump wrote:
This new virsh command uses the new virNetworkUpdate() API to modify an existing network definition, and optionally have those modifications take effect immediately without restarting the network.
An example usage:
virsh net-update mynet add-last ip-dhcp-host \ "<host mac='00:11:22:33:44:55' ip='192.168.122.45'/>" \ --live --config
If you like, you can instead put the xml into a file, and call like this:
virsh net-update mynet add ip-dhcp-host /tmp/myxml.xml --live --config
(since an xml element must always start with "<", but a filename rarely does, virsh just checks the first character of the supplied "--xml" argument, and decides whether to use the text directly or as a filename. In the rare event that someone really does have a filename starting with "<", they can specify it as "./<xxxx".) I like this version the best.
+ * "net-update" command + */ +static const vshCmdInfo info_network_update[] = { + {"help", N_("update parts of an existing network's configuration")}, + {"desc", ""}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_network_update[] = { + {"network", VSH_OT_DATA, VSH_OFLAG_REQ, N_("network name or uuid")}, + {"command", VSH_OT_DATA, VSH_OFLAG_REQ, + N_("type of update (add-first, add-last (add), delete, or modify)")}, + {"section", VSH_OT_DATA, VSH_OFLAG_REQ, + N_("which section of network configuration to update")}, + {"xml", VSH_OT_DATA, VSH_OFLAG_REQ, + N_("name of file containing xml (or, if it starts with '<', the complete " + "xml element itself) to add/modify, or to be matched for search")}, Two spaces after ')' looks fishy.
+ + /* The goal is to have a full xml element in the "xml" + * string. This is provided in the --xml option, either directly + * (detected by the first character being "<"), or indirectly by + * supplying a filename (first character isn't "<") that contains + * the desired xml. + */ + + if (vshCommandOptString(cmd, "xml", &xml) < 0) { + vshError(ctl, "%s", _("malformed or missing xml argument")); + goto cleanup; + } + + if (*xml != '<') { + /* contents of xmldata is actually the name of a file that + * contains the xml. + */ + if (virFileReadAll(xml, VSH_MAX_XML_FILE, &xmlFromFile) < 0) + return false; + /* NB: the original xml is just a const char * that points + * to a string owned by the Command object, and will be freed + * by vshCommandFree. so it's safe to lose its pointer here. + */ + xml = xmlFromFile; + } + + if (current) { + if (live || config) { + vshError(ctl, "%s", _("--current must be specified exclusively")); + return false; + } + flags |= VIR_NETWORK_UPDATE_AFFECT_CURRENT; + } else { + if (config) + flags |= VIR_NETWORK_UPDATE_AFFECT_CONFIG; + if (live) + flags |= VIR_NETWORK_UPDATE_AFFECT_LIVE; + } + + if (virNetworkUpdate(network, command, + section, parentIndex, xml, flags) < 0) { + vshError(ctl, _("Failed to update network %s"), + virNetworkGetName(network)); + goto cleanup; + } + + if (flags & VIR_NETWORK_UPDATE_AFFECT_CONFIG) { If you want, this could be simplified to: 'if (config) {'
+ if (flags & VIR_NETWORK_UPDATE_AFFECT_LIVE) + affected = _("persistent config and live state"); + else + affected = _("persistent config"); + } else if (flags & VIR_NETWORK_UPDATE_AFFECT_LIVE) { and likewise simplify these two bitwise ops to 'if (live)'.
ACK (unless I get outvoted by other opinions :).
In the interest of getting maximum testing, I'm pushing this version (fixed up as you suggested, and also adding another fix to avoid leaking a virNetwork object when the requested file doesn't exist). If this method is successful for net-update, it can possibly be added to the other commands that take a file-containing-xml option (the only difference between the commands is in the name of that option, and that name is usually unspecified in commandlines anyway).