| Message ID | 1418244708-7087-10-git-send-email-mwilck@arcor.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
-------- Original Message -------- Subject: [PATCH 09/18] btrfs restore: more graceful error handling in copy_file From: <mwilck@arcor.de> To: <linux-btrfs@vger.kernel.org> Date: 2014?12?11? 04:51 > From: Martin Wilck <mwilck@arcor.de> > > Setting size and attributes of a file makes sense even if some > errors have occured during revovery. > > Also, do something useful with the number of bytes written. > > Signed-off-by: Martin Wilck <mwilck@arcor.de> > --- > cmds-restore.c | 27 ++++++++++++++------------- > 1 files changed, 14 insertions(+), 13 deletions(-) > > diff --git a/cmds-restore.c b/cmds-restore.c > index 8ecd896..10bb8be 100644 > --- a/cmds-restore.c > +++ b/cmds-restore.c > @@ -715,7 +715,7 @@ static int copy_file(struct btrfs_root *root, int fd, struct btrfs_key *key, > return ret; > } else if (ret) { > /* No more leaves to search */ > - btrfs_free_path(path); > + ret = 0; > goto set_size; > } > leaf = path->nodes[0]; > @@ -734,35 +734,36 @@ static int copy_file(struct btrfs_root *root, int fd, struct btrfs_key *key, > if (compression >= BTRFS_COMPRESS_LAST) { > fprintf(stderr, "Don't support compression yet %d\n", > compression); > - btrfs_free_path(path); > - return -1; > + ret = -1; Although the original one uses -1, IMHO it would be more meaningful using -ENOTTY. If someone later adds stderr() for the ret and print it, the author will be graceful. > + goto set_size; > } > > + bytes_written = 0ULL; > if (extent_type == BTRFS_FILE_EXTENT_PREALLOC) > goto next; > if (extent_type == BTRFS_FILE_EXTENT_INLINE) { > ret = copy_one_inline(fd, path, found_key.offset, > &bytes_written); > - if (ret) { > - btrfs_free_path(path); > - return -1; > - } > } else if (extent_type == BTRFS_FILE_EXTENT_REG) { > ret = copy_one_extent(root, fd, leaf, fi, > found_key.offset, &bytes_written); > - if (ret) { > - btrfs_free_path(path); > - return ret; > - } > } else { > printf("Weird extent type %d\n", extent_type); > } > + total_written += bytes_written; > + next_pos = found_key.offset + bytes_written; > + if (ret) { > + fprintf(stderr, "ERROR after writing %llu bytes\n", > + total_written); Just a advice, what about add stderr(-ret) for the info? > + ret = -1; Why change ret to -1? At least some of the ret above can be meaningful like -EIO or -ENOENT. > + goto set_size; > + } > next: > path->slots[0]++; > } > > - btrfs_free_path(path); > set_size: > + btrfs_free_path(path); > if (get_xattrs) { > ret = set_file_xattrs(root, key->objectid, fd, file); > if (ret) > @@ -771,7 +772,7 @@ set_size: > } > > set_fd_attrs(fd, &st, file); > - return 0; > + return ret; > } > > static int search_dir(struct btrfs_root *root, struct btrfs_key *key, Other part looks good for me. Thanks, Qu -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/cmds-restore.c b/cmds-restore.c index 8ecd896..10bb8be 100644 --- a/cmds-restore.c +++ b/cmds-restore.c @@ -715,7 +715,7 @@ static int copy_file(struct btrfs_root *root, int fd, struct btrfs_key *key, return ret; } else if (ret) { /* No more leaves to search */ - btrfs_free_path(path); + ret = 0; goto set_size; } leaf = path->nodes[0]; @@ -734,35 +734,36 @@ static int copy_file(struct btrfs_root *root, int fd, struct btrfs_key *key, if (compression >= BTRFS_COMPRESS_LAST) { fprintf(stderr, "Don't support compression yet %d\n", compression); - btrfs_free_path(path); - return -1; + ret = -1; + goto set_size; } + bytes_written = 0ULL; if (extent_type == BTRFS_FILE_EXTENT_PREALLOC) goto next; if (extent_type == BTRFS_FILE_EXTENT_INLINE) { ret = copy_one_inline(fd, path, found_key.offset, &bytes_written); - if (ret) { - btrfs_free_path(path); - return -1; - } } else if (extent_type == BTRFS_FILE_EXTENT_REG) { ret = copy_one_extent(root, fd, leaf, fi, found_key.offset, &bytes_written); - if (ret) { - btrfs_free_path(path); - return ret; - } } else { printf("Weird extent type %d\n", extent_type); } + total_written += bytes_written; + next_pos = found_key.offset + bytes_written; + if (ret) { + fprintf(stderr, "ERROR after writing %llu bytes\n", + total_written); + ret = -1; + goto set_size; + } next: path->slots[0]++; } - btrfs_free_path(path); set_size: + btrfs_free_path(path); if (get_xattrs) { ret = set_file_xattrs(root, key->objectid, fd, file); if (ret) @@ -771,7 +772,7 @@ set_size: } set_fd_attrs(fd, &st, file); - return 0; + return ret; } static int search_dir(struct btrfs_root *root, struct btrfs_key *key,