[libvirt] [PATCH] virsh: add iface-bridge and iface-unbridge commands
Eric Blake
eblake at redhat.com
Wed Nov 16 18:50:52 UTC 2011
On 11/16/2011 11:04 AM, Laine Stump wrote:
> One of the top questions by libvirt users is how to create a host
> bridge device so that guests can be directly on the physical
> network. There are several example documents that explain how to do
> this manually, but following them often results in confusion and
> failure. virt-manager does a good job of creating a bridge based on an
> existing network device, but not everyone wants to use virt-manager.
>
> This patch adds a new command, iface-bridge that makes it just about
> as simple as possible to create a new bridge device based on an
> existing ethernet/vlan/bond device (including associating IP
> configuration with the bridge rather than the now-attached device),
> and start that new bridge up ready for action, eg:
>
> virsh iface-bridge eth0 br0
>
> For symmetry's sake, it also adds a command to remove a device from a
> bridge, restoring the IP config to the now-unattached device:
>
> virsh iface-unbridge br0
I like it!
>
> (I had a short debate about whether to do "iface-unbridge eth0"
> instead, but that would involve searching through all bridge devices
> for the one that contained eth0, which seems like a bit too much
> trouble).
>
> NOTE: These two commands require that the netcf library be available
> on the host. Hopefully this will provide some extra incentive for
> people using suse, debian, ubuntu, and other similar systems to polish
> up (and push downstream) the ports to those distros recently pushed to
> the upstream netcf repo by Dan Berrange. Anyone interested in helping
> with that effort in any way should join the netcf-devel mailing list
> (subscription info at
> https://fedorahosted.org/mailman/listinfo/netcf-devel)
Love the advertising plug. Definitely keep it as part of the commit
message.
>
> During creation of the bridge, it's possible to specify whether or not
> the STP protocol should be started up on the bridge and, if so, how
> many seconds the bridge should squelch traffic from newly added
> devices while learning new topology (defaults are stp='on' and
> delay='0', which seems to usually work best for bridges used in the
> context of libvirt guests).
>
> There is also an option to not immediately start the bridge (and a
> similar option to not immediately start the un-attached device after
> dfestroying the bridge. Default is to start the new device, because in
s/dfestroying/destroying/
> the case of iface-unbridge not starting is strongly discouraged as it
> will leave the system with no network connectivity on that interface
> (because it's necessary to destroy/undefine the bridge device before
> the unattached device can be defined), and it seemed better to make
> the option for iface-bridge behave consistently.
>
> Aside from adding the code for these two functions, and the two
> entries into the command table, the only other change to virsh.c was
> to add the option name to vshCommandOptInterfaceBy(), because the
> iface-unbridge command names its interface option as "bridge".
>
> After being hounded by Eric to always put documentation in with any
> new code changes, this patch seems a bit naked without some html bits
> describing it :-), but it looks like docs/virshcmdref.html.in has been
> deprecated in favor of the version in the separate repo at
> http://libvirt.org/git/?p=libvirt-virshcmdref.git.
Virsh is typically documented first in tools/virsh.pod. You are correct
that the documentation in libvirt-virshcmdref.git (should) be more
detailed, giving example usage and more commentary, but that depends on
virshcmdref being kept up-to-date (I suppose this is my plea for more
help in this area!). But the incompleteness of libvirt-virshcmdref is
no excuse for not at least listing the new commands in virsh.pod (aka
'man virsh').
> +++ b/tools/virsh.c
> @@ -330,12 +330,14 @@ static virNWFilterPtr vshCommandOptNWFilterBy(vshControl *ctl, const vshCmd *cmd
> VSH_BYUUID|VSH_BYNAME)
>
> static virInterfacePtr vshCommandOptInterfaceBy(vshControl *ctl, const vshCmd *cmd,
> + const char *optname,
> +
> const char **name, int flag);
Spurious blank line.
> +static bool
> +cmdInterfaceBridge(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
Drop ATTRIBUTE_UNUSED; it's bad copy-and-paste, since you do use cmd.
> +/*
> + * "iface-unbridge" command
> + */
> +static const vshCmdInfo info_interface_unbridge[] = {
> + {"help", N_("undefine a bridge device after detaching its slave device")},
> + {"desc", N_("unbridge a network device")},
> + {NULL, NULL}
> +};
> +
> +static const vshCmdOptDef opts_interface_unbridge[] = {
> + {"bridge", VSH_OT_DATA, VSH_OFLAG_REQ, N_("current bridge device name")},
> + {"no-start", VSH_OT_BOOL, 0, N_("don't start the un-slaved interface immediately (not recommended)")},
Long line; might be worth a line wrap before the N_(...).
> @@ -14199,6 +14612,10 @@ static const vshCmdDef nodedevCmds[] = {
> static const vshCmdDef ifaceCmds[] = {
> {"iface-begin", cmdInterfaceBegin, opts_interface_begin,
> info_interface_begin, 0},
> + {"iface-bridge", cmdInterfaceBridge, opts_interface_bridge,
> + info_interface_bridge, 0},
> + {"iface-unbridge", cmdInterfaceUnbridge, opts_interface_unbridge,
> + info_interface_unbridge, 0},
iface-unbridge should sort lower down in the list.
I'll live up to my well-earned reputation :), and request that you don't
push this without first squashing in docs (but to soften the blow,
here's my first cut at something you can squash in). ACK with above
nits fixed and man page docs added.
diff --git i/tools/virsh.pod w/tools/virsh.pod
index 775d302..3ddfc89 100644
--- i/tools/virsh.pod
+++ w/tools/virsh.pod
@@ -1465,6 +1465,17 @@ resort to a name instead).
=over 4
+=item B<iface-bridge> I<interface> I<bridge> [I<--no-stp>] [I<delay>]
+[I<--no-start>]
+
+Create a bridge device named I<bridge>, and attach the existing network
+I<interface> to the new bridge. The new bridge defaults to starting
+immediately, with STP enabled and a delay of 0; these settings can
+be altered with I<--no-stp>, I<--no-start>, and an integer number of
+seconds for I<delay>.
+
+See also B<iface-unbridge> for undoing this operation.
+
=item B<iface-define> I<file>
Define a host interface from an XML I<file>, the interface is just
defined but
@@ -1519,6 +1530,15 @@ I<interface> specifies the interface name.
Start a (previously defined) host interface, such as by running "if-up".
+=item B<iface-unbridge> I<bridge> [I<--no-start>]
+
+Tear down a bridge device named I<bridge>, releasing its underlying
+interface back to normal usage. The underlying interface is restarted
+unless I<--no-start> is present; this flag is present for symmetry, but
+generally not recommended.
+
+See also B<iface-bridge> for creating a bridge.
+
=item B<iface-undefine> I<interface>
Undefine the configuration for an inactive host interface.
--
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: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20111116/6b953f8a/attachment-0001.sig>
More information about the libvir-list
mailing list