[squid-dev] To make squid works in snap world.

[Gary Wang]

Thanks Alex for your quick response.


On Tue, Mar 14, 2017 at 11:49 PM, Alex Rousskov <
rousskov at measurement-factory.com> wrote:

> On 03/14/2017 08:44 AM, Gary Wang wrote:
> >     About the DEFAULT_STATEDIR,
> >     ...
> >     DEFS += -DDEFAULT_STATEDIR=\"$(localstatedir)/run/squid\"
> >     ...
> >     According to the above share memory namespace in snap world, it
> > couldn't help on this either.
>
> Here is how I [mis]interpret the above:
>
> 1. Squid has a DEFAULT_STATEDIR compile-time constant that provides the
> right control knob you need but uses the wrong hard-coded value.
>
> 2. You are adding a compile-time variable DEFAULT_IPC_PREFIX that does
> what DEFAULT_STATEDIR does but can be changed at ./configure time.
>
> This raises an obvious question: Why not make DEFAULT_STATEDIR
> configurable instead?  (If we do make it configurable, we may need to
> renamed it to STATEDIR, IPCDIR, or something like that but that is a
> separate and minor issue.)
>
>    A: Actually, DEFAULT_IPC_PREFIX only changes the shared memory file name
       not the file path. So the usage of these two options are different.
       Meanwhile, DEFAULT_STATEDIR only helps the case that the segment
name is path.
       It can't help this situation where segment name is not path e.g.
 shared memory is created
       in /dev/shm folder on Ubuntu.

http://bazaar.launchpad.net/~gary-wzl77/squid/ipc_prefix/view/head:/src/ipc/mem/Segment.cc#L229

       Another concern is that even though we make DEFAULT_STATEDIR
configurable instead of introducing new option, STATEDIR, IPCDIR is not
suitable as
end-user might specify the dir path for these options however the purpose
of DEFAULT_IPC_PREFIX
is only add prefix to shared memory file name

>
> AFAICT, DEFAULT_STATEDIR is also used for IPC communications in
> src/ipc/Port.cc. Your patch does not change that code. This means that
>
> * either you forgot to change src/ipc/Port.cc to honor the new
> DEFAULT_IPC_PREFIX
>
   A: I will change src/ipc/Port.cc in the next commit once we reach an
agreement
       on the usage of compiler option.


>
> * or src/ipc/Port.cc cannot use DEFAULT_IPC_PREFIX for some reason (and,
> hence, DEFAULT_IPC_PREFIX is the wrong name because the parameter does
> not apply to some of the IPC code).
>
> Alex.
>
>
>
> > === modified file 'configure.ac'
> > --- configure.ac      2017-01-28 03:54:15 +0000
> > +++ configure.ac      2017-03-02 03:51:46 +0000
> > @@ -311,6 +311,15 @@
> >  ])
> >  AC_SUBST(DEFAULT_SWAP_DIR)
> >
> > +DEFAULT_IPC_PREFIX=""
> > +AC_ARG_WITH(ipcprefix,
> > +  AS_HELP_STRING([--with-ipcprefix=NAME],
> > +    [Default namespace prefix for shared memory object for ipc
> communication.
> > +     Default is empty string and the share memory object name will be
> specified by squid. ]), [
> > +   DEFAULT_IPC_PREFIX="$withval"
> > +])
> > +AC_SUBST(DEFAULT_IPC_PREFIX)
> > +
> >  if test "$squid_cv_compiler" = "gcc"; then
> >    GCCVER=`$CC -v 2>&1 | awk '$2 ==  "version" {print $3}'`
> >    GCCVER2=`echo $GCCVER | awk '{print $1 * 100}'`
> >
> > === modified file 'src/ipc/Makefile.am'
> > --- src/ipc/Makefile.am       2017-01-01 00:16:45 +0000
> > +++ src/ipc/Makefile.am       2017-03-02 03:51:46 +0000
> > @@ -68,7 +68,7 @@
> >       mem/Segment.cc \
> >       mem/Segment.h
> >
> > -DEFS += -DDEFAULT_STATEDIR=\"$(localstatedir)/run/squid\"
> > +DEFS += -DDEFAULT_STATEDIR=\"$(localstatedir)/run/squid\"
> -DDEFAULT_IPC_PREFIX=\"$(DEFAULT_IPC_PREFIX)\"
> >
> >  install-data-local:
> >       $(mkinstalldirs) $(DESTDIR)$(localstatedir)/run/squid;
> >
> > === modified file 'src/ipc/mem/Segment.cc'
> > --- src/ipc/mem/Segment.cc    2017-01-01 00:16:45 +0000
> > +++ src/ipc/mem/Segment.cc    2017-03-02 03:51:46 +0000
> > @@ -30,6 +30,8 @@
> >  #include <unistd.h>
> >  #endif
> >
> > +#define DEFAULT_SQUID_IPC_PREFIX  "/" DEFAULT_IPC_PREFIX
> > +
> >  // test cases change this
> >  const char *Ipc::Mem::Segment::BasePath = DEFAULT_STATEDIR;
> >
> > @@ -229,9 +231,9 @@
> >      if (nameIsPath) {
> >          name.append(BasePath);
> >          if (name[name.size()-1] != '/')
> > -            name.append('/');
> > +            name.append(DEFAULT_SQUID_IPC_PREFIX);
> >      } else {
> > -        name.append('/');
> > +        name.append(DEFAULT_SQUID_IPC_PREFIX);
> >          name.append(service_name.c_str());
> >          name.append('-');
> >      }
>
>


-- 
Br
Gary.Wzl
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squid-cache.org/pipermail/squid-dev/attachments/20170315/594b3944/attachment-0001.html>