On Thu, 3 Feb 2005, Jeff Moyer wrote:
> ==> Regarding Re: [patch rfc] address regression in duplicate map entry
> order; Ian Kent <[EMAIL PROTECTED]> adds:
>
> raven> On Mon, 22 Nov 2004, Jeff Moyer wrote:
> >> ==> Regarding Re: [patch rfc] address regression in duplicate map entry
> >> order; [EMAIL PROTECTED] adds:
> >>
> raven> On Fri, 19 Nov 2004, Jeff Moyer wrote:
> >> >> Hi, Ian, list,
> >> >>
> >> >> In my proposed fix, I meant to say:
> >> >>
> >> >> We could expire the cache in lookup_ghost, and then go ahead and only
> >> >> insert new entries (instead of overwriting what was there).
> >> >>
> >> >> So, I've implemented this, though it isn't tested just yet. There
> >> were >> some other bugs.
> >> >>
> >> >> In lookup_multi, we iterate through each map's lookup_ghost function.
> >> >> The problem here is that you will end up doing the following:
> >> >>
> >> >> static int read_map(const char *root, struct lookup_context *ctxt) {
> >> >> char key[KEY_MAX_LEN + 1]; char mapent[MAPENT_MAX_LEN + 1]; char >>
> >> *mapname; FILE *f; int entry; time_t age = time(NULL); >>
> >> <=================
> >> >>
> >> >> mapname = alloca(strlen(ctxt->mapname) + 6); sprintf(mapname,
> >> "file:%s", >> ctxt->mapname);
> >> >>
> >> >> f = fopen(ctxt->mapname, "r"); if (!f) { error(MODPREFIX "could not
> >> open >> map file %s", ctxt->mapname); return 0; }
> >> >>
> >> >> while(1) { entry = read_one(f, key, mapent); if (entry) >>
> >> cache_update(root, key, mapent, age); <==========
> >> >>
> >> >> if (feof(f)) break; }
> >> >>
> >> >> fclose(f);
> >> >>
> >> >> /* Clean stale entries from the cache */ cache_clean(root, age); >>
> >> <=============
> >> >>
> >> >> return 1; }
> >> >>
> >> >> Notice the lines I've pointed out. We decide what the expiration age
> >> >> should be at the beginning of the function. We update entries and
> >> give >> them that "age." And finally, we expire all entries that were
> >> made >> prior to that. The problem arises in that we call the read_map
> >> routine >> for each map in the list in order. What will happen is that
> >> each map, >> as it is processed, will end up with a new value for age.
> >> Thus, we >> expire all of the work done by the previous read_maps! This
> >> is bad. To >> fix this, I've passed in the age field from the
> >> top-level.
> >> >>
> >> >> I've changed ldap's lookup_ghost routine (well, a function it calls,
> >> >> anyway) to use cache_insert_unique instead of cache_add.
> >> >>
> >> >> Please let me know what you think. If you only like some parts, let
> >> me >> know which ones and I'll break them out from the rest of the
> >> patch. >> BTW, this patch is applied on top of your map expiration
> >> changes. I can >> rediff the thing to any version of the code you want.
> >>
> raven> Understand the problem.
> >>
> raven> I've had a look at the patch and it looks good.
> >>
> raven> I had a slightly different idea though.
> >>
> raven> I think the cache_insert_unique might be a problem later as if
> raven> people add multiple attributes (map entries) for multi-mount keys,
> raven> instead of one attribute containing multiple mounts, in what they
> raven> think is "the LDAP way" it probably won't work. I can't say we've
> raven> seen this problem yet but I have been trying to allow for it in the
> raven> code.
> >>
> raven> The other thing that occured to me was that, together with passing
> raven> the time to keep things consistent, processing the maps in reverse
> raven> order in lookup_ghost of the multi map module would fix most of the
> raven> issues. Along with modifying cache_add to append to any existing
> raven> entries, rather than prepend.
> >>
> raven> I spotted something you missed in the yp module. Have a look at the
> raven> lookup_yp patch chunks in the attached map-entry-order patch.
> >>
> raven> Have a look at what I've tried to do and let me know what you
> raven> think. I've done some basic testing but not against the issue we are
> raven> trying to fix. So let me know how it goes.
> >>
> raven> Also, I found a couple of other bugs and have attached a couple of
> raven> other patches as well.
> >> Thanks, Ian! This all looks good to me, though I haven't been able to
> >> test. What version of the tree was this applied to?
>
> raven> That's a rather interesting question.
>
> raven> I originally made the patch against your 55.1 but couldn't get
> raven> rpmbuild to work OK under Gentoo so I added approiate patches to my
> raven> Gentoo portage install and built there. Only minor changes were
> raven> needed, which I believe would have been needed for the rpm as well.
>
> raven> Hopefully, it will also apply to earlier rpms without much trouble
> raven> as the changes appear to be mostly outside of the areas with lots of
> raven> change.
>
> In testing 4.1.4 beta 1, this problem still isn't addressed. Automount
> will use the last of the duplicate entries in an indirect map, precisely
> the opposite behaviour as before.
The last !!??
Bizare ....
At least the idea was good but .....
More things to fix.
Ian
_______________________________________________
autofs mailing list
[email protected]
http://linux.kernel.org/mailman/listinfo/autofs
