* [PATCH] metadump/restore: don't use errno after fwrite/fread failures
@ 2018-03-21 23:24 Eric Sandeen
2018-03-21 23:58 ` Darrick J. Wong
2018-03-22 14:41 ` [PATCH V2] " Eric Sandeen
0 siblings, 2 replies; 5+ messages in thread
From: Eric Sandeen @ 2018-03-21 23:24 UTC (permalink / raw)
To: linux-xfs
fread/fwrite don't set errno, so printing out strerror(errno)
after a failure leads to incorrect and confusing messages:
# xfs_mdrestore pre_repair.meta pre_repair.img
xfs_mdrestore: error reading from file: Success
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
diff --git a/db/metadump.c b/db/metadump.c
index 9d62958..231adc6 100644
--- a/db/metadump.c
+++ b/db/metadump.c
@@ -161,7 +161,7 @@ write_index(void)
*/
metablock->mb_count = cpu_to_be16(cur_index);
if (fwrite(metablock, (cur_index + 1) << BBSHIFT, 1, outf) != 1) {
- print_warning("error writing to file: %s", strerror(errno));
+ print_warning("error writing to target file");
return -errno;
}
diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
index c9d4b0c..262a385 100644
--- a/mdrestore/xfs_mdrestore.c
+++ b/mdrestore/xfs_mdrestore.c
@@ -93,15 +93,14 @@ perform_restore(
block_buffer = (char *)metablock + block_size;
if (fread(block_index, block_size - sizeof(struct xfs_metablock), 1, src_f) != 1)
- fatal("error reading from file: %s\n", strerror(errno));
+ fatal("error reading from metadump file\n");
if (block_index[0] != 0)
fatal("first block is not the primary superblock\n");
- if (fread(block_buffer, mb_count << mbp->mb_blocklog,
- 1, src_f) != 1)
- fatal("error reading from file: %s\n", strerror(errno));
+ if (fread(block_buffer, mb_count << mbp->mb_blocklog, 1, src_f) != 1)
+ fatal("error reading from metadump file\n");
libxfs_sb_from_disk(&sb, (xfs_dsb_t *)block_buffer);
@@ -157,7 +156,7 @@ perform_restore(
break;
if (fread(metablock, block_size, 1, src_f) != 1)
- fatal("error reading from file: %s\n", strerror(errno));
+ fatal("error reading from metadump file\n");
mb_count = be16_to_cpu(metablock->mb_count);
if (mb_count == 0)
@@ -167,7 +166,7 @@ perform_restore(
if (fread(block_buffer, mb_count << mbp->mb_blocklog,
1, src_f) != 1)
- fatal("error reading from file: %s\n", strerror(errno));
+ fatal("error reading from metadump file\n");
bytes_read += block_size + (mb_count << mbp->mb_blocklog);
}
@@ -253,7 +252,7 @@ main(
}
if (fread(&mb, sizeof(mb), 1, src_f) != 1)
- fatal("error reading from file: %s\n", strerror(errno));
+ fatal("error reading from metadump file\n");
if (mb.mb_magic != cpu_to_be32(XFS_MD_MAGIC))
fatal("specified file is not a metadata dump\n");
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] metadump/restore: don't use errno after fwrite/fread failures
2018-03-21 23:24 [PATCH] metadump/restore: don't use errno after fwrite/fread failures Eric Sandeen
@ 2018-03-21 23:58 ` Darrick J. Wong
2018-03-22 2:15 ` Eric Sandeen
2018-03-22 14:41 ` [PATCH V2] " Eric Sandeen
1 sibling, 1 reply; 5+ messages in thread
From: Darrick J. Wong @ 2018-03-21 23:58 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs
On Wed, Mar 21, 2018 at 06:24:09PM -0500, Eric Sandeen wrote:
> fread/fwrite don't set errno, so printing out strerror(errno)
> after a failure leads to incorrect and confusing messages:
>
> # xfs_mdrestore pre_repair.meta pre_repair.img
> xfs_mdrestore: error reading from file: Success
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> diff --git a/db/metadump.c b/db/metadump.c
> index 9d62958..231adc6 100644
> --- a/db/metadump.c
> +++ b/db/metadump.c
> @@ -161,7 +161,7 @@ write_index(void)
> */
> metablock->mb_count = cpu_to_be16(cur_index);
> if (fwrite(metablock, (cur_index + 1) << BBSHIFT, 1, outf) != 1) {
> - print_warning("error writing to file: %s", strerror(errno));
> + print_warning("error writing to target file");
> return -errno;
return -1?
--D
> }
>
> diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
> index c9d4b0c..262a385 100644
> --- a/mdrestore/xfs_mdrestore.c
> +++ b/mdrestore/xfs_mdrestore.c
> @@ -93,15 +93,14 @@ perform_restore(
> block_buffer = (char *)metablock + block_size;
>
> if (fread(block_index, block_size - sizeof(struct xfs_metablock), 1, src_f) != 1)
> - fatal("error reading from file: %s\n", strerror(errno));
> + fatal("error reading from metadump file\n");
>
> if (block_index[0] != 0)
> fatal("first block is not the primary superblock\n");
>
>
> - if (fread(block_buffer, mb_count << mbp->mb_blocklog,
> - 1, src_f) != 1)
> - fatal("error reading from file: %s\n", strerror(errno));
> + if (fread(block_buffer, mb_count << mbp->mb_blocklog, 1, src_f) != 1)
> + fatal("error reading from metadump file\n");
>
> libxfs_sb_from_disk(&sb, (xfs_dsb_t *)block_buffer);
>
> @@ -157,7 +156,7 @@ perform_restore(
> break;
>
> if (fread(metablock, block_size, 1, src_f) != 1)
> - fatal("error reading from file: %s\n", strerror(errno));
> + fatal("error reading from metadump file\n");
>
> mb_count = be16_to_cpu(metablock->mb_count);
> if (mb_count == 0)
> @@ -167,7 +166,7 @@ perform_restore(
>
> if (fread(block_buffer, mb_count << mbp->mb_blocklog,
> 1, src_f) != 1)
> - fatal("error reading from file: %s\n", strerror(errno));
> + fatal("error reading from metadump file\n");
>
> bytes_read += block_size + (mb_count << mbp->mb_blocklog);
> }
> @@ -253,7 +252,7 @@ main(
> }
>
> if (fread(&mb, sizeof(mb), 1, src_f) != 1)
> - fatal("error reading from file: %s\n", strerror(errno));
> + fatal("error reading from metadump file\n");
> if (mb.mb_magic != cpu_to_be32(XFS_MD_MAGIC))
> fatal("specified file is not a metadata dump\n");
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] metadump/restore: don't use errno after fwrite/fread failures
2018-03-21 23:58 ` Darrick J. Wong
@ 2018-03-22 2:15 ` Eric Sandeen
0 siblings, 0 replies; 5+ messages in thread
From: Eric Sandeen @ 2018-03-22 2:15 UTC (permalink / raw)
To: Darrick J. Wong, Eric Sandeen; +Cc: linux-xfs
On 3/21/18 6:58 PM, Darrick J. Wong wrote:
> On Wed, Mar 21, 2018 at 06:24:09PM -0500, Eric Sandeen wrote:
>> fread/fwrite don't set errno, so printing out strerror(errno)
>> after a failure leads to incorrect and confusing messages:
>>
>> # xfs_mdrestore pre_repair.meta pre_repair.img
>> xfs_mdrestore: error reading from file: Success
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> diff --git a/db/metadump.c b/db/metadump.c
>> index 9d62958..231adc6 100644
>> --- a/db/metadump.c
>> +++ b/db/metadump.c
>> @@ -161,7 +161,7 @@ write_index(void)
>> */
>> metablock->mb_count = cpu_to_be16(cur_index);
>> if (fwrite(metablock, (cur_index + 1) << BBSHIFT, 1, outf) != 1) {
>> - print_warning("error writing to file: %s", strerror(errno));
>> + print_warning("error writing to target file");
>> return -errno;
>
> return -1?
Oh derp.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH V2] metadump/restore: don't use errno after fwrite/fread failures
2018-03-21 23:24 [PATCH] metadump/restore: don't use errno after fwrite/fread failures Eric Sandeen
2018-03-21 23:58 ` Darrick J. Wong
@ 2018-03-22 14:41 ` Eric Sandeen
2018-03-22 16:47 ` Darrick J. Wong
1 sibling, 1 reply; 5+ messages in thread
From: Eric Sandeen @ 2018-03-22 14:41 UTC (permalink / raw)
To: linux-xfs
fread/fwrite don't set errno, so printing out strerror(errno)
after a failure leads to incorrect and confusing messages:
# xfs_mdrestore pre_repair.meta pre_repair.img
xfs_mdrestore: error reading from file: Success
Don't return unset errno from write_index, either.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
V2: don't return errno, either.
diff --git a/db/metadump.c b/db/metadump.c
index 9d62958..d33f901 100644
--- a/db/metadump.c
+++ b/db/metadump.c
@@ -161,8 +161,8 @@ write_index(void)
*/
metablock->mb_count = cpu_to_be16(cur_index);
if (fwrite(metablock, (cur_index + 1) << BBSHIFT, 1, outf) != 1) {
- print_warning("error writing to file: %s", strerror(errno));
- return -errno;
+ print_warning("error writing to target file");
+ return -1;
}
memset(block_index, 0, num_indices * sizeof(__be64));
diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
index c9d4b0c..262a385 100644
--- a/mdrestore/xfs_mdrestore.c
+++ b/mdrestore/xfs_mdrestore.c
@@ -93,15 +93,14 @@ perform_restore(
block_buffer = (char *)metablock + block_size;
if (fread(block_index, block_size - sizeof(struct xfs_metablock), 1, src_f) != 1)
- fatal("error reading from file: %s\n", strerror(errno));
+ fatal("error reading from metadump file\n");
if (block_index[0] != 0)
fatal("first block is not the primary superblock\n");
- if (fread(block_buffer, mb_count << mbp->mb_blocklog,
- 1, src_f) != 1)
- fatal("error reading from file: %s\n", strerror(errno));
+ if (fread(block_buffer, mb_count << mbp->mb_blocklog, 1, src_f) != 1)
+ fatal("error reading from metadump file\n");
libxfs_sb_from_disk(&sb, (xfs_dsb_t *)block_buffer);
@@ -157,7 +156,7 @@ perform_restore(
break;
if (fread(metablock, block_size, 1, src_f) != 1)
- fatal("error reading from file: %s\n", strerror(errno));
+ fatal("error reading from metadump file\n");
mb_count = be16_to_cpu(metablock->mb_count);
if (mb_count == 0)
@@ -167,7 +166,7 @@ perform_restore(
if (fread(block_buffer, mb_count << mbp->mb_blocklog,
1, src_f) != 1)
- fatal("error reading from file: %s\n", strerror(errno));
+ fatal("error reading from metadump file\n");
bytes_read += block_size + (mb_count << mbp->mb_blocklog);
}
@@ -253,7 +252,7 @@ main(
}
if (fread(&mb, sizeof(mb), 1, src_f) != 1)
- fatal("error reading from file: %s\n", strerror(errno));
+ fatal("error reading from metadump file\n");
if (mb.mb_magic != cpu_to_be32(XFS_MD_MAGIC))
fatal("specified file is not a metadata dump\n");
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH V2] metadump/restore: don't use errno after fwrite/fread failures
2018-03-22 14:41 ` [PATCH V2] " Eric Sandeen
@ 2018-03-22 16:47 ` Darrick J. Wong
0 siblings, 0 replies; 5+ messages in thread
From: Darrick J. Wong @ 2018-03-22 16:47 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs
On Thu, Mar 22, 2018 at 09:41:43AM -0500, Eric Sandeen wrote:
> fread/fwrite don't set errno, so printing out strerror(errno)
> after a failure leads to incorrect and confusing messages:
>
> # xfs_mdrestore pre_repair.meta pre_repair.img
> xfs_mdrestore: error reading from file: Success
>
> Don't return unset errno from write_index, either.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
>
> V2: don't return errno, either.
>
> diff --git a/db/metadump.c b/db/metadump.c
> index 9d62958..d33f901 100644
> --- a/db/metadump.c
> +++ b/db/metadump.c
> @@ -161,8 +161,8 @@ write_index(void)
> */
> metablock->mb_count = cpu_to_be16(cur_index);
> if (fwrite(metablock, (cur_index + 1) << BBSHIFT, 1, outf) != 1) {
> - print_warning("error writing to file: %s", strerror(errno));
> - return -errno;
> + print_warning("error writing to target file");
> + return -1;
> }
>
> memset(block_index, 0, num_indices * sizeof(__be64));
> diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
> index c9d4b0c..262a385 100644
> --- a/mdrestore/xfs_mdrestore.c
> +++ b/mdrestore/xfs_mdrestore.c
> @@ -93,15 +93,14 @@ perform_restore(
> block_buffer = (char *)metablock + block_size;
>
> if (fread(block_index, block_size - sizeof(struct xfs_metablock), 1, src_f) != 1)
> - fatal("error reading from file: %s\n", strerror(errno));
> + fatal("error reading from metadump file\n");
>
> if (block_index[0] != 0)
> fatal("first block is not the primary superblock\n");
>
>
> - if (fread(block_buffer, mb_count << mbp->mb_blocklog,
> - 1, src_f) != 1)
> - fatal("error reading from file: %s\n", strerror(errno));
> + if (fread(block_buffer, mb_count << mbp->mb_blocklog, 1, src_f) != 1)
> + fatal("error reading from metadump file\n");
>
> libxfs_sb_from_disk(&sb, (xfs_dsb_t *)block_buffer);
>
> @@ -157,7 +156,7 @@ perform_restore(
> break;
>
> if (fread(metablock, block_size, 1, src_f) != 1)
> - fatal("error reading from file: %s\n", strerror(errno));
> + fatal("error reading from metadump file\n");
>
> mb_count = be16_to_cpu(metablock->mb_count);
> if (mb_count == 0)
> @@ -167,7 +166,7 @@ perform_restore(
>
> if (fread(block_buffer, mb_count << mbp->mb_blocklog,
> 1, src_f) != 1)
> - fatal("error reading from file: %s\n", strerror(errno));
> + fatal("error reading from metadump file\n");
>
> bytes_read += block_size + (mb_count << mbp->mb_blocklog);
> }
> @@ -253,7 +252,7 @@ main(
> }
>
> if (fread(&mb, sizeof(mb), 1, src_f) != 1)
> - fatal("error reading from file: %s\n", strerror(errno));
> + fatal("error reading from metadump file\n");
> if (mb.mb_magic != cpu_to_be32(XFS_MD_MAGIC))
> fatal("specified file is not a metadata dump\n");
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-03-22 16:47 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-21 23:24 [PATCH] metadump/restore: don't use errno after fwrite/fread failures Eric Sandeen
2018-03-21 23:58 ` Darrick J. Wong
2018-03-22 2:15 ` Eric Sandeen
2018-03-22 14:41 ` [PATCH V2] " Eric Sandeen
2018-03-22 16:47 ` Darrick J. Wong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox