[libvirt] [PATCH] virsh: new net-update command

[Eric Blake]

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 :).

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 617 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120920/8bb74f12/attachment-0001.sig>