Mailing List Archive: Re: [resource-agents] exportfs change to support wildcard exports (#45)

Re: [resource-agents] exportfs change to support wildcard exports (#45)

Dec 13, 2011, 1:55 PM

Post #1 of 5 (2807 views)

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 should cover most of these issues
(appart from multiple consecutive blanks, or tabs, or newlines,
in the mount point... would that even be "legal"?)

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?
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.

--
: 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/

Re: [resource-agents] exportfs change to support wildcard exports (#45) [ In reply to ]

dejan at suse

Jan 19, 2012, 7:18 PM

Post #2 of 5 (2715 views)

Permalink

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.

> 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.

Cheers,

Dejan

> --
> : 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/
_______________________________________________________
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/

Re: [resource-agents] exportfs change to support wildcard exports (#45) [ In reply to ]

lars.ellenberg at linbit

Jan 21, 2012, 8:52 AM

Post #3 of 5 (2709 views)

Permalink

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!)

--
: Lars Ellenberg
: LINBIT | Your Way to High Availability
: DRBD/HA support and consulting http://www.linbit.com

DRBD® and LINBIT® are registered trademarks of LINBIT, Austria.
_______________________________________________________
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/

Re: [resource-agents] exportfs change to support wildcard exports (#45) [ In reply to ]

dejan at suse

Jan 31, 2012, 7:31 AM

Post #4 of 5 (2712 views)

Permalink

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.

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 :)

> --
> : Lars Ellenberg
> : LINBIT | Your Way to High Availability
> : DRBD/HA support and consulting http://www.linbit.com
>
> DRBD® and LINBIT® are registered trademarks of LINBIT, Austria.
> _______________________________________________________
> 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/
_______________________________________________________
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/

Re: [resource-agents] exportfs change to support wildcard exports (#45) [ In reply to ]

lars.ellenberg at linbit

Jan 31, 2012, 12:21 PM

Post #5 of 5 (2711 views)

Permalink

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/