* [PATCH] xfs_restore: Return on error when restoring file or symlink
@ 2019-12-12 23:32 Frank Sorenson
2019-12-13 22:25 ` Eric Sandeen
0 siblings, 1 reply; 2+ messages in thread
From: Frank Sorenson @ 2019-12-12 23:32 UTC (permalink / raw)
To: linux-xfs; +Cc: sandeen, sorenson
If an error occurs while opening or truncating a regular
file, or while creating a symlink, during a restore, no error
is currently propagated back to the caller, so xfsrestore can
return SUCCESS on a failed restore.
Make restore_reg and restore_symlink return an error code
indicating the restore was incomplete.
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
---
restore/content.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/restore/content.c b/restore/content.c
index c267234..5e30f08 100644
--- a/restore/content.c
+++ b/restore/content.c
@@ -7429,7 +7429,7 @@ done:
/* called to begin a regular file. if no path given, or if just toc,
* don't actually write, just read. also get into that situation if
- * cannot prepare destination. fd == -1 signifies no write. *statp
+ * cannot prepare destination. fd == -1 signifies no write. *rvp
* is set to indicate drive errors. returns FALSE if should abort
* this iteration.
*/
@@ -7486,12 +7486,13 @@ restore_reg(drive_t *drivep,
*fdp = open(path, oflags, S_IRUSR | S_IWUSR);
if (*fdp < 0) {
- mlog(MLOG_NORMAL | MLOG_WARNING,
+ mlog(MLOG_NORMAL | MLOG_ERROR,
_("open of %s failed: %s: discarding ino %llu\n"),
path,
strerror(errno),
bstatp->bs_ino);
- return BOOL_TRUE;
+ *rvp = RV_INCOMPLETE;
+ return BOOL_FALSE;
}
rval = fstat64(*fdp, &stat);
@@ -7510,10 +7511,12 @@ restore_reg(drive_t *drivep,
rval = ftruncate64(*fdp, bstatp->bs_size);
if (rval != 0) {
- mlog(MLOG_VERBOSE | MLOG_WARNING,
+ mlog(MLOG_VERBOSE | MLOG_ERROR,
_("attempt to truncate %s failed: %s\n"),
path,
strerror(errno));
+ *rvp = RV_INCOMPLETE;
+ return BOOL_FALSE;
}
}
}
@@ -8021,7 +8024,8 @@ restore_symlink(drive_t *drivep,
bstatp->bs_ino,
path);
}
- return BOOL_TRUE;
+ *rvp = RV_INCOMPLETE;
+ return BOOL_FALSE;
}
scratchpath[nread] = 0;
if (!tranp->t_toconlypr && path) {
@@ -8045,7 +8049,8 @@ restore_symlink(drive_t *drivep,
bstatp->bs_ino,
path,
strerror(errno));
- return BOOL_TRUE;
+ *rvp = RV_INCOMPLETE;
+ return BOOL_FALSE;
}
/* set the owner and group (if enabled)
--
2.20.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] xfs_restore: Return on error when restoring file or symlink
2019-12-12 23:32 [PATCH] xfs_restore: Return on error when restoring file or symlink Frank Sorenson
@ 2019-12-13 22:25 ` Eric Sandeen
0 siblings, 0 replies; 2+ messages in thread
From: Eric Sandeen @ 2019-12-13 22:25 UTC (permalink / raw)
To: Frank Sorenson, linux-xfs
On 12/12/19 5:32 PM, Frank Sorenson wrote:
> If an error occurs while opening or truncating a regular
> file, or while creating a symlink, during a restore, no error
> is currently propagated back to the caller, so xfsrestore can
> return SUCCESS on a failed restore.
>
> Make restore_reg and restore_symlink return an error code
> indicating the restore was incomplete.
Thanks for looking at this, some stuff below
> Signed-off-by: Frank Sorenson <sorenson@redhat.com>
> ---
> restore/content.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/restore/content.c b/restore/content.c
> index c267234..5e30f08 100644
> --- a/restore/content.c
> +++ b/restore/content.c
> @@ -7429,7 +7429,7 @@ done:
>
> /* called to begin a regular file. if no path given, or if just toc,
> * don't actually write, just read. also get into that situation if
> - * cannot prepare destination. fd == -1 signifies no write. *statp
> + * cannot prepare destination. fd == -1 signifies no write. *rvp
> * is set to indicate drive errors. returns FALSE if should abort
> * this iteration.
> */
> @@ -7486,12 +7486,13 @@ restore_reg(drive_t *drivep,
>
> *fdp = open(path, oflags, S_IRUSR | S_IWUSR);
> if (*fdp < 0) {
> - mlog(MLOG_NORMAL | MLOG_WARNING,
> + mlog(MLOG_NORMAL | MLOG_ERROR,
> _("open of %s failed: %s: discarding ino %llu\n"),
> path,
> strerror(errno),
> bstatp->bs_ino);
> - return BOOL_TRUE;
> + *rvp = RV_INCOMPLETE;
> + return BOOL_FALSE;
> }
I'm really not sure about original intent of the return values here and
when "this iteration should abort"
Before this patch, the function always returned BOOL_TRUE so there's
not much guidance. Presumably all the "log something and return true"
considered these to be transient errors ...
> rval = fstat64(*fdp, &stat);
> @@ -7510,10 +7511,12 @@ restore_reg(drive_t *drivep,
>
> rval = ftruncate64(*fdp, bstatp->bs_size);
> if (rval != 0) {
> - mlog(MLOG_VERBOSE | MLOG_WARNING,
> + mlog(MLOG_VERBOSE | MLOG_ERROR,
> _("attempt to truncate %s failed: %s\n"),
> path,
> strerror(errno));
> + *rvp = RV_INCOMPLETE;
> + return BOOL_FALSE;
> }
> }
> }
so this aborts on an open or frtruncate failure, but continues on
an fstat failure or a set_file_owner failure or an XFS_IOC_FSSETXATTR
failure ...
Was this motivated by ENOSPC, i.e. maybe the open couldn't even create
a new inode? I could see that possibly being a hard error to stop on,
but given the prior behavior of trying to coninue as much as possible
I'm unsure about the BOOL_FALSE's here. Setting RV_INCOMPLETE would
still make sense to me though, I think, for anything that caused the restore
to actually be incomplete.
(And this is all a little speculative as nobody really understands
this code anymore....)
-Eric
> @@ -8021,7 +8024,8 @@ restore_symlink(drive_t *drivep,
> bstatp->bs_ino,
> path);
> }
> - return BOOL_TRUE;
> + *rvp = RV_INCOMPLETE;
> + return BOOL_FALSE;
> }
> scratchpath[nread] = 0;
> if (!tranp->t_toconlypr && path) {
> @@ -8045,7 +8049,8 @@ restore_symlink(drive_t *drivep,
> bstatp->bs_ino,
> path,
> strerror(errno));
> - return BOOL_TRUE;
> + *rvp = RV_INCOMPLETE;
> + return BOOL_FALSE;
> }
>
> /* set the owner and group (if enabled)
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-12-13 22:25 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-12 23:32 [PATCH] xfs_restore: Return on error when restoring file or symlink Frank Sorenson
2019-12-13 22:25 ` Eric Sandeen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.