From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from cn.fujitsu.com ([59.151.112.132]:64800 "EHLO heian.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1752509AbdCFAqF (ORCPT ); Sun, 5 Mar 2017 19:46:05 -0500 Subject: Re: [PATCH v2] btrfs-progs: report I/O errors when closing the filesystem To: Omar Sandoval , , David Sterba References: <105e3aa192a47f3936c2ac47ad8d673745cccaea.1488560489.git.osandov@fb.com> CC: From: Qu Wenruo Message-ID: Date: Mon, 6 Mar 2017 08:42:01 +0800 MIME-Version: 1.0 In-Reply-To: <105e3aa192a47f3936c2ac47ad8d673745cccaea.1488560489.git.osandov@fb.com> Content-Type: text/plain; charset="utf-8"; format=flowed Sender: linux-btrfs-owner@vger.kernel.org List-ID: At 03/04/2017 01:02 AM, Omar Sandoval wrote: > From: Omar Sandoval > > If the final fsync() on the Btrfs device fails, we just swallow the > error and don't alert the user in any way. This was uncovered by xfstest > generic/405, which checks that mkfs fails when it encounters EIO. > > Signed-off-by: Omar Sandoval Please ignore my comment on v1 patch, didn't see the v2 one. Looks good to me. Reviewed-by: Qu Wenruo Thanks, Qu > --- > Changes from v1: return -errno instead of -1 > > disk-io.c | 4 ++-- > volumes.c | 9 +++++++-- > 2 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/disk-io.c b/disk-io.c > index 2a94d4fc..48fb3c6b 100644 > --- a/disk-io.c > +++ b/disk-io.c > @@ -1817,10 +1817,10 @@ int close_ctree_fs_info(struct btrfs_fs_info *fs_info) > free_fs_roots_tree(&fs_info->fs_root_tree); > > btrfs_release_all_roots(fs_info); > - btrfs_close_devices(fs_info->fs_devices); > + ret = btrfs_close_devices(fs_info->fs_devices); > btrfs_cleanup_all_caches(fs_info); > btrfs_free_fs_info(fs_info); > - return 0; > + return ret; > } > > int clean_tree_block(struct btrfs_trans_handle *trans, struct btrfs_root *root, > diff --git a/volumes.c b/volumes.c > index a0a85edd..f7d82d51 100644 > --- a/volumes.c > +++ b/volumes.c > @@ -160,6 +160,7 @@ int btrfs_close_devices(struct btrfs_fs_devices *fs_devices) > { > struct btrfs_fs_devices *seed_devices; > struct btrfs_device *device; > + int ret = 0; > > again: > if (!fs_devices) > @@ -168,7 +169,11 @@ again: > device = list_entry(fs_devices->devices.next, > struct btrfs_device, dev_list); > if (device->fd != -1) { > - fsync(device->fd); > + if (fsync(device->fd) == -1) { > + warning("fsync on device %llu failed: %s", > + device->devid, strerror(errno)); > + ret = -errno; > + } > if (posix_fadvise(device->fd, 0, 0, POSIX_FADV_DONTNEED)) > fprintf(stderr, "Warning, could not drop caches\n"); > close(device->fd); > @@ -197,7 +202,7 @@ again: > free(fs_devices); > } > > - return 0; > + return ret; > } > > void btrfs_close_all_devices(void) >