| Message ID | 20190327072500.11156-2-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs-progs: Fix the nobarrier behavior of write | expand |
On 27.03.19 г. 9:24 ч., Qu Wenruo wrote: > When we failed to write super blocks, we just output something like: > WARNING: failed to write sb: I/O error > Or > WARNING: failed to write all sb data > > There is no info about which device failed and there are two different > error message for the same write error. > > This patch will change it to something more detailed: > ERROR: failed to write super block: I/O error > ERROR: failed to write super block for devid 1 But now we print 2 messages for 1 error. I'd rather have the information conveyed on a single line. I.e call fprintf directly from both callsites that do goto write_err with respective format and rename write_err label to out. > > This provides the basis for later super block flush error handling. > > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > disk-io.c | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > > diff --git a/disk-io.c b/disk-io.c > index 797b9b79ea3c..238b1821be14 100644 > --- a/disk-io.c > +++ b/disk-io.c > @@ -1599,8 +1599,12 @@ static int write_dev_supers(struct btrfs_fs_info *fs_info, > ret = pwrite64(device->fd, fs_info->super_copy, > BTRFS_SUPER_INFO_SIZE, > fs_info->super_bytenr); > - if (ret != BTRFS_SUPER_INFO_SIZE) > + if (ret != BTRFS_SUPER_INFO_SIZE) { > + errno = EIO; > + ret = -errno; > + error("failed to write super block: %m"); > goto write_err; > + } > return 0; > } > > @@ -1622,17 +1626,18 @@ static int write_dev_supers(struct btrfs_fs_info *fs_info, > */ > ret = pwrite64(device->fd, fs_info->super_copy, > BTRFS_SUPER_INFO_SIZE, bytenr); > - if (ret != BTRFS_SUPER_INFO_SIZE) > + if (ret != BTRFS_SUPER_INFO_SIZE) { > + errno = EIO; > + ret = -errno; > + error("failed to write super block %i: %m", i); > goto write_err; > + } > } > > return 0; > > write_err: > - if (ret > 0) > - fprintf(stderr, "WARNING: failed to write all sb data\n"); > - else > - fprintf(stderr, "WARNING: failed to write sb: %m\n"); > + error("failed to write super block for devid %llu", device->devid); > return ret; > } > >
On 2019/3/27 下午4:00, Nikolay Borisov wrote: > > > On 27.03.19 г. 9:24 ч., Qu Wenruo wrote: >> When we failed to write super blocks, we just output something like: >> WARNING: failed to write sb: I/O error >> Or >> WARNING: failed to write all sb data >> >> There is no info about which device failed and there are two different >> error message for the same write error. >> >> This patch will change it to something more detailed: >> ERROR: failed to write super block: I/O error >> ERROR: failed to write super block for devid 1 > > But now we print 2 messages for 1 error. I'd rather have the information > conveyed on a single line. I.e call fprintf directly from both callsites > that do goto write_err with respective format and rename write_err label > to out. Makes sense. I'll go that way. Thanks, Qu > >> >> This provides the basis for later super block flush error handling. >> >> Signed-off-by: Qu Wenruo <wqu@suse.com> >> --- >> disk-io.c | 17 +++++++++++------ >> 1 file changed, 11 insertions(+), 6 deletions(-) >> >> diff --git a/disk-io.c b/disk-io.c >> index 797b9b79ea3c..238b1821be14 100644 >> --- a/disk-io.c >> +++ b/disk-io.c >> @@ -1599,8 +1599,12 @@ static int write_dev_supers(struct btrfs_fs_info *fs_info, >> ret = pwrite64(device->fd, fs_info->super_copy, >> BTRFS_SUPER_INFO_SIZE, >> fs_info->super_bytenr); >> - if (ret != BTRFS_SUPER_INFO_SIZE) >> + if (ret != BTRFS_SUPER_INFO_SIZE) { >> + errno = EIO; >> + ret = -errno; >> + error("failed to write super block: %m"); >> goto write_err; >> + } >> return 0; >> } >> >> @@ -1622,17 +1626,18 @@ static int write_dev_supers(struct btrfs_fs_info *fs_info, >> */ >> ret = pwrite64(device->fd, fs_info->super_copy, >> BTRFS_SUPER_INFO_SIZE, bytenr); >> - if (ret != BTRFS_SUPER_INFO_SIZE) >> + if (ret != BTRFS_SUPER_INFO_SIZE) { >> + errno = EIO; >> + ret = -errno; >> + error("failed to write super block %i: %m", i); >> goto write_err; >> + } >> } >> >> return 0; >> >> write_err: >> - if (ret > 0) >> - fprintf(stderr, "WARNING: failed to write all sb data\n"); >> - else >> - fprintf(stderr, "WARNING: failed to write sb: %m\n"); >> + error("failed to write super block for devid %llu", device->devid); >> return ret; >> } >> >>
diff --git a/disk-io.c b/disk-io.c index 797b9b79ea3c..238b1821be14 100644 --- a/disk-io.c +++ b/disk-io.c @@ -1599,8 +1599,12 @@ static int write_dev_supers(struct btrfs_fs_info *fs_info, ret = pwrite64(device->fd, fs_info->super_copy, BTRFS_SUPER_INFO_SIZE, fs_info->super_bytenr); - if (ret != BTRFS_SUPER_INFO_SIZE) + if (ret != BTRFS_SUPER_INFO_SIZE) { + errno = EIO; + ret = -errno; + error("failed to write super block: %m"); goto write_err; + } return 0; } @@ -1622,17 +1626,18 @@ static int write_dev_supers(struct btrfs_fs_info *fs_info, */ ret = pwrite64(device->fd, fs_info->super_copy, BTRFS_SUPER_INFO_SIZE, bytenr); - if (ret != BTRFS_SUPER_INFO_SIZE) + if (ret != BTRFS_SUPER_INFO_SIZE) { + errno = EIO; + ret = -errno; + error("failed to write super block %i: %m", i); goto write_err; + } } return 0; write_err: - if (ret > 0) - fprintf(stderr, "WARNING: failed to write all sb data\n"); - else - fprintf(stderr, "WARNING: failed to write sb: %m\n"); + error("failed to write super block for devid %llu", device->devid); return ret; }
When we failed to write super blocks, we just output something like: WARNING: failed to write sb: I/O error Or WARNING: failed to write all sb data There is no info about which device failed and there are two different error message for the same write error. This patch will change it to something more detailed: ERROR: failed to write super block: I/O error ERROR: failed to write super block for devid 1 This provides the basis for later super block flush error handling. Signed-off-by: Qu Wenruo <wqu@suse.com> --- disk-io.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)