On Wed, Sep 19, 2012 at 12:32:18PM -0600, Eric Blake wrote:
> 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.
Yes, this is total overkill. When I suggested --xmldata as an option
my intent was that you'd have the following usage
virsh net-update /path/to/xmlfile
virsh net-update --xmldata "some XML data inline"
IMHO, requiring --xml and --file in addition is unfriendly to the
user too
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list