Re: [Xen-devel] [PATCH V3] hotplug: add openvswitch script

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2013-04-23 at 09:46 +0100, Roger Pau Monne wrote:
> > +    ovs-vsctl --timeout=30 -- --if-exists del-port $dev -- add-port 
> > "$bridge" $dev $tag_arg $trunk_arg $vif_details
> > +    ip link set $dev up
> 
> Should we check the return value of this two calls, so we can print a
> nice error message if something fails?

The bridge variant doesn't, but I suppose that's not much of an
argument. There's a do_or_die helper in xen-hotplug-common.sh which logs
on failure, I suppose that would be a good thing to use?

> > +}
> > +
> > +case "$command" in
> > +    add|online)
> > +        setup_virtual_bridge_port $dev
> > +        add_to_openvswitch $dev
> > +        ;;
> > +
> > +    remove|offline)
> > +        ovs-vsctl --timeout=30 -- --if-exists del-port $dev
> 
> Same here.

I think this one wants to be a non-fatal error on failure, do allow as
much cleanup as possible to go ahead (e.g. the iptables rule below). I
think do_without_error is the right answer for this one.

I also noticed that we don't set the link down here. Probably the device
is about to go away but I added it for consistency with the up case, and
with the bridge script...

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel