[libvirt] [RFC] Refactoring bridge driver for portability

[Roman Bogorodskiy]

  Laine Stump wrote:

> On 12/23/2012 07:54 AM, Roman Bogorodskiy wrote:
> > Hi,
> >
> > Few weeks ago when I have submitted my all-in-one huge FreeBSD,
> > Eric made some comments on the networking part:
> >
> > http://www.redhat.com/archives/libvir-list/2012-December/msg00432.html
> >
> > Now when we're done with most of the other things, I'd like
> > to discuss networking in more detail.
> >
> > Basically, the main question that bothers me is what would be the
> > best way to refactor network/bridge_driver.c?
> >
> > Currently it contains a number of things:
> >
> >  - dnsmasq routines, static, but cross-platform, doesn't need
> >    any changes (at least major ones)
> >  - the same for radvd
> >  - minor firewalld bits, that are linux specifc
> >  - iptables routines. There are quite a lot of that code
> >    and it's also gets called from many places in the file
> 
> Note that there is already viriptables.c in src/util. Of course those
> are lower level primitives, but they were really created with the needs
> of bridge_driver.c in mind.
> 
> 
> >
> > I'm thinking about an approach that Eric mentioned for virthread.c:
> >
> > - keep dnsmasq and radvd in bridge_driver.c
> > - move out all linux specific code to bridge_driver_linux.c and
> >   #include it in a way similar to virthread.c
> 
> I must have missed that message. I dislike #including .c files, if
> that's what you're talking about.
> 
> 
> > - in a similar way create bridge_driver_freebsd.c
> >
> > Does it sound reasonable?
> 
> I don't mind the idea of bridge_driver_xxx.c and bridge_driver.c, but I
> think that the appropriate bridge_driver_xxx.c should be added to the
> Makefile rather than #included in bridge_driver.c.
> 
> I would do it by having a common .h file bridge_driver_platform.h which
> had a set of functions that must be defined for any supported platform,
> then rewrite bridge_driver.c so that everything that isn't
> platform-independent is turned into a call to a function or functions
> declared in bridge_driver_platform.h and defined in
> bridge_driver_(linux|freebsd).c
> 
> For example, the firewalld/dbus specific stuff can be considered as "an
> opaque chunk of Linux specific code that must be executed during
> networkStartup()". I think that can be taken care of by declaring a
> function networkStartupPlatform() in bridge_driver_platform.h, then
> defining it in both bridge_driver_linux.c and bridge_driver_freebsd.c
> (where it may simply return success.)
> 
> All of the seemingly iptables-specific functions bridge_driver.c should
> be renamed to use a more generic term in place of "iptables":
> 
> 
>   networkAddIpSpecificIptablesRules
>   networkAddMasqueradingFilterRules
>   networkAddRoutingFilterRules
>   networkReloadFilterRules
>   networkRemoveFilterRules
>   networkRemoveIpSPecificFilterRules
>   networkRemoveGeneralFilterRules
>   [etc]
> 
> Almost all of those functions should be able to remain in
> bridge_driver.c; only the bottom-level functions that are directly
> calling iptables(Add|Remove)*() functions should need to be moved to
> bridge_driver_xxx.c (and re-written appropriately for FreeBSD). (Or
> maybe the concepts behind the functions declared in
> src/util/viriptables.h are generic enough that those functions should
> also be renamed, and a new src/util/virpf.c (or whatever filtering
> package is used on FreeeBSD these days - I haven't kept up with BSD for
> a long time) defined that implements these filter*() functions for that
> platform.

That sounds reasonable, I'll start moving this way.

Thanks

Roman Bogorodskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130103/14c67f32/attachment-0001.sig>