[libvirt] [PATCH 1/2] Move qemuGetDHCPInterfaces to separate file

[Guido Günther]

Hi Laine,

thanks for having a look!

On Tue, Feb 02, 2016 at 01:35:28PM -0500, Laine Stump wrote:
> On 01/31/2016 01:42 PM, Guido Günther wrote:
> >so we can use it from the LXC driver as well.
> >---
> >I couldn't find a nice place to add this so I went for a separate file. I'm
> >happy to move this elsewhere.
> 
> A separate file is fine, but it can't be in the util directory, because
> you're including a file in the conf directory (virdomainobjlist), and files
> in util aren't allowed to reference files in conf (ugh, I see we're doing it
> in several places anyway; I thought those had all been cleared out; I know
> we at least *discussed* it). Also the function is calling public libvirt API
> functions (virNetworkLookupByName and virNetworkGetDHCPLeases,
> virNetworkDHCPLeaseFree, virDomainInterfaceFree) and using datatypes from
> the public API, also not allowed in the util directory.

It kind of occurred to me while moving code around that s.th. like this
might happen but I couldn't come up with good reasons (that wouldn't
allow for exceptions), I couldn't find anything in the contribution
guidelines ether why we wouldn't in util/

   * use files from conf
   * use public API

except for keeping things self contained (i.e. to avoid deadlocks).

> When faced with a similar problem several years back and not seeing a
> reasonable alternative, I "temporarily" created backdoor functions into the
> network driver (networkAllocateActualDevice/networkNotifyActualDevice/networkReleaseActualDevice)
> which are called directly from the hypervisor drivers to allocate physical
> ethernet devices from the pools of devices managed by the network driver.
> This is problematic because a stub function needs to be provided in case the
> network driver isn't built, and also because it creates a hard dependency
> for daemon-driver-network in daemon-driver-qemu and daemon-driver-lxc when
> the network driver *is* built. I actually expected that it would be shot
> down in review and I'd have to come up with something else (or, even better,
> that someone would suggest a cleaner alternative), but it was acked and has
> been in place since 2011 or so with apparently no complaints (commit
> 04711a0f3).
> 
> But of course you have a function that trawls through the domain list as
> well, so it's not really appropriate to have it in the network driver
> either, so...

I wonder if it wouldn't be simpler to just c'n'p the function into the
LXC driver then? I didn't do so in the first place since other drivers
could benefit from this as well?
Cheers,
 -- Guido