From: Andrew Deason Date: Fri, 5 Nov 2010 21:48:28 +0000 (-0500) Subject: vol: Do not give back not-checked-out vols X-Git-Tag: openafs-devel-1_7_1~1291 X-Git-Url: https://git.openafs.org/?p=openafs.git;a=commitdiff_plain;h=e890f090e11d09b6e6b929642cbd92a56fb6e66e vol: Do not give back not-checked-out vols VAttachVolumeByName_r has logic to give back a volume over FSSYNC if we checked out a volume but failed to attach it for whatever reason. However, the logic used for determining if the volume was checked out or not is a bit inaccurate (even moreso than the comments imply), potentially causing us to VOL_ON volumes that don't exist at all. Instead of trying to guess based on various conditions whether or not we checked out the volume, keep track of a variable that is only set when we actually checkout the volume from the fileserver. Then only give back the volume if it is set. Change-Id: I03197eca3e1a31a4b9566552eb9032fdc7cc5909 Reviewed-on: http://gerrit.openafs.org/3274 Tested-by: Andrew Deason Reviewed-by: Derrick Brashear Tested-by: Derrick Brashear --- diff --git a/src/vol/volume.c b/src/vol/volume.c index 6686bf4..d1d6f9f 100644 --- a/src/vol/volume.c +++ b/src/vol/volume.c @@ -175,7 +175,7 @@ extern void *calloc(), *realloc(); /* Forward declarations */ static Volume *attach2(Error * ec, VolId volumeId, char *path, struct DiskPartition64 *partp, Volume * vp, - int isbusy, int mode); + int isbusy, int mode, int *acheckedOut); static void ReallyFreeVolume(Volume * vp); #ifdef AFS_DEMAND_ATTACH_FS static void FreeVolume(Volume * vp); @@ -2239,6 +2239,7 @@ VAttachVolumeByName_r(Error * ec, char *partition, char *name, int mode) char path[64]; int isbusy = 0; VolId volumeId; + int checkedOut; #ifdef AFS_DEMAND_ATTACH_FS VolumeStats stats_save; Volume *svp = NULL; @@ -2403,7 +2404,7 @@ VAttachVolumeByName_r(Error * ec, char *partition, char *name, int mode) /* attach2 is entered without any locks, and returns * with vol_glock_mutex held */ - vp = attach2(ec, volumeId, path, partp, vp, isbusy, mode); + vp = attach2(ec, volumeId, path, partp, vp, isbusy, mode, &checkedOut); if (VCanUseFSSYNC() && vp) { #ifdef AFS_DEMAND_ATTACH_FS @@ -2431,20 +2432,11 @@ VAttachVolumeByName_r(Error * ec, char *partition, char *name, int mode) vp->needsPutBack = 1; #endif /* !AFS_DEMAND_ATTACH_FS */ } - /* OK, there's a problem here, but one that I don't know how to - * fix right now, and that I don't think should arise often. - * Basically, we should only put back this volume to the server if - * it was given to us by the server, but since we don't have a vp, - * we can't run the VolumeWriteable function to find out as we do - * above when computing vp->needsPutBack. So we send it back, but - * there's a path in VAttachVolume on the server which may abort - * if this volume doesn't have a header. Should be pretty rare - * for all of that to happen, but if it does, probably the right - * fix is for the server to allow the return of readonly volumes - * that it doesn't think are really checked out. */ #ifdef FSSYNC_BUILD_CLIENT - if (VCanUseFSSYNC() && vp == NULL && - mode != V_SECRETLY && mode != V_PEEK) { + /* Only give back the vol to the fileserver if we checked it out; attach2 + * will set checkedOut only if we successfully checked it out from the + * fileserver. */ + if (VCanUseFSSYNC() && vp == NULL && checkedOut) { #ifdef AFS_DEMAND_ATTACH_FS /* If we couldn't attach but we scheduled a salvage, we already @@ -2532,6 +2524,7 @@ VAttachVolumeByVp_r(Error * ec, Volume * vp, int mode) VolId volumeId; Volume * nvp = NULL; VolumeStats stats_save; + int checkedOut; *ec = 0; /* volume utility should never call AttachByVp */ @@ -2599,7 +2592,7 @@ VAttachVolumeByVp_r(Error * ec, Volume * vp, int mode) * * NOTE: attach2 is entered without any locks, and returns * with vol_glock_mutex held */ - vp = attach2(ec, volumeId, path, partp, vp, isbusy, mode); + vp = attach2(ec, volumeId, path, partp, vp, isbusy, mode, &checkedOut); /* * the event that an error was encountered, or @@ -2722,6 +2715,9 @@ VUnlockVolume(Volume *vp) * we don't try to lock the vol, or check it out from * FSSYNC or anything like that; 0 otherwise, for 'normal' * operation + * @param[out] acheckedOut If we successfully checked-out the volume from + * the fileserver (if we needed to), this is set + * to 1, otherwise it is untouched. * * @note As part of DAFS volume attachment, the volume header may be either * read- or write-locked to ensure mutual exclusion of certain volume @@ -2734,7 +2730,7 @@ VUnlockVolume(Volume *vp) */ static void attach_volume_header(Error *ec, Volume *vp, struct DiskPartition64 *partp, - int mode, int peek) + int mode, int peek, int *acheckedOut) { struct VolumeDiskHeader diskHeader; struct VolumeHeader header; @@ -2796,6 +2792,7 @@ attach_volume_header(Error *ec, Volume *vp, struct DiskPartition64 *partp, } goto done; } + *acheckedOut = 1; } #endif @@ -2961,7 +2958,7 @@ attach_volume_header(Error *ec, Volume *vp, struct DiskPartition64 *partp, #ifdef AFS_DEMAND_ATTACH_FS static void attach_check_vop(Error *ec, VolumeId volid, struct DiskPartition64 *partp, - Volume *vp) + Volume *vp, int *acheckedOut) { *ec = 0; @@ -2986,7 +2983,7 @@ attach_check_vop(Error *ec, VolumeId volid, struct DiskPartition64 *partp, /* attach header with peek=1 to avoid checking out the volume * or locking it; we just want the header info, we're not * messing with the volume itself at all */ - attach_volume_header(ec, vp, partp, V_PEEK, 1); + attach_volume_header(ec, vp, partp, V_PEEK, 1, acheckedOut); if (*ec) { return; } @@ -3058,6 +3055,9 @@ attach_check_vop(Error *ec, VolumeId volid, struct DiskPartition64 *partp, * otherwise. (see VVolOpSetVBusy_r) * @param[in] mode attachment mode such as V_VOLUPD, V_DUMP, etc (see * volume.h) + * @param[out] acheckedOut If we successfully checked-out the volume from + * the fileserver (if we needed to), this is set + * to 1, otherwise it is 0. * * @return pointer to the semi-attached volume pointer * @retval NULL an error occurred (check value of *ec) @@ -3069,7 +3069,7 @@ attach_check_vop(Error *ec, VolumeId volid, struct DiskPartition64 *partp, */ static Volume * attach2(Error * ec, VolId volumeId, char *path, struct DiskPartition64 *partp, - Volume * vp, int isbusy, int mode) + Volume * vp, int isbusy, int mode, int *acheckedOut) { /* have we read in the header successfully? */ int read_header = 0; @@ -3091,13 +3091,15 @@ attach2(Error * ec, VolId volumeId, char *path, struct DiskPartition64 *partp, vp->diskDataHandle = NULL; vp->linkHandle = NULL; + *acheckedOut = 0; + #ifdef AFS_DEMAND_ATTACH_FS - attach_check_vop(ec, volumeId, partp, vp); + attach_check_vop(ec, volumeId, partp, vp, acheckedOut); if (!*ec) { - attach_volume_header(ec, vp, partp, mode, 0); + attach_volume_header(ec, vp, partp, mode, 0, acheckedOut); } #else - attach_volume_header(ec, vp, partp, mode, 0); + attach_volume_header(ec, vp, partp, mode, 0, acheckedOut); #endif /* !AFS_DEMAND_ATTACH_FS */ if (*ec == VNOVOL) {