Re: [PATCH] Split up platfrom specifics from bridge driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



  Eric Blake wrote:

> On 07/18/2013 03:23 AM, Daniel P. Berrange wrote:
> > On Wed, Jul 17, 2013 at 05:41:56PM -0600, Eric Blake wrote:
> >> So, although I like the split, I can't help but wonder if your rebase
> >> should take the road of adjusting things to use a callback struct,
> >> rather than requiring matching implementations across multiple files.
> > 
> > I think the callback struct approach is useful for the case where you
> > have multiple possible implementations compiled in at once and we need
> > to choose them dynamically. Here though, we're making the choice at
> > compile time, so I think callback + dynamic dispatch is somewhat overkill.
> > 
> > Following of the virthread approach of #include'ing .c file was my
> > suggestion to Roman for how to structure this.
> 
> Fair enough.  It just means that we don't have the compiler helping us
> remember to add both functions at once, so it raises the probability
> that someone will add a Linux-only function and forget to add the noop
> counterpart.  But as long as we detect it before a release (that's what
> release candidate testing is for, right?) we should be okay.
> 
> Roman, I'll accept your code with the .c inclusion, so you don't have to
> do the extra work of a dynamic dispatch rewrite; now it's just waiting
> for a rebased patch.

That sounds good, thanks.

I have started with renaming of 'struct network_driver' (patch is
already on the list). When done with this patch, I'll do 'Iptables' ->
'Firewall' rename and then finally will rebase this split patch.

Roman Bogorodskiy

Attachment: pgpmbOr1eX0Y9.pgp
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]