Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os/filestore: handle error returned from write_fd() #10146

Merged
merged 1 commit into from Apr 5, 2017

Conversation

Projects
None yet
6 participants
@yonghengdexin735
Copy link
Contributor

yonghengdexin735 commented Jul 6, 2016

r = bl.write_fd(**fd, offset) returned negative or zero ,so I fix its judgment.

Signed-off-by: zhang.zezhu zhang.zezhu@zte.com.cn

dout(0) << "write error "
<< cpp_strerror(r) << dendl;
goto lfn_clear;
}else{

This comment has been minimized.

Copy link
@xiexingguo

xiexingguo Jul 6, 2016

Member

Can you drop this (else)?

@@ -3329,12 +3329,17 @@ int FileStore::_write(const coll_t& cid, const ghobject_t& oid,

// write
r = bl.write_fd(**fd, offset);
if (r == 0)
if (r < 0){

This comment has been minimized.

Copy link
@xiexingguo

xiexingguo Jul 6, 2016

Member

need space...

@xiexingguo

This comment has been minimized.

Copy link
Member

xiexingguo commented Jul 6, 2016

BTW, you might need to fixup these two commits also.

@yonghengdexin735 yonghengdexin735 force-pushed the yonghengdexin735:wip-zzz-filestore-write branch from e26d8b7 to c9b7e87 Jul 6, 2016


if (r >= 0 && m_filestore_sloppy_crc) {
int rc = backend->_crc_update_write(**fd, offset, len, bl);
assert(rc >= 0);
}
}

This comment has been minimized.

Copy link
@xiexingguo

xiexingguo Jul 6, 2016

Member

Could you drop the extra space too?

@yonghengdexin735

This comment has been minimized.

Copy link
Contributor Author

yonghengdexin735 commented Jul 6, 2016

@xiexingguo thank you for your comment

@yonghengdexin735 yonghengdexin735 force-pushed the yonghengdexin735:wip-zzz-filestore-write branch 2 times, most recently from 629f724 to 8c49f33 Jul 7, 2016

@yonghengdexin735

This comment has been minimized.

Copy link
Contributor Author

yonghengdexin735 commented Jul 8, 2016

@xiexingguo hi, could you review for me, thanks.

if (r < 0) {
dout(0) << "write error "
<< cpp_strerror(r) << dendl;
goto close_fd;

This comment has been minimized.

Copy link
@xiexingguo

xiexingguo Jul 8, 2016

Member

Well, don't think we need to split this statement into multiple lines since it is obviously within 80 characters, right?

This comment has been minimized.

Copy link
@yonghengdexin735

yonghengdexin735 Jul 8, 2016

Author Contributor

@xiexingguo hi, I had fixup follow you suggest

@yonghengdexin735 yonghengdexin735 force-pushed the yonghengdexin735:wip-zzz-filestore-write branch 2 times, most recently from fa01cce to 4d5214b Jul 8, 2016

@yonghengdexin735 yonghengdexin735 force-pushed the yonghengdexin735:wip-zzz-filestore-write branch from 4d5214b to 81e414b Jul 20, 2016

if (r == 0)
r = bl.length();
if (r < 0) {
dout(0) << "write error " << cpp_strerror(r) << dendl;

This comment has been minimized.

Copy link
@yuyuyu101

yuyuyu101 Sep 8, 2016

Member
dout(0) << __func__ << " write_fd on" << cid << "/" << oid << error: " << cpp_strerror(r) << dendl;

This comment has been minimized.

Copy link
@tchaikov

@yonghengdexin735 yonghengdexin735 force-pushed the yonghengdexin735:wip-zzz-filestore-write branch from ca53b35 to d8acd0b Oct 21, 2016

liewegas pushed a commit to liewegas/ceph that referenced this pull request Nov 18, 2016

ceph-disk: don't change the journal partition uuid
We observe that the new /dev/disk/by-partuuid/<journal_uuid>
symlink is not always created by udev when reusing a journal
partition. Fix by not changing the uuid of a journal partition
in this case -- instead we can reuse the existing uuid (and
journal_symlink) instead. We also now assert that the symlink
exists before further preparing the OSD.

Fixes: ceph#10146
Signed-off-by: Dan van der Ster <daniel.vanderster@cern.ch>
Tested-by: Dan van der Ster <daniel.vanderster@cern.ch>
(cherry picked from commit 29eb135)
if (r == 0)
r = bl.length();
if (r < 0) {
dout(10) << "write_fd on " << cid << "/" << oid << " error: " << cpp_strerror(r) << dendl;

This comment has been minimized.

Copy link
@tchaikov

tchaikov Nov 28, 2016

Contributor

might want to use derr in this case.

@tchaikov

This comment has been minimized.

Copy link
Contributor

tchaikov commented Nov 28, 2016

r = bl.write_fd(**fd, offset) returned negative or zero ,so I fix its judgment.

by looking this comment, this PR is not a fix, but just a cleanup. and please fill the commit message with more details on why this is a fix.

@tchaikov

This comment has been minimized.

@tchaikov tchaikov added core and removed needs-qa labels Nov 28, 2016

@tchaikov

This comment has been minimized.

Copy link
Contributor

tchaikov commented Mar 7, 2017

@yonghengdexin735 yonghengdexin735 force-pushed the yonghengdexin735:wip-zzz-filestore-write branch 4 times, most recently from f295ef0 to 776f86e Mar 8, 2017

@yonghengdexin735

This comment has been minimized.

Copy link
Contributor Author

yonghengdexin735 commented Mar 8, 2017

@tchaikov done

@tchaikov tchaikov changed the title os/filestore/FileStore.cc: fix the judgment for write_fd() returned os/filestore: handle error returned from write_fd() Mar 8, 2017

@tchaikov

This comment has been minimized.

Copy link
Contributor

tchaikov commented Mar 8, 2017

you might want to address #10146 (comment) also.

@yonghengdexin735 yonghengdexin735 force-pushed the yonghengdexin735:wip-zzz-filestore-write branch 2 times, most recently from a53c6b7 to 3284df5 Mar 9, 2017

@yonghengdexin735

This comment has been minimized.

Copy link
Contributor Author

yonghengdexin735 commented Mar 13, 2017

@tchaikov OK , DONE

if (r == 0)
r = bl.length();
if (r < 0) {
derr << __func__ << "write_fd on " << cid << "/" << oid

This comment has been minimized.

Copy link
@yuyuyu101

yuyuyu101 Mar 25, 2017

Member

left space between "func" and "write_fd"....

This comment has been minimized.

Copy link
@yonghengdexin735

yonghengdexin735 Mar 29, 2017

Author Contributor

@yuyuyu101 thanks,updated

@liewegas
Copy link
Member

liewegas left a comment

needs whitespace fix, otherwise looks good

@yonghengdexin735 yonghengdexin735 force-pushed the yonghengdexin735:wip-zzz-filestore-write branch from 3284df5 to 6745bee Mar 29, 2017

@yonghengdexin735

This comment has been minimized.

Copy link
Contributor Author

yonghengdexin735 commented Mar 29, 2017

@liewegas thanks, updated

lfn_close(fd);
goto out;
}

This comment has been minimized.

Copy link
@tchaikov

tchaikov Mar 29, 2017

Contributor

please remove this empty line.

This comment has been minimized.

Copy link
@yonghengdexin735

yonghengdexin735 Mar 29, 2017

Author Contributor

@tchaikov hi ,deleted

@yonghengdexin735 yonghengdexin735 force-pushed the yonghengdexin735:wip-zzz-filestore-write branch from 6745bee to 9756217 Mar 29, 2017

lfn_close(fd);
goto out;
}

This comment has been minimized.

Copy link
@tchaikov

tchaikov Mar 29, 2017

Contributor

@yonghengdexin735, it's not deleted. did you push a wrong commit?

os/filestore:cleanup the judgment for write_fd() returned
Signed-off-by: yonghengdexin735 <zhang.zezhu@zte.com.cn>

@yonghengdexin735 yonghengdexin735 force-pushed the yonghengdexin735:wip-zzz-filestore-write branch from 9756217 to d7f64f9 Mar 29, 2017

@yuriw yuriw merged commit 04c5374 into ceph:master Apr 5, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.