Re: [libvirt] [PATCH] Add forwarders attribute to <dns /> element.

On Mon, Sep 2, 2013 at 8:54 AM, Daniel P. Berrange <berrange(a)redhat.com&gt; wrote: > On Mon, Sep 02, 2013 at 08:50:30AM -0300, Diego Woitasen wrote: >> Useful to set custom forwarders instead of using the contents of >> /etc/resolv.conf. It helps me to setup dnsmasq as local nameserver to resolv VM >> domain names from domain 0, when domain option is used. >> >> Signed-off-by: Diego Woitasen <diego.woitasen(a)vhgroup.net&gt; >> --- >> src/conf/network_conf.c | 34 +++++++++++++++++++++- >> src/conf/network_conf.h | 1 + >> src/network/bridge_driver.c | 8 +++++ >> .../nat-network-dns-forwarders.conf | 15 ++++++++++ >> .../nat-network-dns-forwarders.xml | 9 ++++++ >> tests/networkxml2conftest.c | 1 + >> 6 files changed, 67 insertions(+), 1 deletion(-) >> create mode 100644 tests/networkxml2confdata/nat-network-dns-forwarders.conf >> create mode 100644 tests/networkxml2confdata/nat-network-dns-forwarders.xml >> > > >> @@ -1058,6 +1065,20 @@ virNetworkDNSDefParseXML(const char *networkName, >> } >> } >> >> + forwarders = virXPathString("string(./@forwarders)", ctxt); >> + if (forwarders) { >> + def->forwarders = virStringSplit(forwarders, ",", 0); > > It is frowned upon to create XML where one attribute encodes multiple > separate values. For example, makes it impossible for apps to extract > a single value using xpath queies. > > If we need to support multiple addresses for forwarders, then this > suggests we need a child element underneath <dns> eg perhaps > > <dns> > <forwarder addr='8.8.8.8'/> > <forwarder addr='1.2.3.4'/> > <forwarder addr='234:223::4242'/> > </dns> > > > Daniel > -- > |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| > |: http://libvirt.org -o- http://virt-manager.org :| > |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| > |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| Ok, I'll fix it. What do you think about the feature?