| Message ID | 25a01901-b2b0-35f1-6cc7-0c1d173a28e4@jp.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2017年09月26日 13:46, Misono, Tomohiro wrote: > Fix subvol del --commit-after to work properly: > - SYNC ioctl will be issued even when last delete is failed > - SYNC ioctl will be issued on each file system only once in the end > > To achieve this, get_fsid() and add_seen_fsid() is called after each delete > to keep only one fd for each fs. > > In the end, seen_fsid_hash will be traversed and SYNC is issued on each fs. > > Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com> > --- > cmds-subvolume.c | 77 ++++++++++++++++++++++++++++++++++++++++---------------- > 1 file changed, 56 insertions(+), 21 deletions(-) > > diff --git a/cmds-subvolume.c b/cmds-subvolume.c > index 666f6e0..bcbd737 100644 > --- a/cmds-subvolume.c > +++ b/cmds-subvolume.c > @@ -263,6 +263,10 @@ static int cmd_subvol_delete(int argc, char **argv) > DIR *dirstream = NULL; > int verbose = 0; > int commit_mode = 0; > + u8 fsid[BTRFS_FSID_SIZE]; > + char uuidbuf[BTRFS_UUID_UNPARSED_SIZE]; > + struct seen_fsid *seen_fsid_hash[SEEN_FSID_HASH_SIZE] = {NULL,}; > + enum { COMMIT_AFTER = 1, COMMIT_EACH = 2 }; > > while (1) { > int c; > @@ -279,10 +283,10 @@ static int cmd_subvol_delete(int argc, char **argv) > > switch(c) { > case 'c': > - commit_mode = 1; > + commit_mode = COMMIT_AFTER; > break; > case 'C': > - commit_mode = 2; > + commit_mode = COMMIT_EACH; The commit_mode cleanup can be a separate patch. (More patches always look cooler) Other part looks good enough. Feel free to add my reviewed by tags after splitting the cleanup patch. Reviewed-by: Qu Wenruo <quwenruo.btrfs@gmx.com> Thanks, Qu > break; > case 'v': > verbose++; > @@ -298,7 +302,7 @@ static int cmd_subvol_delete(int argc, char **argv) > if (verbose > 0) { > printf("Transaction commit: %s\n", > !commit_mode ? "none (default)" : > - commit_mode == 1 ? "at the end" : "after each"); > + commit_mode == COMMIT_AFTER ? "at the end" : "after each"); > } > > cnt = optind; > @@ -338,50 +342,81 @@ again: > } > > printf("Delete subvolume (%s): '%s/%s'\n", > - commit_mode == 2 || (commit_mode == 1 && cnt + 1 == argc) > - ? "commit" : "no-commit", dname, vname); > + commit_mode == COMMIT_EACH ? "commit" : "no-commit", dname, vname); > memset(&args, 0, sizeof(args)); > strncpy_null(args.name, vname); > res = ioctl(fd, BTRFS_IOC_SNAP_DESTROY, &args); > - if(res < 0 ){ > + if (res < 0) { > error("cannot delete '%s/%s': %s", dname, vname, > strerror(errno)); > ret = 1; > goto out; > } > > - if (commit_mode == 1) { > + if (commit_mode == COMMIT_EACH) { > res = wait_for_commit(fd); > if (res < 0) { > - error("unable to wait for commit after '%s': %s", > + error("unable to wait for commit after deleting '%s': %s", > path, strerror(errno)); > ret = 1; > } > + } else if (commit_mode == COMMIT_AFTER) { > + res = get_fsid(dname, fsid, 0); > + if (res < 0) { > + error("unable to get fsid for '%s': %s", path, strerror(-res)); > + error("delete suceeded but commit may not be done in the end"); > + ret = 1; > + goto out; > + } > + > + if (add_seen_fsid(fsid, seen_fsid_hash, fd, dirstream) == 0) { > + if (verbose > 0) { > + uuid_unparse(fsid, uuidbuf); > + printf(" new fs is found for '%s', fsid: %s\n", path, uuidbuf); > + } > + // this is the first time a subvolume on this filesystem is deleted > + // keep fd in order to issue SYNC ioctl in the end > + goto keep_fd; > + } > } > > out: > + close_file_or_dir(fd, dirstream); > +keep_fd: > + fd = -1; > + dirstream = NULL; > free(dupdname); > free(dupvname); > dupdname = NULL; > dupvname = NULL; > cnt++; > - if (cnt < argc) { > - close_file_or_dir(fd, dirstream); > - /* avoid double free */ > - fd = -1; > - dirstream = NULL; > + if (cnt < argc) > goto again; > - } > > - if (commit_mode == 2 && fd != -1) { > - res = wait_for_commit(fd); > - if (res < 0) { > - error("unable to do final sync after deletion: %s", > - strerror(errno)); > - ret = 1; > + if (commit_mode == COMMIT_AFTER) { > + // traverse seen_fsid_hash and issue SYNC ioctl on each filesystem > + int slot; > + struct seen_fsid *seen; > + > + for (slot = 0; slot < SEEN_FSID_HASH_SIZE; slot++) { > + seen = seen_fsid_hash[slot]; > + while (seen) { > + res = wait_for_commit(seen->fd); > + if (res < 0) { > + uuid_unparse(seen->fsid, uuidbuf); > + error("unable to do final sync after deletion: %s, fsid: %s", > + strerror(errno), uuidbuf); > + ret = 1; > + } else if (verbose > 0) { > + uuid_unparse(seen->fsid, uuidbuf); > + printf("final sync is done for fsid: %s\n", uuidbuf); > + } > + seen = seen->next; > + } > } > + // fd will also be closed in free_seen_fsid > + free_seen_fsid(seen_fsid_hash); > } > - close_file_or_dir(fd, dirstream); > > return ret; > } > -- 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-subvolume.c b/cmds-subvolume.c index 666f6e0..bcbd737 100644 --- a/cmds-subvolume.c +++ b/cmds-subvolume.c @@ -263,6 +263,10 @@ static int cmd_subvol_delete(int argc, char **argv) DIR *dirstream = NULL; int verbose = 0; int commit_mode = 0; + u8 fsid[BTRFS_FSID_SIZE]; + char uuidbuf[BTRFS_UUID_UNPARSED_SIZE]; + struct seen_fsid *seen_fsid_hash[SEEN_FSID_HASH_SIZE] = {NULL,}; + enum { COMMIT_AFTER = 1, COMMIT_EACH = 2 }; while (1) { int c; @@ -279,10 +283,10 @@ static int cmd_subvol_delete(int argc, char **argv) switch(c) { case 'c': - commit_mode = 1; + commit_mode = COMMIT_AFTER; break; case 'C': - commit_mode = 2; + commit_mode = COMMIT_EACH; break; case 'v': verbose++; @@ -298,7 +302,7 @@ static int cmd_subvol_delete(int argc, char **argv) if (verbose > 0) { printf("Transaction commit: %s\n", !commit_mode ? "none (default)" : - commit_mode == 1 ? "at the end" : "after each"); + commit_mode == COMMIT_AFTER ? "at the end" : "after each"); } cnt = optind; @@ -338,50 +342,81 @@ again: } printf("Delete subvolume (%s): '%s/%s'\n", - commit_mode == 2 || (commit_mode == 1 && cnt + 1 == argc) - ? "commit" : "no-commit", dname, vname); + commit_mode == COMMIT_EACH ? "commit" : "no-commit", dname, vname); memset(&args, 0, sizeof(args)); strncpy_null(args.name, vname); res = ioctl(fd, BTRFS_IOC_SNAP_DESTROY, &args); - if(res < 0 ){ + if (res < 0) { error("cannot delete '%s/%s': %s", dname, vname, strerror(errno)); ret = 1; goto out; } - if (commit_mode == 1) { + if (commit_mode == COMMIT_EACH) { res = wait_for_commit(fd); if (res < 0) { - error("unable to wait for commit after '%s': %s", + error("unable to wait for commit after deleting '%s': %s", path, strerror(errno)); ret = 1; } + } else if (commit_mode == COMMIT_AFTER) { + res = get_fsid(dname, fsid, 0); + if (res < 0) { + error("unable to get fsid for '%s': %s", path, strerror(-res)); + error("delete suceeded but commit may not be done in the end"); + ret = 1; + goto out; + } + + if (add_seen_fsid(fsid, seen_fsid_hash, fd, dirstream) == 0) { + if (verbose > 0) { + uuid_unparse(fsid, uuidbuf); + printf(" new fs is found for '%s', fsid: %s\n", path, uuidbuf); + } + // this is the first time a subvolume on this filesystem is deleted + // keep fd in order to issue SYNC ioctl in the end + goto keep_fd; + } } out: + close_file_or_dir(fd, dirstream); +keep_fd: + fd = -1; + dirstream = NULL; free(dupdname); free(dupvname); dupdname = NULL; dupvname = NULL; cnt++; - if (cnt < argc) { - close_file_or_dir(fd, dirstream); - /* avoid double free */ - fd = -1; - dirstream = NULL; + if (cnt < argc) goto again; - } - if (commit_mode == 2 && fd != -1) { - res = wait_for_commit(fd); - if (res < 0) { - error("unable to do final sync after deletion: %s", - strerror(errno)); - ret = 1; + if (commit_mode == COMMIT_AFTER) { + // traverse seen_fsid_hash and issue SYNC ioctl on each filesystem + int slot; + struct seen_fsid *seen; + + for (slot = 0; slot < SEEN_FSID_HASH_SIZE; slot++) { + seen = seen_fsid_hash[slot]; + while (seen) { + res = wait_for_commit(seen->fd); + if (res < 0) { + uuid_unparse(seen->fsid, uuidbuf); + error("unable to do final sync after deletion: %s, fsid: %s", + strerror(errno), uuidbuf); + ret = 1; + } else if (verbose > 0) { + uuid_unparse(seen->fsid, uuidbuf); + printf("final sync is done for fsid: %s\n", uuidbuf); + } + seen = seen->next; + } } + // fd will also be closed in free_seen_fsid + free_seen_fsid(seen_fsid_hash); } - close_file_or_dir(fd, dirstream); return ret; }
Fix subvol del --commit-after to work properly: - SYNC ioctl will be issued even when last delete is failed - SYNC ioctl will be issued on each file system only once in the end To achieve this, get_fsid() and add_seen_fsid() is called after each delete to keep only one fd for each fs. In the end, seen_fsid_hash will be traversed and SYNC is issued on each fs. Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com> --- cmds-subvolume.c | 77 ++++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 56 insertions(+), 21 deletions(-)