[ovs-dev] InterfaceReconfigureVswitch.py fix didn't make it into xen-api?

[Ian Campbell]

On Fri, 2011-02-11 at 18:23 +0000, Ben Pfaff wrote: 
> The following fix to InterfaceReconfigureVswitch.py was committed to
> the OVS repository in July 2010.  It has an ack from Ian and I CC'd it
> to Dominic, but it looks to me like it didn't make it upstream into
> xen-api.hg.  Could you check on that for me?

Thanks for bringing it to our attention.

I'm not sure where this patch got too. I'll chase it up on Monday.

> As background, we'd like OVS to not have to replace these scripts at
> all, but of course we'll have to if we don't get all of our bugfixes
> included upstream.

Absolutely!

BTW xen-api development has recently moved to git at github.com. Would
you be willing to use the github tools to create pull requests against
xen-api.git, which is supposed to help track things and stop them
getting forgotten about/missed, in the future?

If not (I haven't used it myself yet, so I don't know how much of a
burden it is to use) then CCing the xen-api list as you have been should
continue to be sufficient too. This patch well-predates any advice for
you to do that so it wouldn't have helped here.

As a backstop patches to these scripts should at least get picked up
when a new version of openvswitch is sync'd into XCP.

Thanks,
Ian.

> 
> Thanks,
> 
> Ben.
> 
> --8<--------------------------cut here-------------------------->8--
> 
> From: Ben Pfaff <blp at nicira.com>
> Date: Fri, 16 Jul 2010 09:22:23 -0700
> Subject: [PATCH] xenserver: Kill bond slaves' dhclients when bringing up bond master.
> 
> This fixes the converse of the problem addressed by commit fe19e820
> "xenserver: Kill bond master's dhclient when bringing up bond slave".  In
> that commit's log message, I claimed that the converse was not a problem,
> but I was wrong.  I must have screwed up in testing, because it really is
> a problem.  This commit fixes it.
> 
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> Acked-by: Ian Campbell <Ian.Campbell at citrix.com>
> CC: Dominic Curran <dominic.curran at citrix.com>
> Reported-by: Michael Mao <mmao at nicira.com>
> Bug #2668.
> ---
>  AUTHORS                                            |    1 +
>  ...ensource_libexec_InterfaceReconfigureVswitch.py |   18 +++++++++++++-----
>  2 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/AUTHORS b/AUTHORS
> index b0f31d9..97f7c67 100644
> --- a/AUTHORS
> +++ b/AUTHORS
> @@ -37,6 +37,7 @@ Jan Medved              jmedved at juniper.net
>  Jeongkeun Lee           jklee at hp.com
>  Joan Cirer              joan at ev0.net
>  John Galgay             john at galgay.net
> +Michael Mao             mmao at nicira.com
>  Paulo Cravero           pcravero at as2594.net
>  Peter Balland           peter at nicira.com
>  Ram Jothikumar          rjothikumar at nicira.com
> diff --git a/xenserver/opt_xensource_libexec_InterfaceReconfigureVswitch.py b/xenserver/opt_xensource_libexec_InterfaceReconfigureVswitch.py
> index 1e45759..ef2f1fe 100644
> --- a/xenserver/opt_xensource_libexec_InterfaceReconfigureVswitch.py
> +++ b/xenserver/opt_xensource_libexec_InterfaceReconfigureVswitch.py
> @@ -431,17 +431,25 @@ class DatapathVswitch(Datapath):
>      def bring_down_existing(self):
>          # interface-reconfigure is never explicitly called to down a
>          # bond master.  However, when we are called to up a slave it
> -        # is implicit that we are destroying the master.
> +        # is implicit that we are destroying the master.  Conversely,
> +        # when we are called to up a bond is is implicit that we are
> +        # taking down the slaves.
>          #
> -        # This is (only) important in the case where the bond master
> -        # uses DHCP.  We need to kill the dhclient process, otherwise
> -        # bringing the bond master back up later will fail because
> -        # ifup will refuse to start a duplicate dhclient.
> +        # This is (only) important in the case where the device being
> +        # implicitly taken down uses DHCP.  We need to kill the
> +        # dhclient process, otherwise performing the inverse operation
> +        # later later will fail because ifup will refuse to start a
> +        # duplicate dhclient.
>          bond_masters = pif_get_bond_masters(self._pif)
>          for master in bond_masters:
>              log("action_up: bring down bond master %s" % (pif_netdev_name(master)))
>              run_command(["/sbin/ifdown", pif_bridge_name(master)])
>  
> +        bond_slaves = pif_get_bond_slaves(self._pif)
> +        for slave in bond_slaves:
> +            log("action_up: bring down bond slave %s" % (pif_netdev_name(slave)))
> +            run_command(["/sbin/ifdown", pif_bridge_name(slave)])
> +
>      def configure(self):
>          # Bring up physical devices. ovs-vswitchd initially enables or
>          # disables bond slaves based on whether carrier is detected