On Fri, 2019-12-13 at 11:23 +0800, Ian Kent wrote:
> On Fri, 2019-12-13 at 10:18 +0800, Ian Kent wrote:
> > On Thu, 2019-12-12 at 10:07 +0800, Ian Kent wrote:
> > > On Wed, 2019-12-11 at 05:54 -0500, Doug Nazar wrote:
> > > > On 2019-12-09 23:49, Ian Kent wrote:
> > > > > I also need to work out why you don't get caught by the
> > > > > negative
> > > > > map entry check that's meant to prevent multiple retries for
> > > > > a
> > > > > failing map entry for a configured time.
> > > >
> > > > Sorry, I should have been more explicit. The several minute
> > > > wait
> > > > was
> > > > to
> > > > exceed the negative cache timeout. That part was working fine.
> > > >
> > > > > > And even the entry delete below it should be ok because it
> > > > > > will
> > > > > > just lookup (aka. run the program map again to get the map
> > > > > > entry)
> > > > > > and then update the multi-mount during the entry parse.
> > > > > >
> > > > > > So while the change above isn't strictly the way this
> > > > > > should
> > > > > > be
> > > > > > handled it probably should be ok.
> > > > > >
> > > > > > I haven't worked out how to handle it immediately after the
> > > > > > fail
> > > > > > just yet but the change above probably should be kept as
> > > > > > part
> > > > > > of
> > > > > > that as well, not sure yet.
> > > > > >
> > > > > > Ian
> > > >
> > > > I did that based on my greps, that seems to be a fairly common
> > > > check.
> > > > However, it kind of felt wrong, in the sense that the 2nd
> > > > attempt,
> > > > shouldn't depend on any previous status. I was just having
> > > > trouble
> > > > trying to figure out the lifetime rules for the various
> > > > fields/states.
> > >
> > > Yes, the root of a multi-mount has all sorts of special case
> > > handling
> > > in a number of places.
> >
> > I think this is what should be done.
> >
> > Could you check and see if it behaves as required please, of course
> > the CHANGELOG hunk needs to be dropped.
> >
> > autofs-5.1.6 - fix program map multi-mount lookup after mount fail
> >
> > From: Ian Kent <raven@xxxxxxxxxx>
> >
> > For the case of a singleton multi-mount program map lookup
> > following
> > a mount fail (and the negative timeout has passed) the lookup key
> > is
> > what's expected for an indirect map key but the the root offset map
> > entry already exists. This causes autofs to think it has an
> > incorrect
> > lookup key and it fails the lookup when it should take the
> > opptunity
> > to delete and update the cache entry since it's not actually in use
> > yet.
> >
> > Also, if a check is done to see if the lookup is for the root
> > offset,
> > deleting the entry fails because it contains an offset so it
> > doesn't
> > get updated.
>
> Actually that's not entirely accurate.
>
> Not deleting the entry (and it's offsets) should result in a
> "duplcate
> offset detected for key" (spelling needs to be corrected) error but I
> think the check is not quite right so that doesn't happen.
>
> So the change here is meant to take account of that being fixed at
> some point.
That's not quite right either, it would result in a warning and the
offset being updated. But the owner of the multi-mount can get out
of sync with the offsets that have been added (which isn't a problem
as long as the offsets can be deleted on expire) so the right thing
to do is clean it out and start a fresh so the most up to date map
entry is being used.
I'll update the description in the patch.
>
> > Signed-off-by: Ian Kent <raven@xxxxxxxxxx>
> > ---
> > CHANGELOG | 1 +
> > modules/lookup_program.c | 7 +++++--
> > 2 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/CHANGELOG b/CHANGELOG
> > index 981a0333..3c784d34 100644
> > --- a/CHANGELOG
> > +++ b/CHANGELOG
> > @@ -1,6 +1,7 @@
> > xx/xx/2020 autofs-5.1.7
> > - make bind mounts propagation slave by default.
> > - update ldap READMEs and schema definitions.
> > +- fix program map multi-mount lookup after mount fail.
> >
> > 07/10/2019 autofs-5.1.6
> > - support strictexpire mount option.
> > diff --git a/modules/lookup_program.c b/modules/lookup_program.c
> > index fcb1af74..ca209488 100644
> > --- a/modules/lookup_program.c
> > +++ b/modules/lookup_program.c
> > @@ -646,7 +646,7 @@ int lookup_mount(struct autofs_point *ap, const
> > char *name, int name_len, void *
> > name_len, ent, ctxt->parse->context);
> > goto out_free;
> > } else {
> > - if (me->multi) {
> > + if (me->multi && me->multi != me) {
> > cache_unlock(mc);
> > warn(ap->logopt, MODPREFIX
> > "unexpected lookup for active
> > multi-mount"
> > @@ -656,8 +656,11 @@ int lookup_mount(struct autofs_point *ap,
> > const
> > char *name, int name_len, void *
> > cache_unlock(mc);
> > cache_writelock(mc);
> > me = cache_lookup_distinct(mc, name);
> > - if (me)
> > + if (me) {
> > + if (me->multi)
> > + cache_delete_offset_list(mc,
> > name);
> > cache_delete(mc, name);
> > + }
> > cache_unlock(mc);
> > }
> > }
> >
