[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-ha-dev
Subject:    Re: [Linux-ha-dev] [resource-agents] exportfs change to support wildcard exports (#45)
From:       Lars Ellenberg <lars.ellenberg () linbit ! com>
Date:       2012-01-31 20:21:48
Message-ID: 20120131202148.GQ31519 () barkeeper1-xen ! linbit
[Download RAW message or body]

On Tue, Jan 31, 2012 at 04:31:09PM +0100, Dejan Muhamedagic wrote:
> Hey Lars,
> 
> On Sat, Jan 21, 2012 at 05:52:28PM +0100, Lars Ellenberg wrote:
> > On Fri, Jan 20, 2012 at 04:18:02AM +0100, Dejan Muhamedagic wrote:
> > > Hi Lars,
> > > 
> > > On Tue, Dec 13, 2011 at 10:55:57PM +0100, Lars Ellenberg wrote:
> > > > Taking this to the mailing list to give it a wider audience.
> > > > 
> > > > On Tue, Dec 13, 2011 at 09:59:11AM -0800, acqant wrote:
> > > > > --- a/heartbeat/exportfs
> > > > > +++ b/heartbeat/exportfs
> > > > > @@ -181,9 +181,11 @@ END
> > > > > 
> > > > >  exportfs_monitor ()
> > > > >  {
> > > > > +       local clientspec_re
> > > > >        # "grep -z" matches across newlines, which is necessary as
> > > > >        # exportfs output wraps lines for long export directory names
> > > > > -       exportfs | grep -zqs "${OCF_RESKEY_directory}[[:space:]]*${OCF_RESKEY_clientspec}"
> > > > > +       clientspec_re=`echo ${OCF_RESKEY_clientspec} | sed 's/*/[*]/'`
> > > > > +       exportfs | grep -zqs "${OCF_RESKEY_directory}[[:space:]]*${clientspec_re}"
> > > > > 
> > > > >  #Adapt grep status code to OCF return code
> > > > >        case $? in
> > > > 
> > > > > Or you can view, comment on it, or merge it online at:
> > > > > 
> > > > >   https://github.com/ClusterLabs/resource-agents/pull/45
> > > > 
> > > > Thinking about it, I've got a problem with this whole grepping thing here.
> > > > 
> > > > grep -z does not just "match accross newlines",
> > > > it matches records separated by NUL in that file
> > > > (which would be very unexpected).
> > > > 
> > > > So it matches the full file.
> > > > 
> > > > No anchors on the regex whatsoever.
> > > > 
> > > > Client spec will typically have dots in them,
> > > > both hostname and ip address form,
> > > > which would also need to be matched literally.
> > > > 
> > > > If you have two exports /bar and /foo/bar,
> > > > to the same (or similar enough, see above) client spec,
> > > > the grep for /bar will also match on /foo/bar.
> > > > 
> > > > The mount point may also contain dots or other special chars.
> > > > 
> > > > I don't like that, really :(
> > > > 
> > > > Suggestion:
> > > > 
> > > > Why not "unwrap" the exportfs output first,
> > > > so we get one record per line,
> > > > then match literal (grep -F)?
> > > 
> > > That sounds good to me. I wonder if the author of the patch is
> > > subscribed here.
> > 
> > I first commented on his pull request on github, then basically
> > forwarded what I said there slightly edited to the list here.
> > 
> > > > That should cover most of these issues
> > > > (appart from multiple consecutive blanks, or tabs, or newlines,
> > > > in the mount point... would that even be "legal"?)
> > > 
> > > I don't think we'd need to support that.
> > > 
> > > > exportfs | fmt -w 1000 -t -u |
> > > > 	grep -x -F "${OCF_RESKEY_directory} ${OCF_RESKEY_clientspec}"
> > > > 
> > > > I'm not completely sure about the fmt trick:
> > > > Availability should not be a problem (coreutils).
> > > > But, is the exportfs output and fmt behaviour really consistent enough
> > > > to have that work on all platforms?
> > > 
> > > The original usage was probably just "fmt -1000" but that won't
> > > do. IIRC, fmt on AIX was just like that.
> > > 
> > > > But since both exportfs and fmt predate linux, maybe that just works?
> > > > 
> > > > If necessary, we can pull off the unwrap with sed in a more "controlled"
> > > > fashion as well.
> > > 
> > > That'd be preferable (and you're a sed expert :) awk or perl
> > > would also do.
> > 
> > As you wish ;-)
> > 
> > exportfs |
> > 	sed -e '$! N; s/\n[[:space:]]\+/ /; t; s/[[:space:]]\+\([^[:space:]]\+\)\(\n\|$\)/ \1\2/g; P;D;' |
> > 	grep -x -F "${OCF_RESKEY_directory} ${OCF_RESKEY_clientspec}"
> > 
> > (please someone double check that gobbledygook!)
> 
> I think that it looks OK. Can you please also push this to the
> repository.

Will do.
Just kick me, if it does not show up within the next few days.

> Cheers,
> 
> Dejan
> 
> P.S. It would be good if sed had an option such as -E for grep,
> so that one could reduce backslashism and at least slightly
> improve expression readability :)

Oh, it does: sed -r ...
only I'm just not sure how portable that is (supposed to be).
If someone can confirm that "sed -r" is supposed to be present
on all platforms we want to care about, we could use that.
Only it does not make much of a difference, imo:

- 	sed -e '$! N; s/\n[[:space:]]\+/ /; t; s/[[:space:]]\+\([^[:space:]]\+\)\(\n\|$\)/ \1\2/g; P;D;' |
+	sed -r -e '$! N; s/\n[[:space:]]+/ /; t; s/[[:space:]]+([^[:space:]]+)(\n|$)/ \1\2/g; P;D;' |

-- 
: Lars Ellenberg
: LINBIT | Your Way to High Availability
: DRBD/HA support and consulting http://www.linbit.com
_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev@lists.linux-ha.org
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/
[prev in list] [next in list] [prev in thread] [next in thread]