Re: [libvirt] [PATCH] virsh: add iface-bridge and iface-unbridge commands

2011-11-21 Thread Eric Blake
On 11/18/2011 12:25 AM, Kashyap Chamarthy wrote:
 On 11/17/2011 02:35 AM, Laine Stump wrote:
 On 11/16/2011 01:50 PM, Eric Blake wrote:
 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.

 I squashed in your virsh.pod change (plus a couple additions), fixed the 
 nits, and pushed.

 Thanks!


 P.S. to anyone who tries these commands - do us both a favor and run virsh 
 iface-begin
 beforehand, then run virsh iface-commit only after you're sure it's done 
 the right thing
 (otherwise run virsh iface-rollback or just reboot (I think I've just 
 realized the just
 reboot part isn't working on F16 due to the conversion to systemd...)
 
 Laine, is there an srpm or something with the above fixes which I can use to 
 do a quick
 libvirt scratch build and give it a whirl ?

DV maintains hourly snapshots of libvirt.git, documented here:
http://libvirt.org/downloads.html

Those are tarballs rather than srpms.  However, libvirt tarballs include
a working libvirt.spec, so that you could unpack things, run
./configure, then 'make rpm' to give yourself a working srpm.  I haven't
tested this, but I think that also means you could use 'rpmbuild -ta
libvirt-git-snapshot.tar.gz' and get rpms from that approach.

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



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] virsh: add iface-bridge and iface-unbridge commands

2011-11-16 Thread Laine Stump
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 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)

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
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.
---
 tools/virsh.c |  430 -
 1 files changed, 425 insertions(+), 5 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index af80e4b..ddfd341 100644
--- a/tools/virsh.c
+++ 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);
 
 /* default is lookup by Name and MAC */
 #define vshCommandOptInterface(_ctl, _cmd, _name)\
-vshCommandOptInterfaceBy(_ctl, _cmd, _name,  \
-   VSH_BYMAC|VSH_BYNAME)
+vshCommandOptInterfaceBy(_ctl, _cmd, NULL, _name,\
+ VSH_BYMAC|VSH_BYNAME)
 
 static virStoragePoolPtr vshCommandOptPoolBy(vshControl *ctl, const vshCmd 
*cmd,
 const char *optname, const char **name, int flag);
@@ -6807,7 +6809,7 @@ cmdInterfaceName(vshControl *ctl, const vshCmd *cmd)
 
 if (!vshConnectionUsability(ctl, ctl-conn))
 return false;
-if (!(iface = vshCommandOptInterfaceBy(ctl, cmd, NULL,
+if (!(iface = vshCommandOptInterfaceBy(ctl, cmd, NULL, NULL,
VSH_BYMAC)))
 return false;
 
@@ -6837,7 +6839,7 @@ cmdInterfaceMAC(vshControl *ctl, const vshCmd *cmd)
 
 if (!vshConnectionUsability(ctl, ctl-conn))
 return false;
-if (!(iface = vshCommandOptInterfaceBy(ctl, cmd, NULL,
+if (!(iface = vshCommandOptInterfaceBy(ctl, cmd, NULL, NULL,
VSH_BYNAME)))
 return false;
 
@@ -7137,6 +7139,417 @@ 

Re: [libvirt] [PATCH] virsh: add iface-bridge and iface-unbridge commands

2011-11-16 Thread Eric Blake
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 

Re: [libvirt] [PATCH] virsh: add iface-bridge and iface-unbridge commands

2011-11-16 Thread Laine Stump

On 11/16/2011 01:50 PM, Eric Blake wrote:
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.


I squashed in your virsh.pod change (plus a couple additions), fixed the 
nits, and pushed.


Thanks!


P.S. to anyone who tries these commands - do us both a favor and run 
virsh iface-begin beforehand, then run virsh iface-commit only after 
you're sure it's done the right thing (otherwise run virsh 
iface-rollback or just reboot (I think I've just realized the just 
reboot part isn't working on F16 due to the conversion to systemd...)


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list