OpenAFS Master Repository branch, master, updated. openafs-devel-1_5_76-4557-g82a30ed

[Gerrit Code Review]

The following commit has been merged in the master branch:
commit 82a30ed17c9cf56e319d5e13ca2e18d82f8b239c
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Thu Oct 4 14:15:34 2012 -0500

    DAFS: Free header on partially-attached vol salv
    
    When we VRequestSalvage_r a volume, normally the header is freed when
    the volume goes offline. This happens when we VOfflineForSalvage_r,
    either via VCheckSalvage when nUsers drops to 0, or in
    VRequestSalvage_r itself if nUsers is already 0. We cannot free the
    header under normal circumstances, since someone else may have a ref
    on vp, which implies that the vol header object is okay to use.
    
    However, for VOL_SALVAGE_NO_OFFLINE, we skip all of that. For
    VOL_SALVAGE_NO_OFFLINE, the volume has only been partially attached,
    so it does not go through the full offlining process, so we don't ever
    hit the normal VPutVolume_r handlers etc. So, in the current code, we
    don't free the header. But our nUsers drops to 0 anyway, and when
    nUsers is 0, our header is supposed to be on the LRU (if we have one).
    "oops"
    
    Rectify this by freeing the volume header when VOL_SALVAGE_NO_OFFLINE
    is set. Add some comments to try to be very clear about what's going
    on.
    
    Note that similar behavior was removed in commit
    4552dc552687267fce3c7a6a9c7f4a1e9395c8e5 via a similar flag called
    VOL_SALVAGE_INVALIDATE_HEADER. I believe now that this is the same
    scenario that VOL_SALVAGE_INVALIDATE_HEADER was trying to solve.
    However, VOL_SALVAGE_INVALIDATE_HEADER was not always used correctly,
    and its purpose was not really adequately explained, which contributed
    to the idea that its very existence was buggy.
    
    Previously, when VOL_SALVAGE_INVALIDATE_HEADER existed, it was used
    incorrectly in the VRequestSalvage_r calls in GetVolume,
    VForceOffline_r, and VAllocBitmapEntry_r. All of these call sites
    could have a vp with other references held on it, and so invalidating
    the header there can cause segfaults when the header is freed. So
    ideally, commit 4552dc552687267fce3c7a6a9c7f4a1e9395c8e5 would have
    just removed the flag from those call sites.
    
    This change effectively restores the behavior that
    VOL_SALVAGE_INVALIDATE_HEADER provided. But no new flags are gained,
    since this behavior is what we want for the VOL_SALVAGE_NO_OFFLINE
    flag. This is not a coincidence; for the 'normal' case, we will free
    the header whenever we offline the volume. But for the 'do not
    offline' case, obviously that will never happen, so we need to do it
    separately. So, these two flags are really the same thing.
    
    Change-Id: Ia369850a33c0e781a3ab2f22017b8559410ff8bf
    Reviewed-on: http://gerrit.openafs.org/8204
    Reviewed-by: Andrew Deason <adeason@sinenomine.net>
    Tested-by: BuildBot <buildbot@rampaginggeek.com>
    Reviewed-by: Daria Brashear <shadow@your-file-system.com>

 src/vol/volume.c |   31 +++++++++++++++++++++++++++++++
 1 files changed, 31 insertions(+), 0 deletions(-)

-- 
OpenAFS Master Repository