Re: [libvirt] [PATCH 2/2] Network: Add support for DNS hosts definition
On 03/31/2011 10:16 AM, Paolo Bonzini wrote:
> +<zeroOrMore>
> +<element name="host">
> +<attribute name="ip"><ref
name="ipv4-addr"/></attribute>
> +<zeroOrMore>
> +<element name="hostname"><text/></element>
> +</zeroOrMore>
oneOrMore hostname elements seems preferrable.
Well, this could be better. That's right.
> + if ((fd = open(hostsFile,
You can use fopen and fprintf.
Well, I don't know since it's rarely used in to fopen and fprintf form
and I'm not sure whether syntax checking enables it or not. I know there
are some native functions disabled so this one can be as well but open()
is not disabled since it's used somewhere - that's what I'm sure about.
This function also should be in src/util/dnsmasq.c (e.g. with
addnhost*
APIs in that file and dnsmasqAddHost as a public API). See the
hostsfile* APIs there.
Oh, I'll have a look to this one.
Then you could further split the patch this way:
- one patch that moves the creation of the dnsmasqContext into
networkSaveDnsmasqHostsfile;
What do you mean by dnsmasqContents? The code to generate the hostsfile,
i.e. the one I added?
- one patch that makes src/util/dnsmasq.c create the hostsfile only
after the first call to dnsmasqAddDhcpHost
You mean that the first call to dnsmasqAddDhcpHost should call the
function you mentioned about? Also, this is named dnsmasqAddDhcpHost()
.. what if somebody will use <dns> tag but without <dhcp> tag?
- this would be the third patch in the series, and it would add
dnsmasqAddHost calls in networkSaveDnsmasqHostsfile
So this patch will be just adding the networkSaveDnsmasqHostsfile() call
to the dnsmasqAddHost() and nothing else ?
> diff --git a/tests/networkxml2xmlin/nat-network-dns-hosts.xml
b/tests/networkxml2xmlin/nat-network-dns-hosts.xml
> new file mode 100644
> index 0000000..fe545cf
Uhm, libvirt has no tests that actually check whether the dnsmasq
command line works? That's a bit bad, perhaps you can add them...
Well, unfortunately there are no tests to check whether the dnsmasq
command line works. What would you prefer? To run it with some bogus PID
file and port and check the command execution error code and if it's 0
(and PID is working) then to kill $PID and make the test pass, otherwise
fail the test or something similar?
Michal
--
Michal Novotny <minovotn(a)redhat.com>, RHCE
Virtualization Team (xen userspace), Red Hat