Re: [libvirt] [PATCH] leaseshelper: add enhancements to support all events

>> cmd = virCommandNew(dnsmasqCapsGetBinaryPath(caps)); >> virCommandAddArgFormat(cmd, "--conf-file=%s", configfile); >> - virCommandAddArgFormat(cmd, "--dhcp-script=%s", leaseshelper_path); >> + virCommandAddArgFormat(cmd, "--dhcp-script=%s", pseudo_leaseshelper_path); >> + virCommandAddArgFormat(cmd, "--leasefile-ro"); > > Does dnsmasq pass through the rest of the environment we pass to it at > this point? The leaseshelper extracts quite a lot of stuff from the > environment variables that are passed by dnsmasq. In case it does we > could use an env variable to pass the interface name instead of linking > the helper and using different names. If I use the following line of code in the function networkBuildDhcpDaemonCommandLine ... setenv("VIR_BRIDGE_NAME", network->def->bridge, 1); .. then yes, the helper program invoked by dnsmasq does get to see this variable. (It does so in Fedora20). (Looks like I acted in haste, should've waited for more replies on RFC, sigh)

So, in the next version that I will send, should I first make a wrapper by the name virSetEnvBlockSUID for setenv, just like we have virGetEnvBlockSUID for getenv?

> A second issue that comes into my mind is compatibility with already > existing files. Does this work when you already have a lease file? (I > didn't try it, I'm just asking). If we use the --leasefile-ro option, then although this method will work, but no, the lease file will *not* be read/written at all. So even if an old one exists, its of no use. But then again, the use of --leasefile-ro is mandatory, so that all events are captured by our helper program. For example, renew of a lease.

Regards, Nehal J Wani