Thread: Patch: update Bonjour support to the newer non-deprecated API
I've grown a bit tired of reading the "'DNSServiceRegistrationCreate' is
deprecated" warnings in OS X builds, and I'm also wondering whether any
of the recently reported problems with Snow Leopard might trace to our
use of an API that Apple has been deprecating since 2003. Hence,
attached is a patch that replaces our use of the DNSServiceDiscovery.h
API with the more modern dns_sd.h API. As-is, this would break Bonjour
functionality in OS X 10.2 and before, but I really doubt that anyone
still cares about that --- any objections out there?
Like the original coding, this will bind the Bonjour service name on
all available interfaces. I suspect that we ought to do something
different if we have been told to bind to a subset of interfaces,
but it's not entirely clear to me how to get the appropriate "interface
indexes" given the information we have (particularly, the listening
socket FDs). In any case, fixing that seems like added functionality.
In principle this might enable use of Bonjour on non-Apple OSes, but
I'm not personally interested enough to test that ...
Comments, objections?
regards, tom lane
Index: configure.in
===================================================================
RCS file: /cvsroot/pgsql/configure.in,v
retrieving revision 1.609
diff -c -r1.609 configure.in
*** configure.in 26 Aug 2009 22:24:42 -0000 1.609
--- configure.in 7 Sep 2009 03:00:24 -0000
***************
*** 1066,1072 ****
fi
if test "$with_bonjour" = yes ; then
! AC_CHECK_HEADER(DNSServiceDiscovery/DNSServiceDiscovery.h, [], [AC_MSG_ERROR([header file
<DNSServiceDiscovery/DNSServiceDiscovery.h>is required for Bonjour])])
fi
# for contrib/uuid-ossp
--- 1066,1072 ----
fi
if test "$with_bonjour" = yes ; then
! AC_CHECK_HEADER(dns_sd.h, [], [AC_MSG_ERROR([header file <dns_sd.h> is required for Bonjour])])
fi
# for contrib/uuid-ossp
Index: src/backend/postmaster/postmaster.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.594
diff -c -r1.594 postmaster.c
*** src/backend/postmaster/postmaster.c 31 Aug 2009 19:41:00 -0000 1.594
--- src/backend/postmaster/postmaster.c 7 Sep 2009 03:00:24 -0000
***************
*** 89,95 ****
#endif
#ifdef USE_BONJOUR
! #include <DNSServiceDiscovery/DNSServiceDiscovery.h>
#endif
#include "access/transam.h"
--- 89,95 ----
#endif
#ifdef USE_BONJOUR
! #include <dns_sd.h>
#endif
#include "access/transam.h"
***************
*** 309,324 ****
extern int optreset; /* might not be declared by system headers */
#endif
/*
* postmaster.c - function prototypes
*/
static void getInstallationPaths(const char *argv0);
static void checkDataDir(void);
-
- #ifdef USE_BONJOUR
- static void reg_reply(DNSServiceRegistrationReplyErrorType errorCode,
- void *context);
- #endif
static void pmdaemonize(void);
static Port *ConnCreate(int serverFd);
static void ConnFree(Port *port);
--- 309,323 ----
extern int optreset; /* might not be declared by system headers */
#endif
+ #ifdef USE_BONJOUR
+ static DNSServiceRef bonjour_sdref = NULL;
+ #endif
+
/*
* postmaster.c - function prototypes
*/
static void getInstallationPaths(const char *argv0);
static void checkDataDir(void);
static void pmdaemonize(void);
static Port *ConnCreate(int serverFd);
static void ConnFree(Port *port);
***************
*** 855,869 ****
#ifdef USE_BONJOUR
/* Register for Bonjour only if we opened TCP socket(s) */
! if (ListenSocket[0] != -1 && bonjour_name != NULL)
{
! DNSServiceRegistrationCreate(bonjour_name,
! "_postgresql._tcp.",
! "",
! htons(PostPortNumber),
! "",
! (DNSServiceRegistrationReply) reg_reply,
! NULL);
}
#endif
--- 854,891 ----
#ifdef USE_BONJOUR
/* Register for Bonjour only if we opened TCP socket(s) */
! if (ListenSocket[0] != -1)
{
! DNSServiceErrorType err;
!
! /*
! * We pass 0 for interface_index, which will result in registering
! * for all available interfaces. It's not entirely clear from the
! * DNS-SD docs whether this would be appropriate if we have bound
! * to just a subset of the available interfaces.
! */
! err = DNSServiceRegister(&bonjour_sdref,
! 0,
! 0,
! bonjour_name,
! "_postgresql._tcp.",
! NULL,
! NULL,
! htons(PostPortNumber),
! 0,
! NULL,
! NULL,
! NULL);
! if (err != kDNSServiceErr_NoError)
! elog(LOG, "DNSServiceRegister() failed: error code %ld",
! (long) err);
! /*
! * We don't bother to read the mDNS daemon's reply, and we expect
! * that it will automatically terminate our registration when the
! * socket is closed at postmaster termination. So there's nothing
! * more to be done here. However, the bonjour_sdref is kept around
! * so that forked children can close their copies of the socket.
! */
}
#endif
***************
*** 1192,1209 ****
}
- #ifdef USE_BONJOUR
-
- /*
- * empty callback function for DNSServiceRegistrationCreate()
- */
- static void
- reg_reply(DNSServiceRegistrationReplyErrorType errorCode, void *context)
- {
- }
- #endif /* USE_BONJOUR */
-
-
/*
* Fork away from the controlling terminal (silent_mode option)
*
--- 1214,1219 ----
***************
*** 2004,2009 ****
--- 2014,2025 ----
syslogPipe[0] = 0;
#endif
}
+
+ #ifdef USE_BONJOUR
+ /* If using Bonjour, close the connection to the mDNS daemon */
+ if (bonjour_sdref)
+ close(DNSServiceRefSockFD(bonjour_sdref));
+ #endif
}
On Sep 6, 2009, at 8:17 PM, Tom Lane wrote: > In principle this might enable use of Bonjour on non-Apple OSes, but > I'm not personally interested enough to test that ... > > Comments, objections? +1 Seems like a no-brainer. Best, David
Tom Lane wrote: > In principle this might enable use of Bonjour on non-Apple OSes, but > I'm not personally interested enough to test that ... With this patch and Avahi's compatibility headers I can successfully compile --enable-bonjour on my Debian system. I haven't tested it. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera wrote: > Tom Lane wrote: > > > In principle this might enable use of Bonjour on non-Apple OSes, but > > I'm not personally interested enough to test that ... > > With this patch and Avahi's compatibility headers I can successfully > compile --enable-bonjour on my Debian system. I haven't tested it. Hmm, but it doesn't link unless I manually add -ldns_sd to the link command. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera wrote: > Hmm, but it doesn't link unless I manually add -ldns_sd to the link > command. And then it throws a warning on start, and fails to work anyway: *** WARNING *** The program 'postgres' uses the Apple Bonjour compatibility layer of Avahi. *** WARNING *** Please fix your application to use the native API of Avahi! *** WARNING *** For more information see <http://0pointer.de/avahi-compat?s=libdns_sd&e=postgres> LOG: DNSServiceRegister() failed: error code -65540 So we're not better than when we started, but it doesn't cause any problem if not enabled. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Alvaro Herrera <alvherre@commandprompt.com> writes: > *** WARNING *** The program 'postgres' uses the Apple Bonjour compatibility layer of Avahi. > *** WARNING *** Please fix your application to use the native API of Avahi! > *** WARNING *** For more information see <http://0pointer.de/avahi-compat?s=libdns_sd&e=postgres> > LOG: DNSServiceRegister() failed: error code -65540 Hmm, I read in their documentation that the dns_sd.h interface was deprecated, but not that it had been intentionally disabled. Seems like they want to drive users away rather than attract them. > So we're not better than when we started, but it doesn't cause any > problem if not enabled. The patch as I gave it intentionally didn't change any user-visible behavior, but one thing that is bothering me is that if USE_BONJOUR is selected, the postmaster will *always* try to advertise itself via DNS-SD. There's no provision for enabling the feature or not at run time, which is a bad thing for packagers: they have to decide for their users whether to turn it on. There was discussion in connection with the Avahi patch last year to the effect that some people thought advertising the postmaster might be a security issue for them. So I'm thinking we ought to fix that while we're messing with it. The two possibilities for that seem to be to change the meaning of bonjour_name = '' (have it mean "no advertisement" instead of "default to service name = computer's name"), or to add a separate boolean GUC. If the latter, is the default 'on' or 'off'? Opinions? regards, tom lane
Tom Lane wrote: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > *** WARNING *** The program 'postgres' uses the Apple Bonjour compatibility layer of Avahi. > > *** WARNING *** Please fix your application to use the native API of Avahi! > > *** WARNING *** For more information see <http://0pointer.de/avahi-compat?s=libdns_sd&e=postgres> > > LOG: DNSServiceRegister() failed: error code -65540 > > Hmm, I read in their documentation that the dns_sd.h interface was > deprecated, but not that it had been intentionally disabled. > Seems like they want to drive users away rather than attract them. I think it is supposed to work; the code suggests that it should. I can't quite find out what the error number is supposed to mean though. The source is here: http://avahi.sourcearchive.com/documentation/0.6.25-1ubuntu1/avahi-compat-libdns__sd_2compat_8c-source.html ... ah! here it is -- BadParam: http://avahi.sourcearchive.com/documentation/0.6.25-1ubuntu1/dns__sd_8h-source.html > The patch as I gave it intentionally didn't change any user-visible > behavior, but one thing that is bothering me is that if USE_BONJOUR > is selected, the postmaster will *always* try to advertise itself > via DNS-SD. There's no provision for enabling the feature or not > at run time, which is a bad thing for packagers: they have to decide > for their users whether to turn it on. There was discussion in > connection with the Avahi patch last year to the effect that some > people thought advertising the postmaster might be a security issue > for them. So I'm thinking we ought to fix that while we're messing > with it. > > The two possibilities for that seem to be to change the meaning of > bonjour_name = '' (have it mean "no advertisement" instead of > "default to service name = computer's name"), or to add a separate > boolean GUC. If the latter, is the default 'on' or 'off'? Opinions? I have a mild preference towards having a new GUC to shut it off explicitely; and the default should be off to avoid the possible security hole (equivalent to having listen_addresses default to localhost, I think. On the other hand, if listen_addresses is set to that, there is no security hole. I assume we're only publishing on addresses we're listening on, not all addresses?) -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Mon, Sep 07, 2009 at 12:50:37PM -0400, Tom Lane wrote: > > The two possibilities for that seem to be to change the meaning of > bonjour_name = '' (have it mean "no advertisement" instead of > "default to service name = computer's name"), or to add a separate > boolean GUC. If the latter, is the default 'on' or 'off'? > Opinions? +1 for separate boolean GUC, default off. I know extra GUCs aren't great, but I don't see another way :/ Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
David Fetter <david@fetter.org> writes:
> On Mon, Sep 07, 2009 at 12:50:37PM -0400, Tom Lane wrote:
>> The two possibilities for that seem to be to change the meaning of
>> bonjour_name = '' (have it mean "no advertisement" instead of
>> "default to service name = computer's name"), or to add a separate
>> boolean GUC. If the latter, is the default 'on' or 'off'?
>> Opinions?
> +1 for separate boolean GUC, default off. I know extra GUCs aren't
> great, but I don't see another way :/
Yeah, the default service name seems like a potentially useful behavior
in some contexts, so taking that behavior away doesn't seem nice.
Separate bool it is...
regards, tom lane
Alvaro Herrera <alvherre@commandprompt.com> writes: > Tom Lane wrote: >> Hmm, I read in their documentation that the dns_sd.h interface was >> deprecated, but not that it had been intentionally disabled. > I think it is supposed to work; the code suggests that it should. I > can't quite find out what the error number is supposed to mean though. > The source is here: > http://avahi.sourcearchive.com/documentation/0.6.25-1ubuntu1/avahi-compat-libdns__sd_2compat_8c-source.html > ... ah! here it is -- BadParam: > http://avahi.sourcearchive.com/documentation/0.6.25-1ubuntu1/dns__sd_8h-source.html Interesting. I coded the call to default everything it could, but maybe Avahi is a little less forgiving about NULL parameters. In particular it wouldn't surprise me if they thought that there *must* be a callback function --- we're playing fast and loose with the API by not bothering to check for a response from the daemon. regards, tom lane
Alvaro Herrera <alvherre@commandprompt.com> writes: > The source is here: > http://avahi.sourcearchive.com/documentation/0.6.25-1ubuntu1/avahi-compat-libdns__sd_2compat_8c-source.html After reading that code, I am content to remain forcibly non compatible with it. If we relied on this it would inject threading into the postmaster, which was precisely the thing that got the previous Avahi patch rejected last year :-( Apple's implementation of the same API does not appear to require any threading: http://www.opensource.apple.com/source/mDNSResponder/mDNSResponder-176.3/mDNSShared/dnssd_clientstub.c Hm, seems to be Apache license ... regards, tom lane
Tom Lane wrote: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > The source is here: > > http://avahi.sourcearchive.com/documentation/0.6.25-1ubuntu1/avahi-compat-libdns__sd_2compat_8c-source.html > > After reading that code, I am content to remain forcibly non compatible > with it. If we relied on this it would inject threading into the > postmaster, which was precisely the thing that got the previous Avahi > patch rejected last year :-( Should we inject some sort of compile-time rejection of the compatibility API? Like, say #ifdef AVAHI_SERVICE_COOKIE #error The Avahi implementation is incompatible #endif inside some #ifdef BONJOUR block? -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
I wrote:
> I've grown a bit tired of reading the "'DNSServiceRegistrationCreate' is
> deprecated" warnings in OS X builds, and I'm also wondering whether any
> of the recently reported problems with Snow Leopard might trace to our
> use of an API that Apple has been deprecating since 2003. Hence,
> attached is a patch that replaces our use of the DNSServiceDiscovery.h
> API with the more modern dns_sd.h API.
BTW, I cannot find any evidence that our old Bonjour code doesn't work
on Snow Leopard: it compiles, with just the same deprecation warning
as before, and the postmaster runs, and you can see the Bonjour
registration appear with something like 'dns-sd -B _postgresql' ---
and disappear again when the postmaster exits. I had guessed that
Jerry LeVan's DNS issues might be somehow related to this code, but
apparently not.
So I'm inclined to change the code now that we've got a replacement
that doesn't cause the deprecation warning, but there doesn't seem to
be any need to back-patch it.
I will also see about adding a separate boolean control GUC, as
discussed.
regards, tom lane
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Should we inject some sort of compile-time rejection of the
> compatibility API? Like, say
> #ifdef AVAHI_SERVICE_COOKIE
> #error The Avahi implementation is incompatible
> #endif
> inside some #ifdef BONJOUR block?
I looked into this and couldn't find any obvious candidate symbol to
test for. I'm not that excited about throwing roadblocks in the way
of somebody who wants to try it, anyway --- my feeling is more like
"if it breaks you get to keep both pieces".
regards, tom lane
Tom Lane wrote: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > Should we inject some sort of compile-time rejection of the > > compatibility API? Like, say > > > #ifdef AVAHI_SERVICE_COOKIE > > #error The Avahi implementation is incompatible > > #endif > > > inside some #ifdef BONJOUR block? > > I looked into this and couldn't find any obvious candidate symbol to > test for. I'm not that excited about throwing roadblocks in the way > of somebody who wants to try it, anyway --- my feeling is more like > "if it breaks you get to keep both pieces". Agreed -- that seems reasonable considering that one needs to explicitely pass --enable-bonjour anyway. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.