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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list