Remove d_place from DNSResourceRecord #4549
Conversation
zeha
force-pushed
the
dnsrr
branch
5 times, most recently
from
5d46e73
to
542a822
Compare
|
Rebased. |
Now that DNSResourceRecords are only supposed to be created by backends, they certainly have no say in the DNSPacket positioning.
The slave code did not pay attention to d_place in the received DNSResourceRecords anyway, so throwing away that info is alright.
|
Rebased. |
|
I feel this makes it all more clean. @mind04, any comments? |
pdns/mastercommunicator.cc
Outdated
| B->lookup(QType(QType::NS),domain); | ||
| while(B->get(rr)) | ||
| di.backend->lookup(QType(QType::NS), di.zone); | ||
| while(di.backend->get(rr)) | ||
| nsset.insert(rr.content); | ||
|
|
There was a problem hiding this comment.
I don't like the change from B to di.backend here. This change is modifying notify behaviour in multi backend setups. If there are two backends, and the NS records for a given name live in the first backend, with no NS records in the second backend (with the SOA) powerdns was using the records from the first backend for the notify.
After this change no notifications are sent out for the domain because there are no NS records in the second backend (with the SOA).
It is inconsistent if no notifications are send out but powerdns is returning NS records for a normal query. If both backend have differnt NS records thing are even more interesting. Notifications are sent to the nameservers from the second backend and a NS query is returning the ones from the first...
Overriding records is a common use case for multi backend setups, so I my opinion this change is not improving thing.
…nd04's concerns. It also makes the code compile again.
Now that DNSResourceRecords are only supposed to be created by backends, they certainly have no say in the DNSPacket positioning.
The DNSResourceRecord(DNSRecord&) constructor is renamed to DNSResourceRecord::fromWire and records going into packets are only ever converted from DNSResourceRecord (backend type) -> DNSRecord (wire type). This makes it safe to remove d_place from DNSResourceRecord and default to ANSWER in DNSRecord.
Finally FindNS::lookup(..., UeberBackend) goes away. Master/Slave support now really requires backends filling out DomainInfo.backend in getUpdatedMasters, all in-tree backends do that AFAICT.
Also drops serialize.