Differences between revisions 18 and 19
Revision 18 as of 2008-06-23 16:41:40
Size: 4488
Editor: FelixFrank
Comment: snippet
Revision 19 as of 2008-06-25 16:25:29
Size: 4695
Editor: FelixFrank
Comment: question on InitializeCBService()
Deletions are marked like this. Additions are marked like this.
Line 76: Line 76:
 * {o} venus/fs.c:155
  * why is InitializeCBService() protected by `#ifdef AFS_RXOSD_SUPPORT`?
  * ListOffline() depends on it, but the command definition is not protected. Which one's the error here?

TableOfContents

SVN test repository

(!) Original openafs-1.4.7 sources were checked into revision 2.

The URL is file://localhost/afs/ifh.de/user/f/ffrank/osd_svn. ACLs currently allow user ffrank only.

Creating the patch

{{{ cd trunk/openafs-1.4.7 svn diff -r 2:HEAD -x -b >| ../full-patch }}} The -x -b option causes svn to ignore whitespace near eol (like diff -w).

iokaste crash recovery 2008-04-17

  • syslog unconclusive
  • AFS logs held no hints, either
    • update 2008-05-26: the problems with fs replaceosd were likely related to version mismatches between io and iokaste (fs != rxosd)

patch analysis

general problems and remarks

  • OSD awareness is not strictly protected by #ifdefs. VolSplit for example sports osd flags wether compiled in or not.

    • should/can the xg files respect definition of AFS_RXOSD_SUPPORT?
    • is it feasible to engross the declaration of, say, UV_CreateVolume2 in a way that uses OSD-specific parameter(s) only if enabled?

      • inserting alternative calls will be fairly easy on the other hand
  • can fs fidvnode be done without OSD support enabled?

  • the fid* subcommands to fs sort of pollute its range of subcommands, fs help has gone quite long

    • could not a fid be specified in place of the file name in any case? or is ambiguity too great an issue?
      • in the latter case, ambiguity could be caught and reported as an error, forcing the user to specify a parameter to choose
  • AFSFetchStatus.SyncCount is still used in WINNT/afsd/cm_[sd]cache.c

    • however, these are only initializations, the values are apparently never used, so it should be OK to erase/replace the initializations? /!\ TODO /!\

unidentified snippets

  • {o} src/viced/afsfileprocs.c:2472

    • Hartmut inserted an #ifdef AFS_NT40_ENV protecting two functions and some definitions. Probably a fix.

  • {o} src/viced/afsfileprocs.c:229

    • this function is declared here in Hartmuts code. not yet sure why. probably because of the use introduced into common_StoreData64. not yet sure what kind of fixes are hidden here and to what extent they're required. Watch patches for calls to common_StoreData64!

  • {o} src/viced/afsfileprocs.c:3715

    • as mentioned above, the function common_StoreData64 was patched quite a bit. might have to ask Hartmut.

  • {o} src/viced/afsfileprocs.c:4858

    • looks like a bug fix
  • {o} src/viced/afsfileprocs.c:5283

    • this line moved inside the following if block. bug fix?
  • /!\ src/viced/afsfileprocs.c:7379

  • {o} fs.c

    • Hartmut seems to pass NULL instead of 0 to cmd_CreateSyntax conventionally. Where the hell is the declaration for this? Looks like a style fix/correction.

  • {o} vol/vnode.c

    • This new log message is more compelling. I'd have included this in the fs_listvnode patch but it looks like a general fix and should go into a general patch as that.
  • {o} fsint/afsint.xg:136

    • Looks like a common sense fix to exclude negative values here.
  • {o} general question: why does fs vnode not work without OSD support in code.

    • have i missed something?
    • are vnodes fundamentally different with Hartmut's code? this couldn't be - it's interoperable w/ vanilla openafs when not build with --object-storage
  • {o} volser/vsutils.c:469

    • this also has a fix-ish look to it
  • {o} venus/kdump.c

    • not understood yet what this does at all
  • {o} vol/fssync.c

    • exactly what is that "good idea" and what do the other changes do?
  • {o} venus/fs.c:155

    • why is InitializeCBService() protected by #ifdef AFS_RXOSD_SUPPORT?

    • ListOffline() depends on it, but the command definition is not protected. Which one's the error here?

isolating vos splitvol

  • sought through the patch, identifying places where
    • the client command was changed
    • the server was given the extra RPC
  • used vimdiff to introduce changes into

    • rxgen file
    • vos implementation
    • volser implementation

    and added the new vol_split.c code file, therefore

    • also updating volser's Makefile.in

  • Conclusion

    It turns out that the volsplit functionality depends on the reverse lookup functionality, also new with the full patch. Thus isolating volsplit failed at this stage and must be postponed until reverse lookups have been isolated.

    Addendum

    Apparently, InverseLookup is only used by vos splitvol, so both possibly belong in the same sub patch. For better

AfsOsd/Logs (last edited 2009-02-16 11:05:51 by FelixFrank)