Re: [Xen-devel] Proposal: vif-local

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Fri, 2011-05-06 at 18:06 +0100, W. Michael Petullo wrote:
> >> For site-specific reasons, I use the network-route/vif-route scripts. I
> >> have found that we need to maintain a few custom firewall rules in order
> >> to make things operate in an acceptable manner. I'd like to see a place
> >> to put such scripts and any other site-specific setup related to bringing
> >> up a vif. Keeping this separate from vif-route is useful so that the
> >> installed scripts may be kept unmodified.
> >> 
> >> What I have come up with is vif-local, a script that lives in
> >> /etc/xen/scripts. I modified vif-route to call vif-local right before
> >> it logs "Successful..."
>  
> > I think it would be better to be more general and support a vif-post.d
> > style directory which can contain scripts all of which are called (with
> > a defined set of paramters/env variables).
>  
> > Not sure if we want vif-{route,bridge,etc}-post.d or not, perhaps that's
> > overkill. Using -post.d leaves open the option to add -pre.d in the
> > future as necessary.
> 
> I have attached a patch against Xen 4.1.0 that implements a vif-post.d
> system. I only support the Linux hotplug case at this point.

Thanks. I've got a few comments.

The header of "${XEN_SCRIPT_DIR}/vif-post.d/00-vif-local" describes a
command line parameter "(add|remove|online|offline)" but none of the
invocations actually pass one.

I think it would be better to encapsulate the functionality in a
"call_hooks <devtype> <hook> <other args...>" function in
xen-hotplug-common.sh, calling it as "call_hooks vif post ..." rather
than open coding that loop everywhere.

I think generally it is a good idea to have an explicit suffix (e.g.
".hook") for this sort of thing since then you can use *.hook to get the
list of files which saves manually filtering out *~ *.rpmsave *.dpkg-bak
*.disabled-by-admin *.some-random-suffix-intended-to-disable-the-script
etc.

You probably want to quote $f in case some nutter uses a space in the
hook filename.

You don't actually install 00-vif-local but I think that's a good thing
since the default is an empty script so we save a fork/exec by not
running it.

Lastly we need a Signed-off-by per the DCO (section 11 of
http://lwn.net/Articles/139918/) as well as a suitable changelog message
before we can apply any patch.

Thanks,
Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel