DAFS: Free header on partially-attached vol salv
authorAndrew Deason <adeason@sinenomine.net>
Thu, 4 Oct 2012 19:15:34 +0000 (14:15 -0500)
committerDaria Brashear <shadow@your-file-system.com>
Wed, 21 Jan 2015 15:39:37 +0000 (10:39 -0500)
commit82a30ed17c9cf56e319d5e13ca2e18d82f8b239c
treeec29526390386124ce92afb80b5f51f6fe351e0f
parent71072c2bb373a6ae5edec91884985c3cfc478147
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