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.
>
> 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);
> }
> }
>
