WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: "W. Michael Petullo" <mike@xxxxxxxx>
Subject: Re: [Xen-devel] Proposal: vif-local
From: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>
Date: Mon, 9 May 2011 09:45:22 +0100
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Mon, 09 May 2011 01:47:12 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20110506170602.GA4198@xxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Organization: Citrix Systems, Inc.
References: <20110505171542.GA17115@xxxxxxxxx> <1304671321.26692.117.camel@xxxxxxxxxxxxxxxxxxxxxx> <20110506170602.GA4198@xxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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