From: John Spray Date: Tue, 9 Dec 2014 19:52:58 +0000 (+0000) Subject: client: propagate flush errors to fclose/fsync X-Git-Tag: v0.91~47^2~2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=6fdf890355c6956957d1de89abdbbbe71d633f16;p=ceph.git client: propagate flush errors to fclose/fsync Ensure that all calls to _flush get an explicit context (rather than implicit PutInode used before), which tags any error onto the Inode object. Later, read the error from the Inode object in _release_fh and ensure all callers of _release_fh properly handle its return code. Signed-off-by: John Spray --- diff --git a/src/client/Client.cc b/src/client/Client.cc index fac0811aa5c2e..7951d0188f1f8 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -1983,26 +1983,35 @@ void Client::handle_client_reply(MClientReply *reply) mount_cond.Signal(); } - void Client::handle_osd_map(MOSDMap *m) { if (objecter->osdmap_full_flag()) { ldout(cct, 1) << __func__ << ": FULL: cancelling outstanding operations" << dendl; - - // For all inodes with a pending flush write op (i.e. one of the ones we - // will cancel), we've got to purge_set their data from ObjectCacher - // so that it doesn't re-issue the write in response to the ENOSPC error. - - // We can *only* do this if there is a file handle open, because otherwise - // there is nobody to surface the error code to, and we would be silently - // dropping data - // Cancel all outstanding ops with -ENOSPC: it is necessary to do this rather than blocking, // because otherwise when we fill up we potentially lock caps forever on files with // dirty pages, and we need to be able to release those caps to the MDS so that it can // delete files and free up space. epoch_t cancelled_epoch = objecter->op_cancel_writes(-ENOSPC); + // For all inodes with a pending flush write op (i.e. one of the ones we + // will cancel), we've got to purge_set their data from ObjectCacher + // so that it doesn't re-issue the write in response to the ENOSPC error. + // Fortunately since we're cancelling *everything*, we don't need to know + // which ops belong to which ObjectSet, we can just blow all the un-flushed + // cached data away and mark any dirty inodes' async_err field with -ENOSPC + // (i.e. we only need to know which inodes had outstanding ops, not the exact + // op-to-inode relation) + for (unordered_map::iterator i = inode_map.begin(); + i != inode_map.end(); i++) + { + Inode *inode = i->second; + if (inode->oset.dirty_or_tx) { + ldout(cct, 4) << __func__ << ": FULL: inode 0x" << std::hex << i->first << std::dec + << " has dirty objects, purging and setting ENOSPC" << dendl; + objectcacher->purge_set(&inode->oset); + inode->async_err = -ENOSPC; + } + } set_cap_epoch_barrier(cancelled_epoch); } @@ -3067,37 +3076,16 @@ void Client::_release(Inode *in) } } - -class C_Client_PutInode : public Context { - Client *client; - Inode *in; -public: - C_Client_PutInode(Client *c, Inode *i) : client(c), in(i) { - in->get(); - } - void finish(int) { - // I am used via ObjectCacher, which is responsible for taking - // the client lock before calling me back. - assert(client->client_lock.is_locked_by_me()); - client->put_inode(in); - } -}; - bool Client::_flush(Inode *in, Context *onfinish) { ldout(cct, 10) << "_flush " << *in << dendl; if (!in->oset.dirty_or_tx) { ldout(cct, 10) << " nothing to flush" << dendl; - if (onfinish) - onfinish->complete(0); + onfinish->complete(0); return true; } - if (!onfinish) { - onfinish = new C_Client_PutInode(this, in); - } - if (objecter->osdmap_full_flag()) { ldout(cct, 1) << __func__ << ": FULL, purging for ENOSPC" << dendl; objectcacher->purge_set(&in->oset); @@ -4054,6 +4042,34 @@ void Client::_invalidate_inode_parents(Inode *in) } } +/** + * For asynchronous flushes, check for errors from the IO and + * update the inode if necessary + */ +class C_Client_FlushComplete : public Context { + private: + Client *client; + Inode *inode; + + public: + C_Client_FlushComplete(Client *c, Inode *in) : client(c), inode(in) + { + inode->get(); + } + + void finish(int r) { + assert(client->client_lock.is_locked_by_me()); + if (r != 0) { + client_t const whoami = client->whoami; // For the benefit of ldout prefix + ldout(client->cct, 1) << "I/O error from flush on inode " << inode + << " 0x" << std::hex << inode->ino << std::dec + << ": " << r << "(" << cpp_strerror(r) << ")" << dendl; + inode->async_err = r; + } + client->put_inode(inode); + } +}; + void Client::handle_cap_grant(MetaSession *session, Inode *in, Cap *cap, MClientCaps *m) { mds_rank_t mds = session->mds_num; @@ -4121,9 +4137,8 @@ void Client::handle_cap_grant(MetaSession *session, Inode *in, Cap *cap, MClient cap->issued = new_caps; cap->implemented |= new_caps; - - if (((used & ~new_caps) & CEPH_CAP_FILE_BUFFER) && - !_flush(in)) { + if (((used & ~new_caps) & CEPH_CAP_FILE_BUFFER) + && !_flush(in, new C_Client_FlushComplete(this, in))) { // waitin' for flush } else if ((old_caps & ~new_caps) & CEPH_CAP_FILE_CACHE) { _release(in); @@ -4499,8 +4514,9 @@ void Client::unmount() while (!fd_map.empty()) { Fh *fh = fd_map.begin()->second; fd_map.erase(fd_map.begin()); - ldout(cct, 0) << " destroying lost open file " << fh << " on " << *fh->inode << dendl; - _release_fh(fh); + int release_err = _release_fh(fh); + ldout(cct, 0) << " destroyed lost open file " << fh << " on " << *fh->inode << "(async_err = " << release_err << ")" << dendl; + } _ll_drop_pins(); @@ -4526,7 +4542,7 @@ void Client::unmount() if (!in->caps.empty()) { in->get(); _release(in); - _flush(in); + _flush(in, new C_Client_FlushComplete(this, in)); put_inode(in); } } @@ -6396,7 +6412,7 @@ int Client::_release_fh(Fh *f) if (in->snapid == CEPH_NOSNAP) { if (in->put_open_ref(f->mode)) { - _flush(in); + _flush(in, new C_Client_FlushComplete(this, in)); // release clean pages too, if we dont want RDCACHE if (in->cap_refs[CEPH_CAP_FILE_CACHE] == 0 && !(in->caps_wanted() & CEPH_CAP_FILE_CACHE) && @@ -6412,10 +6428,19 @@ int Client::_release_fh(Fh *f) _release_filelocks(f); + // Finally, read any async err (i.e. from flushes) from the inode + int err = in->async_err; + if (err != 0) { + ldout(cct, 1) << "_release_fh " << f << " on inode " << *in << " caught async_err = " + << cpp_strerror(err) << dendl; + } else { + ldout(cct, 10) << "_release_fh " << f << " on inode " << *in << " no async_err state" << dendl; + } + put_inode(in); delete f; - return 0; + return err; } int Client::_open(Inode *in, int flags, mode_t mode, Fh **fhp, int uid, int gid) @@ -6473,10 +6498,10 @@ int Client::close(int fd) Fh *fh = get_filehandle(fd); if (!fh) return -EBADF; - _release_fh(fh); + int err = _release_fh(fh); fd_map.erase(fd); ldout(cct, 3) << "close exit(" << fd << ")" << dendl; - return 0; + return err; } @@ -7142,8 +7167,16 @@ done: int Client::_flush(Fh *f) { - // no-op, for now. hrm. - return 0; + Inode *in = f->inode; + int err = in->async_err; + if (err != 0) { + ldout(cct, 1) << __func__ << ": " << f << " on inode " << *in << " caught async_err = " + << cpp_strerror(err) << dendl; + } else { + ldout(cct, 10) << __func__ << ": " << f << " on inode " << *in << " no async_err state" << dendl; + } + + return err; } int Client::truncate(const char *relpath, loff_t length) @@ -7245,6 +7278,13 @@ int Client::_fsync(Fh *f, bool syncdataonly) ldout(cct, 1) << "ino " << in->ino << " failed to commit to disk! " << cpp_strerror(-r) << dendl; } + + if (in->async_err) { + ldout(cct, 1) << "ino " << in->ino << " marked with error from background flush! " + << cpp_strerror(in->async_err) << dendl; + r = in->async_err; + } + return r; } @@ -9309,7 +9349,8 @@ int Client::ll_create(Inode *parent, const char *name, mode_t mode, r = check_permissions(in, flags, uid, gid); if (r < 0) { if (fhp && *fhp) { - _release_fh(*fhp); + int release_r = _release_fh(*fhp); + assert(release_r == 0); // during create, no async data ops should have happened } goto out; } @@ -9702,8 +9743,7 @@ int Client::ll_release(Fh *fh) tout(cct) << "ll_release (fh)" << std::endl; tout(cct) << (unsigned long)fh << std::endl; - _release_fh(fh); - return 0; + return _release_fh(fh); } int Client::ll_getlk(Fh *fh, struct flock *fl, uint64_t owner) diff --git a/src/client/Client.h b/src/client/Client.h index b2ed1eefd2d63..0d9132ba247bd 100644 --- a/src/client/Client.h +++ b/src/client/Client.h @@ -402,7 +402,7 @@ protected: void put_inode(Inode *in, int n=1); void close_dir(Dir *dir); - friend class C_Client_PutInode; // calls put_inode() + friend class C_Client_FlushComplete; // calls put_inode() friend class C_Client_CacheInvalidate; // calls ino_invalidate_cb friend class C_Client_DentryInvalidate; // calls dentry_invalidate_cb friend class C_Block_Sync; // Calls block map and protected helpers @@ -546,7 +546,7 @@ protected: * * @returns true if the data was already flushed, false otherwise. */ - bool _flush(Inode *in, Context *c=NULL); + bool _flush(Inode *in, Context *c); void _flush_range(Inode *in, int64_t off, uint64_t size); void _flushed(Inode *in); void flush_set_callback(ObjectCacher::ObjectSet *oset); diff --git a/src/client/Inode.h b/src/client/Inode.h index 62cc0d016714e..e8432c3fd53bd 100644 --- a/src/client/Inode.h +++ b/src/client/Inode.h @@ -235,7 +235,8 @@ struct Inode { oset((void *)this, newlayout->fl_pg_pool, ino), reported_size(0), wanted_max_size(0), requested_max_size(0), _ref(0), ll_ref(0), dir(0), dn_set(), - fcntl_locks(NULL), flock_locks(NULL) + fcntl_locks(NULL), flock_locks(NULL), + async_err(0) { memset(&dir_layout, 0, sizeof(dir_layout)); memset(&layout, 0, sizeof(layout)); @@ -276,6 +277,9 @@ struct Inode { bool have_valid_size(); Dir *open_dir(); + // Record errors to be exposed in fclose/fflush + int async_err; + void dump(Formatter *f) const; }; diff --git a/src/client/fuse_ll.cc b/src/client/fuse_ll.cc index 8161485dc7511..154cca7a9cbf8 100644 --- a/src/client/fuse_ll.cc +++ b/src/client/fuse_ll.cc @@ -464,8 +464,10 @@ static void fuse_ll_write(fuse_req_t req, fuse_ino_t ino, const char *buf, static void fuse_ll_flush(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi) { - // NOOP - fuse_reply_err(req, 0); + CephFuse::Handle *cfuse = (CephFuse::Handle *)fuse_req_userdata(req); + Fh *fh = reinterpret_cast(fi->fh); + int r = cfuse->client->ll_flush(fh); + fuse_reply_err(req, -r); } #ifdef FUSE_IOCTL_COMPAT