Re: [v9.1] sepgsql - userspace access vector cache

From: Kohei Kaigai <Kohei(dot)Kaigai(at)EMEA(dot)NEC(dot)COM>
To: Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>, Yeb Havinga <yebhavinga(at)gmail(dot)com>, PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [v9.1] sepgsql - userspace access vector cache
Date: 2011-08-19 15:26:01
Message-ID: D0C1A1F8BF513F469926E6C71461D9EC03F707@EX10MBX02.EU.NEC.COM
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> -----Original Message-----
> From: Robert Haas [mailto:robertmhaas(at)gmail(dot)com]
> Sent: 19. August 2011 15:55
> To: Tom Lane
> Cc: Kohei KaiGai; Kohei Kaigai; Yeb Havinga; PgHacker
> Subject: Re: [HACKERS] [v9.1] sepgsql - userspace access vector cache
>
> On Fri, Aug 19, 2011 at 10:31 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> > On Fri, Aug 19, 2011 at 10:20 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >>> Why not just:
> >>
> >>> SHLIB_LINK = -lselinux
> >>
> >> I wouldn't have any particular objection to that (although I think it's
> >> supposed to be += here).
> >
> > Oh, right.
> >
> >> I don't see that any of the other changes
> >> Kaigai proposed are helpful, though.
> >
> > I was coming to the same conclusion.  I sort of liked his idea of
> > sticking a conditional #error directive in the header files to make it
> > more clear why it was failing.  But on closer examination there's
> > really no benefit: it gets lost in a sea of other failures, and if you
> > have to look through the failure messages anyway you may as well
> > notice that the #include of <selinux/selinux.h> failed as anything
> > else.  So I think changing that line to link with libselinux
> > unconditionally is about as well as we can do.
> >
> >>> Similarly, in the case of xml2 we have:
> >>
> >>> SHLIB_LINK += $(filter -lxslt, $(LIBS)) $(filter -lxml2, $(LIBS))
> >>
> >>> For xslt, it probably makes sense to filter it out if it wasn't found,
> >>> because the code has ifdefs for USE_XSLT that do something sensible if
> >>> the library is not there.  But I fail to see what the point is of
> >>> filtering out xml2, because surely we're doomed if that's not there...
> >>> or am I confused?
> >>
> >> Hmm.  I think it's just that way to make the code look parallel for both
> >> libraries.  But I can see potential value in making -lxml2 unconditional
> >> --- as you say, that would result in a link failure instead of a
> >> silently broken library.
> >
> > Right.
>
> On further review, if the initial configure was done without
> --with-libxml, xml2 is doomed anyway. There's no value to changing
> anything there one way or the other, because it relies not only on
> symbols from the library, but also on symbols that are only defined
> when PostgreSQL itself is configured with xml support. In other
> words, there's no chance of undetected failure here.
>
> On the other hand, we've worked pretty hard to keep sepgsql at arm's
> length from the core server, so as it stands, it's quite easy to build
> a busted sepgsql module.
>
> This probably explains why no one's complained about this before, and
> I think the appropriate fix is to change just sepgsql. I would like
> to back-patch that (one-line) change into 9.1 as well, to eliminate
> this as a foot-gun for anyone who may be packaging that module for the
> first time.
>
I almost agree with this change.

One point I'm worrying about is a case when contrib/sepgsql is compiled
with older libselinux than minimum requirement. In this case, we may not
notice the broken module unless user tries to load it actually.
Is there a good idea to ensure compile failure when we try to build sepgsql
module when libselinux-2.0.98 or older was installed?

Of course, using --with-selinux on ./configure time is the best way...

Thanks,
--
NEC Europe Ltd, SAP Global Competence Center
KaiGai Kohei <kohei(dot)kaigai(at)emea(dot)nec(dot)com>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2011-08-19 15:26:04 Re: [v9.1] sepgsql - userspace access vector cache
Previous Message Robert Haas 2011-08-19 15:24:43 Re: the big picture for index-only scans