Re: [libvirt] [PATCH] virsh: new net-update command
On 09/19/2012 12:08 PM, 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.
+
+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, delete, or modify)")},
s/add/add-first, add-last/
+ {"section", VSH_OT_DATA, VSH_OFLAG_REQ,
+ N_("which section of network configuration to update")},
+ {"xml", VSH_OT_BOOL, 0, N_("xml is specified directly on
commandline")},
+ {"file", VSH_OT_BOOL, 0, N_("file containing xml is specified on
commandline")},
+ {"xmldata", VSH_OT_DATA, VSH_OFLAG_REQ,
+ N_("complete xml element (or name of file containing xml) to add/modify,
"
+ "or to be matched for search")},
Interesting choice to make --xml and --file be boolean flags, and
'[--xmldata] data' be the string that becomes either the file name or
the xml content. I might have done just two optional VSH_OT_DATA
arguments for --xml and --file and then manually checked that exactly
one of the two was supplied, instead of using three arguments. But what
you did works, so no need to change it.
+ /* the goal is to have a full xml element in the
"xmldata"
+ * string. This can either be directly given with the --xml
+ * option, or indirectly with the --file option (default,
+ * indicates that the xmldata on the commandline is really the
+ * name of a file whose contents are the desired xml).
+ */
+
+ if (!isXml)
+ isFile = true;
+ if (isXml && isFile) {
+ vshError(ctl, "%s", _("you can specify file (default) or xml,
"
+ "but not both"));
+ goto cleanup;
+ }
+ if (vshCommandOptString(cmd, "xmldata", (const char **)&xmldata) <
0) {
This cast shouldn't be necessary.
+ vshError(ctl, "%s", _("malformed xmldata
argument"));
+ goto cleanup;
+ }
+
+ if (isFile) {
+ /* contents of xmldata is actually the name of a file that
+ * contains the xml.
+ */
+ if (virFileReadAll(xmldata, VSH_MAX_XML_FILE, &xmldataFromFile) < 0)
+ return false;
+ /* NB: the original xmldata is freed by the vshCommand
+ * infrastructure, so it's safe to lose its pointer here.
Misleading comment; rather, xmldata is a 'const char *' and doesn't need
to be freed.
However, there's one major omission preventing me acking this: virsh.pod
needs to document the new command.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 617 bytes)