[OpenAFS-devel] VLDB flags inconsistencies?

[Benjamin Kaduk]

On Wed, 30 Jul 2014, Nathaniel W Filardo wrote:

> Hey all,
>
> As part of the cleanup in http://gerrit.openafs.org/#change,11331 I stumbled
> upon a few things that probably should go here for review (thanks to Ben for
> suggesting).

I see Jeff has already commented on the gerrit change requesting separate 
fixes for these two issues.  That seems like a good plan to me.

> src/volser/vos.c:2155 says "if (entry.serverFlags[j] != ITSROVOL)", but that
> field is a bitmask, so I think that that should be "if
> (!(entry.serverFlags[j] & ITSROVOL))" (but also renamed to VLSF_ROVOL,
> naturally) so that DeleteVolume will not skip RO volumes with other VLSF
> bits asserted (e.g., VLSF_DONTUSE or VLSF_NEWREPSITE).  Does that seem
> right?

This is in the branch that is looking up the entry in the VLDB to fill out 
partition and/or server informatin for the delete operation.  That said, I 
still can't think of a reason why we would want to ignore entries with 
other flags set, so it does look like changing "!=" to "&" is the right 
thing to do.

> src/volser/vsprocs.c:6217 says "entry.serverFlags[idx] = ITSBACKVOL;".
> ITSBACKVOL and VLSF_BACKVOL are almost never otherwise used in the codebase,
> and in particular the rest of the codebase seems to assume that the backup
> volume has VLSF_RWVOL asserted, not VLSF_BACKVOL.  I believe that
> CheckVolume is here trying to recreate a backup volume record that will get
> detected as such, and so should read "entry.serverFlags[idx] = VLSF_RWVOL;".
> Does that, too, seem right?

The revert of dcafea769a1cb70c7b1f8763ae4f7b7744b2a436 that already hit 
gerrit looks plausible to me.  I guess I should go comment on it.

> If these behavioral changes seem right, I'll move them out to their own
> commits, rather than leaving them in 11331.

Please do, thanks.

-Ben