hi Phil, Pete,
Sorry for this breakage. I do not recollect this fix nor why I did it
this way anymore although I anticipated it would
bite us sometime when I fixed it ;).
I tested that replacing strrchr with strchr seems to fix both your use
case as well as the one that Michael reported
from earlier last year.
Index: src/client/sysint/sys-lookup.sm
===================================================================
RCS file: /anoncvs/pvfs2/src/client/sysint/sys-lookup.sm,v
retrieving revision 1.70
diff -u -r1.70 sys-lookup.sm
--- src/client/sysint/sys-lookup.sm 30 Aug 2007 00:13:42 -0000 1.70
+++ src/client/sysint/sys-lookup.sm 4 Jan 2008 07:40:56 -0000
@@ -360,7 +360,7 @@
slash_str = orig_pathname;
for (i = 0; i < cur_seg_index; i++) {
- slash_str = strrchr(slash_str, '/');
+ slash_str = strchr(slash_str, '/');
if (slash_str == NULL) {
break;
}
That said, I am not quite sure if something else may break due to this..:)
thanks,
Murali
On Jan 3, 2008 2:30 PM, Phil Carns <[EMAIL PROTECTED]> wrote:
>
>
> > Here is the diff (excerpt) that added the code from 1.68 to 1.69:
> >
> >
> > Index: src/client/sysint/sys-lookup.sm
> > --- src/client/sysint/sys-lookup.sm 13 Apr 2007 05:14:16 -0000 1.68
> > +++ src/client/sysint/sys-lookup.sm 20 Jun 2007 06:08:51 -0000
> > @@ -378,7 +378,16 @@
> > cur_seg->seg_name = strdup(cur_seg_name);
> > assert(cur_seg->seg_name);
> >
> > - seg_remaining = strstr(orig_pathname, cur_seg_name);
> > + slash_str = orig_pathname;
> > + for (i = 0; i < cur_seg_index; i++) {
> > + slash_str = strrchr(slash_str, '/');
> > + if (slash_str == NULL) {
> > + break;
> > + }
> > + slash_str++;
> > + }
> > + //seg_remaining = strstr(orig_pathname, cur_seg_name);
> > + seg_remaining = slash_str;
> > if (seg_remaining)
> > {
> > gossip_debug(GOSSIP_LOOKUP_DEBUG,
> >
> >
> > If I change the strrchr to strchr, my case works, but it breaks the
> > bugfix reported previously (but without segv). If I revert the
> > change, going back to the original strstr, my case also still works,
> > and the previous bug also reoccurs (with segv). I'm not
> > understanding this fully, but we seem to be getting closer. Or
> > maybe the problem is in the interpretation of seg_remaining in other
> > parts of the lookup code.
>
> In the case where you keep the logic that is currently in head but just
> change strrchr() to strchr(), what breaks in from the previously
> reported bug?
>
> I tried making that change and it looks like both cases work fine, but I
> may be missing something.
>
> Maybe it has something to do with the number of metadata servers? I am
> currently trying this with just one server.
>
> On a code cleanup note, it looks like maybe we could get rid of the
> "lookup_init" state and its associated LOOKUP_NCACHE_HIT_FASTPATH. It
> is trying to look up the entire path up front in the ncache before doing
> any work, but this isn't likely to gain anything since the ncache now
> only caches one segment at a time. I guess it could hit in the single
> segment lookup case, but that would happen anyway in the first iteration
> of the normal state machine. I guess this extra state isn't technically
> hurting anything, but it is redundant code that looks strange in the
> gossip output since it attempts to do ncache lookups with slashes in the
> name.
>
> -Phil
>
> _______________________________________________
> Pvfs2-developers mailing list
> [email protected]
> http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
>
_______________________________________________
Pvfs2-developers mailing list
[email protected]
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
