From a5eeee6d2ef87d95722950989a2d261efc72b959 Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Mon, 30 Apr 2018 17:30:56 -0500 Subject: [PATCH] LINUX: Return NULL for afs_linux_raw_open error Currently, afs_linux_raw_open (and by extension, LINUX's implementation of osi_UFSOpen) panic when they are unable to open the given cache file. To allow callers to handle the error more gracefully, change afs_linux_raw_open and osi_UFSOpen to return NULL on error, instead of panic'ing. Expand the language a little on the message logged while we're here, since the system might keep running after this situation now. This commit also changes all callers that did not already handle afs_linux_raw_open/osi_UFSOpen errors to assert on errors, so we still panic for all situations where we encounter an error. More graceful behavior will be added in future commits; this commit does not change the behavior on its own. An error on opening cache files can legitimately happen when there is corruption in the filesystem backing the disk cache, but possibly the easiest way to generate an error is if the filesystem has been forcibly mounted readonly (which can happen at runtime due to filesystem corruption or various hardware faults). The latter will generate -EROFS (-30) errors, but of course other errors are probably possible. Reviewed-on: https://gerrit.openafs.org/13045 Tested-by: BuildBot Reviewed-by: Benjamin Kaduk (cherry picked from commit f6af4a155d3636e8f812e40c7169dd8902ae64be) Change-Id: I5f9a71a96cd9c875f4b024562dfa714f9cc27e2f Reviewed-on: https://gerrit.openafs.org/13071 Reviewed-by: Mark Vitale Reviewed-by: Michael Meffie Reviewed-by: Benjamin Kaduk Tested-by: BuildBot --- src/afs/LINUX/osi_file.c | 18 +++++++++++++++--- src/afs/LINUX/osi_vnodeops.c | 2 ++ src/afs/VNOPS/afs_vnop_symlink.c | 1 + src/afs/VNOPS/afs_vnop_write.c | 1 + src/afs/afs_buffer.c | 3 +++ src/afs/afs_dcache.c | 6 ++++++ src/afs/afs_disconnected.c | 1 + src/afs/afs_fetchstore.c | 1 + src/afs/afs_init.c | 1 + src/afs/afs_segments.c | 2 ++ 10 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/afs/LINUX/osi_file.c b/src/afs/LINUX/osi_file.c index 0374c1cce..7069297a4 100644 --- a/src/afs/LINUX/osi_file.c +++ b/src/afs/LINUX/osi_file.c @@ -66,8 +66,12 @@ afs_linux_raw_open(afs_dcache_id_t *ainode) #else filp = dentry_open(dget(dp), mntget(afs_cacheMnt), O_RDWR); #endif - if (IS_ERR(filp)) - osi_Panic("Can't open file: %d\n", (int) PTR_ERR(filp)); + if (IS_ERR(filp)) { + afs_warn("afs: Cannot open cache file (code %d). Trying to continue, " + "but AFS accesses may return errors or panic the system\n", + (int) PTR_ERR(filp)); + filp = NULL; + } dput(dp); @@ -99,8 +103,16 @@ osi_UFSOpen(afs_dcache_id_t *ainode) memset(afile, 0, sizeof(struct osi_file)); afile->filp = afs_linux_raw_open(ainode); - afile->size = i_size_read(FILE_INODE(afile->filp)); + if (afile->filp) { + afile->size = i_size_read(FILE_INODE(afile->filp)); + } AFS_GLOCK(); + + if (!afile->filp) { + osi_FreeLargeSpace(afile); + return NULL; + } + afile->offset = 0; afile->proc = (int (*)())0; return (void *)afile; diff --git a/src/afs/LINUX/osi_vnodeops.c b/src/afs/LINUX/osi_vnodeops.c index 969a27b27..9a070f28f 100644 --- a/src/afs/LINUX/osi_vnodeops.c +++ b/src/afs/LINUX/osi_vnodeops.c @@ -2249,6 +2249,7 @@ afs_linux_readpage_fastpath(struct file *fp, struct page *pp, int *codep) /* XXX - I suspect we should be locking the inodes before we use them! */ AFS_GUNLOCK(); cacheFp = afs_linux_raw_open(&tdc->f.inode); + osi_Assert(cacheFp); if (!cacheFp->f_dentry->d_inode->i_mapping->a_ops->readpage) { cachefs_noreadpage = 1; AFS_GLOCK(); @@ -2677,6 +2678,7 @@ afs_linux_readpages(struct file *fp, struct address_space *mapping, AFS_GUNLOCK(); if (tdc) { cacheFp = afs_linux_raw_open(&tdc->f.inode); + osi_Assert(cacheFp); if (!cacheFp->f_dentry->d_inode->i_mapping->a_ops->readpage) { cachefs_noreadpage = 1; goto out; diff --git a/src/afs/VNOPS/afs_vnop_symlink.c b/src/afs/VNOPS/afs_vnop_symlink.c index 1dd461125..02ba3b43a 100644 --- a/src/afs/VNOPS/afs_vnop_symlink.c +++ b/src/afs/VNOPS/afs_vnop_symlink.c @@ -57,6 +57,7 @@ afs_DisconCreateSymlink(struct vcache *avc, char *aname, afs_AdjustSize(tdc, len); tdc->validPos = len; tfile = afs_CFileOpen(&tdc->f.inode); + osi_Assert(tfile); afs_CFileWrite(tfile, 0, aname, len); afs_CFileClose(tfile); ReleaseWriteLock(&tdc->lock); diff --git a/src/afs/VNOPS/afs_vnop_write.c b/src/afs/VNOPS/afs_vnop_write.c index be6c63adf..c7c56dc15 100644 --- a/src/afs/VNOPS/afs_vnop_write.c +++ b/src/afs/VNOPS/afs_vnop_write.c @@ -335,6 +335,7 @@ afs_write(struct vcache *avc, struct uio *auio, int aio, error = code; ZapDCE(tdc); /* bad data */ cfile = afs_CFileOpen(&tdc->f.inode); + osi_Assert(cfile); afs_CFileTruncate(cfile, 0); afs_CFileClose(cfile); afs_AdjustSize(tdc, 0); /* sets f.chunkSize to 0 */ diff --git a/src/afs/afs_buffer.c b/src/afs/afs_buffer.c index 2220ae301..8a1141d53 100644 --- a/src/afs/afs_buffer.c +++ b/src/afs/afs_buffer.c @@ -239,6 +239,7 @@ DRead(struct dcache *adc, int page, struct DirBuffer *entry) return ENOENT; /* past the end */ } tfile = afs_CFileOpen(&adc->f.inode); + osi_Assert(tfile); code = afs_CFileRead(tfile, tb->page * AFS_BUFFER_PAGESIZE, tb->data, AFS_BUFFER_PAGESIZE); @@ -372,6 +373,7 @@ afs_newslot(struct dcache *adc, afs_int32 apage, struct buffer *lp) if (lp->dirty) { /* see DFlush for rationale for not getting and locking the dcache */ tfile = afs_CFileOpen(&lp->inode); + osi_Assert(tfile); afs_CFileWrite(tfile, lp->page * AFS_BUFFER_PAGESIZE, lp->data, AFS_BUFFER_PAGESIZE); lp->dirty = 0; @@ -461,6 +463,7 @@ DFlushBuffer(struct buffer *ab) struct osi_file *tfile; tfile = afs_CFileOpen(&ab->inode); + osi_Assert(tfile); afs_CFileWrite(tfile, ab->page * AFS_BUFFER_PAGESIZE, ab->data, AFS_BUFFER_PAGESIZE); ab->dirty = 0; /* Clear the dirty flag */ diff --git a/src/afs/afs_dcache.c b/src/afs/afs_dcache.c index 60d2a3106..321e37f5a 100644 --- a/src/afs/afs_dcache.c +++ b/src/afs/afs_dcache.c @@ -1188,6 +1188,7 @@ afs_FreeDiscardedDCache(void) * Truncate the element to reclaim its space */ tfile = afs_CFileOpen(&tdc->f.inode); + osi_Assert(tfile); afs_CFileTruncate(tfile, 0); afs_CFileClose(tfile); afs_AdjustSize(tdc, 0); @@ -1619,6 +1620,7 @@ afs_AllocDiscardDSlot(afs_int32 lock) if ((lock & 2)) { /* Truncate the chunk so zeroes get filled properly */ file = afs_CFileOpen(&tdc->f.inode); + osi_Assert(file); afs_CFileTruncate(file, 0); afs_CFileClose(file); afs_AdjustSize(tdc, 0); @@ -2076,6 +2078,7 @@ afs_GetDCache(struct vcache *avc, afs_size_t abyte, /* no data in file to read at this position */ UpgradeSToWLock(&tdc->lock, 607); file = afs_CFileOpen(&tdc->f.inode); + osi_Assert(file); afs_CFileTruncate(file, 0); afs_CFileClose(file); afs_AdjustSize(tdc, 0); @@ -2295,6 +2298,7 @@ afs_GetDCache(struct vcache *avc, afs_size_t abyte, */ DZap(tdc); /* pages in cache may be old */ file = afs_CFileOpen(&tdc->f.inode); + osi_Assert(file); afs_RemoveVCB(&avc->f.fid); tdc->f.states |= DWriting; tdc->dflags |= DFFetching; @@ -3616,6 +3620,8 @@ afs_MakeShadowDir(struct vcache *avc, struct dcache *adc) /* Open the files. */ tfile_src = afs_CFileOpen(&adc->f.inode); tfile_dst = afs_CFileOpen(&new_dc->f.inode); + osi_Assert(tfile_src); + osi_Assert(tfile_dst); /* And now copy dir dcache data into this dcache, * 4k at a time. diff --git a/src/afs/afs_disconnected.c b/src/afs/afs_disconnected.c index 957bf9394..fd6abe2c5 100644 --- a/src/afs/afs_disconnected.c +++ b/src/afs/afs_disconnected.c @@ -707,6 +707,7 @@ afs_ProcessOpCreate(struct vcache *avc, struct vrequest *areq, } ObtainReadLock(&tdc->lock); tfile = afs_CFileOpen(&tdc->f.inode); + osi_Assert(tfile); code = afs_CFileRead(tfile, 0, ttargetName, tlen); ttargetName[tlen-1] = '\0'; afs_CFileClose(tfile); diff --git a/src/afs/afs_fetchstore.c b/src/afs/afs_fetchstore.c index df31f4348..c9a73a81b 100644 --- a/src/afs/afs_fetchstore.c +++ b/src/afs/afs_fetchstore.c @@ -270,6 +270,7 @@ afs_GenericStoreProc(struct storeOps *ops, void *rock, size = tdc->f.chunkBytes; tfile = afs_CFileOpen(&tdc->f.inode); + osi_Assert(tfile); while ( size > 0 ) { code = (*ops->prepare)(rock, size, &tlen); diff --git a/src/afs/afs_init.c b/src/afs/afs_init.c index acc7bef66..6262f928a 100644 --- a/src/afs/afs_init.c +++ b/src/afs/afs_init.c @@ -330,6 +330,7 @@ afs_InitVolumeInfo(char *afile) if (code) return code; tfile = afs_CFileOpen(&volumeInode); + osi_Assert(tfile); afs_CFileTruncate(tfile, 0); afs_CFileClose(tfile); return 0; diff --git a/src/afs/afs_segments.c b/src/afs/afs_segments.c index 9992a84a5..5a32da4b0 100644 --- a/src/afs/afs_segments.c +++ b/src/afs/afs_segments.c @@ -651,6 +651,7 @@ afs_ExtendSegments(struct vcache *avc, afs_size_t alen, struct vrequest *areq) toAdd = AFS_CHUNKTOSIZE(tdc->f.chunk) - offset; } tfile = afs_CFileOpen(&tdc->f.inode); + osi_Assert(tfile); while(tdc->validPos < avc->f.m.Length + toAdd) { afs_size_t towrite; @@ -819,6 +820,7 @@ afs_TruncateAllSegments(struct vcache *avc, afs_size_t alen, UpgradeSToWLock(&tdc->lock, 673); tdc->f.states |= DWriting; tfile = afs_CFileOpen(&tdc->f.inode); + osi_Assert(tfile); afs_CFileTruncate(tfile, (afs_int32)newSize); afs_CFileClose(tfile); afs_AdjustSize(tdc, (afs_int32)newSize); -- 2.39.5